mbox series

[v2,00/24] Set 3: Fix another set of SCSI related W=1 warnings

Message ID 20200713080001.128044-1-lee.jones@linaro.org
Headers show
Series Set 3: Fix another set of SCSI related W=1 warnings | expand

Message

Lee Jones July 13, 2020, 7:59 a.m. UTC 
This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

Slowly working through the SCSI related ones.  There are many.

This brings the total of W=1 SCSI wanings from 1690 in v5.8-rc1 to 1109.

Changelog:

v1 => v2
 - Collected *-bys
 - Removed inert function-calls when removing unused variables
   - As suggested by James Bottomley
 
Lee Jones (24):
  scsi: aacraid: aachba: Repair two kerneldoc headers
  scsi: aacraid: commctrl: Fix a few kerneldoc issues
  scsi: aacraid: dpcsup: Fix logical bug when !DBG
  scsi: aacraid: dpcsup: Remove unused variable 'status'
  scsi: aacraid: dpcsup: Demote partially documented function header
  scsi: aic94xx: aic94xx_seq: Document 'lseq' and repair
    asd_update_port_links() header
  scsi: aacraid: commsup: Fix a bunch of function header issues
  scsi: aic94xx: aic94xx_scb: Fix a couple of formatting and bitrot
    issues
  scsi: aacraid: rx: Fill in the very parameter descriptions for
    rx_sync_cmd()
  scsi: pm8001: pm8001_ctl: Provide descriptions for the many
    undocumented 'attr's
  scsi: ipr: Fix a mountain of kerneldoc misdemeanours
  scsi: virtio_scsi: Demote seemingly unintentional kerneldoc header
  scsi: ipr: Remove a bunch of set but checked variables
  scsi: ipr: Fix struct packed-not-aligned issues
  scsi: myrs: Demote obvious misuse of kerneldoc to standard comment
    blocks
  scsi: megaraid: Fix a whole bunch of function header formatting issues
  scsi: be2iscsi: be_iscsi: Fix API/documentation slip
  scsi: be2iscsi: be_main: Fix misdocumentation of 'pcontext'
  scsi: be2iscsi: be_mgmt: Add missing function parameter description
  scsi: lpfc: lpfc_nvme: Correct some pretty obvious misdocumentation
  scsi: aic7xxx: aic79xx_osm: Remove unused variable 'ahd'
  scsi: aic7xxx: aic79xx_osm: Remove unused variables 'wait' and
    'paused'
  scsi: aic7xxx: aic79xx_osm: Fix 'amount_xferred' set but not used
    issue
  scsi: aic7xxx: aic79xx_osm: Remove set but unused variabes
    'saved_scsiid' and 'saved_modes'

 drivers/scsi/aacraid/aachba.c      |   5 +-
 drivers/scsi/aacraid/commctrl.c    |  14 +-
 drivers/scsi/aacraid/commsup.c     |  12 +-
 drivers/scsi/aacraid/dpcsup.c      |  15 +-
 drivers/scsi/aacraid/rx.c          |  12 +-
 drivers/scsi/aic7xxx/aic79xx_osm.c |  14 +-
 drivers/scsi/aic94xx/aic94xx_scb.c |   6 +-
 drivers/scsi/aic94xx/aic94xx_seq.c |   6 +-
 drivers/scsi/be2iscsi/be_iscsi.c   |  11 +-
 drivers/scsi/be2iscsi/be_main.c    |   4 +-
 drivers/scsi/be2iscsi/be_mgmt.c    |   3 +-
 drivers/scsi/ipr.c                 |  90 +++++++-----
 drivers/scsi/ipr.h                 |   4 +-
 drivers/scsi/lpfc/lpfc_nvme.c      |  38 +++--
 drivers/scsi/megaraid.c            | 218 ++++++++++++++---------------
 drivers/scsi/myrs.c                |  34 ++---
 drivers/scsi/pm8001/pm8001_ctl.c   |  14 ++
 drivers/scsi/virtio_scsi.c         |   2 +-
 18 files changed, 273 insertions(+), 229 deletions(-)

-- 
2.25.1

Comments

Martin K. Petersen July 15, 2020, 10:14 p.m. UTC | #1 
On Mon, 13 Jul 2020 08:59:37 +0100, Lee Jones wrote:

> This set is part of a larger effort attempting to clean-up W=1

> kernel builds, which are currently overwhelmingly riddled with

> niggly little warnings.

> 

> Slowly working through the SCSI related ones.  There are many.

> 

> This brings the total of W=1 SCSI wanings from 1690 in v5.8-rc1 to 1109.

> 

> [...]


Applied to 5.9/scsi-queue. Thanks for cleaning things up!

I do think that in general it makes little sense for low-level drivers
to document internal functions using kerneldoc. As a general approach
I would prefer to just switch drivers to '/*' or to remove stale
comments instead of trying to keep up with the "docrot" warnings.

kerneldoc cleanups are great for functions that actually have more
than one user (core code, libraries, common code used by multiple
drivers, etc.). Whereas for driver internals I would much rather
emphasize function arguments with well-chosen, descriptive names
instead of a kerneldoc "@p: pointer to a foobar" comment that will
inevitably become stale next time an interface changes.

[01/24] scsi: aacraid: Repair two kerneldoc headers
        https://git.kernel.org/mkp/scsi/c/b115958d91f5
[02/24] scsi: aacraid: Fix a few kerneldoc issues
        https://git.kernel.org/mkp/scsi/c/cf93fffac261
[03/24] scsi: aacraid: Fix logical bug when !DBG
        https://git.kernel.org/mkp/scsi/c/2fee77e5b820
[04/24] scsi: aacraid: Remove unused variable 'status'
        https://git.kernel.org/mkp/scsi/c/0123c7c62d6c
[05/24] scsi: aacraid: Demote partially documented function header
        https://git.kernel.org/mkp/scsi/c/71aa4d3e0e78
[06/24] scsi: aic94xx: Document 'lseq' and repair asd_update_port_links() header
        https://git.kernel.org/mkp/scsi/c/966fdadf6fea
[07/24] scsi: aacraid: Fix a bunch of function header issues
        https://git.kernel.org/mkp/scsi/c/f1134f0eb184
[08/24] scsi: aic94xx: Fix a couple of formatting and bitrot issues
        https://git.kernel.org/mkp/scsi/c/d2e510505006
[09/24] scsi: aacraid: Fill in the very parameter descriptions for rx_sync_cmd()
        https://git.kernel.org/mkp/scsi/c/ae272a95133a
[10/24] scsi: pm8001: Provide descriptions for the many undocumented 'attr's
        https://git.kernel.org/mkp/scsi/c/e1c3e0f8a2ae
[11/24] scsi: ipr: Fix a mountain of kerneldoc misdemeanours
        https://git.kernel.org/mkp/scsi/c/a96099e2c164
[12/24] scsi: virtio_scsi: Demote seemingly unintentional kerneldoc header
        https://git.kernel.org/mkp/scsi/c/e31f2661ff41
[13/24] scsi: ipr: Remove a bunch of set but checked variables
        https://git.kernel.org/mkp/scsi/c/4dc833999e37
[14/24] scsi: ipr: Fix struct packed-not-aligned issues
        https://git.kernel.org/mkp/scsi/c/f3bdc59f9b11
[15/24] scsi: myrs: Demote obvious misuse of kerneldoc to standard comment blocks
        https://git.kernel.org/mkp/scsi/c/8a692fdb1d04
[17/24] scsi: be2iscsi: Fix API/documentation slip
        https://git.kernel.org/mkp/scsi/c/abad069ef0da
[18/24] scsi: be2iscsi: Fix misdocumentation of 'pcontext'
        https://git.kernel.org/mkp/scsi/c/dbc019a48f97
[19/24] scsi: be2iscsi: Add missing function parameter description
        https://git.kernel.org/mkp/scsi/c/7405edfdfb96
[20/24] scsi: lpfc: Correct some pretty obvious misdocumentation
        https://git.kernel.org/mkp/scsi/c/09d99705b5d2
[21/24] scsi: aic7xxx: Remove unused variable 'ahd'
        https://git.kernel.org/mkp/scsi/c/91b6e191c4dc
[22/24] scsi: aic7xxx: Remove unused variables 'wait' and 'paused'
        https://git.kernel.org/mkp/scsi/c/532d56c631f1
[23/24] scsi: aic7xxx: Fix 'amount_xferred' set but not used issue
        https://git.kernel.org/mkp/scsi/c/42b840bcfc16

-- 
Martin K. Petersen	Oracle Linux Engineering
Lee Jones July 16, 2020, 7:14 a.m. UTC | #2 
On Wed, 15 Jul 2020, Martin K. Petersen wrote:

> On Mon, 13 Jul 2020 08:59:37 +0100, Lee Jones wrote:

> 

> > This set is part of a larger effort attempting to clean-up W=1

> > kernel builds, which are currently overwhelmingly riddled with

> > niggly little warnings.

> > 

> > Slowly working through the SCSI related ones.  There are many.

> > 

> > This brings the total of W=1 SCSI wanings from 1690 in v5.8-rc1 to 1109.

> > 

> > [...]

> 

> Applied to 5.9/scsi-queue. Thanks for cleaning things up!

> 

> I do think that in general it makes little sense for low-level drivers

> to document internal functions using kerneldoc. As a general approach

> I would prefer to just switch drivers to '/*' or to remove stale

> comments instead of trying to keep up with the "docrot" warnings.

> 

> kerneldoc cleanups are great for functions that actually have more

> than one user (core code, libraries, common code used by multiple

> drivers, etc.). Whereas for driver internals I would much rather

> emphasize function arguments with well-chosen, descriptive names

> instead of a kerneldoc "@p: pointer to a foobar" comment that will

> inevitably become stale next time an interface changes.


I'm inclined to agree.

There is even a script to list the 'offenders':

 `scripts/find-unused-docs.sh`

An effort to demote all unused docs is likely to be somewhat more
controversial than one to re-sync the attribute/parameter
descriptions though.  I choose the path of least resistance for my
clean-up effort.  Nothing stopping us having another pass once they
are all re-synced.

> [01/24] scsi: aacraid: Repair two kerneldoc headers

>         https://git.kernel.org/mkp/scsi/c/b115958d91f5

> [02/24] scsi: aacraid: Fix a few kerneldoc issues

>         https://git.kernel.org/mkp/scsi/c/cf93fffac261

> [03/24] scsi: aacraid: Fix logical bug when !DBG

>         https://git.kernel.org/mkp/scsi/c/2fee77e5b820

> [04/24] scsi: aacraid: Remove unused variable 'status'

>         https://git.kernel.org/mkp/scsi/c/0123c7c62d6c

> [05/24] scsi: aacraid: Demote partially documented function header

>         https://git.kernel.org/mkp/scsi/c/71aa4d3e0e78

> [06/24] scsi: aic94xx: Document 'lseq' and repair asd_update_port_links() header

>         https://git.kernel.org/mkp/scsi/c/966fdadf6fea

> [07/24] scsi: aacraid: Fix a bunch of function header issues

>         https://git.kernel.org/mkp/scsi/c/f1134f0eb184

> [08/24] scsi: aic94xx: Fix a couple of formatting and bitrot issues

>         https://git.kernel.org/mkp/scsi/c/d2e510505006

> [09/24] scsi: aacraid: Fill in the very parameter descriptions for rx_sync_cmd()

>         https://git.kernel.org/mkp/scsi/c/ae272a95133a

> [10/24] scsi: pm8001: Provide descriptions for the many undocumented 'attr's

>         https://git.kernel.org/mkp/scsi/c/e1c3e0f8a2ae

> [11/24] scsi: ipr: Fix a mountain of kerneldoc misdemeanours

>         https://git.kernel.org/mkp/scsi/c/a96099e2c164

> [12/24] scsi: virtio_scsi: Demote seemingly unintentional kerneldoc header

>         https://git.kernel.org/mkp/scsi/c/e31f2661ff41

> [13/24] scsi: ipr: Remove a bunch of set but checked variables

>         https://git.kernel.org/mkp/scsi/c/4dc833999e37

> [14/24] scsi: ipr: Fix struct packed-not-aligned issues

>         https://git.kernel.org/mkp/scsi/c/f3bdc59f9b11

> [15/24] scsi: myrs: Demote obvious misuse of kerneldoc to standard comment blocks

>         https://git.kernel.org/mkp/scsi/c/8a692fdb1d04

> [17/24] scsi: be2iscsi: Fix API/documentation slip

>         https://git.kernel.org/mkp/scsi/c/abad069ef0da

> [18/24] scsi: be2iscsi: Fix misdocumentation of 'pcontext'

>         https://git.kernel.org/mkp/scsi/c/dbc019a48f97

> [19/24] scsi: be2iscsi: Add missing function parameter description

>         https://git.kernel.org/mkp/scsi/c/7405edfdfb96

> [20/24] scsi: lpfc: Correct some pretty obvious misdocumentation

>         https://git.kernel.org/mkp/scsi/c/09d99705b5d2

> [21/24] scsi: aic7xxx: Remove unused variable 'ahd'

>         https://git.kernel.org/mkp/scsi/c/91b6e191c4dc

> [22/24] scsi: aic7xxx: Remove unused variables 'wait' and 'paused'

>         https://git.kernel.org/mkp/scsi/c/532d56c631f1

> [23/24] scsi: aic7xxx: Fix 'amount_xferred' set but not used issue

>         https://git.kernel.org/mkp/scsi/c/42b840bcfc16

> 


-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee Jones July 16, 2020, 7:16 a.m. UTC | #3 
On Wed, 15 Jul 2020, Martin K. Petersen wrote:

> On Mon, 13 Jul 2020 08:59:37 +0100, Lee Jones wrote:

> 

> > This set is part of a larger effort attempting to clean-up W=1

> > kernel builds, which are currently overwhelmingly riddled with

> > niggly little warnings.

> > 

> > Slowly working through the SCSI related ones.  There are many.

> > 

> > This brings the total of W=1 SCSI wanings from 1690 in v5.8-rc1 to 1109.

> > 

> > [...]

> 

> [01/24] scsi: aacraid: Repair two kerneldoc headers

>         https://git.kernel.org/mkp/scsi/c/b115958d91f5

> [02/24] scsi: aacraid: Fix a few kerneldoc issues

>         https://git.kernel.org/mkp/scsi/c/cf93fffac261

> [03/24] scsi: aacraid: Fix logical bug when !DBG

>         https://git.kernel.org/mkp/scsi/c/2fee77e5b820

> [04/24] scsi: aacraid: Remove unused variable 'status'

>         https://git.kernel.org/mkp/scsi/c/0123c7c62d6c

> [05/24] scsi: aacraid: Demote partially documented function header

>         https://git.kernel.org/mkp/scsi/c/71aa4d3e0e78

> [06/24] scsi: aic94xx: Document 'lseq' and repair asd_update_port_links() header

>         https://git.kernel.org/mkp/scsi/c/966fdadf6fea

> [07/24] scsi: aacraid: Fix a bunch of function header issues

>         https://git.kernel.org/mkp/scsi/c/f1134f0eb184

> [08/24] scsi: aic94xx: Fix a couple of formatting and bitrot issues

>         https://git.kernel.org/mkp/scsi/c/d2e510505006

> [09/24] scsi: aacraid: Fill in the very parameter descriptions for rx_sync_cmd()

>         https://git.kernel.org/mkp/scsi/c/ae272a95133a

> [10/24] scsi: pm8001: Provide descriptions for the many undocumented 'attr's

>         https://git.kernel.org/mkp/scsi/c/e1c3e0f8a2ae

> [11/24] scsi: ipr: Fix a mountain of kerneldoc misdemeanours

>         https://git.kernel.org/mkp/scsi/c/a96099e2c164

> [12/24] scsi: virtio_scsi: Demote seemingly unintentional kerneldoc header

>         https://git.kernel.org/mkp/scsi/c/e31f2661ff41

> [13/24] scsi: ipr: Remove a bunch of set but checked variables

>         https://git.kernel.org/mkp/scsi/c/4dc833999e37

> [14/24] scsi: ipr: Fix struct packed-not-aligned issues

>         https://git.kernel.org/mkp/scsi/c/f3bdc59f9b11

> [15/24] scsi: myrs: Demote obvious misuse of kerneldoc to standard comment blocks

>         https://git.kernel.org/mkp/scsi/c/8a692fdb1d04


Something wrong with [16/24]?

> [17/24] scsi: be2iscsi: Fix API/documentation slip

>         https://git.kernel.org/mkp/scsi/c/abad069ef0da

> [18/24] scsi: be2iscsi: Fix misdocumentation of 'pcontext'

>         https://git.kernel.org/mkp/scsi/c/dbc019a48f97

> [19/24] scsi: be2iscsi: Add missing function parameter description

>         https://git.kernel.org/mkp/scsi/c/7405edfdfb96

> [20/24] scsi: lpfc: Correct some pretty obvious misdocumentation

>         https://git.kernel.org/mkp/scsi/c/09d99705b5d2

> [21/24] scsi: aic7xxx: Remove unused variable 'ahd'

>         https://git.kernel.org/mkp/scsi/c/91b6e191c4dc

> [22/24] scsi: aic7xxx: Remove unused variables 'wait' and 'paused'

>         https://git.kernel.org/mkp/scsi/c/532d56c631f1

> [23/24] scsi: aic7xxx: Fix 'amount_xferred' set but not used issue

>         https://git.kernel.org/mkp/scsi/c/42b840bcfc16

> 


-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee Jones July 16, 2020, 8 a.m. UTC | #4 
On Thu, 16 Jul 2020, Lee Jones wrote:

> On Wed, 15 Jul 2020, Martin K. Petersen wrote:

> 

> > On Mon, 13 Jul 2020 08:59:37 +0100, Lee Jones wrote:

> > 

> > > This set is part of a larger effort attempting to clean-up W=1

> > > kernel builds, which are currently overwhelmingly riddled with

> > > niggly little warnings.

> > > 

> > > Slowly working through the SCSI related ones.  There are many.

> > > 

> > > This brings the total of W=1 SCSI wanings from 1690 in v5.8-rc1 to 1109.

> > > 

> > > [...]

> > 

> > [01/24] scsi: aacraid: Repair two kerneldoc headers

> >         https://git.kernel.org/mkp/scsi/c/b115958d91f5

> > [02/24] scsi: aacraid: Fix a few kerneldoc issues

> >         https://git.kernel.org/mkp/scsi/c/cf93fffac261

> > [03/24] scsi: aacraid: Fix logical bug when !DBG

> >         https://git.kernel.org/mkp/scsi/c/2fee77e5b820

> > [04/24] scsi: aacraid: Remove unused variable 'status'

> >         https://git.kernel.org/mkp/scsi/c/0123c7c62d6c

> > [05/24] scsi: aacraid: Demote partially documented function header

> >         https://git.kernel.org/mkp/scsi/c/71aa4d3e0e78

> > [06/24] scsi: aic94xx: Document 'lseq' and repair asd_update_port_links() header

> >         https://git.kernel.org/mkp/scsi/c/966fdadf6fea

> > [07/24] scsi: aacraid: Fix a bunch of function header issues

> >         https://git.kernel.org/mkp/scsi/c/f1134f0eb184

> > [08/24] scsi: aic94xx: Fix a couple of formatting and bitrot issues

> >         https://git.kernel.org/mkp/scsi/c/d2e510505006

> > [09/24] scsi: aacraid: Fill in the very parameter descriptions for rx_sync_cmd()

> >         https://git.kernel.org/mkp/scsi/c/ae272a95133a

> > [10/24] scsi: pm8001: Provide descriptions for the many undocumented 'attr's

> >         https://git.kernel.org/mkp/scsi/c/e1c3e0f8a2ae

> > [11/24] scsi: ipr: Fix a mountain of kerneldoc misdemeanours

> >         https://git.kernel.org/mkp/scsi/c/a96099e2c164

> > [12/24] scsi: virtio_scsi: Demote seemingly unintentional kerneldoc header

> >         https://git.kernel.org/mkp/scsi/c/e31f2661ff41

> > [13/24] scsi: ipr: Remove a bunch of set but checked variables

> >         https://git.kernel.org/mkp/scsi/c/4dc833999e37

> > [14/24] scsi: ipr: Fix struct packed-not-aligned issues

> >         https://git.kernel.org/mkp/scsi/c/f3bdc59f9b11

> > [15/24] scsi: myrs: Demote obvious misuse of kerneldoc to standard comment blocks

> >         https://git.kernel.org/mkp/scsi/c/8a692fdb1d04

> 

> Something wrong with [16/24]?


I see that you reviewed v1.  No need to respond, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog