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@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.
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
