The MStar/SigmaStar SoCs have some very weird internal
bus bridges called RIU and XIU. These seem to be a left
over from when the CPU core was 8051 or MIPS.
Basically they act as a bridge between the ARM CPU and the
lump of standard peripherals (ethernet, usb, sd host etc)
that has been used throughout all of their designs.
RIU has 16bit registers 32bits apart from the CPU view while
XIU has 32bit registers 64bits apart from the CPU view.
Older chips (MSC313) only have RIU, newer chips (MSC313E)
have both RIU and XIU with some IPs accessible via RIU with
the original address and via XIU with an additional address.
To make things really fun some IPs (memory mapped ethernet PHY)
have registers that are completely accessible via RIU but only
partially accessible via XIU.
The main issue is for non-MStar IPs connected to these bridges.
All of the MStar IPs seem to have 16bit registers but the
ethernet controller and usb controller are third party and
have 32bit registers.
The kernel drivers expect the registers to be at normal
offsets and not broken into two parts so they don't work
out of the box here.
I want to hide this stuff as much as possible so it seemed
like a good idea to hide it in a header and use the headers
in the unfortunate drivers.
RFC because maybe this isn't the right approach and I'm
sure the two readw()/writew()s for RIU need to be protected
somehow but I wasn't sure how.
Daniel Palmer (2):
ARM: mstar: Add header with macros for RIU register access
ARM: mstar: Add header with macros for XIU register access
MAINTAINERS | 1 +
include/soc/mstar/riu.h | 28 ++++++++++++++++++++++++++++
include/soc/mstar/xiu.h | 22 ++++++++++++++++++++++
3 files changed, 51 insertions(+)
create mode 100644 include/soc/mstar/riu.h
create mode 100644 include/soc/mstar/xiu.h
--
2.31.0
Registers connected to the CPU via "XIU" (Maybe eXtended Interface
Unit) are 32bits wide with a 64bit stride.
Apparently someone realised that splitting 32bit registers into
two 16bit ones was silly but at the same time didn't want to
fix all of the register offsets used for "RIU" in their code.
This means that any existing driver (i.e. the usb and ethernet)
cannot be used as is and needs to use a special readl()/writel()
to fix up the address of the register that needs to be accessed.
To avoid having this code in every driver add a header with an
implementation of readl()/writel() that patches over the insanity.
Signed-off-by: Daniel Palmer <[email protected]>
---
include/soc/mstar/xiu.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 include/soc/mstar/xiu.h
diff --git a/include/soc/mstar/xiu.h b/include/soc/mstar/xiu.h
new file mode 100644
index 000000000000..d658338b8006
--- /dev/null
+++ b/include/soc/mstar/xiu.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _SOC_MSTAR_XIU_H_
+#define _SOC_MSTAR_XIU_H_
+
+#include <linux/io.h>
+
+static inline u32 xiu_readl(__iomem void *base, unsigned int offset)
+{
+ __iomem void *reg = base + (offset * 2);
+
+ return readl_relaxed(reg);
+}
+
+static inline void xiu_writel(__iomem void *base, unsigned int offset, u32 value)
+{
+ __iomem void *reg = base + (offset * 2);
+
+ writel_relaxed(value, reg);
+}
+
+#endif
--
2.31.0
Registers connected to the CPU via "RIU" (Maybe Register Interface
Unit) are 16bits wide with a 32bit stride.
For IPs that came from 3rd parties that have natively 32bit
registers they are annoyingly mapped with the 32bit register
split into two 16bit registers.
This means that any existing driver (i.e. the usb and ethernet)
cannot be used as is and needs to use a special readl()/writel()
to fix up the address of the register that needs to be accessed,
do two readw()/writel()s and stitch the values together.
To avoid having this code in every driver add a header with an
implementation of readl()/writel() that patches over the insanity.
Signed-off-by: Daniel Palmer <[email protected]>
---
MAINTAINERS | 1 +
include/soc/mstar/riu.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 include/soc/mstar/riu.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 19dc2eb0d93b..9600291e73a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2155,6 +2155,7 @@ F: drivers/gpio/gpio-msc313.c
F: drivers/pinctrl/pinctrl-msc313.c
F: include/dt-bindings/clock/mstar-*
F: include/dt-bindings/gpio/msc313-gpio.h
+F: include/soc/mstar/
ARM/NEC MOBILEPRO 900/c MACHINE SUPPORT
M: Michael Petchkovsky <[email protected]>
diff --git a/include/soc/mstar/riu.h b/include/soc/mstar/riu.h
new file mode 100644
index 000000000000..5aeea9c1e7eb
--- /dev/null
+++ b/include/soc/mstar/riu.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _SOC_MSTAR_RIU_H_
+#define _SOC_MSTAR_RIU_H_
+
+#include <linux/io.h>
+
+static inline u32 riu_readl(__iomem void *base, unsigned int offset)
+{
+ __iomem void *reg = base + (offset * 2);
+
+ return readw_relaxed(reg + 4) << 16 | readw_relaxed(reg);
+}
+
+static inline void riu_writel(__iomem void *base, unsigned int offset, u32 value)
+{
+ __iomem void *reg = base + (offset * 2);
+
+ /*
+ * Do not change this order. For EMAC at least
+ * the write order must be the lower half and then
+ * the upper half otherwise it doesn't work.
+ */
+ writew_relaxed(value, reg);
+ writew_relaxed(value >> 16, reg + 4);
+}
+
+#endif
--
2.31.0
Hi Arnd,
On Fri, 23 Apr 2021 at 22:48, Arnd Bergmann <[email protected]> wrote:
>
> The __iomem token comes after the type, so this should be 'void __iomem *'.
>
Bit of copy/paste fail. Fixed.
> > + return readw_relaxed(reg + 4) << 16 | readw_relaxed(reg);
>
> This should probably be using 'readw' instead of 'readw_relaxed'. If you
> absolutely need to use one of the relaxed accessors somewhere,
> better add both sets and make sure drivers use the non-relaxed version
> by default.
I'll add a relaxed/non-relaxed version of each.
Because of the heavy memory barrier to access one 32 bit register
we'll hit the barrier twice in the non-relaxed version.
And we don't need to hit the barrier at all because it doesn't
actually matter for IO. Is there something better I can do there?
> Maybe both types of accessors can be in a single header.
That makes sense. I'll merge them. Would this header be something that
could go in alone without anything that uses them in mainline right
now?
Thanks,
Daniel