Re: [PATCH 0/6] soc: Add Baikal-T1 SoC APB/AXI EHB and L2-cache drivers

[Sergey Semin]

Hello Arnd,

On Fri, Mar 06, 2020 at 04:19:47PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 6, 2020 at 2:07 PM <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > From: Serge Semin <fancer.lancer@xxxxxxxxx>
> >
> > Aside from PCIe/SATA/DDR/I2C/CPU-reboot specific settings the Baikal-T1
> > system controller provides three vendor-specific blocks. In particular
> > there are two Errors Handler Blocks to detect and report an info regarding
> > any problems discovered on the AXI and APB buses. These are the main buses
> > utilized by the SoC devices to interact with each other. In addition there
> > is a way to tune the MIPS P5600 CM2 L2-cache up by setting the Tag/Data/WS
> > L2-to-RAM latencies. All of this functionality is implemented in the
> > APB/AXI EHB and L2-cache control block drivers to be a part of the kernel soc
> > subsystem (as being specific to the Baikal-T1 SoC) and introduced in the
> > framework of this patchset.
> >
> > This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
> > commit 98d54f81e36b ("Linux 5.6-rc4").
> 
> I have no objection to the drivers, but I wonder if these should be
> in drivers/bus and drivers/memory instead of drivers/soc, which have
> similar drivers already. The driver for the L2 cache is not really a
> memory controller driver, but it may be close enough, and we
> already have a couple of different things in there.
> 
>           Arnd

Sorry for a delay. I was analyzing and fixing comments, which were raised
in the framework of anther patchsets I've submitted. Some of them including
yours cause bigger changes than just a few fixups and might be resolved
at once by a solution I've described in RFC: https://lkml.org/lkml/2020/3/22/393
You've been in Cc there, so feel free to send your comments.

Regarding ehb drivers. You are right. They should be moved to the drivers/bus
(it has also been described in the RFC). It is more suitable place for them.
I'll do it in v2.

Regarding l2 driver. Do you really think that L2 cache should be in
drivers/memory? First there is no any cache-related drivers in that
subsystem (at least I couldn't find any). Second the Baikal-T1
L2-cache-RAM config block has just indirect connection with RAM.
The block just tunes the L2-cache<->RAM stall clock cycles up on
WS/Tag/Data RAM IO-operations. This config seems more SoC-specific,
than memory-like. Do you think that the driver should still be in
drivers/memory?

On the other hand the block is part of the System Controller. I could
just embed the l2-cache driver functionality into the System Controller
MFD driver. Though honestly IMHO the functionality should live in
a dedicated driver and drivers/soc is a better place for it. I also have
doubts this part will be well accepted by Lee (drivers/mfd maintainer).

So what do you think?

Regards,
-Sergey