* [patch] scsi: hisi_sas: shift vs compare typos
@ 2016-11-29 10:47 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2016-11-29 10:47 UTC (permalink / raw)
To: John Garry, Xiang Chen
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
kernel-janitors
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 <dan.carpenter@oracle.com>
---
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_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)
^ permalink raw reply related [flat|nested] 5+ messages in thread* [patch] scsi: hisi_sas: shift vs compare typos
@ 2016-11-29 10:47 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2016-11-29 10:47 UTC (permalink / raw)
To: John Garry, Xiang Chen
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
kernel-janitors
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 <dan.carpenter@oracle.com>
---
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_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)
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [patch] scsi: hisi_sas: shift vs compare typos
2016-11-29 10:47 ` Dan Carpenter
(?)
@ 2016-11-29 11:20 ` John Garry
-1 siblings, 0 replies; 5+ messages in thread
From: John Garry @ 2016-11-29 11:20 UTC (permalink / raw)
To: Dan Carpenter, Xiang Chen
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
kernel-janitors, Linuxarm, tanxiaofei
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 <dan.carpenter@oracle.com>
> ---
> 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_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)
>
> .
>
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 <john.garry@huawei.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] scsi: hisi_sas: shift vs compare typos
2016-11-29 10:47 ` Dan Carpenter
@ 2016-11-29 16:28 ` Martin K. Petersen
-1 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2016-11-29 16:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: John Garry, Xiang Chen, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, kernel-janitors
>>>>> "Dan" = Dan Carpenter <dan.carpenter@oracle.com> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] scsi: hisi_sas: shift vs compare typos
@ 2016-11-29 16:28 ` Martin K. Petersen
0 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2016-11-29 16:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: John Garry, Xiang Chen, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, kernel-janitors
>>>>> "Dan" == Dan Carpenter <dan.carpenter@oracle.com> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-29 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-29 10:47 [patch] scsi: hisi_sas: shift vs compare typos Dan Carpenter
2016-11-29 10:47 ` Dan Carpenter
2016-11-29 11:20 ` John Garry
2016-11-29 16:28 ` Martin K. Petersen
2016-11-29 16:28 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.