RE: [PATCH 1/8] staging: rtl8723bs: replace RND4 with round_up()

2020-10-04 Thread David Laight
From: Ross Schmidt
> Sent: 04 October 2020 02:18
> 
> Use round_up instead of define RND4.

RND4 was also particularly horrid!
...
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c
> b/drivers/staging/rtl8723bs/core/rtw_security.c
> index 7f74e1d05b3a..159d32ace2bc 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -260,7 +260,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 
> *pxmitframe)
>   arcfour_encrypt(&mycontext, payload+length, 
> crc, 4);
> 
>   pframe += pxmitpriv->frag_len;
> - pframe = (u8 *)RND4((SIZE_PTR)(pframe));
> + pframe = (u8 *)round_up((SIZE_PTR)(pframe), 4);

I also suspect this is equivalent to:
pframe == round_up(pxmitpriv->frag_len, 4);

Does make one wonder whether the skipped bytes need to be
zeroed though.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4] staging: qlge: fix build breakage with dumping enabled

2020-10-04 Thread Coiby Xu

On Sat, Oct 03, 2020 at 02:53:48PM +0900, Benjamin Poirier wrote:

On 2020-10-03 07:59 +0800, Coiby Xu wrote:

This fixes commit 0107635e15ac
("staging: qlge: replace pr_err with netdev_err") which introduced an
build breakage of missing `struct ql_adapter *qdev` for some functions
and a warning of type mismatch with dumping enabled, i.e.,

$ make CFLAGS_MODULE="-DQL_ALL_DUMP -DQL_OB_DUMP -DQL_CB_DUMP \
-DQL_IB_DUMP -DQL_REG_DUMP -DQL_DEV_DUMP" M=drivers/staging/qlge

qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this function); did 
you mean ‘cdev’?
 2051 |  netdev_err(qdev->ndev, "%s\n", __func__);
  | ^~~~
qlge_dbg.c: In function ‘ql_dump_routing_entries’:
qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char *’, but 
argument 3 has type ‘int’ [-Wformat=]
 1435 |"%s: Routing Mask %d = 0x%.08x\n",
  | ~^
  |  |
  |  char *
  | %d
 1436 |i, value);
  |~
  ||
  |int
qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned int’ 
argument [-Wformat=]
 1435 |"%s: Routing Mask %d = 0x%.08x\n",
  | ^
  | |
  | unsigned int

Note that now ql_dump_rx_ring/ql_dump_tx_ring won't check if the passed
parameter is a null pointer.

Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
Reported-by: Benjamin Poirier 
Suggested-by: Benjamin Poirier 
Signed-off-by: Coiby Xu 
---


Reviewed-by: Benjamin Poirier 


Thank you! Btw, I guess when this patch is picked, the "Reviewed-by" tag
will also be included. So I needn't to send another patch, am I right?

--
Best regards,
Coiby
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] media: zoran.rst: place it at the right place this time

2020-10-04 Thread Mauro Carvalho Chehab
I was too quick moving zoran.rst... it ends that the original
patch didn't do the right thing and forgot to update the files
that references it.

Fix it.

Fixes: 6b90346919d4 ("media: zoran: move documentation file to the right place")
Signed-off-by: Mauro Carvalho Chehab 
---
 .../driver-api/media/drivers/{v4l-drivers => }/zoran.rst| 0
 MAINTAINERS | 2 +-
 drivers/staging/media/zoran/Kconfig | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/driver-api/media/drivers/{v4l-drivers => }/zoran.rst 
(100%)

diff --git a/Documentation/driver-api/media/drivers/v4l-drivers/zoran.rst 
b/Documentation/driver-api/media/drivers/zoran.rst
similarity index 100%
rename from Documentation/driver-api/media/drivers/v4l-drivers/zoran.rst
rename to Documentation/driver-api/media/drivers/zoran.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index ba5eb1dff9c2..7a12633747c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19247,7 +19247,7 @@ L:  linux-me...@vger.kernel.org
 S: Maintained
 W: http://mjpeg.sourceforge.net/driver-zoran/
 Q: https://patchwork.linuxtv.org/project/linux-media/list/
-F: Documentation/media/v4l-drivers/zoran.rst
+F: Documentation/driver-api/media/drivers/zoran.rst
 F: drivers/staging/media/zoran/
 
 ZPOOL COMPRESSED PAGE STORAGE API
diff --git a/drivers/staging/media/zoran/Kconfig 
b/drivers/staging/media/zoran/Kconfig
index 492507030276..7874842033ca 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -8,7 +8,7 @@ config VIDEO_ZORAN
  36057/36067 PCI controller chipset. This includes the Iomega
  Buz, Pinnacle DC10+ and the Linux Media Labs LML33. There is
  a driver homepage at . For
- more information, check 
.
+ more information, check 
.
 
  To compile this driver as a module, choose M here: the
  module will be called zr36067.
-- 
2.26.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] media: zoran.rst: place it at the right place this time

2020-10-04 Thread LABBE Corentin
On Sun, Oct 04, 2020 at 06:00:30PM +0200, Mauro Carvalho Chehab wrote:
> I was too quick moving zoran.rst... it ends that the original
> patch didn't do the right thing and forgot to update the files
> that references it.
> 
> Fix it.
> 
> Fixes: 6b90346919d4 ("media: zoran: move documentation file to the right 
> place")
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  .../driver-api/media/drivers/{v4l-drivers => }/zoran.rst| 0
>  MAINTAINERS | 2 +-
>  drivers/staging/media/zoran/Kconfig | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename Documentation/driver-api/media/drivers/{v4l-drivers => }/zoran.rst 
> (100%)
> 
> diff --git a/Documentation/driver-api/media/drivers/v4l-drivers/zoran.rst 
> b/Documentation/driver-api/media/drivers/zoran.rst
> similarity index 100%
> rename from Documentation/driver-api/media/drivers/v4l-drivers/zoran.rst
> rename to Documentation/driver-api/media/drivers/zoran.rst
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ba5eb1dff9c2..7a12633747c8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19247,7 +19247,7 @@ L:linux-me...@vger.kernel.org
>  S:   Maintained
>  W:   http://mjpeg.sourceforge.net/driver-zoran/
>  Q:   https://patchwork.linuxtv.org/project/linux-media/list/
> -F:   Documentation/media/v4l-drivers/zoran.rst
> +F:   Documentation/driver-api/media/drivers/zoran.rst
>  F:   drivers/staging/media/zoran/
>  
>  ZPOOL COMPRESSED PAGE STORAGE API
> diff --git a/drivers/staging/media/zoran/Kconfig 
> b/drivers/staging/media/zoran/Kconfig
> index 492507030276..7874842033ca 100644
> --- a/drivers/staging/media/zoran/Kconfig
> +++ b/drivers/staging/media/zoran/Kconfig
> @@ -8,7 +8,7 @@ config VIDEO_ZORAN
> 36057/36067 PCI controller chipset. This includes the Iomega
> Buz, Pinnacle DC10+ and the Linux Media Labs LML33. There is
> a driver homepage at . For
> -   more information, check 
> .
> +   more information, check 
> .
>  
> To compile this driver as a module, choose M here: the
> module will be called zr36067.
> -- 
> 2.26.2
> 
Hello

Acked-by: Corentin Labbe 

Thanks
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4] staging: qlge: fix build breakage with dumping enabled

2020-10-04 Thread Benjamin Poirier
On 2020-10-04 23:22 +0800, Coiby Xu wrote:
> On Sat, Oct 03, 2020 at 02:53:48PM +0900, Benjamin Poirier wrote:
> > On 2020-10-03 07:59 +0800, Coiby Xu wrote:
> > > This fixes commit 0107635e15ac
> > > ("staging: qlge: replace pr_err with netdev_err") which introduced an
> > > build breakage of missing `struct ql_adapter *qdev` for some functions
> > > and a warning of type mismatch with dumping enabled, i.e.,
> > > 
> > > $ make CFLAGS_MODULE="-DQL_ALL_DUMP -DQL_OB_DUMP -DQL_CB_DUMP \
> > > -DQL_IB_DUMP -DQL_REG_DUMP -DQL_DEV_DUMP" M=drivers/staging/qlge
> > > 
> > > qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
> > > qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this 
> > > function); did you mean ‘cdev’?
> > >  2051 |  netdev_err(qdev->ndev, "%s\n", __func__);
> > >   | ^~~~
> > > qlge_dbg.c: In function ‘ql_dump_routing_entries’:
> > > qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char 
> > > *’, but argument 3 has type ‘int’ [-Wformat=]
> > >  1435 |"%s: Routing Mask %d = 0x%.08x\n",
> > >   | ~^
> > >   |  |
> > >   |  char *
> > >   | %d
> > >  1436 |i, value);
> > >   |~
> > >   ||
> > >   |int
> > > qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned 
> > > int’ argument [-Wformat=]
> > >  1435 |"%s: Routing Mask %d = 0x%.08x\n",
> > >   | ^
> > >   | |
> > >   | unsigned int
> > > 
> > > Note that now ql_dump_rx_ring/ql_dump_tx_ring won't check if the passed
> > > parameter is a null pointer.
> > > 
> > > Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
> > > Reported-by: Benjamin Poirier 
> > > Suggested-by: Benjamin Poirier 
> > > Signed-off-by: Coiby Xu 
> > > ---
> > 
> > Reviewed-by: Benjamin Poirier 
> 
> Thank you! Btw, I guess when this patch is picked, the "Reviewed-by" tag
> will also be included. So I needn't to send another patch, am I right?

I think so. Maintainers usually take care of adding attribution tags
from followup emails and that's what Greg has done for your previous
qlge patch it looks like.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC 1/3] Initialize devlink health dump framework for the dlge driver

2020-10-04 Thread Coiby Xu

On Fri, Aug 21, 2020 at 11:08:22AM +0800, Coiby Xu wrote:

On Sun, Aug 16, 2020 at 11:56:40AM +0900, Benjamin Poirier wrote:

On 2020-08-15 00:05 +0800, Coiby Xu wrote:

Initialize devlink health dump framework for the dlge driver so the
coredump could be done via devlink.

Signed-off-by: Coiby Xu 
---
drivers/staging/qlge/Makefile  |  2 +-
drivers/staging/qlge/qlge.h|  9 +++
drivers/staging/qlge/qlge_health.c | 43 ++
drivers/staging/qlge/qlge_health.h |  2 ++
drivers/staging/qlge/qlge_main.c   | 21 +++
5 files changed, 76 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/qlge/qlge_health.c
create mode 100644 drivers/staging/qlge/qlge_health.h

diff --git a/drivers/staging/qlge/Makefile b/drivers/staging/qlge/Makefile
index 1dc2568e820c..0a1e4c8dd546 100644
--- a/drivers/staging/qlge/Makefile
+++ b/drivers/staging/qlge/Makefile
@@ -5,4 +5,4 @@

obj-$(CONFIG_QLGE) += qlge.o

-qlge-objs := qlge_main.o qlge_dbg.o qlge_mpi.o qlge_ethtool.o
+qlge-objs := qlge_main.o qlge_dbg.o qlge_mpi.o qlge_ethtool.o qlge_health.o
diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index fc8c5ca8935d..055ded6dab60 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2061,6 +2061,14 @@ struct nic_operations {
int (*port_initialize) (struct ql_adapter *);
};



This patch doesn't apply over the latest staging tree. I think your tree
is missing commit d923bb6bf508 ("staging: qlge: qlge.h: Function
definition arguments should have names.")


Thank you for applying the patch to test it! I had incorrect
understanding about the word "RFC" and didn't do a rebase onto
the latest staging tree.




+
+
+struct qlge_devlink {
+struct ql_adapter *qdev;
+struct net_device *ndev;


I don't have experience implementing devlink callbacks but looking at
some other devlink users (mlx4, ionic, ice), all of them use devlink
priv space for their main private structure. That would be struct
ql_adapter in this case. Is there a good reason to stray from that
pattern?


+struct devlink_health_reporter *reporter;
+};
+
/*
 * The main Adapter structure definition.
 * This structure has all fields relevant to the hardware.
@@ -2078,6 +2086,7 @@ struct ql_adapter {
struct pci_dev *pdev;
struct net_device *ndev;/* Parent NET device */

+   struct qlge_devlink *devlink;
/* Hardware information */
u32 chip_rev_id;
u32 fw_rev_id;
diff --git a/drivers/staging/qlge/qlge_health.c 
b/drivers/staging/qlge/qlge_health.c
new file mode 100644
index ..292f6b1827e1
--- /dev/null
+++ b/drivers/staging/qlge/qlge_health.c
@@ -0,0 +1,43 @@
+#include "qlge.h"
+#include "qlge_health.h"
+
+static int
+qlge_reporter_coredump(struct devlink_health_reporter *reporter,
+   struct devlink_fmsg *fmsg, void *priv_ctx,
+   struct netlink_ext_ack *extack)
+{
+   return 0;
+}
+
+static const struct devlink_health_reporter_ops qlge_reporter_ops = {
+   .name = "dummy",
+   .dump = qlge_reporter_coredump,
+};


I think
select NET_DEVLINK
should be added to drivers/staging/qlge/Kconfig


Thank you for reminding me!




+
+int qlge_health_create_reporters(struct qlge_devlink *priv)
+{
+   int err;
+
+   struct devlink_health_reporter *reporter;
+   struct devlink *devlink;
+
+   devlink = priv_to_devlink(priv);
+   reporter =
+   devlink_health_reporter_create(devlink, &qlge_reporter_ops,
+  0,
+  priv);
+   if (IS_ERR(reporter)) {
+   netdev_warn(priv->ndev,
+   "Failed to create reporter, err = %ld\n",
+   PTR_ERR(reporter));
+   return PTR_ERR(reporter);
+   }
+   priv->reporter = reporter;
+
+   if (err)
+   return err;
+
+   return 0;
+}
+
+


Stray newlines


Will fix it in v1.




diff --git a/drivers/staging/qlge/qlge_health.h 
b/drivers/staging/qlge/qlge_health.h
new file mode 100644
index ..07d3bafab845
--- /dev/null
+++ b/drivers/staging/qlge/qlge_health.h
@@ -0,0 +1,2 @@
+#include 
+int qlge_health_create_reporters(struct qlge_devlink *priv);


I would suggest to put this in qlge.h instead of creating a new file.


Although there are only two lines for now, is it possible qlge will add
more devlink code? If that's the case, a file to single out these code
is necessary as is the same to some other drivers,

   $ find drivers -name *health*.h
   drivers/net/ethernet/mellanox/mlx5/core/en/health.h

   $ find drivers -name *devlink*.h
   drivers/net/ethernet/huawei/hinic/hinic_devlink.h
   drivers/net/ethernet/mellanox/mlx5/core/devlink.h
   drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h
   drivers/net/ethernet/intel/ice/ice_devlink.h
   drivers/net/ethernet/pen