Re: [PATCH 2/7] scsi: Use Scsi_Host and channel number as argument for eh_bus_reset_handler()

[Steffen Maier <maier@linux.ibm.com>]

On 5/2/22 23:59, Hannes Reinecke wrote:
> The bus reset should not depend on any command, but rather only
> use the SCSI Host and the channel/bus number as argument.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>   Documentation/scsi/scsi_eh.rst                |  2 +-
>   Documentation/scsi/scsi_mid_low_api.rst       |  5 ++-

>   include/scsi/scsi_host.h                      |  2 +-
>   36 files changed, 150 insertions(+), 163 deletions(-)
> 
> diff --git a/Documentation/scsi/scsi_eh.rst b/Documentation/scsi/scsi_eh.rst
> index 12113cca5ad9..5e2d04e64005 100644
> --- a/Documentation/scsi/scsi_eh.rst
> +++ b/Documentation/scsi/scsi_eh.rst
> @@ -214,7 +214,7 @@ considered to fail always.
> 
>       int (* eh_abort_handler)(struct scsi_cmnd *);
>       int (* eh_device_reset_handler)(struct scsi_cmnd *);
> -    int (* eh_bus_reset_handler)(struct scsi_cmnd *);
> +    int (* eh_bus_reset_handler)(struct Scsi_Host *, int);

IIRC, checkpatch prefers using variable names with prototypes. I also like it 
because it would explain the semantics of e.g. the "int". Is it possible to do 
so in function pointer definitions? [method seemed to work for me with gcc 11.3.0]:

+    int (* eh_bus_reset_handler)(struct Scsi_Host *host, int channel);

>       int (* eh_host_reset_handler)(struct Scsi_Host *);
> 
>   Higher-severity actions are taken only when lower-severity actions
> diff --git a/Documentation/scsi/scsi_mid_low_api.rst b/Documentation/scsi/scsi_mid_low_api.rst
> index 784587ea7eee..1b1c37445580 100644
> --- a/Documentation/scsi/scsi_mid_low_api.rst
> +++ b/Documentation/scsi/scsi_mid_low_api.rst
> @@ -741,7 +741,8 @@ Details::
> 
>       /**
>       *      eh_bus_reset_handler - issue SCSI bus reset
> -    *      @scp: SCSI bus that contains this device should be reset
> +    *      @host: SCSI Host that contains the channel which should be reset
> +    *      @channel: channel to be reset
>       *
>       *      Returns SUCCESS if command aborted else FAILED
>       *
> @@ -754,7 +755,7 @@ Details::
>       *
>       *      Optionally defined in: LLD
>       **/
> -	int eh_bus_reset_handler(struct scsi_cmnd * scp)
> +	int eh_bus_reset_handler(struct Scsi_Host * host, int channel)

exactly like that

> 
> 
>       /**

> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 3b8d9e65ea57..73c9971e7334 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -140,7 +140,7 @@ struct scsi_host_template {
>   	int (* eh_abort_handler)(struct scsi_cmnd *);
>   	int (* eh_device_reset_handler)(struct scsi_cmnd *);
>   	int (* eh_target_reset_handler)(struct scsi_cmnd *);
> -	int (* eh_bus_reset_handler)(struct scsi_cmnd *);
> +	int (* eh_bus_reset_handler)(struct Scsi_Host *, int);

dito

>   	int (* eh_host_reset_handler)(struct Scsi_Host *);
> 
>   	/*


-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z and LinuxONE

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschaeftsfuehrung: David Faller
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294