On 2/18/22 17:41, Damien Le Moal wrote:
> On 2/18/22 16:56, Ajish Koshy wrote:
>> Firmware expects host driver to clear
>> scratchpad rsvd 0 register after non-fatal error
>> is found.
>
> Please use up to 72-ish characters per line.
>
>>
>> This is done when firmware raises fatal error interrupt
>> and indicates non-fatal error. At this
>> point firmware updates scratchpad rsvd 0 register
>> with non-fatal error value. Here host has
>> to clear the register after reading it during non-fatal
>> errors.
>>
>> Renamed
>> MSGU_HOST_SCRATCH_PAD_6 to MSGU_SCRATCH_PAD_RSVD_0
>> MSGU_HOST_SCRATCH_PAD_7 to MSGU_SCRATCH_PAD_RSVD_1
>>
>> Signed-off-by: Ajish Koshy <Ajish.Koshy@xxxxxxxxxxxxx>
>> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
>> ---
>> drivers/scsi/pm8001/pm80xx_hwi.c | 28 ++++++++++++++++++++++------
>> drivers/scsi/pm8001/pm80xx_hwi.h | 9 +++++++--
>> 2 files changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
>> index bbf538fe15b3..df22cfd07262 100644
>> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
>> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
>> @@ -1552,9 +1552,9 @@ pm80xx_fatal_errors(struct pm8001_hba_info *pm8001_ha)
>> {
>> int ret = 0;
>> u32 scratch_pad_rsvd0 = pm8001_cr32(pm8001_ha, 0,
>> - MSGU_HOST_SCRATCH_PAD_6);
>> + MSGU_SCRATCH_PAD_RSVD_0);
>
> I know the code is full of such style, but could you please indent the
> function arguments together ? that is:
>
> u32 scratch_pad_rsvd0 = pm8001_cr32(pm8001_ha, 0,
> MSGU_SCRATCH_PAD_RSVD_0);
>
> This makes the code far easier to read.
>
>> u32 scratch_pad_rsvd1 = pm8001_cr32(pm8001_ha, 0,
>> - MSGU_HOST_SCRATCH_PAD_7);
>> + MSGU_SCRATCH_PAD_RSVD_1);
>
> Same here, and many places below too.
>
>> u32 scratch_pad1 = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_1);
>> u32 scratch_pad2 = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_2);
>> u32 scratch_pad3 = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_3);
>> @@ -1663,9 +1663,9 @@ pm80xx_chip_soft_rst(struct pm8001_hba_info *pm8001_ha)
>> PCI_VENDOR_ID_ATTO &&
>> pm8001_ha->pdev->subsystem_vendor != 0) {
>> ibutton0 = pm8001_cr32(pm8001_ha, 0,
>> - MSGU_HOST_SCRATCH_PAD_6);
>> + MSGU_SCRATCH_PAD_RSVD_0);
>> ibutton1 = pm8001_cr32(pm8001_ha, 0,
>> - MSGU_HOST_SCRATCH_PAD_7);
>> + MSGU_SCRATCH_PAD_RSVD_1);
>> if (!ibutton0 && !ibutton1) {
>> pm8001_dbg(pm8001_ha, FAIL,
>> "iButton Feature is not Available!!!\n");
>> @@ -4138,9 +4138,9 @@ static void print_scratchpad_registers(struct pm8001_hba_info *pm8001_ha)
>> pm8001_dbg(pm8001_ha, FAIL, "MSGU_HOST_SCRATCH_PAD_5: 0x%x\n",
>> pm8001_cr32(pm8001_ha, 0, MSGU_HOST_SCRATCH_PAD_5));
>> pm8001_dbg(pm8001_ha, FAIL, "MSGU_RSVD_SCRATCH_PAD_0: 0x%x\n",
>> - pm8001_cr32(pm8001_ha, 0, MSGU_HOST_SCRATCH_PAD_6));
>> + pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_RSVD_0));
>> pm8001_dbg(pm8001_ha, FAIL, "MSGU_RSVD_SCRATCH_PAD_1: 0x%x\n",
>> - pm8001_cr32(pm8001_ha, 0, MSGU_HOST_SCRATCH_PAD_7));
>> + pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_RSVD_1));
>> }
>>
>> static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>> @@ -4162,6 +4162,22 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>> pm8001_handle_event(pm8001_ha, NULL, IO_FATAL_ERROR);
>> print_scratchpad_registers(pm8001_ha);
>> return ret;
>> + } else {
>> + /*read scratchpad rsvd 0 register*/
>> + regval = pm8001_cr32(pm8001_ha, 0,
>> + MSGU_SCRATCH_PAD_RSVD_0);> + switch (regval) {
>> + case NON_FATAL_SPBC_LBUS_ECC_ERR:
>> + case NON_FATAL_BDMA_ERR:
>> + case NON_FATAL_THERM_OVERTEMP_ERR:
>> + /*Clear the register*/
>> + pm8001_cw32(pm8001_ha, 0,
>> + MSGU_SCRATCH_PAD_RSVD_0,
>> + 0x00000000);
>> + break;
>> + default:
>> + break;
>> + }
>> }
>> }
>> circularQ = &pm8001_ha->outbnd_q_tbl[vec];
>> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h
>> index c7e5d93bea92..f0818540b2cd 100644
>> --- a/drivers/scsi/pm8001/pm80xx_hwi.h
>> +++ b/drivers/scsi/pm8001/pm80xx_hwi.h
>> @@ -1366,8 +1366,8 @@ typedef struct SASProtocolTimerConfig SASProtocolTimerConfig_t;
>> #define MSGU_HOST_SCRATCH_PAD_3 0x60
>> #define MSGU_HOST_SCRATCH_PAD_4 0x64
>> #define MSGU_HOST_SCRATCH_PAD_5 0x68
>> -#define MSGU_HOST_SCRATCH_PAD_6 0x6C
>> -#define MSGU_HOST_SCRATCH_PAD_7 0x70
>> +#define MSGU_SCRATCH_PAD_RSVD_0 0x6C
>> +#define MSGU_SCRATCH_PAD_RSVD_1 0x70
>>
>> #define MSGU_SCRATCHPAD1_RAAE_STATE_ERR(x) ((x & 0x3) == 0x2)
>> #define MSGU_SCRATCHPAD1_ILA_STATE_ERR(x) (((x >> 2) & 0x3) == 0x2)
>> @@ -1435,6 +1435,11 @@ typedef struct SASProtocolTimerConfig SASProtocolTimerConfig_t;
>> #define SCRATCH_PAD_ERROR_MASK 0xFFFFFC00 /* Error mask bits */
>> #define SCRATCH_PAD_STATE_MASK 0x00000003 /* State Mask bits */
>>
>> +/*state definition for Scratchpad Rsvd 0, Offset 0x6C, Non-fatal*/
>> +#define NON_FATAL_SPBC_LBUS_ECC_ERR 0x70000001
>> +#define NON_FATAL_BDMA_ERR 0xE0000001
>> +#define NON_FATAL_THERM_OVERTEMP_ERR 0x80000001
>
> Could you align the values similarly to other macros above and below
> this hunk ? Again, code readability.
My bad. The values are aligned... My mailer was playing tricks on me :)
>
>> +
>> /* main configuration offset - byte offset */
>> #define MAIN_SIGNATURE_OFFSET 0x00 /* DWORD 0x00 */
>> #define MAIN_INTERFACE_REVISION 0x04 /* DWORD 0x01 */
>
>
--
Damien Le Moal
Western Digital Research
