scsi: hisi_sas: shift vs compare typos
Subsystems (3)
PATCH 1/1 scsi: hisi_sas: shift vs compare typos
There are some typos where we intended "<<" but have "<". Seems likely to cause a bunch of problems.
---
There is another static checker warning to fix perhaps:
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c:2015 phy_up_v2_hw() warn: was expecting a 64 bit value instead of '(2 | 1)'
The code is doing:
phy->phy_type &= ~(PORT_TYPE_SAS | PORT_TYPE_SATA);
phy->phy_type is a u64 and PORT_TYPE_SAS is "1U << 1". So this code is clearing out the high 32 bits of ->phy_type uninitentionally. It's simple enough to make it 1ULL << 1 but the question is, do we really need ->phy_type to be a 64 bit variable? I'm pretty sure that a u32 will suffice.
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +6 -6
@@ -64,19 +64,19 @@ HGC_LM_DFX_STATUS2_ITCTLIST_OFF)#define HGC_CQE_ECC_ADDR 0x13c#define HGC_CQE_ECC_1B_ADDR_OFF 0 -#define HGC_CQE_ECC_1B_ADDR_MSK (0x3f < HGC_CQE_ECC_1B_ADDR_OFF) +#define HGC_CQE_ECC_1B_ADDR_MSK (0x3f << HGC_CQE_ECC_1B_ADDR_OFF)#define HGC_CQE_ECC_MB_ADDR_OFF 8 -#define HGC_CQE_ECC_MB_ADDR_MSK (0x3f < HGC_CQE_ECC_MB_ADDR_OFF) +#define HGC_CQE_ECC_MB_ADDR_MSK (0x3f << HGC_CQE_ECC_MB_ADDR_OFF)#define HGC_IOST_ECC_ADDR 0x140#define HGC_IOST_ECC_1B_ADDR_OFF 0 -#define HGC_IOST_ECC_1B_ADDR_MSK (0x3ff < HGC_IOST_ECC_1B_ADDR_OFF) +#define HGC_IOST_ECC_1B_ADDR_MSK (0x3ff << HGC_IOST_ECC_1B_ADDR_OFF)#define HGC_IOST_ECC_MB_ADDR_OFF 16 -#define HGC_IOST_ECC_MB_ADDR_MSK (0x3ff < HGC_IOST_ECC_MB_ADDR_OFF) +#define HGC_IOST_ECC_MB_ADDR_MSK (0x3ff << HGC_IOST_ECC_MB_ADDR_OFF)#define HGC_DQE_ECC_ADDR 0x144#define HGC_DQE_ECC_1B_ADDR_OFF 0 -#define HGC_DQE_ECC_1B_ADDR_MSK (0xfff < HGC_DQE_ECC_1B_ADDR_OFF) +#define HGC_DQE_ECC_1B_ADDR_MSK (0xfff << HGC_DQE_ECC_1B_ADDR_OFF)#define HGC_DQE_ECC_MB_ADDR_OFF 16 -#define HGC_DQE_ECC_MB_ADDR_MSK (0xfff < HGC_DQE_ECC_MB_ADDR_OFF) +#define HGC_DQE_ECC_MB_ADDR_MSK (0xfff << HGC_DQE_ECC_MB_ADDR_OFF)#define HGC_INVLD_DQE_INFO 0x148#define HGC_INVLD_DQE_INFO_FB_CH0_OFF 9#define HGC_INVLD_DQE_INFO_FB_CH0_MSK (0x1 << HGC_INVLD_DQE_INFO_FB_CH0_OFF)
Quoted 8 lines · Dan Carpenter
There are some typos where we intended "<<" but have "<". Seems likely to cause a bunch of problems.
Fixes d3b688d3c69d ("scsi: hisi_sas: add v2 hw support for ECC and AXI bus fatal error")Signed-off-by Dan Carpenter <[email protected]>---
There is another static checker warning to fix perhaps:
Quoted 6 lines
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c:2015 phy_up_v2_hw() warn: was expecting a 64 bit value instead of '(2 | 1)'The code is doing:
phy->phy_type &= ~(PORT_TYPE_SAS | PORT_TYPE_SATA);
Right, I think that a u32 would suffice. We can generate a separate patch for this.
Quoted 38 lines
phy->phy_typeis a u64 andPORT_TYPE_SASis "1U << 1". So this code is clearing out the high 32 bits of ->phy_type uninitentionally. It's simple enough to make it 1ULL << 1 but the question is, do we really need ->phy_type to be a 64 bit variable? I'm pretty sure that a u32 will suffice.drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +6
-6@@ -64,19 +64,19 @@ HGC_LM_DFX_STATUS2_ITCTLIST_OFF)#define HGC_CQE_ECC_ADDR 0x13c#define HGC_CQE_ECC_1B_ADDR_OFF 0 -#define HGC_CQE_ECC_1B_ADDR_MSK (0x3f < HGC_CQE_ECC_1B_ADDR_OFF) +#define HGC_CQE_ECC_1B_ADDR_MSK (0x3f << HGC_CQE_ECC_1B_ADDR_OFF)#define HGC_CQE_ECC_MB_ADDR_OFF 8 -#define HGC_CQE_ECC_MB_ADDR_MSK (0x3f < HGC_CQE_ECC_MB_ADDR_OFF) +#define HGC_CQE_ECC_MB_ADDR_MSK (0x3f << HGC_CQE_ECC_MB_ADDR_OFF)#define HGC_IOST_ECC_ADDR 0x140#define HGC_IOST_ECC_1B_ADDR_OFF 0 -#define HGC_IOST_ECC_1B_ADDR_MSK (0x3ff < HGC_IOST_ECC_1B_ADDR_OFF) +#define HGC_IOST_ECC_1B_ADDR_MSK (0x3ff << HGC_IOST_ECC_1B_ADDR_OFF)#define HGC_IOST_ECC_MB_ADDR_OFF 16 -#define HGC_IOST_ECC_MB_ADDR_MSK (0x3ff < HGC_IOST_ECC_MB_ADDR_OFF) +#define HGC_IOST_ECC_MB_ADDR_MSK (0x3ff << HGC_IOST_ECC_MB_ADDR_OFF)#define HGC_DQE_ECC_ADDR 0x144#define HGC_DQE_ECC_1B_ADDR_OFF 0 -#define HGC_DQE_ECC_1B_ADDR_MSK (0xfff < HGC_DQE_ECC_1B_ADDR_OFF) +#define HGC_DQE_ECC_1B_ADDR_MSK (0xfff << HGC_DQE_ECC_1B_ADDR_OFF)#define HGC_DQE_ECC_MB_ADDR_OFF 16 -#define HGC_DQE_ECC_MB_ADDR_MSK (0xfff < HGC_DQE_ECC_MB_ADDR_OFF) +#define HGC_DQE_ECC_MB_ADDR_MSK (0xfff << HGC_DQE_ECC_MB_ADDR_OFF)#define HGC_INVLD_DQE_INFO 0x148#define HGC_INVLD_DQE_INFO_FB_CH0_OFF 9#define HGC_INVLD_DQE_INFO_FB_CH0_MSK (0x1 << HGC_INVLD_DQE_INFO_FB_CH0_OFF)
Quoted 2 lines
.
Cheers
These were introduced in recent patch applied to 4.10 queue, and would only affect rare occurance of AXI/ECC error.
Martin K. Petersen
AppliedQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Dan" = Dan Carpenter <[email protected]> writes:
Dan> There are some typos where we intended "<<" but have "<". Seems Dan> likely to cause a bunch of problems.
Applied to 4.10/scsi-queue.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering