Messages in this thread | | | Subject | Re: [PATCH RFT] scsi: pm8001: Fix FW crash for maxcpus=1 | From | John Garry <> | Date | Wed, 5 Jan 2022 11:28:14 +0000 |
| |
On 05/01/2022 04:03, Damien Le Moal wrote: > On 1/5/22 03:26, John Garry wrote: >> According to the comment in check_fw_ready() we should not check the >> IOP1_READY field in register SCRATCH_PAD_1 for 8008 or 8009 controllers. >> >> However we check this very field in process_oq() for processing the highest >> index interrupt vector. Change that function to not check IOP1_READY for >> those mentioned controllers, but do check ILA_READY in both cases. >> >> The reason I assume that this was not hit earlier was because we always >> allocated 64 MSI(X), and just did not pass the vector index check in >> process_oq(), i.e. the handler never ran for vector index 63. >> >> Signed-off-by: John Garry<john.garry@huawei.com> >> >> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c >> index 2101fc5761c3..77b8bb30615b 100644 >> --- a/drivers/scsi/pm8001/pm80xx_hwi.c >> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c >> @@ -4162,9 +4162,16 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec) >> u32 regval; >> >> if (vec == (pm8001_ha->max_q_num - 1)) { >> + u32 mipsall_ready; >> + >> + if ((pm8001_ha->chip_id == chip_8008) || >> + (pm8001_ha->chip_id == chip_8009)) > nit: no need for the inner brackets here.
ok, I can fix that.
But I would also like opinion from microchip guys/maintainer on why this code is here at all. Seems strange in the way we check in this register in the interrupt handler for only a specific vector and, also, why we check at all in an interrupt handler. > >> + mipsall_ready = SCRATCH_PAD_MIPSALL_READY_8PORT; >> + else >> + mipsall_ready = SCRATCH_PAD_MIPSALL_READY_16PORT; >> + >> regval = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_1); >> - if ((regval & SCRATCH_PAD_MIPSALL_READY) != >> - SCRATCH_PAD_MIPSALL_READY) { >> + if ((regval & mipsall_ready) != mipsall_ready) { >> pm8001_ha->controller_fatal_error = true; >> pm8001_dbg(pm8001_ha, FAIL, >> "Firmware Fatal error! Regval:0x%x\n", >> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h >> index c7e5d93bea92..c41ed039c92a 100644 >> --- a/drivers/scsi/pm8001/pm80xx_hwi.h >> +++ b/drivers/scsi/pm8001/pm80xx_hwi.h >> @@ -1405,8 +1405,12 @@ typedef struct SASProtocolTimerConfig SASProtocolTimerConfig_t; >> #define SCRATCH_PAD_BOOT_LOAD_SUCCESS 0x0 >> #define SCRATCH_PAD_IOP0_READY 0xC00 >> #define SCRATCH_PAD_IOP1_READY 0x3000 >> -#define SCRATCH_PAD_MIPSALL_READY (SCRATCH_PAD_IOP1_READY | \ >> +#define SCRATCH_PAD_MIPSALL_READY_16PORT (SCRATCH_PAD_IOP1_READY | \ >> SCRATCH_PAD_IOP0_READY | \ >> + SCRATCH_PAD_ILA_READY | \ >> + SCRATCH_PAD_RAAE_READY) >> +#define SCRATCH_PAD_MIPSALL_READY_8PORT (SCRATCH_PAD_IOP0_READY | \ >> + SCRATCH_PAD_ILA_READY | \ >> SCRATCH_PAD_RAAE_READY) >> >> /* boot loader state */ > Otherwise, looks OK to me. > I tested with and without max_cpus=1 with a ATTO Technology, Inc. > ExpressSAS 12Gb/s SAS/SATA HBA (rev 06) adapter and everything is OK. > That adapter uses chip_8072 though, not 8008 or 8009. > > Feel free to add: > > Reviewed-by: Damien Le Moal<damien.lemoal@opensource.wdc.com> > Tested-by: Damien Le Moal<damien.lemoal@opensource.wdc.com>
Thanks!
john
| |