On Wed, Oct 11, 2023 at 1:20 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On Wed, 11 Oct 2023 at 18:49, Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Sun, Aug 27, 2023 at 02:50:18PM +0300, Dmitry Baryshkov wrote:
> > > Add a simple driver that handles scaling of L2 frequency and voltages.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > > ---
> >
> > [...]
> >
> > > +static const struct of_device_id krait_l2_match_table[] = {
> > > + { .compatible = "qcom,krait-l2-cache" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, krait_l2_match_table);
> > > +
> > > +static struct platform_driver krait_l2_driver = {
> > > + .probe = krait_l2_probe,
> > > + .remove = krait_l2_remove,
> > > + .driver = {
> > > + .name = "qcom-krait-l2",
> > > + .of_match_table = krait_l2_match_table,
> > > + .sync_state = icc_sync_state,
> > > + },
> > > +};
> >
> > As I mentioned in the other thread, cache devices already have a struct
> > device. Specifically, they have a struct device (no subclass) on the
> > cpu_subsys bus type. So there should be no need for a platform device
> > and second struct device.
> >
> > See drivers/acpi/processor_driver.c for an example. Or grep any use of
> > "cpu_subsys".
>
> Most likely you mean drivers/base/cacheinfo.c. I saw this code, I
> don't think it makes a good fit here. The cacheinfo devices provide
> information only, they are not tied to DT nodes in any way.
They are completely tied to DT nodes beyond L1.
> cpu_subsys
> doesn't provide a way to match drivers with subsys devices in the
> non-ACPI case, etc.
That's a 2 line addition to add DT support.
> Moreover, the whole cacheinfo subsys is
> non-existing on arm32, there is no cacheinfo implementation there,
> thanks to the overall variety of architectures.
Humm, well I don't think it would be too hard to add, but I won't ask
you to do that. All the info comes from DT or can come from DT, so it
should be just a matter of arm32 calling the cacheinfo init.
> Thus said, I don't think cacheinfo makes a good fit for the case of
> scaling L2 cache.
I still disagree. It's not really cacheinfo. That is what creates the
devices, but it's the cpu_subsys bus type. Why do you care that it is
platform bus vs. cpu_subsys?
On a separate issue, I'd propose you move this to drivers/cache/
instead of the dumping ground that is drivers/soc/. It's nothing more
than a location to collect cache related drivers ATM because we seem
to be accumulating more of them.
Rob
