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

Steffen Maier
2022-05-06T11:20:43Z

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