diff mbox series

mpt3sas: Perform additional retries if Doorbell read returns 0

Message ID 20230615083010.45837-1-ranjan.kumar@broadcom.com
State Superseded
Headers show
Series mpt3sas: Perform additional retries if Doorbell read returns 0 | expand

Commit Message

Ranjan Kumar June 15, 2023, 8:30 a.m. UTC
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 ")'
Cc: stable@vger.kernel.org

Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 50 ++++++++++++++++-------------
 drivers/scsi/mpt3sas/mpt3sas_base.h |  4 ++-
 2 files changed, 31 insertions(+), 23 deletions(-)

Comments

Greg KH June 15, 2023, 8:47 a.m. UTC | #1
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
Sathya Prakash Veerichetty June 22, 2023, 3:26 p.m. UTC | #2
On Thu, Jun 15, 2023 at 2:47 AM Greg KH <gregkh@linuxfoundation.org> 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@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?
>
>
Greg,  the volatile definition is present for a long time and we don't
want to change it in this patch, we will review and see whether we can
remove it later.

> >  {
> >       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
Greg KH June 22, 2023, 4:08 p.m. UTC | #3
On Thu, Jun 22, 2023 at 09:26:50AM -0600, Sathya Prakash Veerichetty wrote:
> -- 
> This electronic communication and the information and any files transmitted 
> with it, or attached to it, are confidential and are intended solely for 
> the use of the individual or entity to whom it is addressed and may contain 
> information that is confidential, legally privileged, protected by privacy 
> laws, or otherwise restricted from disclosure to anyone else. If you are 
> not the intended recipient or the person responsible for delivering the 
> e-mail to the intended recipient, you are hereby notified that any use, 
> copying, distributing, dissemination, forwarding, printing, or copying of 
> this e-mail is strictly prohibited. If you received this e-mail in error, 
> please return the e-mail to the sender, delete it from your computer, and 
> destroy any printed copy of it.

Now deleted.
Sathya Prakash Veerichetty June 22, 2023, 4:15 p.m. UTC | #4
On Thu, Jun 22, 2023 at 10:08 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jun 22, 2023 at 09:26:50AM -0600, Sathya Prakash Veerichetty wrote:
> > --
> > This electronic communication and the information and any files transmitted
> > with it, or attached to it, are confidential and are intended solely for
> > the use of the individual or entity to whom it is addressed and may contain
> > information that is confidential, legally privileged, protected by privacy
> > laws, or otherwise restricted from disclosure to anyone else. If you are
> > not the intended recipient or the person responsible for delivering the
> > e-mail to the intended recipient, you are hereby notified that any use,
> > copying, distributing, dissemination, forwarding, printing, or copying of
> > this e-mail is strictly prohibited. If you received this e-mail in error,
> > please return the e-mail to the sender, delete it from your computer, and
> > destroy any printed copy of it.
>
> Now deleted.
you didn't receive this in error, so you can keep it, not sure from
when this footer is getting added, I will check on the options to
remove it :(
Greg KH June 22, 2023, 5:04 p.m. UTC | #5
On Thu, Jun 22, 2023 at 10:15:58AM -0600, Sathya Prakash Veerichetty wrote:
> On Thu, Jun 22, 2023 at 10:08 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jun 22, 2023 at 09:26:50AM -0600, Sathya Prakash Veerichetty wrote:
> > > --
> > > This electronic communication and the information and any files transmitted
> > > with it, or attached to it, are confidential and are intended solely for
> > > the use of the individual or entity to whom it is addressed and may contain
> > > information that is confidential, legally privileged, protected by privacy
> > > laws, or otherwise restricted from disclosure to anyone else. If you are
> > > not the intended recipient or the person responsible for delivering the
> > > e-mail to the intended recipient, you are hereby notified that any use,
> > > copying, distributing, dissemination, forwarding, printing, or copying of
> > > this e-mail is strictly prohibited. If you received this e-mail in error,
> > > please return the e-mail to the sender, delete it from your computer, and
> > > destroy any printed copy of it.
> >
> > Now deleted.
> you didn't receive this in error, so you can keep it,

No I can not, as this obviously is confidential, and as such, I need to
delete it because that is incompatible with kernel development.

> not sure from
> when this footer is getting added, I will check on the options to
> remove it :(
> 
> -- 
> This electronic communication and the information and any files transmitted 
> with it, or attached to it, are confidential and are intended solely for 
> the use of the individual or entity to whom it is addressed and may contain 
> information that is confidential, legally privileged, protected by privacy 
> laws, or otherwise restricted from disclosure to anyone else. If you are 
> not the intended recipient or the person responsible for delivering the 
> e-mail to the intended recipient, you are hereby notified that any use, 
> copying, distributing, dissemination, forwarding, printing, or copying of 
> this e-mail is strictly prohibited. If you received this e-mail in error, 
> please return the e-mail to the sender, delete it from your computer, and 
> destroy any printed copy of it.


I'll go delete this as well.
Sathya Prakash Veerichetty July 5, 2023, 5:09 p.m. UTC | #6
Hi Greg,
The email footer issue is resolved and Ranjan has submitted a new
revision of patches, we can discuss in the thread if you have further
questions on the patch.

Thanks
Sathya
Greg KH July 5, 2023, 5:16 p.m. UTC | #7
On Wed, Jul 05, 2023 at 11:09:30AM -0600, Sathya Prakash Veerichetty wrote:
> Hi Greg,
> The email footer issue is resolved and Ranjan has submitted a new
> revision of patches, we can discuss in the thread if you have further
> questions on the patch.

I have no context here at all, sorry.  What in the world is this all
about?

What would you do if you got a random email like this?

Remember, some of us get 1000+ emails a day to deal with...

greg k-h
Sathya Prakash Veerichetty July 5, 2023, 5:28 p.m. UTC | #8
On Thu, Jun 15, 2023 at 2:47 AM Greg KH <gregkh@linuxfoundation.org> 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@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?
>
>
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
Sathya Prakash Veerichetty July 5, 2023, 5:30 p.m. UTC | #9
On Wed, Jul 5, 2023 at 11:16 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jul 05, 2023 at 11:09:30AM -0600, Sathya Prakash Veerichetty wrote:
> > Hi Greg,
> > The email footer issue is resolved and Ranjan has submitted a new
> > revision of patches, we can discuss in the thread if you have further
> > questions on the patch.
>
> I have no context here at all, sorry.  What in the world is this all
> about?
>
> What would you do if you got a random email like this?
>
> Remember, some of us get 1000+ emails a day to deal with...
>
Sorry about that, I replied your comments on this patch and you had to
delete that due to the confidentiality footer added by my company's
email server, I had fixed the problem and sent the response again, we
have also provided new version of the patch with one of your comment
addressed,
> greg k-h
diff mbox series

Patch

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)
 {
 	u32 i = 0, ret_val;
 
 	do {
 		ret_val = readl(addr);
 		i++;
-	} while (ret_val == 0 && i < 3);
+	} while (ret_val == 0 && i < retry_count);
 
 	return ret_val;
 }
 
 static inline u32
-_base_readl(const volatile void __iomem *addr)
+_base_readl(const volatile void __iomem *addr, u8 retry_count)
 {
 	return readl(addr);
 }
@@ -940,7 +940,7 @@  mpt3sas_halt_firmware(struct MPT3SAS_ADAPTER *ioc)
 
 	dump_stack();
 
-	doorbell = ioc->base_readl(&ioc->chip->Doorbell);
+	doorbell = ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY);
 	if ((doorbell & MPI2_IOC_STATE_MASK) == MPI2_IOC_STATE_FAULT) {
 		mpt3sas_print_fault_code(ioc, doorbell &
 		    MPI2_DOORBELL_DATA_MASK);
@@ -1617,10 +1617,10 @@  mpt3sas_base_mask_interrupts(struct MPT3SAS_ADAPTER *ioc)
 	u32 him_register;
 
 	ioc->mask_interrupts = 1;
-	him_register = ioc->base_readl(&ioc->chip->HostInterruptMask);
+	him_register = ioc->base_readl(&ioc->chip->HostInterruptMask, READL_RETRY_COUNT_OF_THREE);
 	him_register |= MPI2_HIM_DIM + MPI2_HIM_RIM + MPI2_HIM_RESET_IRQ_MASK;
 	writel(him_register, &ioc->chip->HostInterruptMask);
-	ioc->base_readl(&ioc->chip->HostInterruptMask);
+	ioc->base_readl(&ioc->chip->HostInterruptMask, READL_RETRY_COUNT_OF_THREE);
 }
 
 /**
@@ -1634,7 +1634,7 @@  mpt3sas_base_unmask_interrupts(struct MPT3SAS_ADAPTER *ioc)
 {
 	u32 him_register;
 
-	him_register = ioc->base_readl(&ioc->chip->HostInterruptMask);
+	him_register = ioc->base_readl(&ioc->chip->HostInterruptMask, READL_RETRY_COUNT_OF_THREE);
 	him_register &= ~MPI2_HIM_RIM;
 	writel(him_register, &ioc->chip->HostInterruptMask);
 	ioc->mask_interrupts = 0;
@@ -6686,7 +6686,7 @@  mpt3sas_base_get_iocstate(struct MPT3SAS_ADAPTER *ioc, int cooked)
 {
 	u32 s, sc;
 
-	s = ioc->base_readl(&ioc->chip->Doorbell);
+	s = ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY);
 	sc = s & MPI2_IOC_STATE_MASK;
 	return cooked ? sc : s;
 }
@@ -6760,7 +6760,8 @@  _base_wait_for_doorbell_int(struct MPT3SAS_ADAPTER *ioc, int timeout)
 	count = 0;
 	cntdn = 1000 * timeout;
 	do {
-		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus);
+		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus,
+							READL_RETRY_COUNT_OF_THREE);
 		if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) {
 			dhsprintk(ioc,
 				  ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n",
@@ -6786,7 +6787,8 @@  _base_spin_on_doorbell_int(struct MPT3SAS_ADAPTER *ioc, int timeout)
 	count = 0;
 	cntdn = 2000 * timeout;
 	do {
-		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus);
+		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus,
+							READL_RETRY_COUNT_OF_THREE);
 		if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) {
 			dhsprintk(ioc,
 				  ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n",
@@ -6824,14 +6826,16 @@  _base_wait_for_doorbell_ack(struct MPT3SAS_ADAPTER *ioc, int timeout)
 	count = 0;
 	cntdn = 1000 * timeout;
 	do {
-		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus);
+		int_status = ioc->base_readl(&ioc->chip->HostInterruptStatus,
+							READL_RETRY_COUNT_OF_THREE);
 		if (!(int_status & MPI2_HIS_SYS2IOC_DB_STATUS)) {
 			dhsprintk(ioc,
 				  ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n",
 					   __func__, count, timeout));
 			return 0;
 		} else if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) {
-			doorbell = ioc->base_readl(&ioc->chip->Doorbell);
+			doorbell = ioc->base_readl(&ioc->chip->Doorbell,
+							READL_RETRY_COUNT_OF_THIRTY);
 			if ((doorbell & MPI2_IOC_STATE_MASK) ==
 			    MPI2_IOC_STATE_FAULT) {
 				mpt3sas_print_fault_code(ioc, doorbell);
@@ -6871,7 +6875,7 @@  _base_wait_for_doorbell_not_used(struct MPT3SAS_ADAPTER *ioc, int timeout)
 	count = 0;
 	cntdn = 1000 * timeout;
 	do {
-		doorbell_reg = ioc->base_readl(&ioc->chip->Doorbell);
+		doorbell_reg = ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY);
 		if (!(doorbell_reg & MPI2_DOORBELL_USED)) {
 			dhsprintk(ioc,
 				  ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n",
@@ -7019,13 +7023,13 @@  _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 	__le32 *mfp;
 
 	/* make sure doorbell is not in use */
-	if ((ioc->base_readl(&ioc->chip->Doorbell) & MPI2_DOORBELL_USED)) {
+	if ((ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY) & MPI2_DOORBELL_USED)) {
 		ioc_err(ioc, "doorbell is in use (line=%d)\n", __LINE__);
 		return -EFAULT;
 	}
 
 	/* clear pending doorbell interrupts from previous state changes */
-	if (ioc->base_readl(&ioc->chip->HostInterruptStatus) &
+	if (ioc->base_readl(&ioc->chip->HostInterruptStatus, READL_RETRY_COUNT_OF_THREE) &
 	    MPI2_HIS_IOC2SYS_DB_STATUS)
 		writel(0, &ioc->chip->HostInterruptStatus);
 
@@ -7068,7 +7072,7 @@  _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 	}
 
 	/* read the first two 16-bits, it gives the total length of the reply */
-	reply[0] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell)
+	reply[0] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY)
 	    & MPI2_DOORBELL_DATA_MASK);
 	writel(0, &ioc->chip->HostInterruptStatus);
 	if ((_base_wait_for_doorbell_int(ioc, 5))) {
@@ -7076,7 +7080,7 @@  _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 			__LINE__);
 		return -EFAULT;
 	}
-	reply[1] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell)
+	reply[1] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY)
 	    & MPI2_DOORBELL_DATA_MASK);
 	writel(0, &ioc->chip->HostInterruptStatus);
 
@@ -7087,10 +7091,10 @@  _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 			return -EFAULT;
 		}
 		if (i >=  reply_bytes/2) /* overflow case */
-			ioc->base_readl(&ioc->chip->Doorbell);
+			ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY);
 		else
 			reply[i] = le16_to_cpu(
-			    ioc->base_readl(&ioc->chip->Doorbell)
+			    ioc->base_readl(&ioc->chip->Doorbell, READL_RETRY_COUNT_OF_THIRTY)
 			    & MPI2_DOORBELL_DATA_MASK);
 		writel(0, &ioc->chip->HostInterruptStatus);
 	}
@@ -7949,14 +7953,15 @@  _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 			goto out;
 		}
 
-		host_diagnostic = ioc->base_readl(&ioc->chip->HostDiagnostic);
+		host_diagnostic = ioc->base_readl(&ioc->chip->HostDiagnostic,
+								READL_RETRY_COUNT_OF_THIRTY);
 		drsprintk(ioc,
 			  ioc_info(ioc, "wrote magic sequence: count(%d), host_diagnostic(0x%08x)\n",
 				   count, host_diagnostic));
 
 	} while ((host_diagnostic & MPI2_DIAG_DIAG_WRITE_ENABLE) == 0);
 
-	hcb_size = ioc->base_readl(&ioc->chip->HCBSize);
+	hcb_size = ioc->base_readl(&ioc->chip->HCBSize, READL_RETRY_COUNT_OF_THREE);
 
 	drsprintk(ioc, ioc_info(ioc, "diag reset: issued\n"));
 	writel(host_diagnostic | MPI2_DIAG_RESET_ADAPTER,
@@ -7969,7 +7974,8 @@  _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 	for (count = 0; count < (300000000 /
 		MPI2_HARD_RESET_PCIE_SECOND_READ_DELAY_MICRO_SEC); count++) {
 
-		host_diagnostic = ioc->base_readl(&ioc->chip->HostDiagnostic);
+		host_diagnostic = ioc->base_readl(&ioc->chip->HostDiagnostic,
+								READL_RETRY_COUNT_OF_THIRTY);
 
 		if (host_diagnostic == 0xFFFFFFFF) {
 			ioc_info(ioc,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 05364aa15ecd..3b8ec4fd2d21 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -160,6 +160,8 @@ 
 
 #define IOC_OPERATIONAL_WAIT_COUNT	10
 
+#define READL_RETRY_COUNT_OF_THIRTY	30
+#define READL_RETRY_COUNT_OF_THREE	3
 /*
  * NVMe defines
  */
@@ -994,7 +996,7 @@  typedef void (*NVME_BUILD_PRP)(struct MPT3SAS_ADAPTER *ioc, u16 smid,
 typedef void (*PUT_SMID_IO_FP_HIP) (struct MPT3SAS_ADAPTER *ioc, u16 smid,
 	u16 funcdep);
 typedef void (*PUT_SMID_DEFAULT) (struct MPT3SAS_ADAPTER *ioc, u16 smid);
-typedef u32 (*BASE_READ_REG) (const volatile void __iomem *addr);
+typedef u32 (*BASE_READ_REG) (const volatile void __iomem *addr, u8 retry_count);
 /*
  * To get high iops reply queue's msix index when high iops mode is enabled
  * else get the msix index of general reply queues.