Big endian swap for Marvell 6440 SAS driver

PATCH In review Cédric Cano 2 participants 4 messages Jun 10, 2011 – Jun 14, 2011 SCSI

lore

Share & tools

lore

Cédric Cano

author

2011-06-10 17:51 UTC #0 lore raw

PATCH 1/1 Big endian swap for Marvell 6440 SAS driver

Hi,

Marvell SAS driver doesn't work on powerpc architecture due to big endian swap. I've tested it with the MV6440 chip. Here you can find the patch for Linux 2.6.39.1.

Cedric Cano

Signed-off-by Cedric Cano <[email protected]>

---

scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +8 -0

@@ -1143,6 +1143,14 @@
     MVS_CHIP_DISP->write_port_cfg_addr(mvi, i, PHYR_SATA_SIG0);
     s[0] = MVS_CHIP_DISP->read_port_cfg_data(mvi, i);
 
+    /* swap for big endian devices because of use of these data in bytes */
+#ifdef __BIG_ENDIAN
+    s[0] = swab32(s[0]);
+    s[1] = swab32(s[1]);
+    s[2] = swab32(s[2]);
+    s[3] = swab32(s[3]);
+#endif /* __BIG_ENDIAN */
+
     /* Workaround: take some ATAPI devices for ATA */
     if (((s[1] & 0x00FFFFFF) == 0x00EB1401) && (*(u8 *)&s[3] == 0x01))
         s[1] = 0x00EB1401 | (*((u8 *)&s[1] + 3) & 0x10);

---

Benjamin Herrenschmidt

2011-06-11 22:46 UTC #1 lore raw

Quoted 13 lines · Cédric Cano

Hi,
Marvell SAS driver doesn't work on powerpc architecture due to big endian swap. I've tested it with the MV6440 chip.
Here you can find the patch for Linux 2.6.39.1.
Cedric Cano
Signed-off-by Cedric Cano <[email protected]>
---

scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 line 1152

Quoted 10 lines

scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +7 -0

@@ -1143,6 +1143,14 @@
      MVS_CHIP_DISP->write_port_cfg_addr(mvi, i, PHYR_SATA_SIG0);
      s[0] = MVS_CHIP_DISP->read_port_cfg_data(mvi, i);
 
+    /* swap for big endian devices because of use of these data in bytes */
+#ifdef __BIG_ENDIAN
+    s[0] = swab32(s[0]);
+    s[1] = swab32(s[1]);
+    s[2] = swab32(s[2]);
+    s[3] = swab32(s[3]);
+#endif /* __BIG_ENDIAN */

Hi Cedric !

Thanks for this. CC'ing linux-scsi.

Note that it would probably have been better to use le32_to_cpu, which avoids the ifdef completely.

Cheers,
Ben.

Quoted 9 lines

scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +0 -1
      /* Workaround: take some ATAPI devices for ATA */
      if (((s[1] & 0x00FFFFFF) == 0x00EB1401) && (*(u8 *)&s[3] == 0x01))
          s[1] = 0x00EB1401 | (*((u8 *)&s[1] + 3) & 0x10);
 
---
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

Cédric Cano

author

2011-06-14 06:42 UTC #2 lore raw

Hi Ben,

You're alright! I hadn't thought to use le32_to_cpu. Do you want I make a new patch?

Cedric

Quoted 26 lines · Benjamin Herrenschmidt

Quoted 13 lines · Cédric Cano
Hi,
Marvell SAS driver doesn't work on powerpc architecture due to big endian swap. I've tested it with the MV6440 chip.
Here you can find the patch for Linux 2.6.39.1.
Cedric Cano
Signed-off-by Cedric Cano<[email protected]>
---
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 line 1152
Quoted 10 lines
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +7 -0
@@ -1143,6 +1143,14 @@
       MVS_CHIP_DISP->write_port_cfg_addr(mvi, i, PHYR_SATA_SIG0);
       s[0] = MVS_CHIP_DISP->read_port_cfg_data(mvi, i);
 
+    /* swap for big endian devices because of use of these data in bytes */
+#ifdef __BIG_ENDIAN
+    s[0] = swab32(s[0]);
+    s[1] = swab32(s[1]);
+    s[2] = swab32(s[2]);
+    s[3] = swab32(s[3]);
+#endif /* __BIG_ENDIAN */
Hi Cedric !

Quoted 19 lines

Thanks for this. CC'ing linux-scsi.
Note that it would probably have been better to use le32_to_cpu, which avoids the ifdef completely.
Cheers,
Ben.
Quoted 9 lines
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +0 -1
       /* Workaround: take some ATAPI devices for ATA */
       if (((s[1]&  0x00FFFFFF) == 0x00EB1401)&&  (*(u8 *)&s[3] == 0x01))
           s[1] = 0x00EB1401 | (*((u8 *)&s[1] + 3)&  0x10);
 
---
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

Benjamin Herrenschmidt

2011-06-14 07:05 UTC #3 lore raw

Quoted 4 lines · Cédric Cano

Hi Ben,
You're alright! I hadn't thought to use le32_to_cpu. Do you want I make a new patch?

Well, I'm not the maintainer of the subsystem or of the driver, so I'm not going to be the one accepting/merging it :-)

I'm just reviewing the patch, but yes, I think you should, and you should send it to the right people, ie, I think at least linux-scsi and possibly the author/maintainer of the driver itself.

Cheers,
Ben.

Quoted 48 lines

Cedric
Quoted 45 lines · Benjamin Herrenschmidt
Quoted 13 lines · Cédric Cano
Hi,
Marvell SAS driver doesn't work on powerpc architecture due to big endian swap. I've tested it with the MV6440 chip.
Here you can find the patch for Linux 2.6.39.1.
Cedric Cano
Signed-off-by Cedric Cano<[email protected]>
---
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 line 1152
Quoted 10 lines
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +7 -0
@@ -1143,6 +1143,14 @@
       MVS_CHIP_DISP->write_port_cfg_addr(mvi, i, PHYR_SATA_SIG0);
       s[0] = MVS_CHIP_DISP->read_port_cfg_data(mvi, i);
 
+    /* swap for big endian devices because of use of these data in bytes */
+#ifdef __BIG_ENDIAN
+    s[0] = swab32(s[0]);
+    s[1] = swab32(s[1]);
+    s[2] = swab32(s[2]);
+    s[3] = swab32(s[3]);
+#endif /* __BIG_ENDIAN */
Hi Cedric !
Thanks for this. CC'ing linux-scsi.
Note that it would probably have been better to use le32_to_cpu, which avoids the ifdef completely.
Cheers,
Ben.
Quoted 9 lines
scsi/mvsas/mv_sas.c 2011-06-10 15:50:12.000000000 +0200 renamed from scsi/mvsas/mv_sas.c 2011-06-10 15:50:04.000000000 +0200 +0 -1
       /* Workaround: take some ATAPI devices for ATA */
       if (((s[1]&  0x00FFFFFF) == 0x00EB1401)&&  (*(u8 *)&s[3] == 0x01))
           s[1] = 0x00EB1401 | (*((u8 *)&s[1] + 3)&  0x10);
 
---
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev