RFC: Fix big endian MMIO primitives.

RFC: Fix big endian MMIO primitives.

[Krzysztof Halasa]

cpu_read*() and cpu_wrire*() are precisely equal to __raw_read*() and
__raw_write*().

Striving for correctness we can replace all those __raw_*() with cpu_*()
as that's exactly what the former ones do. Also we can change read*()
and write*() into explicit leXX_*(). Both changes could affect porting
from Linux, though.

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/include/io.h b/include/io.h
index 39b5e61..8d885de 100644
--- a/include/io.h
+++ b/include/io.h
@@ -4,20 +4,11 @@
 #include <asm/io.h>
 
 /* cpu_read/cpu_write: cpu native io accessors */
-#if __BYTE_ORDER == __BIG_ENDIAN
-#define cpu_readb(a)		readb(a)
-#define cpu_readw(a)		in_be16(a)
-#define cpu_readl(a)		in_be32(a)
-#define cpu_writeb(v, a)	writeb((v), (a))
-#define cpu_writew(v, a)	out_be16((a), (v))
-#define cpu_writel(v, a)	out_be32((a), (v))
-#else
-#define cpu_readb(a)		readb(a)
-#define cpu_readw(a)		readw(a)
-#define cpu_readl(a)		readl(a)
-#define cpu_writeb(v, a)	writeb((v), (a))
-#define cpu_writew(v, a)	writew((v), (a))
-#define cpu_writel(v, a)	writel((v), (a))
-#endif
+#define cpu_readb(a)		__raw_readb(a)
+#define cpu_readw(a)		__raw_readw(a)
+#define cpu_readl(a)		__raw_readl(a)
+#define cpu_writeb(v, a)	__raw_writeb((v), (a))
+#define cpu_writew(v, a)	__raw_writew((v), (a))
+#define cpu_writel(v, a)	__raw_writel((v), (a))
 
 #endif /* __IO_H */

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: RFC: Fix big endian MMIO primitives.

[Sascha Hauer]

On Sat, May 05, 2012 at 11:34:13PM +0200, Krzysztof Halasa wrote:
> cpu_read*() and cpu_wrire*() are precisely equal to __raw_read*() and
> __raw_write*().
> 
> Striving for correctness we can replace all those __raw_*() with cpu_*()
> as that's exactly what the former ones do. Also we can change read*()
> and write*() into explicit leXX_*(). Both changes could affect porting
> from Linux, though.

Maybe we should rather use the __raw_* variants in the cfi driver aswell
which is the only user of the functions below. For some reason I
believed that the __raw_* variants also do little endian accesses which
is wrong.
I don't like the naming of the __raw_* variants very much as the
underscores and 'raw' suggests that these are internal functions which
one should rather not use, but in fact these are the correct functions
in most SoC (non PCI) drivers. Anyway, since Linux has this functions we
should use them aswell, everything else probably leads to more
confusion.

Sascha


> 
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>
> 
> diff --git a/include/io.h b/include/io.h
> index 39b5e61..8d885de 100644
> --- a/include/io.h
> +++ b/include/io.h
> @@ -4,20 +4,11 @@
>  #include <asm/io.h>
>  
>  /* cpu_read/cpu_write: cpu native io accessors */
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -#define cpu_readb(a)		readb(a)
> -#define cpu_readw(a)		in_be16(a)
> -#define cpu_readl(a)		in_be32(a)
> -#define cpu_writeb(v, a)	writeb((v), (a))
> -#define cpu_writew(v, a)	out_be16((a), (v))
> -#define cpu_writel(v, a)	out_be32((a), (v))
> -#else
> -#define cpu_readb(a)		readb(a)
> -#define cpu_readw(a)		readw(a)
> -#define cpu_readl(a)		readl(a)
> -#define cpu_writeb(v, a)	writeb((v), (a))
> -#define cpu_writew(v, a)	writew((v), (a))
> -#define cpu_writel(v, a)	writel((v), (a))
> -#endif
> +#define cpu_readb(a)		__raw_readb(a)
> +#define cpu_readw(a)		__raw_readw(a)
> +#define cpu_readl(a)		__raw_readl(a)
> +#define cpu_writeb(v, a)	__raw_writeb((v), (a))
> +#define cpu_writew(v, a)	__raw_writew((v), (a))
> +#define cpu_writel(v, a)	__raw_writel((v), (a))
>  
>  #endif /* __IO_H */
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: RFC: Fix big endian MMIO primitives.

[Krzysztof Halasa]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> Maybe we should rather use the __raw_* variants in the cfi driver aswell
> which is the only user of the functions below.

We can do that for the sake of compatibility with Linux.

> For some reason I
> believed that the __raw_* variants also do little endian accesses which
> is wrong.
> I don't like the naming of the __raw_* variants very much as the
> underscores and 'raw' suggests that these are internal functions which
> one should rather not use, but in fact these are the correct functions
> in most SoC (non PCI) drivers. Anyway, since Linux has this functions we
> should use them aswell, everything else probably leads to more
> confusion.

I don't know.

I would rename:
__raw_* -> cpu_*() as they are just plain and simple accessors with
native endianness.

readl() and friends -> le32_readl() etc.
The 'l' is somewhat redundant, the size is already determined by '32'
(and 16, 8). Maybe le32_read() or read_le32()?

Your call. We can just limit this renaming to cpu_* -> __raw_*.


To be honest, I would like this stuff renamed in Linux as well. Perhaps
some day.
-- 
Krzysztof Halasa

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: RFC: Fix big endian MMIO primitives.

[Sascha Hauer]

On Sun, May 06, 2012 at 08:01:58PM +0200, Krzysztof Halasa wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> > Maybe we should rather use the __raw_* variants in the cfi driver aswell
> > which is the only user of the functions below.
> 
> We can do that for the sake of compatibility with Linux.
> 
> > For some reason I
> > believed that the __raw_* variants also do little endian accesses which
> > is wrong.
> > I don't like the naming of the __raw_* variants very much as the
> > underscores and 'raw' suggests that these are internal functions which
> > one should rather not use, but in fact these are the correct functions
> > in most SoC (non PCI) drivers. Anyway, since Linux has this functions we
> > should use them aswell, everything else probably leads to more
> > confusion.
> 
> I don't know.
> 
> I would rename:
> __raw_* -> cpu_*() as they are just plain and simple accessors with
> native endianness.
> 
> readl() and friends -> le32_readl() etc.
> The 'l' is somewhat redundant, the size is already determined by '32'
> (and 16, 8). Maybe le32_read() or read_le32()?
> 
> Your call. We can just limit this renaming to cpu_* -> __raw_*.
> 
> 
> To be honest, I would like this stuff renamed in Linux as well. Perhaps
> some day.

I tend to take your patch as is. The cpu_* accessors have a clear
meaning and don't conflict with anything in Linux, so why not just have
them around. Anyway, I'll sleep over it before applying it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: RFC: Fix big endian MMIO primitives.

[Sascha Hauer]

On Sat, May 05, 2012 at 11:34:13PM +0200, Krzysztof Halasa wrote:
> cpu_read*() and cpu_wrire*() are precisely equal to __raw_read*() and
> __raw_write*().
> 
> Striving for correctness we can replace all those __raw_*() with cpu_*()
> as that's exactly what the former ones do. Also we can change read*()
> and write*() into explicit leXX_*(). Both changes could affect porting
> from Linux, though.
> 
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

Applied, thanks

Sascha

> 
> diff --git a/include/io.h b/include/io.h
> index 39b5e61..8d885de 100644
> --- a/include/io.h
> +++ b/include/io.h
> @@ -4,20 +4,11 @@
>  #include <asm/io.h>
>  
>  /* cpu_read/cpu_write: cpu native io accessors */
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -#define cpu_readb(a)		readb(a)
> -#define cpu_readw(a)		in_be16(a)
> -#define cpu_readl(a)		in_be32(a)
> -#define cpu_writeb(v, a)	writeb((v), (a))
> -#define cpu_writew(v, a)	out_be16((a), (v))
> -#define cpu_writel(v, a)	out_be32((a), (v))
> -#else
> -#define cpu_readb(a)		readb(a)
> -#define cpu_readw(a)		readw(a)
> -#define cpu_readl(a)		readl(a)
> -#define cpu_writeb(v, a)	writeb((v), (a))
> -#define cpu_writew(v, a)	writew((v), (a))
> -#define cpu_writel(v, a)	writel((v), (a))
> -#endif
> +#define cpu_readb(a)		__raw_readb(a)
> +#define cpu_readw(a)		__raw_readw(a)
> +#define cpu_readl(a)		__raw_readl(a)
> +#define cpu_writeb(v, a)	__raw_writeb((v), (a))
> +#define cpu_writew(v, a)	__raw_writew((v), (a))
> +#define cpu_writel(v, a)	__raw_writel((v), (a))
>  
>  #endif /* __IO_H */
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox