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

Conor Dooley
2022-12-29T14:42:25Z

[-- Attachment #1.1: Type: text/plain, Size: 3287 bytes --]

On Thu, Dec 29, 2022 at 03:05:37PM +0100, Arnd Bergmann wrote:
> On Sat, Dec 17, 2022, at 23:52, Conor Dooley wrote:
> > On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
> >> On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> >> > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> >>
> >> I don't particularly like drivers/soc/ to become more of a dumping
> >> ground for random drivers. If there are several SoCs that have the
> >> same requirement to do a particular thing, the logical step would
> >> be to put them into a proper subsystem, with a well-defined interface
> >> to dma-mapping and virtualization frameworks.
> >>
> >> The other things we have in drivers/soc/ are usually either
> >> soc_device drivers for identifying the system, or they export
> >> interfaces used by soc specific drivers.
> >
> > Sounds like that's two "not in my back yard" votes from the maintainers
> > in question..
> > Doing drivers/cache would allow us to co-locate the RISC-V cache
> > management bits since it is not just going to be the ax45mp l2 driver
> > that will need to have them.
> >
> > Would it be okay to put this driver in soc/andestech for now & then move
> > it, and the SiFive one, once we've got patches posted for cache
> > management with that?
>
> I actually had a look at both of these drivers now and
> found that they do entirely different things, so I would
> revise what I had said earlier. Sorry for not having paid
> enough attention at first.

Eh, I wouldn't consider it to be your fault as, I at least, have been
ignoring this difference as...

> The Sifive L2 cache driver handles an interrupt from the
> cache controller that is trigger by data corruption
> (corectable or uncorrectable). This is used as an
> implementation detail of drivers/edac/sifive_edac.c
> and could probably just be merged into that file.
>
> The Andes cache driver in this series on the other hand
> does not do EDAC at all but instead handles cache maintenance
> for the dma-mapping interface by hooking into the
> inline-asm implementation details of arch/riscv/mm/dma-noncoherent.c
> as an errata fix.

...we (Microchip) need to add similar cache maintenance to the SiFive
driver. Should have posted patches by now, but conferences + Christmas
have delayed that a bit.

> If we expect more nonstandard ways
> to manage cache controllers for this, I think this
> needs a proper interface in arch/riscv or drivers/cache.

The Zicbo* extensions for cache management arrived after people had
already been shipping SoCs that need stuff that isn't cache coherent.
On top of the two already mentioned, I am told there are two other
non-Zicbo* cache management solutions from SiFive alone - so I think it
is likely that we'll have variants here, unfortunately.

> This could be done the same way as arch/arm/include/asm/cacheflush.h
> with CPU specific cache management callback pointers, but
> can't really be a separate device driver without interacting
> with low-level architecture code.

I'll take a look at our patches again this week (they're 5.15 based, so
well out of date, vendor tree stuff). I'll try to whip up something
based on top of this series.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]