Re: [PATCH 1/8] arch/sh: add sh7786_mm_sel() function

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Hello,

On Mon, 8 Jan 2018 13:28:42 +0100, Mason wrote:

> > +static inline u32 sh7786_mm_sel(void)
> > +{
> > +	return __raw_readl(0xFC400020) & 0x7;  
> 
> I thought we were never supposed to use __raw_readl(), rather use readl_relaxed() instead?
> 
> The difference is that readl_relaxed() takes care of endianness, which would seem relevant
> since you have both endianness. Am I missing something?

Yes. That I/O accessors in Linux are a mess.

On ARM for example, when the CPU runs big-endian, the devices are still
little-endian, so we use readl/writel (or their relaxed variants) to do
the endianness conversion.

However, on SH, platform devices have an endianness that follow the
CPU one. So if you run little-endian, your devices are little endian,
if you run big-endian, your devices are big endian. Hence the use of
__raw_readl().

So you could ask me: but if SH behaves like this, why is readl/writel
doing an endianness conversion on this architecture? Well because
readl/writel were originally introduced for PCI devices, and PCI
devices are always little-endian, even when your SH core runs
big-endian.

Basically, I think the I/O accessors are just a mess, and the only way
to solve this mess properly would be to have a "struct device *" as
argument to those accessors, that tells the accessor how the device
behaves in terms of endianness.

Best regards, 

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com