Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

From: Conor Dooley
Date: Thu Dec 15 2022 - 14:54:40 EST


On Thu, Dec 15, 2022 at 05:46:42PM +0000, Lad, Prabhakar wrote:
> Hi Geert,
>
> On Thu, Dec 15, 2022 at 11:10 AM Geert Uytterhoeven
> <geert@xxxxxxxxxxxxxx> wrote:
> >
> > Hi Prabhakar,
> >
> > On Thu, Dec 15, 2022 at 12:06 PM Lad, Prabhakar
> > <prabhakar.csengg@xxxxxxxxx> wrote:
> > > On Thu, Dec 15, 2022 at 10:36 AM Geert Uytterhoeven
> > > <geert@xxxxxxxxxxxxxx> wrote:
> > > > On Mon, Dec 12, 2022 at 12:58 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > > >
> > > > > I/O Coherence Port (IOCP) provides an AXI interface for connecting
> > > > > external non-caching masters, such as DMA controllers. The accesses
> > > > > from IOCP are coherent with D-Caches and L2 Cache.
> > > > >
> > > > > IOCP is a specification option and is disabled on the Renesas RZ/Five
> > > > > SoC due to this reason IP blocks using DMA will fail.
> > > > >
> > > > > The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> > > > > block that allows dynamic adjustment of memory attributes in the runtime.
> > > > > It contains a configurable amount of PMA entries implemented as CSR
> > > > > registers to control the attributes of memory locations in interest.
> > > > > Below are the memory attributes supported:
> > > > > * Device, Non-bufferable
> > > > > * Device, bufferable
> > > > > * Memory, Non-cacheable, Non-bufferable
> > > > > * Memory, Non-cacheable, Bufferable
> > > > > * Memory, Write-back, No-allocate
> > > > > * Memory, Write-back, Read-allocate
> > > > > * Memory, Write-back, Write-allocate
> > > > > * Memory, Write-back, Read and Write-allocate
> > > > >
> > > > > More info about PMA (section 10.3):
> > > > > Link: http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> > > > >
> > > > > As a workaround for SoCs with IOCP disabled CMO needs to be handled by
> > > > > software. Firstly OpenSBI configures the memory region as
> > > > > "Memory, Non-cacheable, Bufferable" and passes this region as a global
> > > > > shared dma pool as a DT node. With DMA_GLOBAL_POOL enabled all DMA
> > > > > allocations happen from this region and synchronization callbacks are
> > > > > implemented to synchronize when doing DMA transactions.
> > > > >
> > > > > Example PMA region passes as a DT node from OpenSBI:
> > > > > reserved-memory {
> > > > > #address-cells = <2>;
> > > > > #size-cells = <2>;
> > > > > ranges;
> > > > >
> > > > > pma_resv0@58000000 {
> > > > > compatible = "shared-dma-pool";
> > > > > reg = <0x0 0x58000000 0x0 0x08000000>;
> > > > > no-map;
> > > > > linux,dma-default;
> > > > > };
> > > > > };
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > arch/riscv/include/asm/cacheflush.h | 8 +
> > > > > arch/riscv/include/asm/errata_list.h | 28 ++-
> > > > > drivers/soc/renesas/Kconfig | 6 +
> > > > > drivers/soc/renesas/Makefile | 2 +
> > > > > drivers/soc/renesas/rzfive/Kconfig | 6 +
> > > > > drivers/soc/renesas/rzfive/Makefile | 3 +
> > > > > drivers/soc/renesas/rzfive/ax45mp_cache.c | 256 ++++++++++++++++++++++
> > > >
> > > > Given this touches arch/riscv/include/asm/, I don't think the
> > > > code belongs under drivers/soc/renesas/.
> > > >
> > > Ok. Do you have any suggestions on where you want me to put this code?
> >
> > As it plugs into core riscv functionality, I think it should be under
> > arch/riscv/.
> > if the RISC-V maintainers object to that, another option is
> > drivers/soc/andestech/ or (new) drivers/cache/
> >
> RISC-V maintainers had already made it clear to not to include vendor
> specific stuff in the arch/riscv folder, so I'll consider putting this
> into drivers/cache/ folder to sync with the bindings.
>
> Conor/Palmer - do you have any objections/suggestions?

I'm not its maintainer so sorta moot what I say, but having drivers in
arch/riscv makes little sense to me..
Putting stuff in drivers/cache does sound like a good idea since the
binding is going there too.

The SiFive ccache driver is in drivers/soc and it was suggested to me
this week that there's likely going to be a second SiFive cache driver
at some point in the near future. Plus Microchip are going to have to
add cache management stuff to the existing SiFive ccache driver.
Having them be their own thing makes sense in my mind - especially since
they're not tied to SoCs sold by Andes or SiFive.

I had a quick, and I mean *quick* look through other soc drivers to see
if there were any other cache controller drivers but nothing stood out
to me. Maybe someone else has more of a clue there. Ditto for misc, had
a look but nothing seemed obvious.

If we do do drivers/cache & move the SiFive stuff there too, someone
needs to look after it. I guess I can if noone else feels strongly
since I seem to be the Responsible Adult for the SiFive ones already?

Attachment: signature.asc
Description: PGP signature