On 6/28/23 16:05, 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.
The magic value "3" for retry count was already that, magic. Why would things work better with 30 ? What is the reasoning ? Isn't a udelay needed to avoid that many retries ?
Fixes: b899202901a8 ("mpt3sas: Add separate function for aero doorbell reads ") Cc: stable@vger.kernel.org Signed-off-by: Ranjan Kumar ranjan.kumar@broadcom.com
[..]
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
Less than ideal naming I think. If the values need to be changed again, a lot of code will need to change. What about soemthing like:
#define READL_RETRY_COUNT 30 #define READL_RETRY_SHORT_COUNT 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.