Re: [patch] scsi: hisi_sas: shift vs compare typos

2016-11-29 Thread Martin K. Petersen 
> "Dan" == Dan Carpenter  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.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] scsi: hisi_sas: shift vs compare typos

2016-11-29 Thread John Garry 

On 29/11/2016 10:47, Dan Carpenter wrote:

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 
---
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);


Right, I think that a u32 would suffice. We can generate a separate 
patch for this.




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.

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 15487f2..93876c0 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -64,19 +64,19 @@
 HGC_LM_DFX_STATUS2_ITCTLIST_OFF)
 #define HGC_CQE_ECC_ADDR   0x13c
 #define HGC_CQE_ECC_1B_ADDR_OFF0
-#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_OFF8
-#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_OFF0
-#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_OFF16
-#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)

.



Cheers

These were introduced in recent patch applied to 4.10 queue, and would 
only affect rare occurance of AXI/ECC error.


Signed-off-by: John Garry 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch] scsi: hisi_sas: shift vs compare typos

2016-11-29 Thread 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 
---
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.

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 15487f2..93876c0 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -64,19 +64,19 @@
 HGC_LM_DFX_STATUS2_ITCTLIST_OFF)
 #define HGC_CQE_ECC_ADDR   0x13c
 #define HGC_CQE_ECC_1B_ADDR_OFF0
-#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_OFF8
-#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_OFF0
-#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_OFF16
-#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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html