Re: [PATCH v9 1/5] riscv_cbqri: Add capacity controller probe and allocation device ops

[Drew Fustini <fustini@kernel.org>]

On Mon, Oct 12, 2026 at 09:42:59PM +0530, Sunil V L wrote:
> Hi Drew!

Hi Sunil, thanks for reviewing!

> On Sat, Oct 3, 2026 at 12:45 PM Drew Fustini <fustini@kernel.org> wrote:
> >
> > Add support for the RISC-V CBQRI capacity controller. A platform driver
> > passes a cbqri_controller_info descriptor together with the cache level
> > to riscv_cbqri_register_cc_dt(), which probes the controller and adds it
> > to the controller list.
> >
> > Assisted-by: LLM
> > Co-developed-by: Adrien Ricciardi <aricciardi@baylibre.com>
> > Signed-off-by: Adrien Ricciardi <aricciardi@baylibre.com>
> > Signed-off-by: Drew Fustini <fustini@kernel.org>
> > ---
> >  MAINTAINERS                      |   3 +
> >  drivers/resctrl/Kconfig          |   9 +
> >  drivers/resctrl/Makefile         |   3 +
> >  drivers/resctrl/cbqri_devices.c  | 574 +++++++++++++++++++++++++++++++++++++++
> >  drivers/resctrl/cbqri_internal.h | 122 +++++++++
> >  include/linux/riscv_cbqri.h      |  45 +++
> >  6 files changed, 756 insertions(+)
[..]
> > diff --git a/drivers/resctrl/cbqri_devices.c b/drivers/resctrl/cbqri_devices.c
> > new file mode 100644
> > index 000000000000..87d4ecf8f141
> > --- /dev/null
> > +++ b/drivers/resctrl/cbqri_devices.c
[..]
> > +static int cbqri_probe_controller(struct cbqri_controller *ctrl)
> > +{
[..]
> > +
> > +       if (!request_mem_region(ctrl->addr, ctrl->size, "cbqri_controller")) {
> > +               pr_err("request_mem_region failed for %pa\n", &ctrl->addr);
> > +               return -EBUSY;
> > +       }
> > +
> Apologies for jumping on this late. But why not simply use
> devm_platform_ioremap_resource() ?

This is an artifact of my fuller RFC series which implemented RQSC as
well. For ACPI, it registers controllers directly from the RQSC table
without a platform device.

But you make a good point and this is what MPAM does as well. I will
switch to devm_platform_get_and_ioremap_resource() and add a base field
to cbqri_controller_info.

> > +/**
> > + * riscv_cbqri_register_cc_dt() - register a DT-described capacity controller
> > + * @info:        registration descriptor. info->cache_id is used as the
> > + *               resctrl domain id. info->type must be CAPACITY.
> > + * @cache_level: cache level (2 or 3) the controller backs, mapped to the
> > + *               resctrl L2/L3 resource by the resctrl glue.
> > + *
> > + * The cache level and id (info->cache_id) are supplied directly by the
> > + * caller, which resolves them from cacheinfo. Domain membership is resolved
> > + * per CPU at hotplug, so no cpu mask is stored. There is no firmware table
> > + * to resolve any of this from.
> > + *
> > + * Return: 0 on success, -EEXIST when a controller for the same cache level
> > + * and id is already registered, or another negative errno on failure.
> > + */
> > +int riscv_cbqri_register_cc_dt(const struct cbqri_controller_info *info,
> > +                              u32 cache_level)
> >
> What is DT specific code in this function? IMO, it is better to gather
> all information in the main
> driver itself into a generic "info" structure. That way, all ACPI vs
> DT differences will be handled in
> one place and everything further will be generic.

There is nothing that is DT specific beyond needing to pass the cache
level whereas ACPI can determine it from PPTT.

But I think it would make sense to be more generic. I will move
cache_level into cbqri_controller_info and drop the _dt suffix.

[..]
> > +       /*
> > +        * SRMCFG encodes RCID in 12 bits. Reject an out-of-range count rather
> > +        * than silently truncating in every FIELD_PREP(SRMCFG_RCID_MASK, closid)
> > +        * on the schedule-in fast path.
> > +        */
> > +       if (ctrl->rcid_count > FIELD_MAX(SRMCFG_RCID_MASK) + 1) {
> > +               pr_warn("CC at %pa has RCID count %u beyond the 12-bit SRMCFG field, skipping\n",
> > +                       &ctrl->addr, ctrl->rcid_count);
> > +               cbqri_controller_destroy(ctrl);
> >
> Any reason to have explicit cleanups ?

No. I will move the info->rcid_count check before allocation and use
__free() for the controller.

[..]
> > diff --git a/drivers/resctrl/cbqri_internal.h b/drivers/resctrl/cbqri_internal.h
[..]
> > +extern struct list_head cbqri_controllers;
> > +extern struct mutex cbqri_controllers_lock;
> > +
> >
> Can these be made static in cbqri_devices.c and provide API for cbqri_resctrl.c?

Yes, the only use outside of cbqri_devices.c is
cbqri_resctrl_pick_caches(). I will make both static and add a helper
that returns the registered controllers.

Thanks,
Drew