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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux