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

[Maciej Wieczor-Retman]

On 2026-04-07 at 13:37:32 -0700, srinivas pandruvada wrote:
>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@xxxxxxxxx>
>> > >
>> > > 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.
>
>"
>

Cool, thanks, I'll do that.

-- 
Kind regards
Maciej Wieczór-Retman