On Thu, Jun 15, 2023 at 2:47 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx
> >
> > Signed-off-by: Ranjan Kumar <ranjan.kumar@xxxxxxxxxxxx>
>
> 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?
>
>
This is taken care in the new revision of the patch
> > {
> > 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.
>
This is nothing to do with the system speed, this is our hardware
specific behavior and we are confident that the increased retry count
is sufficient from our hardware perspective for any new systems too.
> thanks,
>
> greg k-h
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
