On 7/5/23 21:37, Ranjan kumar wrote:
Hi Damien, Regarding delay: As Sathya already mentioned as 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. So, we want to go with this change .
Fine. Adding a comment above the macro definitions to explain something like that would be nice.
Apart from that, I will change the name as suggested .
Thanks & Regards, Ranjan
Please avoid top posting.
On Thu, 29 Jun 2023 at 05:24, Damien Le Moal dlemoal@kernel.org wrote:
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.
-- Damien Le Moal Western Digital Research