Skip to contentSkip to conversation

staging: gdm724x: Remove test for host endian

PATCH In review Jaime Arrocha 3 participants 3 messages May 26, 2015 Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 staging: gdm724x: Remove test for host endian

This is the first patch of two. Both patches perform a small clean up done to the section for host endian test. Instead of handling endianness internally, kernel functions were added for use.
The second patch depends on the first one, it is just a small piece that is no longer needed.

---
 drivers/staging/gdm724x/gdm_endian.c |   52 +++++++++++++++-------------------
 1 file changed, 23 insertions(+), 29 deletions(-)
drivers/staging/gdm724x/gdm_endian.c +23 -29
index f6cc90a..609a433 100644
@@ -11,57 +11,51 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/slab.h>
+#include<asm/byteorder.h>
+#ifdef __LITTLE_ENDIAN
+#include<linux/byteorder/little_endian.h>
+#else
+#include<linux/byteorder/big_endian.h>
+#endif
+
 #include "gdm_endian.h"
 
 void gdm_set_endian(struct gdm_endian *ed, u8 dev_endian)
 {
-	u8 a[2] = {0x12, 0x34};
-	u8 b[2] = {0, };
-	u16 c = 0x1234;
-
 	if (dev_endian == ENDIANNESS_BIG)
 		ed->dev_ed = ENDIANNESS_BIG;
 	else
 		ed->dev_ed = ENDIANNESS_LITTLE;
-
-	memcpy(b, &c, 2);
-
-	if (a[0] != b[0])
-		ed->host_ed = ENDIANNESS_LITTLE;
-	else
-		ed->host_ed = ENDIANNESS_BIG;
-
 }
 
 u16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
 {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian16_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __cpu_to_le16(x);
+	else
+		return __cpu_to_be16(x);
 }
 
 u16 gdm_dev16_to_cpu(struct gdm_endian *ed, u16 x)
 {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian16_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __le16_to_cpu(x);
+	else
+		return __cpu_to_be16(x);
 }
 
 u32 gdm_cpu_to_dev32(struct gdm_endian *ed, u32 x)
 {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian32_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __cpu_to_le32(x);
+	else
+		return __cpu_to_be32(x);
 }
 
 u32 gdm_dev32_to_cpu(struct gdm_endian *ed, u32 x)
 {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian32_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __le32_to_cpu(x);
+	else
+		return __be32_to_cpu(x);
 }
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 6 lines · Jaime Arrocha

This is the first patch of two. Both patches perform a small clean up done to the section for host endian test. Instead of handling endianness internally, kernel functions were added for use.

The second patch depends on the first one, it is just a small piece that is no longer needed.

This kind of dependencies are built in the the name patch 1/2 and 2/2. We don't want this kind of meta commentary in the permanent changelog. If it were needed then it would go under the --- cut off line.

And anyway, fold patch 1 & 2 together into one patch. Do "one thing" per patch instead of half a thing per patch.

Quoted 3 lines

---

<--- meta commentary goes here.

drivers/staging/gdm724x/gdm_endian.c line 14

Quoted 12 lines
 drivers/staging/gdm724x/gdm_endian.c |   52 +++++++++++++++-------------------
 1 file changed, 23 insertions(+), 29 deletions(-)
drivers/staging/gdm724x/gdm_endian.c +0 -1
index f6cc90a..609a433 100644
@@ -11,57 +11,51 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/slab.h>

Is this related to endianness?

drivers/staging/gdm724x/gdm_endian.c line 19

Quoted 6 lines
drivers/staging/gdm724x/gdm_endian.c +6 -0
+#include<asm/byteorder.h>
+#ifdef __LITTLE_ENDIAN
+#include<linux/byteorder/little_endian.h>
+#else
+#include<linux/byteorder/big_endian.h>
+#endif

Why do we need this? Also the spacing is wrong.

drivers/staging/gdm724x/gdm_endian.c line 36

Quoted 10 lines
drivers/staging/gdm724x/gdm_endian.c +4 -4
 u16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
 {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian16_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __cpu_to_le16(x);
+	else
+		return __cpu_to_be16(x);

Use cpu_to_le16() no underscore versions everywhere. The other is for code which is shared with usespace.

regards,
dan carpenter

#2 lore raw
Quoted 8 lines · Dan Carpenter
Quoted 6 lines · Jaime Arrocha

This is the first patch of two. Both patches perform a small clean up done to the section for host endian test. Instead of handling endianness internally, kernel functions were added for use.

The second patch depends on the first one, it is just a small piece that is no longer needed.

Quoted 7 lines

This kind of dependencies are built in the the name patch 1/2 and 2/2. We don't want this kind of meta commentary in the permanent changelog. If it were needed then it would go under the --- cut off line.

And anyway, fold patch 1 & 2 together into one patch. Do "one thing" per patch instead of half a thing per patch.

Thanks for the feedback. I'll do the corrections needed.

Quoted 17 lines
Quoted 3 lines

---

<--- meta commentary goes here.

Quoted 11 lines
  drivers/staging/gdm724x/gdm_endian.c |   52 +++++++++++++++-------------------
  1 file changed, 23 insertions(+), 29 deletions(-)
drivers/staging/gdm724x/gdm_endian.c +0 -0
index f6cc90a..609a433 100644
@@ -11,57 +11,51 @@
   * GNU General Public License for more details.
   */
 
Quoted 4 lines

drivers/staging/gdm724x/gdm_endian.c line 14

Quoted 1 line
drivers/staging/gdm724x/gdm_endian.c +0 -1
-#include <linux/slab.h>

Is this related to endianness?

This was needed because the original code uses memcpy() inside gdm_set_endian() to test for host endianness.

Quoted 9 lines

drivers/staging/gdm724x/gdm_endian.c line 19

Quoted 6 lines
drivers/staging/gdm724x/gdm_endian.c +6 -0
+#include<asm/byteorder.h>
+#ifdef __LITTLE_ENDIAN
+#include<linux/byteorder/little_endian.h>
+#else
+#include<linux/byteorder/big_endian.h>
+#endif

Why do we need this? Also the spacing is wrong.

Yes,this needs to go. It was from an old change where I was obtaining host endianness. It will be replaced by linux/byteorder/generic.h

Quoted 18 lines

drivers/staging/gdm724x/gdm_endian.c line 36

Quoted 10 lines
drivers/staging/gdm724x/gdm_endian.c +4 -4
  u16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
  {
-	if (ed->dev_ed == ed->host_ed)
-		return x;
-
-	return Endian16_Swap(x);
+	if (ed->dev_ed == ENDIANNESS_LITTLE)
+		return __cpu_to_le16(x);
+	else
+		return __cpu_to_be16(x);

Use cpu_to_le16() no underscore versions everywhere. The other is for code which is shared with usespace.

regards,
dan carpenter