Re: [PATCH v2 2/2] platform/x86/intel-uncore-freq: Expose instance ID in the sysfs

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

On Tue, 2026-04-07 at 18:19 +0000, Maciej Wieczor-Retman wrote:
> On 2026-04-07 at 11:03:06 -0700, srinivas pandruvada wrote:
> > On Thu, 2026-04-02 at 19:59 +0000, Maciej Wieczor-Retman wrote:
> > > From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> > > 
> > > Insufficient data is exported to allow direct access to TPMI
> > > registers
> > > through MMIO. On non-partitioned systems domain_id can be used
> > > both
> > > for
> > > mapping CPUs to their compute die IDs and for mapping die indices
> > > to
> > > their MMIO memory blocks mapped
> > >  into userspace.
> > 
> > not mapped. But presented to user space via TPMI debugfs.
> > >  However on partitioned
> > > systems it can't be used for mapping MMIO blocks anymore.
> > Again not mapping.
> 
> Okay, I'll fix these two messages.
> 
> ...
> > > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-
> > > frequency-
> > > tpmi.c
> > > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-
> > > frequency-
> > > tpmi.c
> > > @@ -385,7 +385,19 @@ static u8 io_die_index_next;
> > >  /* Lock to protect io_die_start, io_die_index_next */
> > >  static DEFINE_MUTEX(domain_lock);
> > >  
> > > -static void set_domain_id(int id,  int num_resources,
> > > +static void set_instance_id(int id, struct
> > > tpmi_uncore_cluster_info
> > > *cluster_info)
> > > +{
> > > +	/*
> > > +	 * On non-partitioned systems domain_id can be used for
> > > mapping both
> > > +	 * CPUs to compute die IDs and physical die indexes to
> > > MMIO
> > > mapped
> > > +	 * memory. However on partitioned systems domain_id
> > > loses
> > > the second
> > > +	 * association. Therefore instance_id should be used for
> > > that instead,
> > > +	 * while domain_id should still be used to match CPUs to
> > > compute dies.
> > > +	 */
> > > +	cluster_info->uncore_data.instance_id = id;
> > > +}
> > 
> > Do you need a function for single line assignment? Why not just
> > assign
> > in the uncore_probe().
> 
> I thought uncore_probe() would be less cluttered if I move this
> assignment to a
> separate function, especially with this longer comment.
> 
> But I can move it to uncore_probe(). Do you think the comment is
> helpful or
> should it be shortened? I thought it'd be good to keep some context
> information
> in here too.

I think static inline should be OK.

https://www.kernel.org/doc/html/v5.8/process/coding-style.html#the-inline-disease

"
A reasonable rule of thumb is to not put inline at functions that have
more than 3 lines of code in them. An exception to this rule are the
cases where a parameter is known to be a compiletime constant, and as a
result of this constantness you know the compiler will be able to
optimize most of your function away at compile time. For a good example
of this later case, see the kmalloc() inline function.

"