Message ID | 20220218075627.54589-1-Ajish.Koshy@microchip.com |
---|---|
State | Superseded |
Headers | show |
Series | scsi: pm80xx: handle non-fatal errors | expand |
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@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > --- > 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. > + > /* main configuration offset - byte offset */ > #define MAIN_SIGNATURE_OFFSET 0x00 /* DWORD 0x00 */ > #define MAIN_INTERFACE_REVISION 0x04 /* DWORD 0x01 */
Hi Damien, Thank you for your comments here. Shall incorporate the changes suggested in V2 patch. > >> 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. OK > > > >> > >> 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@microchip.com> > >> Signed-off-by: Viswas G <Viswas.G@microchip.com> > >> --- > >> 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. OK > > > >> 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 Thanks, Ajish
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); u32 scratch_pad_rsvd1 = pm8001_cr32(pm8001_ha, 0, - MSGU_HOST_SCRATCH_PAD_7); + MSGU_SCRATCH_PAD_RSVD_1); 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 + /* main configuration offset - byte offset */ #define MAIN_SIGNATURE_OFFSET 0x00 /* DWORD 0x00 */ #define MAIN_INTERFACE_REVISION 0x04 /* DWORD 0x01 */