[RFC] soc: renesas: Add RZ/V2M SYS driver

[RFC] soc: renesas: Add RZ/V2M SYS driver

[Phil Edworthy]

The System Configuration (SYS) module on the Renesas RZ/V2M (r9a09g011)
contains registers for many different aspects of the SoC.

Some of the peripherals on the SoC are only 32-bit address capable bus
masters. To select the lower 4GiB or upper 4GiB of memory, the
SYS PERI0_BANK and SYS_PERI1_BANK registers can be programmed to set
the 33rd address bit.
Due to the use of firmware with the SoC, uboot is often set up such that
these peripherals can only access the upper 4GiB. In order to allow
Linux to use bounce buffers for drivers, we set aside some memory in the
lower 4GiB for Linux.
Thus this requires the SYS PERIx_BANK registers to be reprogrammed.
---
 drivers/soc/renesas/Kconfig         |  4 ++
 drivers/soc/renesas/Makefile        |  1 +
 drivers/soc/renesas/r9a09g011-sys.c | 67 +++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)
 create mode 100644 drivers/soc/renesas/r9a09g011-sys.c

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index c50a6ce1b99d..b9e3dc879ddc 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -327,6 +327,7 @@ config ARCH_R9A09G011
 	bool "ARM64 Platform support for RZ/V2M"
 	select PM
 	select PM_GENERIC_DOMAINS
+	select SYS_R9A09G011
 	help
 	  This enables support for the Renesas RZ/V2M SoC.
 
@@ -440,4 +441,7 @@ config SYSC_R8A774B1
 	bool "System Controller support for RZ/G2N" if COMPILE_TEST
 	select SYSC_RCAR
 
+config SYS_R9A09G011
+	bool "System Controller support for RZ/V2M" if COMPILE_TEST
+
 endif # SOC_RENESAS
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index 535868c9c7e4..7e269ab6343e 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_SYSC_R8A779G0)	+= r8a779g0-sysc.o
 ifdef CONFIG_SMP
 obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
 endif
+obj-$(CONFIG_SYS_R9A09G011)	+= r9a09g011-sys.o
 
 # Family
 obj-$(CONFIG_RST_RCAR)		+= rcar-rst.o
diff --git a/drivers/soc/renesas/r9a09g011-sys.c b/drivers/soc/renesas/r9a09g011-sys.c
new file mode 100644
index 000000000000..6a72ab15cc89
--- /dev/null
+++ b/drivers/soc/renesas/r9a09g011-sys.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/V2M SYS driver
+ *
+ * Copyright (C) 2022  Renesas Electronics Corporation
+ */
+
+#include <linux/io.h>
+#include <linux/of_address.h>
+
+#define SYS_PERI0_BANK		0x30
+#define SDI0_SHIFT		0
+#define SDI1_SHIFT		2
+#define EMMC_SHIFT		4
+#define USB_HOST_SHIFT		8
+#define USB_PERI_SHIFT		10
+#define PCIE_SHIFT		12
+
+#define SYS_PERI1_BANK		0x34
+#define ETH_SHIFT		0
+
+#define BANK_LOWER_4GB		0
+#define BANK_UPPER_4GB		1
+
+static const struct of_device_id renesas_socs[] __initconst = {
+	{ .compatible = "renesas,r9a09g011-sys" },
+	{ /* sentinel */ }
+};
+
+static void write_peri_bank(void __iomem *addr, uint32_t val, int shift)
+{
+	/* Set the write enable bits */
+	writel(((3 << 16) | val) << shift, addr);
+}
+
+static int __init r9a09g011_init(void)
+{
+	const struct of_device_id *match;
+	struct device_node *np;
+	void __iomem *base;
+	int error = 0;
+
+	np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
+	if (!np)
+		return -ENODEV;
+
+	base = of_iomap(np, 0);
+	if (!base) {
+		pr_warn("%pOF: Cannot map regs\n", np);
+		error = -ENOMEM;
+		goto out_put;
+	}
+
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, SDI0_SHIFT);
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, SDI1_SHIFT);
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, EMMC_SHIFT);
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, USB_HOST_SHIFT);
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, USB_PERI_SHIFT);
+	write_peri_bank(base + SYS_PERI0_BANK, BANK_LOWER_4GB, PCIE_SHIFT);
+	write_peri_bank(base + SYS_PERI1_BANK, BANK_LOWER_4GB, ETH_SHIFT);
+
+out_put:
+	of_node_put(np);
+	return error;
+}
+
+core_initcall(r9a09g011_init);
-- 
2.34.1

Re: [RFC] soc: renesas: Add RZ/V2M SYS driver

[Geert Uytterhoeven]

Hi Phil,

Thanks for your patch!

On Tue, Jun 28, 2022 at 7:40 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> The System Configuration (SYS) module on the Renesas RZ/V2M (r9a09g011)
> contains registers for many different aspects of the SoC.
>
> Some of the peripherals on the SoC are only 32-bit address capable bus
> masters. To select the lower 4GiB or upper 4GiB of memory, the
> SYS PERI0_BANK and SYS_PERI1_BANK registers can be programmed to set
> the 33rd address bit.
> Due to the use of firmware with the SoC, uboot is often set up such that
> these peripherals can only access the upper 4GiB. In order to allow
> Linux to use bounce buffers for drivers, we set aside some memory in the
> lower 4GiB for Linux.
> Thus this requires the SYS PERIx_BANK registers to be reprogrammed.

Does this interfere with the firmware?
If yes, why is this not bad?
If not, why can't U-Boot set this up correctly for Linux?

As most RAM available to Linux is in the upper 4 GiB, wouldn't it be
better to use the upper 4 GiB, so bounce buffer can be avoided for most
transfers? Or is it impossible to set up bounce buffers in the upper 4 GiB?

> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g011-sys.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/V2M SYS driver
> + *
> + * Copyright (C) 2022  Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +
> +#define SYS_PERI0_BANK         0x30
> +#define SDI0_SHIFT             0
> +#define SDI1_SHIFT             2
> +#define EMMC_SHIFT             4
> +#define USB_HOST_SHIFT         8
> +#define USB_PERI_SHIFT         10
> +#define PCIE_SHIFT             12
> +
> +#define SYS_PERI1_BANK         0x34
> +#define ETH_SHIFT              0

These fields look like perfect users of GENMASK() and FIELD_PREP().

#define PERI0_SDI0    GENMASK(1, 0)

> +
> +#define BANK_LOWER_4GB         0
> +#define BANK_UPPER_4GB         1

I'm not sure these are useful.  The values are just the high
address bits.

> +
> +static const struct of_device_id renesas_socs[] __initconst = {
> +       { .compatible = "renesas,r9a09g011-sys" },
> +       { /* sentinel */ }
> +};
> +
> +static void write_peri_bank(void __iomem *addr, uint32_t val, int shift)
> +{
> +       /* Set the write enable bits */
> +       writel(((3 << 16) | val) << shift, addr);

writel((field << 16) | FIELD_PREP(field, addr_high), addr)?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [RFC] soc: renesas: Add RZ/V2M SYS driver

[Phil Edworthy]

Hi Geert,

Thank you for looking at this.

On 01 July 2022 09:37 Geert Uytterhoeven wrote:
> On Tue, Jun 28, 2022 at 7:40 PM Phil Edworthy wrote:
> > The System Configuration (SYS) module on the Renesas RZ/V2M (r9a09g011)
> > contains registers for many different aspects of the SoC.
> >
> > Some of the peripherals on the SoC are only 32-bit address capable bus
> > masters. To select the lower 4GiB or upper 4GiB of memory, the
> > SYS PERI0_BANK and SYS_PERI1_BANK registers can be programmed to set
> > the 33rd address bit.
> > Due to the use of firmware with the SoC, uboot is often set up such that
> > these peripherals can only access the upper 4GiB. In order to allow
> > Linux to use bounce buffers for drivers, we set aside some memory in the
> > lower 4GiB for Linux.
> > Thus this requires the SYS PERIx_BANK registers to be reprogrammed.
> 
> Does this interfere with the firmware?
> If yes, why is this not bad?
> If not, why can't U-Boot set this up correctly for Linux?
Yes, there is potentially a problem with the firmware trying to write to
the registers at the same time. It’s unlikely, but possible.
You make a very good point about U-Boot setting it correctly.

> As most RAM available to Linux is in the upper 4 GiB, wouldn't it be
> better to use the upper 4 GiB, so bounce buffer can be avoided for most
> transfers? Or is it impossible to set up bounce buffers in the upper 4
> GiB?
Avoiding bounce buffers would be best. I guess I have been guilty of
trying to ease my work as some drivers need non-trivial changes. In
particular, the usb xhci driver.

Perhaps we can continue development for the time being with U-Boot
setting the bank addr registers so the peripherals access the lower
4GiB and give Linux some mem in the lower 4GiB for bounce buffers.

Can we look at making the drivers use the higher 4GiB later on?


> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g011-sys.c
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/V2M SYS driver
> > + *
> > + * Copyright (C) 2022  Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_address.h>
> > +
> > +#define SYS_PERI0_BANK         0x30
> > +#define SDI0_SHIFT             0
> > +#define SDI1_SHIFT             2
> > +#define EMMC_SHIFT             4
> > +#define USB_HOST_SHIFT         8
> > +#define USB_PERI_SHIFT         10
> > +#define PCIE_SHIFT             12
> > +
> > +#define SYS_PERI1_BANK         0x34
> > +#define ETH_SHIFT              0
> 
> These fields look like perfect users of GENMASK() and FIELD_PREP().
Another macro I am not familiar with! Thanks for pointing it out.


> #define PERI0_SDI0    GENMASK(1, 0)
> 
> > +
> > +#define BANK_LOWER_4GB         0
> > +#define BANK_UPPER_4GB         1
> 
> I'm not sure these are useful.  The values are just the high
> address bits.
> 
> > +
> > +static const struct of_device_id renesas_socs[] __initconst = {
> > +       { .compatible = "renesas,r9a09g011-sys" },
> > +       { /* sentinel */ }
> > +};
> > +
> > +static void write_peri_bank(void __iomem *addr, uint32_t val, int
> shift)
> > +{
> > +       /* Set the write enable bits */
> > +       writel(((3 << 16) | val) << shift, addr);
> 
> writel((field << 16) | FIELD_PREP(field, addr_high), addr)?
> 

Thanks
Phil

Re: [RFC] soc: renesas: Add RZ/V2M SYS driver

[Geert Uytterhoeven]

Hi Phil,

On Fri, Jul 1, 2022 at 11:15 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 01 July 2022 09:37 Geert Uytterhoeven wrote:
> > On Tue, Jun 28, 2022 at 7:40 PM Phil Edworthy wrote:
> > > The System Configuration (SYS) module on the Renesas RZ/V2M (r9a09g011)
> > > contains registers for many different aspects of the SoC.
> > >
> > > Some of the peripherals on the SoC are only 32-bit address capable bus
> > > masters. To select the lower 4GiB or upper 4GiB of memory, the
> > > SYS PERI0_BANK and SYS_PERI1_BANK registers can be programmed to set
> > > the 33rd address bit.
> > > Due to the use of firmware with the SoC, uboot is often set up such that
> > > these peripherals can only access the upper 4GiB. In order to allow
> > > Linux to use bounce buffers for drivers, we set aside some memory in the
> > > lower 4GiB for Linux.
> > > Thus this requires the SYS PERIx_BANK registers to be reprogrammed.
> >
> > Does this interfere with the firmware?
> > If yes, why is this not bad?
> > If not, why can't U-Boot set this up correctly for Linux?
> Yes, there is potentially a problem with the firmware trying to write to
> the registers at the same time. It’s unlikely, but possible.

I'm mainly worried about the firmware relying on the u-boot settings,
and using the devices?  But I guess that won't happen, if they're assigned
to Linux?

> You make a very good point about U-Boot setting it correctly.

Typically things like this are supposed to be handled by U-Boot,
in a correct way.

> > As most RAM available to Linux is in the upper 4 GiB, wouldn't it be
> > better to use the upper 4 GiB, so bounce buffer can be avoided for most
> > transfers? Or is it impossible to set up bounce buffers in the upper 4
> > GiB?
>
> Avoiding bounce buffers would be best. I guess I have been guilty of
> trying to ease my work as some drivers need non-trivial changes. In
> particular, the usb xhci driver.
>
> Perhaps we can continue development for the time being with U-Boot
> setting the bank addr registers so the peripherals access the lower
> 4GiB and give Linux some mem in the lower 4GiB for bounce buffers.
>
> Can we look at making the drivers use the higher 4GiB later on?

Sure.

> > > --- /dev/null
> > > +++ b/drivers/soc/renesas/r9a09g011-sys.c
> > > @@ -0,0 +1,67 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Renesas RZ/V2M SYS driver
> > > + *
> > > + * Copyright (C) 2022  Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/of_address.h>
> > > +
> > > +#define SYS_PERI0_BANK         0x30
> > > +#define SDI0_SHIFT             0
> > > +#define SDI1_SHIFT             2
> > > +#define EMMC_SHIFT             4
> > > +#define USB_HOST_SHIFT         8
> > > +#define USB_PERI_SHIFT         10
> > > +#define PCIE_SHIFT             12
> > > +
> > > +#define SYS_PERI1_BANK         0x34
> > > +#define ETH_SHIFT              0
> >
> > These fields look like perfect users of GENMASK() and FIELD_PREP().
>
> Another macro I am not familiar with! Thanks for pointing it out.

> > > +static void write_peri_bank(void __iomem *addr, uint32_t val, int
> > shift)
> > > +{
> > > +       /* Set the write enable bits */
> > > +       writel(((3 << 16) | val) << shift, addr);
> >
> > writel((field << 16) | FIELD_PREP(field, addr_high), addr)?

Oops, this won't work, as FIELD_PREP() needs a compile-time constant.
Of course you can still pass the result of FIELD_PREP() as a parameter
to the function instead.

Or push for "[PATCH 01/17] bitfield: Add non-constant field_{prep,get}()
helpers"[1] ;-)  Or open code the proposed field_prep().

[1] https://lore.kernel.org/all/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [RFC] soc: renesas: Add RZ/V2M SYS driver

[Phil Edworthy]

Hi Geert,

On 01 July 2022 10:25 Geert Uytterhoeven wrote:
> On Fri, Jul 1, 2022 at 11:15 AM Phil Edworthy wrote:
> > On 01 July 2022 09:37 Geert Uytterhoeven wrote:
> > > On Tue, Jun 28, 2022 at 7:40 PM Phil Edworthy wrote:
> > > > The System Configuration (SYS) module on the Renesas RZ/V2M
> > > > (r9a09g011) contains registers for many different aspects of the
> SoC.
> > > >
> > > > Some of the peripherals on the SoC are only 32-bit address capable
> > > > bus masters. To select the lower 4GiB or upper 4GiB of memory, the
> > > > SYS PERI0_BANK and SYS_PERI1_BANK registers can be programmed to
> > > > set the 33rd address bit.
> > > > Due to the use of firmware with the SoC, uboot is often set up
> > > > such that these peripherals can only access the upper 4GiB. In
> > > > order to allow Linux to use bounce buffers for drivers, we set
> > > > aside some memory in the lower 4GiB for Linux.
> > > > Thus this requires the SYS PERIx_BANK registers to be reprogrammed.
> > >
> > > Does this interfere with the firmware?
> > > If yes, why is this not bad?
> > > If not, why can't U-Boot set this up correctly for Linux?
> > Yes, there is potentially a problem with the firmware trying to write
> > to the registers at the same time. It's unlikely, but possible.
> 
> I'm mainly worried about the firmware relying on the u-boot settings, and
> using the devices?  But I guess that won't happen, if they're assigned to
> Linux?
That's right. The firmware doesn't handle anything related to SD, USB, PCIe
or Eth. I don't know about VCD (multimedia codec) or GRP (graphics engine),
but I am reasonable sure they will be assigned to one of firmware or Linux.


> > You make a very good point about U-Boot setting it correctly.
> 
> Typically things like this are supposed to be handled by U-Boot, in a
> correct way.
> 
> > > As most RAM available to Linux is in the upper 4 GiB, wouldn't it be
> > > better to use the upper 4 GiB, so bounce buffer can be avoided for
> > > most transfers? Or is it impossible to set up bounce buffers in the
> > > upper 4 GiB?
> >
> > Avoiding bounce buffers would be best. I guess I have been guilty of
> > trying to ease my work as some drivers need non-trivial changes. In
> > particular, the usb xhci driver.
> >
> > Perhaps we can continue development for the time being with U-Boot
> > setting the bank addr registers so the peripherals access the lower
> > 4GiB and give Linux some mem in the lower 4GiB for bounce buffers.
> >
> > Can we look at making the drivers use the higher 4GiB later on?
> 
> Sure.
Ok, great.

 
> > > > --- /dev/null
> > > > +++ b/drivers/soc/renesas/r9a09g011-sys.c
> > > > @@ -0,0 +1,67 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Renesas RZ/V2M SYS driver
> > > > + *
> > > > + * Copyright (C) 2022  Renesas Electronics Corporation  */
> > > > +
> > > > +#include <linux/io.h>
> > > > +#include <linux/of_address.h>
> > > > +
> > > > +#define SYS_PERI0_BANK         0x30
> > > > +#define SDI0_SHIFT             0
> > > > +#define SDI1_SHIFT             2
> > > > +#define EMMC_SHIFT             4
> > > > +#define USB_HOST_SHIFT         8
> > > > +#define USB_PERI_SHIFT         10
> > > > +#define PCIE_SHIFT             12
> > > > +
> > > > +#define SYS_PERI1_BANK         0x34
> > > > +#define ETH_SHIFT              0
> > >
> > > These fields look like perfect users of GENMASK() and FIELD_PREP().
> >
> > Another macro I am not familiar with! Thanks for pointing it out.
> 
> > > > +static void write_peri_bank(void __iomem *addr, uint32_t val, int
> > > shift)
> > > > +{
> > > > +       /* Set the write enable bits */
> > > > +       writel(((3 << 16) | val) << shift, addr);
> > >
> > > writel((field << 16) | FIELD_PREP(field, addr_high), addr)?
> 
> Oops, this won't work, as FIELD_PREP() needs a compile-time constant.
> Of course you can still pass the result of FIELD_PREP() as a parameter to
> the function instead.
> 
> Or push for "[PATCH 01/17] bitfield: Add non-constant field_{prep,get}()
> helpers"[1] ;-)  Or open code the proposed field_prep().

Thanks!
Phil