[U-Boot] (no subject)

[U-Boot] (no subject)

[Daniel Mack]

The following patch series is needed to build U-Boot for ARM platforms
with CONFIG_CMD_UBIFS set. The UBIFS layer uses bit operation functions
which are currently only implemented for PPC, and the ARM bit operation
definitions are unused and wrong.

[PATCH 1/3] ARM: remove unused bit operations
[PATCH 2/3] Add generic bit operations
[PATCH 3/3] ARM: add unaligned macros

[U-Boot] [PATCH 1/3] ARM: remove unused bit operations

[Daniel Mack]

Remove include/asm-arm/bitops.h a bunch of 'external' marked functions
from include/asm-arm/bitops.h. They are not implemented anywhere in the
sources, so this forward declaration is wrong.

Also remove the functions __set_bit, __clear_bit, __change_bit,
__test_and_set_bit, __test_and_clear_bit and __test_and_change_bit.

All these functions can be implemented in a generic fashion which will
be done in the next patch.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 include/asm-arm/bitops.h |   70 ----------------------------------------------
 1 files changed, 0 insertions(+), 70 deletions(-)

diff --git a/include/asm-arm/bitops.h b/include/asm-arm/bitops.h
index 4b8bab2..3ffd4d5 100644
--- a/include/asm-arm/bitops.h
+++ b/include/asm-arm/bitops.h
@@ -20,76 +20,6 @@
 #define smp_mb__before_clear_bit()	do { } while (0)
 #define smp_mb__after_clear_bit()	do { } while (0)
 
-/*
- * Function prototypes to keep gcc -Wall happy.
- */
-extern void set_bit(int nr, volatile void * addr);
-
-static inline void __set_bit(int nr, volatile void *addr)
-{
-	((unsigned char *) addr)[nr >> 3] |= (1U << (nr & 7));
-}
-
-extern void clear_bit(int nr, volatile void * addr);
-
-static inline void __clear_bit(int nr, volatile void *addr)
-{
-	((unsigned char *) addr)[nr >> 3] &= ~(1U << (nr & 7));
-}
-
-extern void change_bit(int nr, volatile void * addr);
-
-static inline void __change_bit(int nr, volatile void *addr)
-{
-	((unsigned char *) addr)[nr >> 3] ^= (1U << (nr & 7));
-}
-
-extern int test_and_set_bit(int nr, volatile void * addr);
-
-static inline int __test_and_set_bit(int nr, volatile void *addr)
-{
-	unsigned int mask = 1 << (nr & 7);
-	unsigned int oldval;
-
-	oldval = ((unsigned char *) addr)[nr >> 3];
-	((unsigned char *) addr)[nr >> 3] = oldval | mask;
-	return oldval & mask;
-}
-
-extern int test_and_clear_bit(int nr, volatile void * addr);
-
-static inline int __test_and_clear_bit(int nr, volatile void *addr)
-{
-	unsigned int mask = 1 << (nr & 7);
-	unsigned int oldval;
-
-	oldval = ((unsigned char *) addr)[nr >> 3];
-	((unsigned char *) addr)[nr >> 3] = oldval & ~mask;
-	return oldval & mask;
-}
-
-extern int test_and_change_bit(int nr, volatile void * addr);
-
-static inline int __test_and_change_bit(int nr, volatile void *addr)
-{
-	unsigned int mask = 1 << (nr & 7);
-	unsigned int oldval;
-
-	oldval = ((unsigned char *) addr)[nr >> 3];
-	((unsigned char *) addr)[nr >> 3] = oldval ^ mask;
-	return oldval & mask;
-}
-
-extern int find_first_zero_bit(void * addr, unsigned size);
-extern int find_next_zero_bit(void * addr, int size, int offset);
-
-/*
- * This routine doesn't need to be atomic.
- */
-static inline int test_bit(int nr, const void * addr)
-{
-    return ((unsigned char *) addr)[nr >> 3] & (1U << (nr & 7));
-}
 
 /*
  * ffz = Find First Zero in word. Undefined if no zero exists,
-- 
1.6.3.1

[U-Boot] [PATCH 2/3] Add generic bit operations

[Daniel Mack]

This adds generic bit operations for all platforms and enables includes
the implementations from asm-arm.

Code taken from Linux kernel.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 include/asm-arm/bitops.h     |    2 +
 include/asm-generic/bitops.h |  151 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-generic/bitops.h

diff --git a/include/asm-arm/bitops.h b/include/asm-arm/bitops.h
index 3ffd4d5..97abea6 100644
--- a/include/asm-arm/bitops.h
+++ b/include/asm-arm/bitops.h
@@ -15,6 +15,8 @@
 #ifndef __ASM_ARM_BITOPS_H
 #define __ASM_ARM_BITOPS_H
 
+#include "asm-generic/bitops.h"
+
 #ifdef __KERNEL__
 
 #define smp_mb__before_clear_bit()	do { } while (0)
diff --git a/include/asm-generic/bitops.h b/include/asm-generic/bitops.h
new file mode 100644
index 0000000..088e5da
--- /dev/null
+++ b/include/asm-generic/bitops.h
@@ -0,0 +1,151 @@
+#ifndef _ASM_GENERIC_BITOPS_H_
+#define _ASM_GENERIC_BITOPS_H_
+
+#include <asm/types.h>
+
+#define BIT(nr)			(1UL << (nr))
+#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
+#define BITS_PER_BYTE		8
+#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+
+/**
+ * set_bit - Set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * Unlike set_bit(), this function is non-atomic and may be reordered.
+ * If it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ */
+static inline void set_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+	*p  |= mask;
+}
+
+static inline void clear_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+	*p &= ~mask;
+}
+
+/**
+ * change_bit - Toggle a bit in memory
+ * @nr: the bit to change
+ * @addr: the address to start counting from
+ *
+ * Unlike change_bit(), this function is non-atomic and may be reordered.
+ * If it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ */
+static inline void change_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+	*p ^= mask;
+}
+
+/**
+ * test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic and can be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail.  You must protect multiple accesses with a lock.
+ */
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old | mask;
+	return (old & mask) != 0;
+}
+
+/**
+ * test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic and can be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail.  You must protect multiple accesses with a lock.
+ */
+static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old & ~mask;
+	return (old & mask) != 0;
+}
+
+/* WARNING: non atomic and it can be reordered! */
+static inline int test_and_change_bit(int nr,
+				    volatile unsigned long *addr)
+{
+	unsigned long mask = BIT_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old ^ mask;
+	return (old & mask) != 0;
+}
+
+/**
+ * test_bit - Determine whether a bit is set
+ * @nr: bit number to test
+ * @addr: Address to start counting from
+ */
+static inline int test_bit(int nr, const volatile unsigned long *addr)
+{
+	return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
+}
+
+/**
+ * fls - find last (most-significant) bit set
+ * @x: the word to search
+ *
+ * This is defined the same way as ffs.
+ * Note fls(0) = 0, fls(1) = 1, fls(0x80000000) = 32.
+ */
+
+static inline int fls(int x)
+{
+	int r = 32;
+
+	if (!x)
+		return 0;
+	if (!(x & 0xffff0000u)) {
+		x <<= 16;
+		r -= 16;
+	}
+	if (!(x & 0xff000000u)) {
+		x <<= 8;
+		r -= 8;
+	}
+	if (!(x & 0xf0000000u)) {
+		x <<= 4;
+		r -= 4;
+	}
+	if (!(x & 0xc0000000u)) {
+		x <<= 2;
+		r -= 2;
+	}
+	if (!(x & 0x80000000u)) {
+		x <<= 1;
+		r -= 1;
+	}
+	return r;
+}
+
+#endif /* _ASM_GENERIC_BITOPS_H_ */
-- 
1.6.3.1

[U-Boot] [PATCH 3/3] ARM: add unaligned macros

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 include/asm-arm/unaligned.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-arm/unaligned.h

diff --git a/include/asm-arm/unaligned.h b/include/asm-arm/unaligned.h
new file mode 100644
index 0000000..dd7d852
--- /dev/null
+++ b/include/asm-arm/unaligned.h
@@ -0,0 +1,14 @@
+#ifndef _ASM_ARM_UNALIGNED_H
+#define _ASM_ARM_UNALIGNED_H
+
+#ifdef __KERNEL__
+
+#include <linux/unaligned/access_ok.h>
+#include <linux/unaligned/generic.h>
+
+#define get_unaligned	__get_unaligned_le
+#define put_unaligned	__put_unaligned_le
+
+#endif	/* __KERNEL__ */
+#endif	/* _ASM_ARM_UNALIGNED_H */
+
-- 
1.6.3.1

[U-Boot] [PATCH 2/3] Add generic bit operations

[Wolfgang Denk]

Dear Daniel Mack,

In message <1244111241-32735-3-git-send-email-daniel@caiaq.de> you wrote:
> This adds generic bit operations for all platforms and enables includes
> the implementations from asm-arm.

Be careful. I am not sure if a generic definition is even possible.
In any case, it is extremely risky to use in the wrong way.

It's nothing but faux ami.

My recommendation is NEVER to use any code based on bit numbers (even
if this is what some chip documentation may refer to)  but  ONLY  use
appropriate masks instead.

> new file mode 100644
> index 0000000..088e5da
> --- /dev/null
> +++ b/include/asm-generic/bitops.h
> @@ -0,0 +1,151 @@
> +#ifndef _ASM_GENERIC_BITOPS_H_
> +#define _ASM_GENERIC_BITOPS_H_
> +
> +#include <asm/types.h>
> +
> +#define BIT(nr)			(1UL << (nr))
> +#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
> +#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
> +#define BITS_PER_BYTE		8
> +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))

You see, this is plain wrong on PowerPC.

On PowerPC, bit number 0 is the most significant bit, not the least
significant one as you assume here. So using this well-intended code
on a PowerPC system will most likely get you in trouble.

[And you cannot even use a generic definition for PowerPC,  as  some-
thing  like  "#define BIT(nr) (0x80000000 >> (nr))" will only work on
32 bit systems but be wrong on 64 bit ones.]


Let's get rid of this stuff, it is confusing.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The one charm of marriage is that it makes a  life  of  deception  a
neccessity."                                            - Oscar Wilde

[U-Boot] [PATCH 2/3] Add generic bit operations

[Wolfgang Denk]

Dear Daniel Mack,

In message <1244111241-32735-3-git-send-email-daniel@caiaq.de> you wrote:
> This adds generic bit operations for all platforms and enables includes
> the implementations from asm-arm.

I should have read the patch to end...

> +static inline void set_bit(int nr, volatile unsigned long *addr)
> +{
> +	unsigned long mask = BIT_MASK(nr);
> +	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> +
> +	*p  |= mask;
> +}
> +
> +static inline void clear_bit(int nr, volatile unsigned long *addr)
> +{
> +	unsigned long mask = BIT_MASK(nr);
> +	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> +
> +	*p &= ~mask;
> +}

Such code has no chance of being accepted anyway.

It tries to be generic and does not care if you use it on memory or
device addresses - but for device addresses, the use of proper I/O
accessor functions is mandatory.


Full NAK.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I really hate this damned machine     It never does quite what I want
I wish that they would sell it.              But only what I tell it.

[U-Boot] [PATCH 2/3] Add generic bit operations

[Daniel Mack]

On Thu, Jun 04, 2009 at 01:45:22PM +0200, Wolfgang Denk wrote:
> > +#define BIT(nr)			(1UL << (nr))
> > +#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
> > +#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
> > +#define BITS_PER_BYTE		8
> > +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
> 
> You see, this is plain wrong on PowerPC.
> 
> On PowerPC, bit number 0 is the most significant bit, not the least
> significant one as you assume here. So using this well-intended code
> on a PowerPC system will most likely get you in trouble.

Well, the idea is to let those platforms use the generic operations that
do not bring their owm ones. The code above is not on use by PPC, so it
doesn't harm either.

> [And you cannot even use a generic definition for PowerPC,  as  some-
> thing  like  "#define BIT(nr) (0x80000000 >> (nr))" will only work on
> 32 bit systems but be wrong on 64 bit ones.]

That is why PPC implements that in its own fashion. Fair enough.

> Let's get rid of this stuff, it is confusing.

Hmm, and how?

Daniel

[U-Boot] [PATCH 2/3] Add generic bit operations

[Daniel Mack]

On Thu, Jun 04, 2009 at 01:47:17PM +0200, Wolfgang Denk wrote:
> > +static inline void clear_bit(int nr, volatile unsigned long *addr)
> > +{
> > +	unsigned long mask = BIT_MASK(nr);
> > +	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> > +
> > +	*p &= ~mask;
> > +}
> 
> Such code has no chance of being accepted anyway.
> 
> It tries to be generic and does not care if you use it on memory or
> device addresses - but for device addresses, the use of proper I/O
> accessor functions is mandatory.

And the functions I removed from asm-arm/bitops.h did that?

[U-Boot] [PATCH 2/3] Add generic bit operations

[Wolfgang Denk]

Dear Daniel Mack,

In message <20090604114818.GH26688@buzzloop.caiaq.de> you wrote:
>
> > > +#define BIT(nr)			(1UL << (nr))
> > > +#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
> > > +#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
> > > +#define BITS_PER_BYTE		8
> > > +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
> > 
> > You see, this is plain wrong on PowerPC.
> > 
> > On PowerPC, bit number 0 is the most significant bit, not the least
> > significant one as you assume here. So using this well-intended code
> > on a PowerPC system will most likely get you in trouble.
> 
> Well, the idea is to let those platforms use the generic operations that
> do not bring their owm ones. The code above is not on use by PPC, so it
> doesn't harm either.

But it is not a generic operation. The notion of "bit number" is a
generic concept, but here we have a machine dependent implementation
that sails under the name "asm-generic/bitops.h".

But it is NOT generic.

> > Let's get rid of this stuff, it is confusing.
> 
> Hmm, and how?

Just don't use it. Use masks instead of bit numbers.

What's wrong with using 0x00008000 instead of BIT(15) (which would be
0x00010000 on 32 bit Power systems).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Prof:        So the American government went to IBM to come up with a
             data encryption standard and they came up with ...
Student:     EBCDIC!

[U-Boot] [PATCH 3/3] ARM: add unaligned macros

[Daniel Mack]

On Thu, Jun 04, 2009 at 12:27:21PM +0200, Daniel Mack wrote:
> ---
>  include/asm-arm/unaligned.h |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>  create mode 100644 include/asm-arm/unaligned.h

This one was too easy, updated patch below.

With that one applied, the lzo1x decompressor and the whole ubifs works
fine on an ARM PXA3xx.

Daniel

[U-Boot] [PATCH 2/3] Add generic bit operations

[Daniel Mack]

On Thu, Jun 04, 2009 at 01:59:22PM +0200, Wolfgang Denk wrote:
> > And the functions I removed from asm-arm/bitops.h did that?
> 
> No. Let's be happy that we have eliminated some poor code, and if we
> add a replacement, let's make sure not to repeat the mistakes of the
> past again.

Ok. I just saw ubifs implementing its own set_bit() functions and
considered that the wrong place for such functions to live in. ext2
seems to do the same things, also minix. Platforms which want to
enable support for these filesystems have to do an evil #define to
map ext2_set_bit() to the platform specific version. Which is all
bogus, you might agree. And because of that situation, ubifs can't
currently build for anything else than ppc
(according to 'git grep -w fls include/asm*').

Hence I thought it might be a good idea (or at least a good start
thereof) to put the functions where I believe they belong to - a
'generic' place.

Anyway - applying the first patch of this series would at least prevent
others from being mislead by dead and wrong code.

Daniel

[U-Boot] [PATCH 2/3] Add generic bit operations

[Stefan Roese]

On Thursday 04 June 2009 20:00:42 Daniel Mack wrote:
> On Thu, Jun 04, 2009 at 01:59:22PM +0200, Wolfgang Denk wrote:
> > > And the functions I removed from asm-arm/bitops.h did that?
> >
> > No. Let's be happy that we have eliminated some poor code, and if we
> > add a replacement, let's make sure not to repeat the mistakes of the
> > past again.
>
> Ok. I just saw ubifs implementing its own set_bit() functions and
> considered that the wrong place for such functions to live in. ext2
> seems to do the same things, also minix. Platforms which want to
> enable support for these filesystems have to do an evil #define to
> map ext2_set_bit() to the platform specific version. Which is all
> bogus, you might agree.

Ack. It would a good thing to have those functions available for all 
architectures so that common code like UBIFS etc can use it. Please note that 
UBIFS uses these bit functions for memory operations only (not IO access). So 
we don't have to use accessor functions etc here. The "standard" Linux 
implementation should do.

> And because of that situation, ubifs can't
> currently build for anything else than ppc
> (according to 'git grep -w fls include/asm*').
>
> Hence I thought it might be a good idea (or at least a good start
> thereof) to put the functions where I believe they belong to - a
> 'generic' place.

Yes, I do like this approach. And from my point of view, we should continue 
this way.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 3/3] ARM: add unaligned macros

[Wolfgang Denk]

Dear Daniel Mack,

In message <20090604174208.GM26688@buzzloop.caiaq.de> you wrote:
>
> +static inline u16 get_unaligned_le16(const void *p)
> +{
> +	return __get_unaligned_cpu16((const u8 *)p);
> +}
> +
> +static inline u32 get_unaligned_le32(const void *p)
> +{
> +	return __get_unaligned_cpu32((const u8 *)p);
> +}
> +
> +static inline u64 get_unaligned_le64(const void *p)
> +{
> +	return __get_unaligned_cpu64((const u8 *)p);
> +}

Are these 3 really all "u8" pointers, or is this a copy & paste error?


Is there any guarantee that such macros are never used on device
registers and the like?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"It is better to have tried and failed than to have  failed  to  try,
but the result's the same."                           - Mike Dennison

[U-Boot] [PATCH 2/3] Add generic bit operations

[Wolfgang Denk]

Dear Daniel Mack,

In message <20090604180042.GN26688@buzzloop.caiaq.de> you wrote:
>
> Ok. I just saw ubifs implementing its own set_bit() functions and
> considered that the wrong place for such functions to live in. ext2
> seems to do the same things, also minix. Platforms which want to
> enable support for these filesystems have to do an evil #define to
> map ext2_set_bit() to the platform specific version. Which is all
> bogus, you might agree. And because of that situation, ubifs can't
> currently build for anything else than ppc
> (according to 'git grep -w fls include/asm*').

Yes, we have a mess here, and we need toclean this up ASAP.

I'm trying to find out if there is any agreement what a portable set
of I/O and bit macro definitions should look like.

> Anyway - applying the first patch of this series would at least prevent
> others from being mislead by dead and wrong code.

ACK on that.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are three ways to get something  done:  do  it  yourself,  hire
someone, or forbid your kids to do it.

[U-Boot] [PATCH 3/3] ARM: add unaligned macros

[Daniel Mack]

On Thu, Jun 04, 2009 at 09:03:47PM +0200, Wolfgang Denk wrote:
> > +static inline u16 get_unaligned_le16(const void *p)
> > +{
> > +	return __get_unaligned_cpu16((const u8 *)p);
> > +}
> > +
> > +static inline u32 get_unaligned_le32(const void *p)
> > +{
> > +	return __get_unaligned_cpu32((const u8 *)p);
> > +}
> > +
> > +static inline u64 get_unaligned_le64(const void *p)
> > +{
> > +	return __get_unaligned_cpu64((const u8 *)p);
> > +}
> 
> Are these 3 really all "u8" pointers, or is this a copy & paste error?

Yes, this is how it came from the Linux kernel and my tests show that
these access methods work well.

> Is there any guarantee that such macros are never used on device
> registers and the like?

Well - how can I guarantee that? Anyway - the functions can be enhanced
later to make them work with different types of memories. For now, they
implement a working set of functions to allow ubifs (and probably other
code as well) to be compiled and ran on ARMs.

Daniel

[U-Boot] [PATCH 3/3] ARM: add unaligned macros

[Stefan Roese]

On Thursday 04 June 2009 21:23:31 Daniel Mack wrote:
> > Is there any guarantee that such macros are never used on device
> > registers and the like?
>
> Well - how can I guarantee that? Anyway - the functions can be enhanced
> later to make them work with different types of memories. For now, they
> implement a working set of functions to allow ubifs (and probably other
> code as well) to be compiled and ran on ARMs.

Yes. I suggest that we just document that these functions (and the 
set_bit()... ones) don't implement any memory barriers/sync operations and 
therefore should be handled with care when used on IO registers etc (on 
platforms that need such barriers like PPC).

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 2/3] Add generic bit operations

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:27 Thu 04 Jun     , Daniel Mack wrote:
> This adds generic bit operations for all platforms and enables includes
> the implementations from asm-arm.
> 
> Code taken from Linux kernel.
> 
the __set_bit, __clear_bit, __change_bit & co generic is used on arm ok

but be aware that we use it in U-Boot so NACK

and please do not rename __xxx by xxx they do not mean the same think

the __xxx can be re-ordered and the xxx not

IMHO please keep the linux naming to simplify think in the futur
and please specify which file it's import and which kernel version

Best Regards,
J.

[U-Boot] [PATCH 2/3] Add generic bit operations

[Daniel Mack]

On Fri, Jun 05, 2009 at 10:44:21PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > This adds generic bit operations for all platforms and enables includes
> > the implementations from asm-arm.
> > 
> > Code taken from Linux kernel.
> > 
> the __set_bit, __clear_bit, __change_bit & co generic is used on arm ok

Do you mean it worked before? Or are you referring to the version I
posted which you think is ok?

> but be aware that we use it in U-Boot so NACK

You use what? The functions I removed? I doubt that. The functions
marked 'external' have no implementation, and I also don't believe the
static inlines did the right thing (note the shift operations).

> and please do not rename __xxx by xxx they do not mean the same think
> 
> the __xxx can be re-ordered and the xxx not

Sorry, I don't get it - what's your point about the status quo in the
sources and about the patches?

Daniel