drivers/rtc/rtc-pl031.c uses __raw_readl()?

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Linus Walleij]

Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
the __raw_[read|write]l() macro instead of plain [read|write]l()?

Is it OK if I just patch this so it looks like the sister pl030 driver?

Linus Walleij

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Linus Walleij]

2009/11/11 Linus Walleij <linus.ml.walleij@gmail.com>:

> Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
> the __raw_[read|write]l() macro instead of plain [read|write]l()?

Reading the macro definitions I come to the conclusion that readl()
is always little-endian and __raw_readl() is the machine endianness,
so that if you put your ARM in BE mode or synthesize this PrimeCell
on PPC, it will not work. PrimeCell registers are always LE I believe.

Now none of that was exactly obvious so please correct me if I'm
wrong!

I sent a patch you can slam on.

Linus Walleij

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Russell King - ARM Linux]

On Thu, Nov 12, 2009 at 09:43:43AM +0100, Linus Walleij wrote:
> 2009/11/11 Linus Walleij <linus.ml.walleij@gmail.com>:
> 
> > Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
> > the __raw_[read|write]l() macro instead of plain [read|write]l()?
> 
> Reading the macro definitions I come to the conclusion that readl()
> is always little-endian and __raw_readl() is the machine endianness,
> so that if you put your ARM in BE mode or synthesize this PrimeCell
> on PPC, it will not work. PrimeCell registers are always LE I believe.
> 
> Now none of that was exactly obvious so please correct me if I'm
> wrong!

I think it's a question for Catalin - unfortunately the AMBA specifications
(which I guess is where there's a definitive statement on the endianness
for APB peripherals) is only available to registered users via the ARM
website.

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Uwe Kleine-König]

On Thu, Nov 12, 2009 at 09:43:43AM +0100, Linus Walleij wrote:
> 2009/11/11 Linus Walleij <linus.ml.walleij@gmail.com>:
> 
> > Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
> > the __raw_[read|write]l() macro instead of plain [read|write]l()?
> 
> Reading the macro definitions I come to the conclusion that readl()
> is always little-endian and __raw_readl() is the machine endianness,
Actually the readl part is not necessarily true I think.

At least arch/arm/mach-bcmring/include/mach/io.h has:

	/* Do not enable mem_pci for a big endian arm architecture or unexpected byteswaps will */
	/* happen in readw/writew etc. */

	#define readb(c)        __raw_readb(c)
	...

But maybe this is a bug in mach-bcmring?!  Or a work-around of drivers
using readb et al in wrong sitatations?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Leo (Hao) Chen]

On Tue, Nov 17, 2009 at 02:02:56AM -0800, Uwe Kleine-K?nig wrote:
> On Thu, Nov 12, 2009 at 09:43:43AM +0100, Linus Walleij wrote:
> > 2009/11/11 Linus Walleij <linus.ml.walleij@gmail.com>:
> > 
> > > Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
> > > the __raw_[read|write]l() macro instead of plain [read|write]l()?
> > 
> > Reading the macro definitions I come to the conclusion that readl()
> > is always little-endian and __raw_readl() is the machine endianness,
> Actually the readl part is not necessarily true I think.
> 
> At least arch/arm/mach-bcmring/include/mach/io.h has:
> 
> 	/* Do not enable mem_pci for a big endian arm architecture or unexpected byteswaps will */
> 	/* happen in readw/writew etc. */
> 
> 	#define readb(c)        __raw_readb(c)
> 	...
> 
> But maybe this is a bug in mach-bcmring?!  Or a work-around of drivers
> using readb et al in wrong sitatations?

Our code was based on older io.h. No need to keep our own readb
definition any more. We'll clean them up by using the
generic readb defined in asm/io.h.

-- 

Leo Hao Chen

------------------------
Life is short, run long.

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Russell King - ARM Linux]

On Fri, Nov 13, 2009 at 11:26:37PM +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 12, 2009 at 09:43:43AM +0100, Linus Walleij wrote:
> > 2009/11/11 Linus Walleij <linus.ml.walleij@gmail.com>:
> > 
> > > Is there some special reason as to why drivers/rtc/rtc-pl031.c uses
> > > the __raw_[read|write]l() macro instead of plain [read|write]l()?
> > 
> > Reading the macro definitions I come to the conclusion that readl()
> > is always little-endian and __raw_readl() is the machine endianness,
> > so that if you put your ARM in BE mode or synthesize this PrimeCell
> > on PPC, it will not work. PrimeCell registers are always LE I believe.
> > 
> > Now none of that was exactly obvious so please correct me if I'm
> > wrong!
> 
> I think it's a question for Catalin - unfortunately the AMBA specifications
> (which I guess is where there's a definitive statement on the endianness
> for APB peripherals) is only available to registered users via the ARM
> website.

So where do we stand on this?  Are Primecell registers always LE?  Or
can they be BE if running in one of the BE modes?

I'm not merging the patch until we have a definitive answer on this.

drivers/rtc/rtc-pl031.c uses __raw_readl()?

[Russell King - ARM Linux]

On Thu, Nov 19, 2009 at 09:43:59AM +0000, Russell King - ARM Linux wrote:
> So where do we stand on this?  Are Primecell registers always LE?  Or
> can they be BE if running in one of the BE modes?
> 
> I'm not merging the patch until we have a definitive answer on this.

Linus, you just said in the patch system that this is in akpm's tree.

Since we don't know whether this patch is even right or not, it's not
something which should be there.  It's not a bug fix nor is it a known
improvement.  It's just a hunch.

It may be that the existing code is correct and the other primecell
drivers that are using readb() rather than __raw_readb() need fixing.
We just don't know at present.