[patch 2/2] [SCSI] bfa: test is always false in bfa_sfp_media_get()

[patch 2/2] [SCSI] bfa: test is always false in bfa_sfp_media_get()

[Dan Carpenter]

e10g.r.e10g_unall is only 1 bit so "if (e10g.r.e10g_unall & 0x80)"
is always false.  e10g is a union between an unsigned char and named
bitfields.  The intent here was to test the named bitfield by
itself.

We can change the previous tests as well.  That improves the
readability and it looks like it fixes a endian bug as well.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
I don't understand where __BIGENDIAN is defined.  Only this driver
references it.  How does that work?

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 052373b..090a19d 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -3589,11 +3589,11 @@ bfa_sfp_media_get(struct bfa_sfp_s *sfp)
 			 (xmtr_tech & SFP_XMTR_TECH_SA))
 			*media = BFA_SFP_MEDIA_SW;
 		/* Check 10G Ethernet Compilance code */
-		else if (e10g.b & 0x10)
+		else if (e10g.r.e10g_sr)
 			*media = BFA_SFP_MEDIA_SW;
-		else if (e10g.b & 0x60)
+		else if (e10g.r.e10g_lr && e10g.r.e10g_lrm)
 			*media = BFA_SFP_MEDIA_LW;
-		else if (e10g.r.e10g_unall & 0x80)
+		else if (e10g.r.e10g_unall)
 			*media = BFA_SFP_MEDIA_UNKNOWN;
 		else
 			bfa_trc(sfp, 0);

RE: [patch 2/2] [SCSI] bfa: test is always false in bfa_sfp_media_get()

[Jing Huang]

>-----Original Message-----
>From: Dan Carpenter [mailto:error27@gmail.com]
>Sent: Wednesday, July 06, 2011 12:40 AM
>To: Jing Huang
>Cc: James E.J. Bottomley; open list:BROCADE BFA FC SC...; linux-
>kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
>Subject: [patch 2/2] [SCSI] bfa: test is always false in bfa_sfp_media_get()
>
>e10g.r.e10g_unall is only 1 bit so "if (e10g.r.e10g_unall & 0x80)"
>is always false.  e10g is a union between an unsigned char and named
>bitfields.  The intent here was to test the named bitfield by
>itself.
>
>We can change the previous tests as well.  That improves the
>readability and it looks like it fixes a endian bug as well.
>
>Signed-off-by: Dan Carpenter <error27@gmail.com>
>---
>I don't understand where __BIGENDIAN is defined.  Only this driver
>references it.  How does that work?
>
>diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
>index 052373b..090a19d 100644
>--- a/drivers/scsi/bfa/bfa_ioc.c
>+++ b/drivers/scsi/bfa/bfa_ioc.c
>@@ -3589,11 +3589,11 @@ bfa_sfp_media_get(struct bfa_sfp_s *sfp)
> 			 (xmtr_tech & SFP_XMTR_TECH_SA))
> 			*media = BFA_SFP_MEDIA_SW;
> 		/* Check 10G Ethernet Compilance code */
>-		else if (e10g.b & 0x10)
>+		else if (e10g.r.e10g_sr)
> 			*media = BFA_SFP_MEDIA_SW;
>-		else if (e10g.b & 0x60)
>+		else if (e10g.r.e10g_lr && e10g.r.e10g_lrm)
> 			*media = BFA_SFP_MEDIA_LW;
>-		else if (e10g.r.e10g_unall & 0x80)
>+		else if (e10g.r.e10g_unall)
> 			*media = BFA_SFP_MEDIA_UNKNOWN;
> 		else
> 			bfa_trc(sfp, 0);

Hi Dan,

__BIGENIDIAN is a bug and it needs to be replaced with __BIG_ENDIAN.
The first two changes will introduce endianness issue without having __BIGENDIAN fixed.
The third change is good but also needs the same fix to make the code endianness netural,.

Thanks
Jing

[patch 2/2 v2] [SCSI] bfa: fix some endian bugs

[Dan Carpenter]

The initial problem that I noticed was that
"if (e10g.r.e10g_unall & 0x80)" is always false because e10g_unall
is only one bit wide.

It should be testing just "if (e10g.r.e10g_unall)" instead, but that
also has a problem on big endian systems because there was a typo
where it said __BIGENDIAN and it should have said __BIG_ENDIAN.

Also I updated the other two tests in the if else block to match as
well.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/bna/bfi.h b/drivers/net/bna/bfi.h
index 6050379..b2611ef 100644
--- a/drivers/net/bna/bfi.h
+++ b/drivers/net/bna/bfi.h
@@ -104,7 +104,7 @@ union bfi_addr_u {
  * Scatter Gather Element
  */
 struct bfi_sge {
-#ifdef __BIGENDIAN
+#ifdef __BIG_ENDIAN
 	u32	flags:2,
 			rsvd:2,
 			sg_len:28;
diff --git a/drivers/scsi/bfa/bfa_defs.h b/drivers/scsi/bfa/bfa_defs.h
index ed8d31b..1cefb11 100644
--- a/drivers/scsi/bfa/bfa_defs.h
+++ b/drivers/scsi/bfa/bfa_defs.h
@@ -731,7 +731,7 @@ struct sfp_mem_s {
 union sfp_xcvr_e10g_code_u {
 	u8		b;
 	struct {
-#ifdef __BIGENDIAN
+#ifdef __BIG_ENDIAN
 		u8	e10g_unall:1;   /* 10G Ethernet compliance */
 		u8	e10g_lrm:1;
 		u8	e10g_lr:1;
@@ -811,7 +811,7 @@ union sfp_xcvr_fc2_code_u {
 union sfp_xcvr_fc3_code_u {
 	u8		b;
 	struct {
-#ifdef __BIGENDIAN
+#ifdef __BIG_ENDIAN
 		u8	rsv4:1;
 		u8	mb800:1;    /* 800 Mbytes/sec */
 		u8	mb1600:1;   /* 1600 Mbytes/sec */
diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index d6c2bf3..bc678c7 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -3589,11 +3589,11 @@ bfa_sfp_media_get(struct bfa_sfp_s *sfp)
 			 (xmtr_tech & SFP_XMTR_TECH_SA))
 			*media = BFA_SFP_MEDIA_SW;
 		/* Check 10G Ethernet Compilance code */
-		else if (e10g.b & 0x10)
+		else if (e10g.r.e10g_sr)
 			*media = BFA_SFP_MEDIA_SW;
-		else if (e10g.b & 0x60)
+		else if (e10g.r.e10g_lr && e10g.r.e10g_lrm)
 			*media = BFA_SFP_MEDIA_LW;
-		else if (e10g.r.e10g_unall & 0x80)
+		else if (e10g.r.e10g_unall)
 			*media = BFA_SFP_MEDIA_UNKNOWN;
 		else
 			bfa_trc(sfp, 0);