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

Message ID 20171204150907.24345-2-thomas.petazzoni@free-electrons.com
State Not Applicable
Headers show
Series
  • [1/8] arch/sh: add sh7786_mm_sel() function
Related show

Commit Message

Thomas Petazzoni Dec. 4, 2017, 3:09 p.m.
The SH7786 has different physical memory layout configurations,
configurable through the MMSELR register. The configuration is
typically defined by the bootloader, so Linux generally doesn't care.

Except that depending on the configuration, some PCI MEM areas may or
may not be available. This commit adds a helper function that allows
to retrieve the current physical memory layout configuration. It will
be used in a following patch to exclude unusable PCI MEM areas during
the PCI initialization.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/sh/include/cpu-sh4/cpu/sh7786.h | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Mason Jan. 8, 2018, 12:28 p.m. | #1
On 04/12/2017 16:09, Thomas Petazzoni wrote:

> The SH7786 has different physical memory layout configurations,
> configurable through the MMSELR register. The configuration is
> typically defined by the bootloader, so Linux generally doesn't care.
> 
> Except that depending on the configuration, some PCI MEM areas may or
> may not be available. This commit adds a helper function that allows
> to retrieve the current physical memory layout configuration. It will
> be used in a following patch to exclude unusable PCI MEM areas during
> the PCI initialization.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/sh/include/cpu-sh4/cpu/sh7786.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/sh/include/cpu-sh4/cpu/sh7786.h b/arch/sh/include/cpu-sh4/cpu/sh7786.h
> index 0df09e638f09..96b8cb1f754a 100644
> --- a/arch/sh/include/cpu-sh4/cpu/sh7786.h
> +++ b/arch/sh/include/cpu-sh4/cpu/sh7786.h
> @@ -14,6 +14,8 @@
>  #ifndef __CPU_SH7786_H__
>  #define __CPU_SH7786_H__
>  
> +#include <linux/io.h>
> +
>  enum {
>  	/* PA */
>  	GPIO_PA7, GPIO_PA6, GPIO_PA5, GPIO_PA4,
> @@ -131,4 +133,9 @@ enum {
>  	GPIO_FN_IRL7, GPIO_FN_IRL6, GPIO_FN_IRL5, GPIO_FN_IRL4,
>  };
>  
> +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?

Regards.
Thomas Petazzoni Jan. 8, 2018, 12:55 p.m. | #2
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

Patch

diff --git a/arch/sh/include/cpu-sh4/cpu/sh7786.h b/arch/sh/include/cpu-sh4/cpu/sh7786.h
index 0df09e638f09..96b8cb1f754a 100644
--- a/arch/sh/include/cpu-sh4/cpu/sh7786.h
+++ b/arch/sh/include/cpu-sh4/cpu/sh7786.h
@@ -14,6 +14,8 @@ 
 #ifndef __CPU_SH7786_H__
 #define __CPU_SH7786_H__
 
+#include <linux/io.h>
+
 enum {
 	/* PA */
 	GPIO_PA7, GPIO_PA6, GPIO_PA5, GPIO_PA4,
@@ -131,4 +133,9 @@  enum {
 	GPIO_FN_IRL7, GPIO_FN_IRL6, GPIO_FN_IRL5, GPIO_FN_IRL4,
 };
 
+static inline u32 sh7786_mm_sel(void)
+{
+	return __raw_readl(0xFC400020) & 0x7;
+}
+
 #endif /* __CPU_SH7786_H__ */