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
