Re: [PATCH RFC v2 04/10] cacheinfo: Expose the code to generate a cache-id from a device_node

[Yin Li]

On 9/14/2026 8:26 PM, Andre Przywara wrote:
> Hi,
>
> On 9/14/26 11:37, Yin Li wrote:
> > From: James Morse <james.morse@xxxxxxx>
> >
> > The MPAM driver identifies caches by id for use with resctrl. It
> > needs to know the cache-id when probe-ing, but the value isn't set
> > in cacheinfo until device_initcall(). Even after device_initcall(),
> > the cache-id is only available if at least one CPU associated with
> > the cache is online.
> >
> > Instead of making the driver wait, expose the code that generates the
> > cache-id. The parts of the MPAM driver that run early can use this to
> > set up the resctrl structures before cacheinfo is ready in
> > device_initcall().
> >
> > Signed-off-by: James Morse <james.morse@xxxxxxx>
> > [ Yin Li: fix context conflicts in cacheinfo.c and cacheinfo.h; guard the
> >    cache_of_calculate_id() declaration with CONFIG_OF to prevent build
> >    errors when CONFIG_OF is not set ]
>
> You can shorten that part in square brackets: doing adjustments due to 
> rebasing is surely implied, and you can shorten the rest, like:
> [ Yin Li: guard cache_of_calculate_id() prototype ]
>
> Speaking of which ...
>
> > Signed-off-by: Yin Li <yin.li@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/base/cacheinfo.c  | 17 ++++++++++++-----
> >   include/linux/cacheinfo.h |  3 +++
> >   2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> > index 9f9c72727a05..f75e7f64038b 100644
> > --- a/drivers/base/cacheinfo.c
> > +++ b/drivers/base/cacheinfo.c
> > @@ -226,8 +226,7 @@ static bool match_cache_node(struct device_node *cpu,
> >   #define arch_compact_of_hwid(_x)    (_x)
> >   #endif
> > -static void cache_of_set_id(struct cacheinfo *this_leaf,
> > -                struct device_node *cache_node)
> > +u32 cache_of_calculate_id(struct device_node *cache_node)
> >   {
> >       struct device_node *cpu;
> >       u32 min_id = ~0;
> > @@ -238,15 +237,23 @@ static void cache_of_set_id(struct cacheinfo 
> > *this_leaf,
> >           id = arch_compact_of_hwid(id);
> >           if (FIELD_GET(GENMASK_ULL(63, 32), id)) {
> >               of_node_put(cpu);
> > -            return;
> > +            return ~0;
> >           }
> >           if (match_cache_node(cpu, cache_node))
> >               min_id = min(min_id, id);
> >       }
> > -    if (min_id != ~0) {
> > -        this_leaf->id = min_id;
> > +    return min_id;
> > +}
> > +
> > +static void cache_of_set_id(struct cacheinfo *this_leaf,
> > +                struct device_node *cache_node)
> > +{
> > +    u32 id = cache_of_calculate_id(cache_node);
> > +
> > +    if (id != ~0) {
> > +        this_leaf->id = id;
> >           this_leaf->attributes |= CACHE_ID;
> >       }
> >   }
> > diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
> > index fc879ac4cc4f..c33bb3c8bd63 100644
> > --- a/include/linux/cacheinfo.h
> > +++ b/include/linux/cacheinfo.h
> > @@ -113,6 +113,9 @@ int acpi_get_cache_info(unsigned int cpu,
> >   #endif
> >   const struct attribute_group *cache_get_priv_group(struct cacheinfo 
> > *this_leaf);
> > +#ifdef CONFIG_OF
>
> Why is that, exactly? First IIUC it's quite uncommon to use #ifdef 
> guards around prototypes (unless they are stubbed without the symbol 
> defined). Using types protected by those symbols if certainly another 
> reason, and it looks like this would be the case here, but I had no 
> trouble building the kernel for x86, where CONFIG_OF is not defined.
> So can you share a .config example (or give a hint) as to where this 
> fails building?
> And if it does, wouldn't it be better to always include <linux/of.h> in 
> that file instead? I think I see a similar pattern elsewhere (rfkill- 
> gpio.c, sound/ac97/bus.c).

Hi Andre,

On the #ifdef CONFIG_OF: I also verified that removing the guard doesn't
break the build. However, since the implementation in cacheinfo.c is
itself guarded by #ifdef CONFIG_OF, exposing the prototype
unconditionally could cause a link error on CONFIG_OF=n builds if called
from outside the ARM64/MPAM path. A more idiomatic approach might be to
use a stub to keep the header consistent with the implementation:

  #ifdef CONFIG_OF
  u32 cache_of_calculate_id(struct device_node *np);
  #else
  static inline u32 cache_of_calculate_id(struct device_node *np)
  {
      return ~0U;
  }
  #endif

Would that work for you? Otherwise I'm happy to just drop the guard in
the next version.

Thanks,
Yin


> Cheers,
> Andre
>
>
> > +u32 cache_of_calculate_id(struct device_node *np);
> > +#endif
> >   /*
> >    * Get the cacheinfo structure for the cache associated with @cpu at

--
Thx and BRs,
Yin