[PATCHv4 6/6] byteorder: add copy_{endian} helpers

[PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Harvey Harrison]

Add helpers for the idiom:
*(__le16 *)ptr = cpu_to_le16(val);

Can now be written as:
copy_le16(ptr, val);

Implemented as macros to allow val to be byteswapped at compile-time
when it is a constant.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 include/linux/byteorder.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/byteorder.h b/include/linux/byteorder.h
index b4713ce..8d3c847 100644
--- a/include/linux/byteorder.h
+++ b/include/linux/byteorder.h
@@ -278,6 +278,17 @@ static inline __be64 __cpu_to_be64p(const __u64 *p)
 # define htons(x) ___htons(x)
 # define ntohs(x) ___ntohs(x)
 
+/*
+ * Defined as macros to allow constant folding of the cpu_to_XXXX when
+ * possible.
+ */
+#define copy_le16(ptr, val)	(*(__le16 *)(ptr) = cpu_to_le16((u16)(val));)
+#define copy_le32(ptr, val)	(*(__le32 *)(ptr) = cpu_to_le32((u32)(val));)
+#define copy_le64(ptr, val)	(*(__le64 *)(ptr) = cpu_to_le64((u64)(val));)
+#define copy_be16(ptr, val)	(*(__be16 *)(ptr) = cpu_to_be16((u16)(val));)
+#define copy_be32(ptr, val)	(*(__be32 *)(ptr) = cpu_to_be32((u32)(val));)
+#define copy_be64(ptr, val)	(*(__be64 *)(ptr) = cpu_to_be64((u64)(val));)
+
 static inline void le16_add_cpu(__le16 *var, u16 val)
 {
 	*var = cpu_to_le16(le16_to_cpup(var) + val);
-- 
1.5.6.rc0.277.g804cf

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Geert Uytterhoeven]

On Mon, 2 Jun 2008, Harvey Harrison wrote:
> Add helpers for the idiom:
> *(__le16 *)ptr = cpu_to_le16(val);
> 
> Can now be written as:
> copy_le16(ptr, val);

Copy a little endian 16-bit value?

It doesn't just copy. It does a byteswap during the copy.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Harvey Harrison]

On Mon, 2008-06-02 at 21:15 +0200, Geert Uytterhoeven wrote:
> On Mon, 2 Jun 2008, Harvey Harrison wrote:
> > Add helpers for the idiom:
> > *(__le16 *)ptr = cpu_to_le16(val);
> > 
> > Can now be written as:
> > copy_le16(ptr, val);
> 
> Copy a little endian 16-bit value?
> 
> It doesn't just copy. It does a byteswap during the copy.
> 

True, I had originally called this put_le16.  akpm suggested copy_le16.
What do people think of copy_as_le16?

Harvey

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Geert Uytterhoeven]

On Wed, 4 Jun 2008, Harvey Harrison wrote:
> On Mon, 2008-06-02 at 21:15 +0200, Geert Uytterhoeven wrote:
> > On Mon, 2 Jun 2008, Harvey Harrison wrote:
> > > Add helpers for the idiom:
> > > *(__le16 *)ptr = cpu_to_le16(val);
> > > 
> > > Can now be written as:
> > > copy_le16(ptr, val);
> > 
> > Copy a little endian 16-bit value?
> > 
> > It doesn't just copy. It does a byteswap during the copy.
> 
> True, I had originally called this put_le16.  akpm suggested copy_le16.
> What do people think of copy_as_le16?

Sounds still a bit strange to me...

As `get' and `put' have connotations of reference counts, what about
`load' and `store', e.g. `load_le32()' and `store_be16()'?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Harvey Harrison]

On Wed, 2008-06-04 at 21:34 +0200, Geert Uytterhoeven wrote:
> On Wed, 4 Jun 2008, Harvey Harrison wrote:
> > On Mon, 2008-06-02 at 21:15 +0200, Geert Uytterhoeven wrote:
> > > On Mon, 2 Jun 2008, Harvey Harrison wrote:
> Sounds still a bit strange to me...
> 
> As `get' and `put' have connotations of reference counts, what about
> `load' and `store', e.g. `load_le32()' and `store_be16()'?
> 

Well, load is covered by le16_to_cpup and friends.

I could live with store_le16 though.  I had originally added get_le16,
put_le16 to match up with the get_unaligned_le16, put_unaligned_le16
functions.  Maybe these would have been better as load_unaligned_*
store_unaligned_* and during this change we could fix the argument
order to be (ptr, val) allowing a gradual changeover without a flag
day.

Andrew?  What do you think of load/store?

Harvey

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Russell King]

On Wed, Jun 04, 2008 at 12:39:45PM -0700, Harvey Harrison wrote:
> On Wed, 2008-06-04 at 21:34 +0200, Geert Uytterhoeven wrote:
> > On Wed, 4 Jun 2008, Harvey Harrison wrote:
> > > On Mon, 2008-06-02 at 21:15 +0200, Geert Uytterhoeven wrote:
> > > > On Mon, 2 Jun 2008, Harvey Harrison wrote:
> > Sounds still a bit strange to me...
> > 
> > As `get' and `put' have connotations of reference counts, what about
> > `load' and `store', e.g. `load_le32()' and `store_be16()'?
> > 
> 
> Well, load is covered by le16_to_cpup and friends.

What about the logical cpu_to_le16p then?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCHv4 6/6] byteorder: add copy_{endian} helpers

[Harvey Harrison]

On Thu, 2008-06-05 at 00:20 +0100, Russell King wrote:
> On Wed, Jun 04, 2008 at 12:39:45PM -0700, Harvey Harrison wrote:
> > On Wed, 2008-06-04 at 21:34 +0200, Geert Uytterhoeven wrote:
> > > On Wed, 4 Jun 2008, Harvey Harrison wrote:
> > > > On Mon, 2008-06-02 at 21:15 +0200, Geert Uytterhoeven wrote:
> > > > > On Mon, 2 Jun 2008, Harvey Harrison wrote:
> > > Sounds still a bit strange to me...
> > > 
> > > As `get' and `put' have connotations of reference counts, what about
> > > `load' and `store', e.g. `load_le32()' and `store_be16()'?
> > > 
> > 
> > Well, load is covered by le16_to_cpup and friends.
> 
> What about the logical cpu_to_le16p then?
> 

Already exists, but it is not like the proposed store_le16, it just
has the types reversed.

u16 le16_to_cpup(__le16 *p)
__le16 cpu_to_le16p(u16 *p)

Currently you see things like:

*(__le16 *)ptr = cpu_to_le16(val);

Hence my idea for a:
store_le16(ptr, val);

Harvey