Re: [PATCH 1/2] byteorder: add a new include/linux/swab.h to define byteswapping functions

Previous thread: [PATCH] cred: remove const qualifiers by Harvey Harrison on Saturday, August 9, 2008 - 4:02 pm. (4 messages)

Next thread: [PATCH 2/2] byteorder: add include/linux/byteorder.h to define endian helpers by Harvey Harrison on Saturday, August 9, 2008 - 4:24 pm. (1 message)
From: Harvey Harrison
Date: Saturday, August 9, 2008 - 4:23 pm

Collect the implementations from include/linux/byteorder/swab.h, swabb.h
in swab.h

The functionality provided covers:
u16 swab16(u16 val) - return a byteswapped 16 bit value
u32 swab32(u32 val) - return a byteswapped 32 bit value
u64 swab64(u64 val) - return a byteswapped 64 bit value
u32 swahw32(u32 val) - return a wordswapped 32 bit value
u32 swahb32(u32 val) - return a high/low byteswapped 32 bit value

Similar to above, but return swapped value from a naturally-aligned pointer
u16 swab16p(u16 *p)
u32 swab32p(u32 *p)
u64 swab64p(u64 *p)
u32 swahw32p(u32 *p)
u32 swahb32p(u32 *p)

Similar to above, but swap the value in-place (in-situ)
void swab16s(u16 *p)
void swab32s(u32 *p)
void swab64s(u64 *p)
void swahw32s(u32 *p)
void swahb32s(u32 *p)

Arches can override any of these with an optimized version by defining an
inline in their asm/byteorder.h (example given for swab16()):

u16 __arch_swab16() {}
 #define __arch_swab16 __arch_swab16

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Linus, please apply so I can start getting each arch moved over to the
consolidated implementation.  This is an opt-in process to avoid breakage,
the old implementation will be removed _after_ all arches have moved over.

 include/linux/swab.h |  309 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 309 insertions(+), 0 deletions(-)

diff --git a/include/linux/swab.h b/include/linux/swab.h
new file mode 100644
index 0000000..270d5c2
--- /dev/null
+++ b/include/linux/swab.h
@@ -0,0 +1,309 @@
+#ifndef _LINUX_SWAB_H
+#define _LINUX_SWAB_H
+
+#include <linux/types.h>
+#include <linux/compiler.h>
+#include <asm/byteorder.h>
+
+/*
+ * casts are necessary for constants, because we never know how for sure
+ * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
+ */
+#define __const_swab16(x) ((__u16)(				\
+	(((__u16)(x) & (__u16)0x00ffU) << 8) |			\
+	(((__u16)(x) & (__u16)0xff00U) >> 8)))
+
+#define __const_swab32(x) ...
From: Hans Verkuil
Date: Saturday, August 30, 2008 - 5:36 am

Hi Harvey,


Now that this is merged into 2.6.27-rc5 I'm getting broken builds for 
linux/drivers/media/dvb/ttpci/av7110.c. It includes 
<linux/byteorder/swabb.h>. Building on the arm gives this error:

In file included from /marune/build/v4l-dvb/v4l/av7110.c:39:
include/linux/byteorder/swabb.h:82:1: warning: "__swahw32" redefined
In file included from include/linux/byteorder.h:5,
                 
from /marune/build/trees/armv5-ixp/linux-2.6.27-rc5/arch/arm/include/asm/byteorder.h:53,
                 from include/linux/kernel.h:19,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from /marune/build/v4l-dvb/v4l/av7110.c:33:
include/linux/swab.h:138:1: warning: this is the location of the 
previous definition
In file included from /marune/build/v4l-dvb/v4l/av7110.c:39:
include/linux/byteorder/swabb.h:86:1: warning: "__swahb32" redefined
In file included from include/linux/byteorder.h:5,
                 
from /marune/build/trees/armv5-ixp/linux-2.6.27-rc5/arch/arm/include/asm/byteorder.h:53,
                 from include/linux/kernel.h:19,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from /marune/build/v4l-dvb/v4l/av7110.c:33:
include/linux/swab.h:149:1: warning: this is the location of the 
previous definition
In file included from /marune/build/v4l-dvb/v4l/av7110.c:39:
include/linux/byteorder/swabb.h:97: error: conflicting types 
for '__swahw32p'
include/linux/swab.h:200: error: previous definition of '__swahw32p' was 
here
include/linux/byteorder/swabb.h:102: error: redefinition of '__swahw32s'
include/linux/swab.h:268: error: previous definition of '__swahw32s' was 
here
include/linux/byteorder/swabb.h:112: error: conflicting types 
for ...
From: Harvey Harrison
Date: Tuesday, September 2, 2008 - 11:35 am

From: Harvey Harrison <harvey.harrison@gmail.com>
Subject: [buildfix-PATCH] byteorder: remove direct includes of linux/byteorder/swabb.h

All of the functionality has been collected in linux/swab.h

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/media/dvb/ttpci/av7110.c |    2 +-
 kernel/rcupreempt.c              |    2 +-
 tests/rcutorture.c               |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/ttpci/av7110.c b/drivers/media/dvb/ttpci/av7110.c
index c0a1746..792a175 100644
--- a/drivers/media/dvb/ttpci/av7110.c
+++ b/drivers/media/dvb/ttpci/av7110.c
@@ -36,7 +36,7 @@
 #include <linux/fs.h>
 #include <linux/timer.h>
 #include <linux/poll.h>
-#include <linux/byteorder/swabb.h>
+#include <linux/swab.h>
 #include <linux/smp_lock.h>
 
 #include <linux/kernel.h>
diff --git a/kernel/rcupreempt.c b/kernel/rcupreempt.c
index ca4bbbe..494d0e8 100644
--- a/kernel/rcupreempt.c
+++ b/kernel/rcupreempt.c
@@ -54,7 +54,7 @@
 #include <linux/cpu.h>
 #include <linux/random.h>
 #include <linux/delay.h>
-#include <linux/byteorder/swabb.h>
+#include <linux/swab.h>
 #include <linux/cpumask.h>
 #include <linux/rcupreempt_trace.h>
 
diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 90b5b12..67856af 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -42,7 +42,7 @@
 #include <linux/freezer.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
-#include <linux/byteorder/swabb.h>
+#include <linux/swab.h>
 #include <linux/stat.h>
 #include <linux/srcu.h>
 #include <linux/slab.h>
-- 
1.6.0.1.400.ga23d3



--

From: Harvey Harrison
Date: Tuesday, September 2, 2008 - 11:50 am

All of the functionality has been collected in linux/swab.h

Needed to prevent build brakage as arches start moving over
to the new byteorder implementation.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Sorry, I only sent you half-a-patch in my last message, this is the
full patch needed.

 drivers/media/dvb/ttpci/av7110.c       |    2 +-
 drivers/media/video/cx18/cx18-driver.h |    2 +-
 drivers/media/video/ivtv/ivtv-driver.h |    2 +-
 drivers/media/video/vpx3220.c          |    2 +-
 kernel/rcupreempt.c                    |    2 +-
 tests/rcutorture.c                     |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb/ttpci/av7110.c
b/drivers/media/dvb/ttpci/av7110.c
index c0a1746..792a175 100644
--- a/drivers/media/dvb/ttpci/av7110.c
+++ b/drivers/media/dvb/ttpci/av7110.c
@@ -36,7 +36,7 @@
 #include <linux/fs.h>
 #include <linux/timer.h>
 #include <linux/poll.h>
-#include <linux/byteorder/swabb.h>
+#include <linux/swab.h>
 #include <linux/smp_lock.h>
 
 #include <linux/kernel.h>
diff --git a/drivers/media/video/cx18/cx18-driver.h
b/drivers/media/video/cx18/cx18-driver.h
index 2635989..8787a5f 100644
--- a/drivers/media/video/cx18/cx18-driver.h
+++ b/drivers/media/video/cx18/cx18-driver.h
@@ -38,7 +38,7 @@
 #include <linux/i2c-algo-bit.h>
 #include <linux/list.h>
 #include <linux/unistd.h>
-#include <linux/byteorder/swab.h>
+#include <linux/swab.h>
 #include <linux/pagemap.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
diff --git a/drivers/media/video/ivtv/ivtv-driver.h
b/drivers/media/video/ivtv/ivtv-driver.h
index 2ceb522..2213473 100644
--- a/drivers/media/video/ivtv/ivtv-driver.h
+++ b/drivers/media/video/ivtv/ivtv-driver.h
@@ -49,7 +49,7 @@
 #include <linux/i2c-algo-bit.h>
 #include <linux/list.h>
 #include <linux/unistd.h>
-#include <linux/byteorder/swab.h>
+#include <linux/swab.h>
 #include <linux/pagemap.h>
 #include <linux/scatterlist.h>
 #include ...
From: Hans Verkuil
Date: Tuesday, September 2, 2008 - 12:42 pm

Hi Harvey,


1) With this patch the i686, x86_64 and powerpc64 build fail on 
av7110.c, as I said in my original mail. Do you have additional patches 
that address this?

2) For the cx18, ivtv and vpx3220 please just remove the swabb.h 
include: it's either not really needed or included through another 
include. The v4l-dvb master repository has been updated to just remove 
these linux/byteorder/swabb.h includes in these three sources.

Regards,



--

From: Harvey Harrison
Date: Tuesday, September 2, 2008 - 4:11 pm

All of the functionality has been collected in linux/swab.h

Needed to prevent build breakage as arches start moving over
to the new byteorder implementation. Ensure that each implementation
offers all byte-swapping methods to avoid build breakage when only
some arches are converted.

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Tested on a converted(ARM) and an unconverted(x86) arch.

 drivers/media/dvb/ttpci/av7110.c        |    2 +-
 drivers/media/video/cx18/cx18-driver.h  |    2 +-
 drivers/media/video/ivtv/ivtv-driver.h  |    2 +-
 drivers/media/video/vpx3220.c           |    3 +--
 include/linux/byteorder/big_endian.h    |    1 +
 include/linux/byteorder/little_endian.h |    1 +
 kernel/rcupreempt.c                     |    2 +-
 tests/rcutorture.c                      |    2 +-
 8 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb/ttpci/av7110.c b/drivers/media/dvb/ttpci/av7110.c
index c0a1746..deb3829 100644
--- a/drivers/media/dvb/ttpci/av7110.c
+++ b/drivers/media/dvb/ttpci/av7110.c
@@ -36,7 +36,6 @@
 #include <linux/fs.h>
 #include <linux/timer.h>
 #include <linux/poll.h>
-#include <linux/byteorder/swabb.h>
 #include <linux/smp_lock.h>
 
 #include <linux/kernel.h>
@@ -52,6 +51,7 @@
 #include <linux/i2c.h>
 #include <linux/kthread.h>
 #include <asm/unaligned.h>
+#include <asm/byteorder.h>
 
 #include <asm/system.h>
 
diff --git a/drivers/media/video/cx18/cx18-driver.h b/drivers/media/video/cx18/cx18-driver.h
index 2635989..bd94d77 100644
--- a/drivers/media/video/cx18/cx18-driver.h
+++ b/drivers/media/video/cx18/cx18-driver.h
@@ -38,10 +38,10 @@
 #include <linux/i2c-algo-bit.h>
 #include <linux/list.h>
 #include <linux/unistd.h>
-#include <linux/byteorder/swab.h>
 #include <linux/pagemap.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <asm/byteorder.h>
 
 #include <linux/dvb/video.h>
 #include <linux/dvb/audio.h>
diff --git ...
Previous thread: [PATCH] cred: remove const qualifiers by Harvey Harrison on Saturday, August 9, 2008 - 4:02 pm. (4 messages)

Next thread: [PATCH 2/2] byteorder: add include/linux/byteorder.h to define endian helpers by Harvey Harrison on Saturday, August 9, 2008 - 4:24 pm. (1 message)