Hi Phil,
On Fri, Jul 1, 2022 at 11:15 AM Phil Edworthy <phil.edworthy@xxxxxxxxxxx> 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@xxxxxxxxx
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
