[PATCH] MAINTAINERS: rectify ZR36067 VIDEO FOR LINUX DRIVER section

2020-10-08 Thread Lukas Bulwahn 
Commit 754f0f1ba8d9 ("media: MAINTAINERS: change maintainer of the zoran
driver") added a new section in MAINTAINERS with an invalid file entry
and at the wrong place for alphabetic ordering.

Hence, ./scripts/get_maintainer.pl --self-test=patterns complains:

  warning: no file matches  F:  Documentation/media/v4l-drivers/zoran.rst

Point the file entry to the right location and move the section to the
right place in MAINTAINERS.

Signed-off-by: Lukas Bulwahn 
---
applies cleanly on next-20201008

Corentin, please ack.
Mauro, please pick this minor non-urgent cleanup patch into your -next tree.

 MAINTAINERS | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 239ae2425cf8..6879ca545677 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19412,6 +19412,13 @@ T: git 
git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/zonefs.git
 F: Documentation/filesystems/zonefs.rst
 F: fs/zonefs/
 
+ZPOOL COMPRESSED PAGE STORAGE API
+M: Dan Streetman 
+L: linux...@kvack.org
+S: Maintained
+F: include/linux/zpool.h
+F: mm/zpool.c
+
 ZR36067 VIDEO FOR LINUX DRIVER
 M: Corentin Labbe 
 L: mjpeg-us...@lists.sourceforge.net
@@ -19419,16 +19426,9 @@ 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/v4l-drivers/zoran.rst
 F: drivers/staging/media/zoran/
 
-ZPOOL COMPRESSED PAGE STORAGE API
-M: Dan Streetman 
-L: linux...@kvack.org
-S: Maintained
-F: include/linux/zpool.h
-F: mm/zpool.c
-
 ZRAM COMPRESSED RAM BLOCK DEVICE DRVIER
 M: Minchan Kim 
 M: Nitin Gupta 
-- 
2.17.1

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

Re: [PATCH v1 2/6] staging: qlge: coredump via devlink health reporter

2020-10-08 Thread Coiby Xu 

On Thu, Oct 08, 2020 at 04:39:40PM +0300, Dan Carpenter wrote:

On Thu, Oct 08, 2020 at 07:58:04PM +0800, Coiby Xu wrote:

-static int
-qlge_reporter_coredump(struct devlink_health_reporter *reporter,
-   struct devlink_fmsg *fmsg, void *priv_ctx,
-   struct netlink_ext_ack *extack)
+static int fill_seg_(struct devlink_fmsg *fmsg,
+   struct mpi_coredump_segment_header *seg_header,
+   u32 *reg_data)
 {
-   return 0;
+   int i;
+   int header_size = sizeof(struct mpi_coredump_segment_header);


Please use the sizeof() directly in the code.  Don't introduce
indirection if you can help it.


+   int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
+   int err;
+



Thank you for the suggestion!

regards,
dan carpenter


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

Re: [PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread Coiby Xu 

On Thu, Oct 08, 2020 at 04:31:42PM +0300, Dan Carpenter wrote:

On Thu, Oct 08, 2020 at 07:58:03PM +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/Kconfig|  1 +
 drivers/staging/qlge/Makefile   |  2 +-
 drivers/staging/qlge/qlge.h |  9 +++
 drivers/staging/qlge/qlge_devlink.c | 38 +
 drivers/staging/qlge/qlge_devlink.h |  8 ++
 drivers/staging/qlge/qlge_main.c| 28 +
 6 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/qlge/qlge_devlink.c
 create mode 100644 drivers/staging/qlge/qlge_devlink.h

diff --git a/drivers/staging/qlge/Kconfig b/drivers/staging/qlge/Kconfig
index a3cb25a3ab80..6d831ed67965 100644
--- a/drivers/staging/qlge/Kconfig
+++ b/drivers/staging/qlge/Kconfig
@@ -3,6 +3,7 @@
 config QLGE
tristate "QLogic QLGE 10Gb Ethernet Driver Support"
depends on ETHERNET && PCI
+   select NET_DEVLINK
help
This driver supports QLogic ISP8XXX 10Gb Ethernet cards.

diff --git a/drivers/staging/qlge/Makefile b/drivers/staging/qlge/Makefile
index 1dc2568e820c..07c1898a512e 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_devlink.o
diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index b295990e361b..290e754450c5 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2060,6 +2060,14 @@ struct nic_operations {
int (*port_initialize)(struct ql_adapter *qdev);
 };

+
+


Having multiple blank lines in a row leads to a static checker warning.
Please run `checkpatch.pl --strict` over your patches.


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

+   struct qlge_devlink *ql_devlink;
/* Hardware information */
u32 chip_rev_id;
u32 fw_rev_id;
diff --git a/drivers/staging/qlge/qlge_devlink.c 
b/drivers/staging/qlge/qlge_devlink.c
new file mode 100644
index ..aa45e7e368c0
--- /dev/null
+++ b/drivers/staging/qlge/qlge_devlink.c
@@ -0,0 +1,38 @@
+#include "qlge.h"
+#include "qlge_devlink.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,


Indented too far.


+};
+
+int qlge_health_create_reporters(struct qlge_devlink *priv)
+{
+   int err;
+


No blanks in the middle of declarations.


+   struct devlink_health_reporter *reporter;
+   struct devlink *devlink;


Generally this driver declares variables in "Reverse Christmas Tree"
order.  The names are orderred by the length of the line from longest
to shortest.

type long_name;
type medium;
type short;


+
+   devlink = priv_to_devlink(priv);
+   reporter =
+   devlink_health_reporter_create(devlink, &qlge_reporter_ops,
+  0,
+  priv);


Break this up like so:

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);


No point in returning an error if the caller doesn't check?


+   }
+   priv->reporter = reporter;
+
+   return err;


err is uninitialized.  "return 0;"


+}
diff --git a/drivers/staging/qlge/qlge_devlink.h 
b/drivers/staging/qlge/qlge_devlink.h
new file mode 100644
index ..c91f7a29e805
--- /dev/null
+++ b/drivers/staging/qlge/qlge_devlink.h
@@ -0,0 +1,8 @@
+#ifndef QLGE_DEVLINK_H
+#define QLGE_DEVLINK_H
+
+#include 
+
+int qlge_health_create_reporters(struct qlge_devlink *priv);
+
+#endif /* QLGE_DEVLINK_H */
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 27da386f9d87..135225530e51 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -42,6 +42,7 @@
 #include 

 #include

Cursos Bonificables (ÚLTIMA CONVOCATORIA 2020)

2020-10-08 Thread FOESCO 
Buenos días


Llegadas estas fechas y como cada año, recordamos a todas las empresas 
Españolas su derecho a consumir el Crédito de Formación del que disponen para 
la formación de sus empleados en activo o en ERTE, antes de su caducidad a 
final de año.


Actualmente se encuentra abierto el plazo de inscripción para la "ÚLTIMA 
CONVOCATORIA 2020" de Cursos Bonificables con cargo al Crédito de Formación 
2020.


AVISO IMPORTANTE: Si vuestra empresa todavía dispone de Crédito de Formación 
2020 esta es la última oportunidad para poder utilizarlo.


Deseáis que os mandemos la información?


Quedamos a la espera de vuestra respuesta.


Saludos cordiales.


Alex Pons
Director FOESCO.

FOESCO Formación Estatal Continua.
Entidad Organizadora: B200592AA
www.foesco.com
e-mail: cur...@foesco.net
Tel: 910 323 794
(Horario de 9h a 15h y de 17h a 20h de Lunes a Viernes)

FOESCO ofrece formación a empresas y trabajadores en activo a través de cursos 
bonificados por la Fundación Estatal para la Formación en el Empleo (antiguo 
FORCEM) que gestiona las acciones formativas de FORMACIÓN CONTINUA para 
trabajadores y se rige por la ley 30/2015 de 9 de Septiembre.

Antes de imprimir este e-mail piense bien si es necesario hacerlo. Before 
printing this e-mail please think twice if you really need it. FOESCO Tfno: 910 
382 880 Email: cur...@foesco.com. La información transmitida en este mensaje 
está dirigida solamente a las personas o entidades que figuran en el 
encabezamiento y contiene información confidencial, por lo que, si usted lo 
recibiera por error, por favor destrúyalo sin copiarlo, usarlo ni distribuirlo, 
comunicándolo inmediatamente al emisor del mensaje. De conformidad con lo 
dispuesto en el Reglamento Europeo del 2016/679, del 27 de Abril de 2016, 
FOESCO le informa que los datos por usted suministrados serán tratados con las 
medidas de seguridad conformes a la normativa vigente que se requiere. Dichos 
datos serán empleados con fines de gestión. Para el ejercicio de sus derechos 
de transparencia, información, acceso, rectificación, supresión o derecho al 
olvido, limitación del tratamiento , portabilidad de datos y oposición de sus 
datos de carácter personal deberá dirigirse a la dirección del Responsable del 
tratamiento a C/ LAGUNA DEL MARQUESADO Nº10, 28021, MADRID, "PULSANDO AQUI" 
 y "ENVIAR" o a traves de la 
dirección de correo electrónico: ba...@foesco.com 

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

[PATCH] staging: rtl8188eu: Fix long lines

2020-10-08 Thread Fan Fei 
This patch fix long lines found by checkpatch.

Signed-off-by: Fan Fei 
---
 drivers/staging/rtl8188eu/core/rtw_cmd.c | 74 
 1 file changed, 51 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_cmd.c 
b/drivers/staging/rtl8188eu/core/rtw_cmd.c
index 5c300865eeb3..a2b88ba242d5 100644
--- a/drivers/staging/rtl8188eu/core/rtw_cmd.c
+++ b/drivers/staging/rtl8188eu/core/rtw_cmd.c
@@ -159,14 +159,16 @@ int rtw_cmd_thread(void *context)
if (padapter->bDriverStopped ||
padapter->bSurpriseRemoved) {
DBG_88E("%s: DriverStopped(%d) SurpriseRemoved(%d) 
break at line %d\n",
-   __func__, padapter->bDriverStopped, 
padapter->bSurpriseRemoved, __LINE__);
+   __func__, padapter->bDriverStopped,
+   padapter->bSurpriseRemoved, __LINE__);
break;
}
 _next:
if (padapter->bDriverStopped ||
padapter->bSurpriseRemoved) {
DBG_88E("%s: DriverStopped(%d) SurpriseRemoved(%d) 
break at line %d\n",
-   __func__, padapter->bDriverStopped, 
padapter->bSurpriseRemoved, __LINE__);
+   __func__, padapter->bDriverStopped,
+   padapter->bSurpriseRemoved, __LINE__);
break;
}
 
@@ -195,14 +197,18 @@ int rtw_cmd_thread(void *context)
if (pcmd->cmdcode < ARRAY_SIZE(rtw_cmd_callback)) {
pcmd_callback = 
rtw_cmd_callback[pcmd->cmdcode].callback;
if (!pcmd_callback) {
-   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, 
("mlme_cmd_hdl(): pcmd_callback = 0x%p, cmdcode = 0x%x\n", pcmd_callback, 
pcmd->cmdcode));
+   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
+("mlme_cmd_hdl(): pcmd_callback = 
0x%p, cmdcode = 0x%x\n",
+ pcmd_callback, pcmd->cmdcode));
rtw_free_cmd_obj(pcmd);
} else {
/* todo: !!! fill rsp_buf to pcmd->rsp if 
(pcmd->rsp!= NULL) */
pcmd_callback(pcmd->padapter, pcmd);/* need 
consider that free cmd_obj in rtw_cmd_callback */
}
} else {
-   RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_, ("%s: 
cmdcode = 0x%x callback not defined!\n", __func__, pcmd->cmdcode));
+   RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
+("%s: cmdcode = 0x%x callback not defined!\n",
+ __func__, pcmd->cmdcode));
rtw_free_cmd_obj(pcmd);
}
 
@@ -264,7 +270,8 @@ u8 rtw_sitesurvey_cmd(struct adapter  *padapter, struct 
ndis_802_11_ssid *ssid,
 
for (i = 0; i < ssid_num && i < RTW_SSID_SCAN_AMOUNT; i++) {
if (ssid[i].ssid_length) {
-   memcpy(&psurveyPara->ssid[i], &ssid[i], 
sizeof(struct ndis_802_11_ssid));
+   memcpy(&psurveyPara->ssid[i], &ssid[i],
+  sizeof(struct ndis_802_11_ssid));
psurveyPara->ssid_num++;
}
}
@@ -276,7 +283,8 @@ u8 rtw_sitesurvey_cmd(struct adapter  *padapter, struct 
ndis_802_11_ssid *ssid,
 
for (i = 0; i < ch_num && i < RTW_CHANNEL_SCAN_AMOUNT; i++) {
if (ch[i].hw_value && !(ch[i].flags & 
RTW_IEEE80211_CHAN_DISABLED)) {
-   memcpy(&psurveyPara->ch[i], &ch[i], 
sizeof(struct rtw_ieee80211_channel));
+   memcpy(&psurveyPara->ch[i], &ch[i],
+  sizeof(struct rtw_ieee80211_channel));
psurveyPara->ch_num++;
}
}
@@ -317,9 +325,11 @@ u8 rtw_createbss_cmd(struct adapter  *padapter)
led_control_8188eu(padapter, LED_CTL_START_TO_LINK);
 
if (pmlmepriv->assoc_ssid.ssid_length == 0)
-   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for 
Any SSid:%s\n", pmlmepriv->assoc_ssid.ssid));
+   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
+(" createbss for Any SSid:%s\n", 
pmlmepriv->assoc_ssid.ssid));
else
-   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for 
SSid:%s\n", pmlmepriv->assoc_ssid.ssid));
+   RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
+(" createbss for SSid:%s\n", 
pmlmepriv->assoc_ssid.ssid));
 
pcmd = kzalloc(sizeof(*pcmd), GFP_ATOMIC);
if (!pcmd) {
@@ -361,7 +371,8 @@ u8 rtw_joinbss

Re: [PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread kernel test robot 
Hi Coiby,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:
https://github.com/0day-ci/linux/commits/Coiby-Xu/staging-qlge-Re-writing-the-debugging-features/20201008-22
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
76c3bdd67d27289b9e407113821eab2a70bbcca6
config: arm64-randconfig-r025-20201008 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
8da0df3d6dcc0dd42740be60b0da4ec201190904)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# 
https://github.com/0day-ci/linux/commit/dda18d66af0554f1f2b69f9a61335f3de9ec5124
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Coiby-Xu/staging-qlge-Re-writing-the-debugging-features/20201008-22
git checkout dda18d66af0554f1f2b69f9a61335f3de9ec5124
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

>> drivers/staging/qlge/qlge_devlink.c:37:9: warning: variable 'err' is 
>> uninitialized when used here [-Wuninitialized]
   return err;
  ^~~
   drivers/staging/qlge/qlge_devlink.c:19:9: note: initialize the variable 
'err' to silence this warning
   int err;
  ^
   = 0
   1 warning generated.

vim +/err +37 drivers/staging/qlge/qlge_devlink.c

16  
17  int qlge_health_create_reporters(struct qlge_devlink *priv)
18  {
19  int err;
20  
21  struct devlink_health_reporter *reporter;
22  struct devlink *devlink;
23  
24  devlink = priv_to_devlink(priv);
25  reporter =
26  devlink_health_reporter_create(devlink, 
&qlge_reporter_ops,
27 0,
28 priv);
29  if (IS_ERR(reporter)) {
30  netdev_warn(priv->ndev,
31  "Failed to create reporter, err = %ld\n",
32  PTR_ERR(reporter));
33  return PTR_ERR(reporter);
34  }
35  priv->reporter = reporter;
36  
  > 37  return err;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 2/6] staging: qlge: coredump via devlink health reporter

2020-10-08 Thread Dan Carpenter 
On Thu, Oct 08, 2020 at 07:58:04PM +0800, Coiby Xu wrote:
> -static int
> -qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> - struct devlink_fmsg *fmsg, void *priv_ctx,
> - struct netlink_ext_ack *extack)
> +static int fill_seg_(struct devlink_fmsg *fmsg,
> + struct mpi_coredump_segment_header *seg_header,
> + u32 *reg_data)
>  {
> - return 0;
> + int i;
> + int header_size = sizeof(struct mpi_coredump_segment_header);

Please use the sizeof() directly in the code.  Don't introduce
indirection if you can help it.

> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
> + int err;
> +

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

Re: [PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread Dan Carpenter 
On Thu, Oct 08, 2020 at 07:58:03PM +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/Kconfig|  1 +
>  drivers/staging/qlge/Makefile   |  2 +-
>  drivers/staging/qlge/qlge.h |  9 +++
>  drivers/staging/qlge/qlge_devlink.c | 38 +
>  drivers/staging/qlge/qlge_devlink.h |  8 ++
>  drivers/staging/qlge/qlge_main.c| 28 +
>  6 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/staging/qlge/qlge_devlink.c
>  create mode 100644 drivers/staging/qlge/qlge_devlink.h
> 
> diff --git a/drivers/staging/qlge/Kconfig b/drivers/staging/qlge/Kconfig
> index a3cb25a3ab80..6d831ed67965 100644
> --- a/drivers/staging/qlge/Kconfig
> +++ b/drivers/staging/qlge/Kconfig
> @@ -3,6 +3,7 @@
>  config QLGE
>   tristate "QLogic QLGE 10Gb Ethernet Driver Support"
>   depends on ETHERNET && PCI
> + select NET_DEVLINK
>   help
>   This driver supports QLogic ISP8XXX 10Gb Ethernet cards.
>  
> diff --git a/drivers/staging/qlge/Makefile b/drivers/staging/qlge/Makefile
> index 1dc2568e820c..07c1898a512e 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_devlink.o
> diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
> index b295990e361b..290e754450c5 100644
> --- a/drivers/staging/qlge/qlge.h
> +++ b/drivers/staging/qlge/qlge.h
> @@ -2060,6 +2060,14 @@ struct nic_operations {
>   int (*port_initialize)(struct ql_adapter *qdev);
>  };
>  
> +
> +

Having multiple blank lines in a row leads to a static checker warning.
Please run `checkpatch.pl --strict` over your patches.

> +struct qlge_devlink {
> +struct ql_adapter *qdev;
> +struct net_device *ndev;
> +struct devlink_health_reporter *reporter;
> +};
> +
>  /*
>   * The main Adapter structure definition.
>   * This structure has all fields relevant to the hardware.
> @@ -2077,6 +2085,7 @@ struct ql_adapter {
>   struct pci_dev *pdev;
>   struct net_device *ndev;/* Parent NET device */
>  
> + struct qlge_devlink *ql_devlink;
>   /* Hardware information */
>   u32 chip_rev_id;
>   u32 fw_rev_id;
> diff --git a/drivers/staging/qlge/qlge_devlink.c 
> b/drivers/staging/qlge/qlge_devlink.c
> new file mode 100644
> index ..aa45e7e368c0
> --- /dev/null
> +++ b/drivers/staging/qlge/qlge_devlink.c
> @@ -0,0 +1,38 @@
> +#include "qlge.h"
> +#include "qlge_devlink.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,

Indented too far.

> +};
> +
> +int qlge_health_create_reporters(struct qlge_devlink *priv)
> +{
> + int err;
> +

No blanks in the middle of declarations.

> + struct devlink_health_reporter *reporter;
> + struct devlink *devlink;

Generally this driver declares variables in "Reverse Christmas Tree"
order.  The names are orderred by the length of the line from longest
to shortest.

type long_name;
type medium;
type short;

> +
> + devlink = priv_to_devlink(priv);
> + reporter =
> + devlink_health_reporter_create(devlink, &qlge_reporter_ops,
> +0,
> +priv);

Break this up like so:

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);

No point in returning an error if the caller doesn't check?

> + }
> + priv->reporter = reporter;
> +
> + return err;

err is uninitialized.  "return 0;"

> +}
> diff --git a/drivers/staging/qlge/qlge_devlink.h 
> b/drivers/staging/qlge/qlge_devlink.h
> new file mode 100644
> index ..c91f7a29e805
> --- /dev/null
> +++ b/drivers/staging/qlge/qlge_devlink.h
> @@ -0,0 +1,8 @@
> +#ifndef QLGE_DEVLINK_H
> +#define QLGE_DEVLINK_H
> +
> +#include 
> +
> +int qlge_health_create_reporters(struct qlge_devlink *priv);
> +
> +#endif /* QLGE_DEVLINK_H */
> diff --git a/drivers/staging/qlge/qlge_main.c 
> b/drivers/staging/qlge/qlge_main.c
> index 27da386f9d87..135225530e51 100644
> -

Re: [PATCH 0/7] wfx: move out from the staging area

2020-10-08 Thread Dan Carpenter 
There are some static checker warnings to look at from linux-next from
Tuesday.

drivers/staging/wfx/hif_tx.c:319 hif_join() error: we previously assumed 
'channel' could be null (see line 315)
drivers/staging/wfx/main.c:228 wfx_send_pdata_pds() warn: potential NULL 
parameter dereference 'tmp_buf'
drivers/staging/wfx/hif_rx.c:177 hif_scan_complete_indication() warn: potential 
NULL parameter dereference 'wvif'
drivers/staging/wfx/data_tx.c:576 wfx_flush() warn: potential NULL parameter 
dereference 'wvif'
drivers/staging/wfx/bus_spi.c:228 wfx_spi_probe() warn: 'bus->core' could be an 
error pointer
drivers/staging/wfx/bus_sdio.c:221 wfx_sdio_probe() warn: 'bus->core' could be 
an error pointer
drivers/staging/wfx/hif_rx.c:26 hif_generic_confirm() warn: negative user 
subtract: 0-u16max - 4
drivers/staging/wfx/hif_rx.c:98 hif_wakeup_indication() warn: 
'gpiod_get_value(wdev->pdata.gpio_wakeup)' returns positive and negative
drivers/staging/wfx/bh.c:24 device_wakeup() warn: 
'gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup)' returns positive and 
negative
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string 
contains non-ascii character '\xc2'
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string 
contains non-ascii character '\xb0'
drivers/staging/wfx/data_tx.c:37 wfx_get_hw_rate() warn: constraint '(struct 
ieee80211_supported_band)->bitrates' overflow 'band->bitrates' 0 <= abs_rl 
'0-127' user_rl '' required = '(struct ieee80211_supported_band)->n_bitrates'

Some of these are unpublished checks that I haven't published because
they are too crap.  The rest of the email is just long explanations.
Skip if not required.

regards,
dan carpenter

#1
drivers/staging/wfx/hif_tx.c:319 hif_join() error: we previously assumed 
'channel' could be null (see line 315)
   311  if (!hif)
   312  return -ENOMEM;
   313  body->infrastructure_bss_mode = !conf->ibss_joined;
   314  body->short_preamble = conf->use_short_preamble;
   315  if (channel && channel->flags & IEEE80211_CHAN_NO_IR)
^^^
Check for NULL.

   316  body->probe_for_join = 0;
   317  else
   318  body->probe_for_join = 1;
   319  body->channel_number = channel->hw_value;
   ^
Unchecked dereference.

   320  body->beacon_interval = cpu_to_le32(conf->beacon_int);
   321  body->basic_rate_set =

#2
drivers/staging/wfx/main.c:228 wfx_send_pdata_pds() warn: potential NULL 
parameter dereference 'tmp_buf'
   227  tmp_buf = kmemdup(pds->data, pds->size, GFP_KERNEL);

No check for allocation failure.

   228  ret = wfx_send_pds(wdev, tmp_buf, pds->size);
   229  kfree(tmp_buf);

#3
drivers/staging/wfx/hif_rx.c:177 hif_scan_complete_indication() warn: potential 
NULL parameter dereference 'wvif'
   170  static int hif_scan_complete_indication(struct wfx_dev *wdev,
   171  const struct hif_msg *hif,
   172  const void *buf)
   173  {
   174  struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
^^^
Smatch thinks wdev_to_wvif() can return NULL.

   175  
   176  WARN_ON(!wvif);
   177  wfx_scan_complete(wvif);
   178  
   179  return 0;
   180  }

#4
drivers/staging/wfx/data_tx.c:576 wfx_flush() warn: potential NULL parameter 
dereference 'wvif'
   572  while ((skb = skb_dequeue(&dropped)) != NULL) {
   573  hif = (struct hif_msg *)skb->data;
   574  wvif = wdev_to_wvif(wdev, hif->interface);
^
Same.
   575  ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
   576  wfx_skb_dtor(wvif, skb);
   577  }
   578  }

#5 and #6
drivers/staging/wfx/bus_spi.c:228 wfx_spi_probe() warn: 'bus->core' could be an 
error pointer
drivers/staging/wfx/bus_sdio.c:221 wfx_sdio_probe() warn: 'bus->core' could be 
an error pointer

The wfx_init_common() function should return NULL instead of error
pointer if devm_gpiod_get_optional() fails.

#7
drivers/staging/wfx/hif_rx.c:26 hif_generic_confirm() warn: negative user 
subtract: 0-u16max - 4
20  static int hif_generic_confirm(struct wfx_dev *wdev,
21 const struct hif_msg *hif, const void 
*buf)
22  {
23  // All confirm messages start with status
24  int status = le32_to_cpup((__le32 *)buf);
25  int cmd = hif->id;
26  int len = le16_to_cpu(hif->len) - 4; // drop header

Can "len" get set to negative 4?

27  
28  WARN(!mutex_is_locked(&wdev->hif_cmd.lock), "data locking 
error");


#8 and #9
drivers/staging/wfx/hif_rx.c:98 hif_wakeup_indication() warn: 
'gpiod_get_value(

Re: [PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread Coiby Xu 

On Thu, Oct 08, 2020 at 08:22:44AM -0400, Willem de Bruijn wrote:

On Thu, Oct 8, 2020 at 7:58 AM 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 



@@ -4556,6 +4559,13 @@ static int qlge_probe(struct pci_dev *pdev,
struct ql_adapter *qdev = NULL;
static int cards_found;
int err = 0;
+   struct devlink *devlink;
+   struct qlge_devlink *ql_devlink;
+
+   devlink = devlink_alloc(&qlge_devlink_ops, sizeof(struct qlge_devlink));
+   if (!devlink)
+   return -ENOMEM;
+   ql_devlink = devlink_priv(devlink);

ndev = alloc_etherdev_mq(sizeof(struct ql_adapter),
 min(MAX_CPUS,


need to goto devlink_free instead of return -ENOMEM here, too.


@@ -4614,6 +4624,16 @@ static int qlge_probe(struct pci_dev *pdev,
free_netdev(ndev);
return err;


and here


Thank you for reviewing this work and the speedy feedback! I'll fix it
in v2.

}
+
+   err = devlink_register(devlink, &pdev->dev);
+   if (err) {
+   goto devlink_free;
+   }
+
+   qlge_health_create_reporters(ql_devlink);
+   ql_devlink->qdev = qdev;
+   ql_devlink->ndev = ndev;
+   qdev->ql_devlink = ql_devlink;
/* Start up the timer to trigger EEH if
 * the bus goes dead
 */
@@ -4624,6 +4644,10 @@ static int qlge_probe(struct pci_dev *pdev,
atomic_set(&qdev->lb_count, 0);
cards_found++;
return 0;
+
+devlink_free:
+   devlink_free(devlink);
+   return err;
 }


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

Re: [PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread Willem de Bruijn 
On Thu, Oct 8, 2020 at 7:58 AM 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 

> @@ -4556,6 +4559,13 @@ static int qlge_probe(struct pci_dev *pdev,
> struct ql_adapter *qdev = NULL;
> static int cards_found;
> int err = 0;
> +   struct devlink *devlink;
> +   struct qlge_devlink *ql_devlink;
> +
> +   devlink = devlink_alloc(&qlge_devlink_ops, sizeof(struct 
> qlge_devlink));
> +   if (!devlink)
> +   return -ENOMEM;
> +   ql_devlink = devlink_priv(devlink);
>
> ndev = alloc_etherdev_mq(sizeof(struct ql_adapter),
>  min(MAX_CPUS,

need to goto devlink_free instead of return -ENOMEM here, too.

> @@ -4614,6 +4624,16 @@ static int qlge_probe(struct pci_dev *pdev,
> free_netdev(ndev);
> return err;

and here

> }
> +
> +   err = devlink_register(devlink, &pdev->dev);
> +   if (err) {
> +   goto devlink_free;
> +   }
> +
> +   qlge_health_create_reporters(ql_devlink);
> +   ql_devlink->qdev = qdev;
> +   ql_devlink->ndev = ndev;
> +   qdev->ql_devlink = ql_devlink;
> /* Start up the timer to trigger EEH if
>  * the bus goes dead
>  */
> @@ -4624,6 +4644,10 @@ static int qlge_probe(struct pci_dev *pdev,
> atomic_set(&qdev->lb_count, 0);
> cards_found++;
> return 0;
> +
> +devlink_free:
> +   devlink_free(devlink);
> +   return err;
>  }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v1 5/6] staging: qlge: clean up debugging code in the QL_ALL_DUMP ifdef land

2020-10-08 Thread Coiby Xu 
The debugging code in the following ifdef land
 - QL_ALL_DUMP
 - QL_REG_DUMP
 - QL_DEV_DUMP
 - QL_CB_DUMP
 - QL_IB_DUMP
 - QL_OB_DUMP

becomes unnecessary because,
 - Device status and general registers can be obtained by ethtool.
 - Coredump can be done via devlink health reporter.
 - Structure related to the hardware (struct ql_adapter) can be obtained
   by crash or drgn.

Suggested-by: Benjamin Poirier 
Signed-off-by: Coiby Xu 
---
 drivers/staging/qlge/qlge.h |  82 
 drivers/staging/qlge/qlge_dbg.c | 688 
 drivers/staging/qlge/qlge_ethtool.c |   2 -
 drivers/staging/qlge/qlge_main.c|   7 +-
 4 files changed, 1 insertion(+), 778 deletions(-)

diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index 0a39801be15a..8aff3ba77730 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2285,86 +2285,4 @@ void ql_check_lb_frame(struct ql_adapter *qdev, struct 
sk_buff *skb);
 int ql_own_firmware(struct ql_adapter *qdev);
 int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget);
 
-/* #define QL_ALL_DUMP */
-/* #define QL_REG_DUMP */
-/* #define QL_DEV_DUMP */
-/* #define QL_CB_DUMP */
-/* #define QL_IB_DUMP */
-/* #define QL_OB_DUMP */
-
-#ifdef QL_REG_DUMP
-void ql_dump_xgmac_control_regs(struct ql_adapter *qdev);
-void ql_dump_routing_entries(struct ql_adapter *qdev);
-void ql_dump_regs(struct ql_adapter *qdev);
-#define QL_DUMP_REGS(qdev) ql_dump_regs(qdev)
-#define QL_DUMP_ROUTE(qdev) ql_dump_routing_entries(qdev)
-#define QL_DUMP_XGMAC_CONTROL_REGS(qdev) ql_dump_xgmac_control_regs(qdev)
-#else
-#define QL_DUMP_REGS(qdev)
-#define QL_DUMP_ROUTE(qdev)
-#define QL_DUMP_XGMAC_CONTROL_REGS(qdev)
-#endif
-
-#ifdef QL_STAT_DUMP
-void ql_dump_stat(struct ql_adapter *qdev);
-#define QL_DUMP_STAT(qdev) ql_dump_stat(qdev)
-#else
-#define QL_DUMP_STAT(qdev)
-#endif
-
-#ifdef QL_DEV_DUMP
-void ql_dump_qdev(struct ql_adapter *qdev);
-#define QL_DUMP_QDEV(qdev) ql_dump_qdev(qdev)
-#else
-#define QL_DUMP_QDEV(qdev)
-#endif
-
-#ifdef QL_CB_DUMP
-void ql_dump_wqicb(struct wqicb *wqicb);
-void ql_dump_tx_ring(struct tx_ring *tx_ring);
-void ql_dump_ricb(struct ricb *ricb);
-void ql_dump_cqicb(struct cqicb *cqicb);
-void ql_dump_rx_ring(struct rx_ring *rx_ring);
-void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id);
-#define QL_DUMP_RICB(ricb) ql_dump_ricb(ricb)
-#define QL_DUMP_WQICB(wqicb) ql_dump_wqicb(wqicb)
-#define QL_DUMP_TX_RING(tx_ring) ql_dump_tx_ring(tx_ring)
-#define QL_DUMP_CQICB(cqicb) ql_dump_cqicb(cqicb)
-#define QL_DUMP_RX_RING(rx_ring) ql_dump_rx_ring(rx_ring)
-#define QL_DUMP_HW_CB(qdev, size, bit, q_id) \
-   ql_dump_hw_cb(qdev, size, bit, q_id)
-#else
-#define QL_DUMP_RICB(ricb)
-#define QL_DUMP_WQICB(wqicb)
-#define QL_DUMP_TX_RING(tx_ring)
-#define QL_DUMP_CQICB(cqicb)
-#define QL_DUMP_RX_RING(rx_ring)
-#define QL_DUMP_HW_CB(qdev, size, bit, q_id)
-#endif
-
-#ifdef QL_OB_DUMP
-void ql_dump_tx_desc(struct ql_adapter *qdev, struct tx_buf_desc *tbd);
-void ql_dump_ob_mac_iocb(struct ql_adapter *qdev, struct ob_mac_iocb_req 
*ob_mac_iocb);
-void ql_dump_ob_mac_rsp(struct ql_adapter *qdev, struct ob_mac_iocb_rsp 
*ob_mac_rsp);
-#define QL_DUMP_OB_MAC_IOCB(qdev, ob_mac_iocb) ql_dump_ob_mac_iocb(qdev, 
ob_mac_iocb)
-#define QL_DUMP_OB_MAC_RSP(qdev, ob_mac_rsp) ql_dump_ob_mac_rsp(qdev, 
ob_mac_rsp)
-#else
-#define QL_DUMP_OB_MAC_IOCB(qdev, ob_mac_iocb)
-#define QL_DUMP_OB_MAC_RSP(qdev, ob_mac_rsp)
-#endif
-
-#ifdef QL_IB_DUMP
-void ql_dump_ib_mac_rsp(struct ql_adapter *qdev, struct ib_mac_iocb_rsp 
*ib_mac_rsp);
-#define QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp) ql_dump_ib_mac_rsp(qdev, 
ib_mac_rsp)
-#else
-#define QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp)
-#endif
-
-#ifdef QL_ALL_DUMP
-void ql_dump_all(struct ql_adapter *qdev);
-#define QL_DUMP_ALL(qdev) ql_dump_all(qdev)
-#else
-#define QL_DUMP_ALL(qdev)
-#endif
-
 #endif /* _QLGE_H_ */
diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 989575743718..a02ecf8fb291 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1313,691 +1313,3 @@ void ql_get_dump(struct ql_adapter *qdev, void *buff)
ql_get_core_dump(qdev);
}
 }
-
-#ifdef QL_REG_DUMP
-static void ql_dump_intr_states(struct ql_adapter *qdev)
-{
-   int i;
-   u32 value;
-
-   for (i = 0; i < qdev->intr_count; i++) {
-   ql_write32(qdev, INTR_EN, qdev->intr_context[i].intr_read_mask);
-   value = ql_read32(qdev, INTR_EN);
-   netdev_err(qdev->ndev, "Interrupt %d is %s\n", i,
-  (value & INTR_EN_EN ? "enabled" : "disabled"));
-   }
-}
-
-#define DUMP_XGMAC(qdev, reg)  \
-do {   \
-   u32 data;   \
-   ql_read_xgmac_reg(qdev, reg, &data);\
-   netdev_err(qdev->ndev,

[PATCH v1 2/6] staging: qlge: coredump via devlink health reporter

2020-10-08 Thread Coiby Xu 
$ devlink health dump show DEVICE reporter coredump -p -j
{
"Core Registers": {
"segment": 1,
"values": [ 
0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
 ]
},
"Test Logic Regs": {
"segment": 2,
"values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
},
"RMII Registers": {
"segment": 3,
"values": [ 
0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
 ]
},
...
"Sem Registers": {
"segment": 50,
"values": [ 0,0,0,0 ]
}
}

Signed-off-by: Coiby Xu 
---
 drivers/staging/qlge/qlge_devlink.c | 131 ++--
 1 file changed, 125 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/qlge/qlge_devlink.c 
b/drivers/staging/qlge/qlge_devlink.c
index aa45e7e368c0..91b6600b94a9 100644
--- a/drivers/staging/qlge/qlge_devlink.c
+++ b/drivers/staging/qlge/qlge_devlink.c
@@ -1,16 +1,135 @@
 #include "qlge.h"
 #include "qlge_devlink.h"
 
-static int
-qlge_reporter_coredump(struct devlink_health_reporter *reporter,
-   struct devlink_fmsg *fmsg, void *priv_ctx,
-   struct netlink_ext_ack *extack)
+static int fill_seg_(struct devlink_fmsg *fmsg,
+   struct mpi_coredump_segment_header *seg_header,
+   u32 *reg_data)
 {
-   return 0;
+   int i;
+   int header_size = sizeof(struct mpi_coredump_segment_header);
+   int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
+   int err;
+
+   err = devlink_fmsg_pair_nest_start(fmsg, seg_header->description);
+   if (err)
+   return err;
+   err = devlink_fmsg_obj_nest_start(fmsg);
+   if (err)
+   return err;
+   err = devlink_fmsg_u32_pair_put(fmsg, "segment", seg_header->seg_num);
+   if (err)
+   return err;
+   err = devlink_fmsg_arr_pair_nest_start(fmsg, "values");
+   if (err)
+   return err;
+   for (i = 0; i < regs_num; i++) {
+   err = devlink_fmsg_u32_put(fmsg, *reg_data);
+   if (err)
+   return err;
+   reg_data++;
+   }
+   err = devlink_fmsg_obj_nest_end(fmsg);
+   if (err)
+   return err;
+   err = devlink_fmsg_arr_pair_nest_end(fmsg);
+   if (err)
+   return err;
+   err = devlink_fmsg_pair_nest_end(fmsg);
+   return err;
+}
+
+#define fill_seg(seg_hdr, seg_regs)   \
+   do {   \
+   err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
+   if (err) { \
+   kvfree(dump);  \
+   return err;\
+   }  \
+   } while (0)
+
+static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *priv_ctx,
+ struct netlink_ext_ack *extack)
+{
+   int err = 0;
+
+   struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
+   struct ql_adapter *qdev = dev->qdev;
+   struct ql_mpi_coredump *dump;
+
+   if (!netif_running(qdev->ndev))
+   return 0;
+
+   dump = kvmalloc(sizeof(*dump), GFP_KERNEL);
+   if (!dump)
+   return -ENOMEM;
+
+   err = ql_core_dump(qdev, dump);
+   if (err) {
+   kvfree(dump);
+   return err;
+   }
+
+   fill_seg(core_regs_seg_hdr, mpi_core_regs);
+   fill_seg(test_logic_regs_seg_hdr, test_logic_regs);
+   fill_seg(rmii_regs_seg_hdr, rmii_regs);
+   fill_seg(fcmac1_regs_seg_hdr, fcmac1_regs);
+   fill_seg(fcmac2_regs_seg_hdr, fcmac2_regs);
+   fill_seg(fc1_mbx_regs_seg_hdr, fc1_mbx_regs);
+   fill_seg(ide_regs_seg_hdr, ide_regs);
+   fill_seg(nic1_mbx_regs_seg_hdr, nic1_mbx_regs);
+   fill_seg(smbus_regs_seg_hdr, smbus_regs);
+   fill_seg(fc2_mbx_regs_seg_hdr, fc2_mbx_regs);
+   fill_seg(nic2_mbx_regs_seg_hdr, nic2_mbx_regs);
+   fill_seg(i2c_regs_seg_hdr, i2c_regs);
+   fill_seg(memc_regs_seg_hdr, memc_regs);
+   fill_seg(pbus_regs_seg_hdr, pbus_regs);
+   fill_seg(mde_regs_seg_hdr, mde_regs);
+   fill_seg(nic_regs_seg_hdr, nic_regs);
+   fill_seg(nic2_regs_seg_hdr, nic2_regs);
+   fill_seg(xgmac1_seg_hdr, xgmac1);
+   fill_seg(xgmac2_seg_hdr, xgmac2

[PATCH v1 6/6] staging: qlge: add documentation for debugging qlge

2020-10-08 Thread Coiby Xu 
Instructions and examples on kernel data structures dumping and coredump.

Signed-off-by: Coiby Xu 
---
 .../networking/device_drivers/index.rst   |   1 +
 .../device_drivers/qlogic/index.rst   |  18 +++
 .../networking/device_drivers/qlogic/qlge.rst | 118 ++
 MAINTAINERS   |   6 +
 4 files changed, 143 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/qlogic/index.rst
 create mode 100644 Documentation/networking/device_drivers/qlogic/qlge.rst

diff --git a/Documentation/networking/device_drivers/index.rst 
b/Documentation/networking/device_drivers/index.rst
index a3113ffd7a16..d8279de7bf25 100644
--- a/Documentation/networking/device_drivers/index.rst
+++ b/Documentation/networking/device_drivers/index.rst
@@ -15,6 +15,7 @@ Contents:
ethernet/index
fddi/index
hamradio/index
+   qlogic/index
wan/index
wifi/index
 
diff --git a/Documentation/networking/device_drivers/qlogic/index.rst 
b/Documentation/networking/device_drivers/qlogic/index.rst
new file mode 100644
index ..ad05b04286e4
--- /dev/null
+++ b/Documentation/networking/device_drivers/qlogic/index.rst
@@ -0,0 +1,18 @@
+.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+QLogic QLGE Device Drivers
+===
+
+Contents:
+
+.. toctree::
+   :maxdepth: 2
+
+   qlge
+
+.. only::  subproject and html
+
+   Indices
+   ===
+
+   * :ref:`genindex`
diff --git a/Documentation/networking/device_drivers/qlogic/qlge.rst 
b/Documentation/networking/device_drivers/qlogic/qlge.rst
new file mode 100644
index ..0b888253d152
--- /dev/null
+++ b/Documentation/networking/device_drivers/qlogic/qlge.rst
@@ -0,0 +1,118 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===
+QLogic QLGE 10Gb Ethernet device driver
+===
+
+This driver use drgn and devlink for debugging.
+
+Dump kernel data structures in drgn
+---
+
+To dump kernel data structures, the following Python script can be used
+in drgn:
+
+.. code-block:: python
+
+   def align(x, a):
+   """the alignment a should be a power of 2
+   """
+   mask = a - 1
+   return (x+ mask) & ~mask
+
+   def struct_size(struct_type):
+   struct_str = "struct {}".format(struct_type)
+   return sizeof(Object(prog, struct_str, address=0x0))
+
+   def netdev_priv(netdevice):
+   NETDEV_ALIGN = 32
+   return netdevice.value_() + align(struct_size("net_device"), 
NETDEV_ALIGN)
+
+   name = 'xxx'
+   qlge_device = None
+   netdevices = prog['init_net'].dev_base_head.address_of_()
+   for netdevice in list_for_each_entry("struct net_device", netdevices, 
"dev_list"):
+   if netdevice.name.string_().decode('ascii') == name:
+   print(netdevice.name)
+
+   ql_adapter = Object(prog, "struct ql_adapter", 
address=netdev_priv(qlge_device))
+
+The struct ql_adapter will be printed in drgn as follows,
+
+>>> ql_adapter
+(struct ql_adapter){
+.ricb = (struct ricb){
+.base_cq = (u8)0,
+.flags = (u8)120,
+.mask = (__le16)26637,
+.hash_cq_id = (u8 [1024]){ 172, 142, 255, 255 },
+.ipv6_hash_key = (__le32 [10]){},
+.ipv4_hash_key = (__le32 [4]){},
+},
+.flags = (unsigned long)0,
+.wol = (u32)0,
+.nic_stats = (struct nic_stats){
+.tx_pkts = (u64)0,
+.tx_bytes = (u64)0,
+.tx_mcast_pkts = (u64)0,
+.tx_bcast_pkts = (u64)0,
+.tx_ucast_pkts = (u64)0,
+.tx_ctl_pkts = (u64)0,
+.tx_pause_pkts = (u64)0,
+...
+},
+.active_vlans = (unsigned long [64]){
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 52780853100545, 18446744073709551615,
+18446619461681283072, 0, 42949673024, 2147483647,
+},
+.rx_ring = (struct rx_ring [17]){
+{
+.cqicb = (struct cqicb){
+.msix_vect = (u8)0,
+.reserved1 = (u8)0,
+.reserved2 = (u8)0,
+.flags = (u8)0,
+.len = (__le16)0,
+.rid = (__le16)0,
+...
+},
+.cq_base = (void *)0x0,
+.cq_base_dma = (dma_addr_t)0,
+}
+

[PATCH v1 1/6] staging: qlge: Initialize devlink health dump framework for the dlge driver

2020-10-08 Thread Coiby Xu 
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/Kconfig|  1 +
 drivers/staging/qlge/Makefile   |  2 +-
 drivers/staging/qlge/qlge.h |  9 +++
 drivers/staging/qlge/qlge_devlink.c | 38 +
 drivers/staging/qlge/qlge_devlink.h |  8 ++
 drivers/staging/qlge/qlge_main.c| 28 +
 6 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/qlge/qlge_devlink.c
 create mode 100644 drivers/staging/qlge/qlge_devlink.h

diff --git a/drivers/staging/qlge/Kconfig b/drivers/staging/qlge/Kconfig
index a3cb25a3ab80..6d831ed67965 100644
--- a/drivers/staging/qlge/Kconfig
+++ b/drivers/staging/qlge/Kconfig
@@ -3,6 +3,7 @@
 config QLGE
tristate "QLogic QLGE 10Gb Ethernet Driver Support"
depends on ETHERNET && PCI
+   select NET_DEVLINK
help
This driver supports QLogic ISP8XXX 10Gb Ethernet cards.
 
diff --git a/drivers/staging/qlge/Makefile b/drivers/staging/qlge/Makefile
index 1dc2568e820c..07c1898a512e 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_devlink.o
diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index b295990e361b..290e754450c5 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2060,6 +2060,14 @@ struct nic_operations {
int (*port_initialize)(struct ql_adapter *qdev);
 };
 
+
+
+struct qlge_devlink {
+struct ql_adapter *qdev;
+struct net_device *ndev;
+struct devlink_health_reporter *reporter;
+};
+
 /*
  * The main Adapter structure definition.
  * This structure has all fields relevant to the hardware.
@@ -2077,6 +2085,7 @@ struct ql_adapter {
struct pci_dev *pdev;
struct net_device *ndev;/* Parent NET device */
 
+   struct qlge_devlink *ql_devlink;
/* Hardware information */
u32 chip_rev_id;
u32 fw_rev_id;
diff --git a/drivers/staging/qlge/qlge_devlink.c 
b/drivers/staging/qlge/qlge_devlink.c
new file mode 100644
index ..aa45e7e368c0
--- /dev/null
+++ b/drivers/staging/qlge/qlge_devlink.c
@@ -0,0 +1,38 @@
+#include "qlge.h"
+#include "qlge_devlink.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,
+};
+
+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;
+
+   return err;
+}
diff --git a/drivers/staging/qlge/qlge_devlink.h 
b/drivers/staging/qlge/qlge_devlink.h
new file mode 100644
index ..c91f7a29e805
--- /dev/null
+++ b/drivers/staging/qlge/qlge_devlink.h
@@ -0,0 +1,8 @@
+#ifndef QLGE_DEVLINK_H
+#define QLGE_DEVLINK_H
+
+#include 
+
+int qlge_health_create_reporters(struct qlge_devlink *priv);
+
+#endif /* QLGE_DEVLINK_H */
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 27da386f9d87..135225530e51 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -42,6 +42,7 @@
 #include 
 
 #include "qlge.h"
+#include "qlge_devlink.h"
 
 char qlge_driver_name[] = DRV_NAME;
 const char qlge_driver_version[] = DRV_VERSION;
@@ -4549,6 +4550,8 @@ static void ql_timer(struct timer_list *t)
mod_timer(&qdev->timer, jiffies + (5 * HZ));
 }
 
+static const struct devlink_ops qlge_devlink_ops;
+
 static int qlge_probe(struct pci_dev *pdev,
  const struct pci_device_id *pci_entry)
 {
@@ -4556,6 +4559,13 @@ static int qlge_probe(struct pci_dev *pdev,
struct ql_adapter *qdev = NULL;
static int cards_found;
int err = 0;
+   struct devlink *devlink;
+   struct qlge_devlink *ql_devlink;
+
+   devlink = devlink_alloc(&qlge_devlink_ops, sizeof(struct qlge_devlink));
+   if (!devlink)
+   return -ENOMEM;
+   ql_devlink = devlink

[PATCH v1 0/6] staging: qlge: Re-writing the debugging features

2020-10-08 Thread Coiby Xu 
This patch set aims to avoid dumping registers, data structures and
coredump to dmesg and also to reduce the code size of the qlge driver.

As pointed out by Benjamin [1],

> At 2000 lines, qlge_dbg.c alone is larger than some entire ethernet
> drivers. Most of what it does is dump kernel data structures or pci
> memory mapped registers to dmesg. There are better facilities for that.
> My thinking is not simply to delete qlge_dbg.c but to replace it, making
> sure that most of the same information is still available. For data
> structures, crash or drgn can be used; possibly with a script for the
> latter which formats the data. For pci registers, they should be
> included in the ethtool register dump and a patch added to ethtool to
> pretty print them. That's what other drivers like e1000e do. For the
> "coredump", devlink health can be used.

So the debugging features are re-written following Benjamin's advice,
   - dump kernel data structures in drgn
   - use devlink to do coredump which also includes device status and
 general registers

[1] https://lkml.org/lkml/2020/6/30/19

Change since RFC:
 - select NET_DEVLINK in Kconfig [Benjamin Poirier]
 - Don't do a coredump when the interface is down [Shung-Hsi Yu]
 - Remove stray newlines [Benjamin Poirier]
 - force_coredump for devlink
 - Remove mpi_core_to_log which will output the coredump to the kernel
   ring buffer
 - Put drgn script under Documentation [Benjamin Poirier]
 - Rename qlge_health.* to qlge_devlink.*

Coiby Xu (6):
  staging: qlge: Initialize devlink health dump framework for the dlge
driver
  staging: qlge: coredump via devlink health reporter
  staging: qlge: support force_coredump option for devlink health dump
  staging: qlge: remove mpi_core_to_log which sends coredump to the
kernel ring buffer
  staging: qlge: clean up debugging code in the QL_ALL_DUMP ifdef land
  staging: qlge: add documentation for debugging qlge

 .../networking/device_drivers/index.rst   |   1 +
 .../device_drivers/qlogic/index.rst   |  18 +
 .../networking/device_drivers/qlogic/qlge.rst | 118 +++
 MAINTAINERS   |   6 +
 drivers/staging/qlge/Kconfig  |   1 +
 drivers/staging/qlge/Makefile |   2 +-
 drivers/staging/qlge/qlge.h   |  94 +--
 drivers/staging/qlge/qlge_dbg.c   | 699 --
 drivers/staging/qlge/qlge_devlink.c   | 164 
 drivers/staging/qlge/qlge_devlink.h   |   8 +
 drivers/staging/qlge/qlge_ethtool.c   |   3 -
 drivers/staging/qlge/qlge_main.c  |  37 +-
 drivers/staging/qlge/qlge_mpi.c   |   6 -
 13 files changed, 355 insertions(+), 802 deletions(-)
 create mode 100644 Documentation/networking/device_drivers/qlogic/index.rst
 create mode 100644 Documentation/networking/device_drivers/qlogic/qlge.rst
 create mode 100644 drivers/staging/qlge/qlge_devlink.c
 create mode 100644 drivers/staging/qlge/qlge_devlink.h

--
2.28.0

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

[PATCH v1 3/6] staging: qlge: support force_coredump option for devlink health dump

2020-10-08 Thread Coiby Xu 
With force_coredump module paramter set, devlink health dump will reset
the MPI RISC first which takes 5 secs to be finished.

Signed-off-by: Coiby Xu 
---
 drivers/staging/qlge/qlge_devlink.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/qlge/qlge_devlink.c 
b/drivers/staging/qlge/qlge_devlink.c
index 91b6600b94a9..54257468bc7f 100644
--- a/drivers/staging/qlge/qlge_devlink.c
+++ b/drivers/staging/qlge/qlge_devlink.c
@@ -56,10 +56,17 @@ static int qlge_reporter_coredump(struct 
devlink_health_reporter *reporter,
struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
struct ql_adapter *qdev = dev->qdev;
struct ql_mpi_coredump *dump;
+   wait_queue_head_t wait;
 
if (!netif_running(qdev->ndev))
return 0;
 
+   if (test_bit(QL_FRC_COREDUMP, &qdev->flags)) {
+   ql_queue_fw_error(qdev);
+   init_waitqueue_head(&wait);
+   wait_event_timeout(wait, 0, 5 * HZ);
+   }
+
dump = kvmalloc(sizeof(*dump), GFP_KERNEL);
if (!dump)
return -ENOMEM;
-- 
2.28.0

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

[PATCH v1 4/6] staging: qlge: remove mpi_core_to_log which sends coredump to the kernel ring buffer

2020-10-08 Thread Coiby Xu 
devlink health could be used to get coredump. No need to send so much
data to the kernel ring buffer.

Signed-off-by: Coiby Xu 
---
 drivers/staging/qlge/qlge.h |  3 ---
 drivers/staging/qlge/qlge_dbg.c | 11 ---
 drivers/staging/qlge/qlge_ethtool.c |  1 -
 drivers/staging/qlge/qlge_main.c|  2 --
 drivers/staging/qlge/qlge_mpi.c |  6 --
 5 files changed, 23 deletions(-)

diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index 290e754450c5..0a39801be15a 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2149,7 +2149,6 @@ struct ql_adapter {
u32 port_init;
u32 link_status;
struct ql_mpi_coredump *mpi_coredump;
-   u32 core_is_dumped;
u32 link_config;
u32 led_config;
u32 max_frame_size;
@@ -2162,7 +2161,6 @@ struct ql_adapter {
struct delayed_work mpi_work;
struct delayed_work mpi_port_cfg_work;
struct delayed_work mpi_idc_work;
-   struct delayed_work mpi_core_to_log;
struct completion ide_completion;
const struct nic_operations *nic_ops;
u16 device_id;
@@ -2253,7 +2251,6 @@ int ql_write_cfg(struct ql_adapter *qdev, void *ptr, int 
size, u32 bit,
 void ql_queue_fw_error(struct ql_adapter *qdev);
 void ql_mpi_work(struct work_struct *work);
 void ql_mpi_reset_work(struct work_struct *work);
-void ql_mpi_core_to_log(struct work_struct *work);
 int ql_wait_reg_rdy(struct ql_adapter *qdev, u32 reg, u32 bit, u32 ebit);
 void ql_queue_asic_error(struct ql_adapter *qdev);
 void ql_set_ethtool_ops(struct net_device *ndev);
diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 42fd13990f3a..989575743718 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1314,17 +1314,6 @@ void ql_get_dump(struct ql_adapter *qdev, void *buff)
}
 }
 
-/* Coredump to messages log file using separate worker thread */
-void ql_mpi_core_to_log(struct work_struct *work)
-{
-   struct ql_adapter *qdev =
-   container_of(work, struct ql_adapter, mpi_core_to_log.work);
-
-   print_hex_dump(KERN_DEBUG, "Core is dumping to log file!\n",
-  DUMP_PREFIX_OFFSET, 32, 4, qdev->mpi_coredump,
-  sizeof(*qdev->mpi_coredump), false);
-}
-
 #ifdef QL_REG_DUMP
 static void ql_dump_intr_states(struct ql_adapter *qdev)
 {
diff --git a/drivers/staging/qlge/qlge_ethtool.c 
b/drivers/staging/qlge/qlge_ethtool.c
index d44b2dae9213..eed116d8895e 100644
--- a/drivers/staging/qlge/qlge_ethtool.c
+++ b/drivers/staging/qlge/qlge_ethtool.c
@@ -616,7 +616,6 @@ static void ql_get_regs(struct net_device *ndev,
struct ql_adapter *qdev = netdev_priv(ndev);
 
ql_get_dump(qdev, p);
-   qdev->core_is_dumped = 0;
if (!test_bit(QL_FRC_COREDUMP, &qdev->flags))
regs->len = sizeof(struct ql_mpi_coredump);
else
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 135225530e51..aaca740d46c4 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -3808,7 +3808,6 @@ static void ql_cancel_all_work_sync(struct ql_adapter 
*qdev)
cancel_delayed_work_sync(&qdev->mpi_reset_work);
cancel_delayed_work_sync(&qdev->mpi_work);
cancel_delayed_work_sync(&qdev->mpi_idc_work);
-   cancel_delayed_work_sync(&qdev->mpi_core_to_log);
cancel_delayed_work_sync(&qdev->mpi_port_cfg_work);
 }
 
@@ -4504,7 +4503,6 @@ static int ql_init_device(struct pci_dev *pdev, struct 
net_device *ndev,
INIT_DELAYED_WORK(&qdev->mpi_work, ql_mpi_work);
INIT_DELAYED_WORK(&qdev->mpi_port_cfg_work, ql_mpi_port_cfg_work);
INIT_DELAYED_WORK(&qdev->mpi_idc_work, ql_mpi_idc_work);
-   INIT_DELAYED_WORK(&qdev->mpi_core_to_log, ql_mpi_core_to_log);
init_completion(&qdev->ide_completion);
mutex_init(&qdev->mpi_mutex);
 
diff --git a/drivers/staging/qlge/qlge_mpi.c b/drivers/staging/qlge/qlge_mpi.c
index 143a886080c5..1cea24201b17 100644
--- a/drivers/staging/qlge/qlge_mpi.c
+++ b/drivers/staging/qlge/qlge_mpi.c
@@ -1269,11 +1269,5 @@ void ql_mpi_reset_work(struct work_struct *work)
return;
}
 
-   if (qdev->mpi_coredump && !ql_core_dump(qdev, qdev->mpi_coredump)) {
-   netif_err(qdev, drv, qdev->ndev, "Core is dumped!\n");
-   qdev->core_is_dumped = 1;
-   queue_delayed_work(qdev->workqueue,
-  &qdev->mpi_core_to_log, 5 * HZ);
-   }
ql_soft_reset_mpi_risc(qdev);
 }
-- 
2.28.0

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

Re: [PATCH 0/7] wfx: move out from the staging area

2020-10-08 Thread Kalle Valo 
Jérôme Pouiller  writes:

> On Thursday 8 October 2020 09:30:06 CEST Kalle Valo wrote:
> [...]
>> Yes, the driver needs to be reviewed in linux-wireless list. I recommend
>> submitting the whole driver in a patchset with one file per patch, which
>> seems to be the easiest way to review a full driver. The final move will
>> be in just one commit moving the driver, just like patch 7 does here. As
>> an example see how wilc1000 review was done.
>
> I see. I suppose it is still a bit complicated to review? Maybe I could
> try to make things easier.
>
> For my submission to staging/ I had taken time to split the driver in an
> understandable series of patches[1]. I think it was easier to review than
> just sending files one by one. I could do the same thing for the
> submission to linux-wireless. It would ask me a bit of work but, since I
> already have a template, it is conceivable.
>
> Do you think it is worth it, or it would be an unnecessary effort?
>
> [1]
> https://lore.kernel.org/driverdev-devel/20190919142527.31797-1-jerome.pouil...@silabs.com/
>  or commits a7a91ca5a23d^..40115bbc40e2

I don't know how others think, but I prefer to review new drivers "one
file per patch" style as I get to see the big picture easily. And
besides, splitting the driver like that would be a huge job for you. I
don't think it's worth your time in this case. And making changes in the
driver during review process becomes even more complex.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/7] wfx: move out from the staging area

2020-10-08 Thread Jérôme Pouiller 
On Thursday 8 October 2020 09:30:06 CEST Kalle Valo wrote:
[...]
> Yes, the driver needs to be reviewed in linux-wireless list. I recommend
> submitting the whole driver in a patchset with one file per patch, which
> seems to be the easiest way to review a full driver. The final move will
> be in just one commit moving the driver, just like patch 7 does here. As
> an example see how wilc1000 review was done.

I see. I suppose it is still a bit complicated to review? Maybe I could
try to make things easier.

For my submission to staging/ I had taken time to split the driver in an
understandable series of patches[1]. I think it was easier to review than
just sending files one by one. I could do the same thing for the
submission to linux-wireless. It would ask me a bit of work but, since I
already have a template, it is conceivable.

Do you think it is worth it, or it would be an unnecessary effort?

[1] 
https://lore.kernel.org/driverdev-devel/20190919142527.31797-1-jerome.pouil...@silabs.com/
 or commits a7a91ca5a23d^..40115bbc40e2

-- 
Jérôme Pouiller


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

Re: [PATCH 0/7] wfx: move out from the staging area

2020-10-08 Thread Kalle Valo 
Kalle Valo  writes:

> Greg Kroah-Hartman  writes:
>
>> On Wed, Oct 07, 2020 at 12:19:36PM +0200, Jerome Pouiller wrote:
>>> From: Jérôme Pouiller 
>>> 
>>> I think the wfx driver is now mature enough to be accepted in the
>>> drivers/net/wireless directory.
>>> 
>>> There is still one item on the TODO list. It is an idea to improve the rate
>>> control in some particular cases[1]. However, the current performances of 
>>> the
>>> driver seem to satisfy everyone. In add, the suggested change is large 
>>> enough.
>>> So, I would prefer to implement it only if it really solves an issue. I 
>>> think it
>>> is not an obstacle to move the driver out of the staging area.
>>> 
>>> In order to comply with the last rules for the DT bindings, I have 
>>> converted the
>>> documentation to yaml. I am moderately happy with the result. Especially, 
>>> for
>>> the description of the binding. Any comments are welcome.
>>> 
>>> The series also update the copyrights dates of the files. I don't know 
>>> exactly
>>> how this kind of changes should be sent. It's a bit weird to change all the
>>> copyrights in one commit, but I do not see any better way.
>>> 
>>> I also include a few fixes I have found these last weeks.
>>> 
>>> [1] https://lore.kernel.org/lkml/3099559.gv3Q75KnN1@pc-42
>>
>> I'll take the first 6 patches here, the last one you should work with
>> the wireless maintainers to get reviewed.
>>
>> Maybe that might want to wait until after 5.10-rc1 is out, with all of
>> these changes in it, making it an easier move.
>
> Yes, the driver needs to be reviewed in linux-wireless list. I recommend
> submitting the whole driver in a patchset with one file per patch, which
> seems to be the easiest way to review a full driver. The final move will
> be in just one commit moving the driver, just like patch 7 does here. As
> an example see how wilc1000 review was done.
>
> Device tree bindings needs to be reviewed by the DT maintainer so CC
> devicetree on that patch.

BTW, I wrote some instructions for new wireless drivers:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#new_driver

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: vchiq: Fix list_for_each exit tests

2020-10-08 Thread Nicolas Saenz Julienne 
On Tue, 2020-10-06 at 16:47 +0300, Dan Carpenter wrote:
> After a list_for_each_entry() loop, the list iterator is always non-NULL
> so these conditions don't work.  If the "waiter" is not found then this
> results in an out of bounds access.
> 
> I have fixed it by introducing a new "found" variable.  In one case, I
> used an else statement for readability.
> 
> Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when 
> accessing bulk_waiter_list")
> Signed-off-by: Dan Carpenter 
> ---

Reviewed-by: Nicolas Saenz Julienne 



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/7] wfx: move out from the staging area

2020-10-08 Thread Kalle Valo 
Greg Kroah-Hartman  writes:

> On Wed, Oct 07, 2020 at 12:19:36PM +0200, Jerome Pouiller wrote:
>> From: Jérôme Pouiller 
>> 
>> I think the wfx driver is now mature enough to be accepted in the
>> drivers/net/wireless directory.
>> 
>> There is still one item on the TODO list. It is an idea to improve the rate
>> control in some particular cases[1]. However, the current performances of the
>> driver seem to satisfy everyone. In add, the suggested change is large 
>> enough.
>> So, I would prefer to implement it only if it really solves an issue. I 
>> think it
>> is not an obstacle to move the driver out of the staging area.
>> 
>> In order to comply with the last rules for the DT bindings, I have converted 
>> the
>> documentation to yaml. I am moderately happy with the result. Especially, for
>> the description of the binding. Any comments are welcome.
>> 
>> The series also update the copyrights dates of the files. I don't know 
>> exactly
>> how this kind of changes should be sent. It's a bit weird to change all the
>> copyrights in one commit, but I do not see any better way.
>> 
>> I also include a few fixes I have found these last weeks.
>> 
>> [1] https://lore.kernel.org/lkml/3099559.gv3Q75KnN1@pc-42
>
> I'll take the first 6 patches here, the last one you should work with
> the wireless maintainers to get reviewed.
>
> Maybe that might want to wait until after 5.10-rc1 is out, with all of
> these changes in it, making it an easier move.

Yes, the driver needs to be reviewed in linux-wireless list. I recommend
submitting the whole driver in a patchset with one file per patch, which
seems to be the easiest way to review a full driver. The final move will
be in just one commit moving the driver, just like patch 7 does here. As
an example see how wilc1000 review was done.

Device tree bindings needs to be reviewed by the DT maintainer so CC
devicetree on that patch.

But do note that there's currently one new driver in review queue, so it
will most likely take some time before wfx is reviewed.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel