Re: [PATCH] mpt3sas: Perform additional retries if Doorbell read returns 0

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jun 15, 2023 at 02:00:10PM +0530, Ranjan Kumar wrote:
> Doorbell and Host diagnostic registers could return 0 even
> after 3 retries and that leads to occasional resets of the
> controllers, hence increased the retry count to thirty.
> 
> 'Fixes: b899202901a8 ("mpt3sas: Add separate function for aero doorbell reads ")'

No ' characters here please.

> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>

No blank line before the signed-off-by and the other fields please.

Didn't checkpatch warn you about this?

> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 50 ++++++++++++++++-------------
>  drivers/scsi/mpt3sas/mpt3sas_base.h |  4 ++-
>  2 files changed, 31 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 53f5492579cb..44e7ccb6f780 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -201,20 +201,20 @@ module_param_call(mpt3sas_fwfault_debug, _scsih_set_fwfault_debug,
>   * while reading the system interface register.
>   */
>  static inline u32
> -_base_readl_aero(const volatile void __iomem *addr)
> +_base_readl_aero(const volatile void __iomem *addr, u8 retry_count)

Are you sure that volatile really does what you think it does here?


>  {
>  	u32 i = 0, ret_val;
>  
>  	do {
>  		ret_val = readl(addr);
>  		i++;
> -	} while (ret_val == 0 && i < 3);
> +	} while (ret_val == 0 && i < retry_count);

So newer systems will complete this failure loop faster than older ones?
That feels very wrong, you will be changing this in a year or so.  Use
time please, not counts.

thanks,

greg k-h