Re: [PATCH v2 1/5] platform/x86/intel-uncore-freq: Add attributes to show agent types

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

On Wed, 2025-05-07 at 16:02 +0300, Ilpo Järvinen wrote:
> On Mon, 28 Apr 2025, Srinivas Pandruvada wrote:
> 
> > Currently, users need detailed hardware information to understand
> > the
> > scope of controls within each uncore domain. Uncore frequency
> > controls
> > manage subsystems such as core, cache, memory, and I/O. The UFS
> > TPMI
> > provides this information, which can be used to present the scope
> > more
> > clearly.
> > 
> > Each uncore domain consists of one or more agent types, with each
> > agent
> > type controlling one or more uncore hardware subsystems. For
> > example, a
> > single agent might control both the core and cache.
> > 
> > Introduce a new attribute called "agent_types." This attribute
> > displays
> > a list of agents, separated by space character.
> > 
> > The string representations for agent types are as follows:
> > 	For core agent: core
> > 	For cache agent: cache
> > 	For memory agent: memory
> > 	For I/O agent: io
> > 
> > These agent types are read during probe time for each cluster and
> > stored
> > as part of the struct uncore_data.
> > 
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > ---
> > v2:
> > No change
> > 
> >  .../uncore-frequency-common.c                 | 24
> > ++++++++++++++++
> >  .../uncore-frequency-common.h                 | 17 ++++++++++-
> >  .../uncore-frequency/uncore-frequency-tpmi.c  | 28
> > +++++++++++++++++++
> >  3 files changed, 68 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-
> > frequency-common.c b/drivers/platform/x86/intel/uncore-
> > frequency/uncore-frequency-common.c
> > index 4e2c6a2d7e6e..cfa3039a0e39 100644
> > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
> > common.c
> > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
> > common.c
> > @@ -43,6 +43,28 @@ static ssize_t show_package_id(struct kobject
> > *kobj, struct kobj_attribute *attr
> >  	return sprintf(buf, "%u\n", data->package_id);
> >  }
> >  
> > +static ssize_t show_agent_types(struct kobject *kobj, struct
> > kobj_attribute *attr, char *buf)
> > +{
> > +	struct uncore_data *data = container_of(attr, struct
> > uncore_data, agent_types_kobj_attr);
> > +	int length = 0;
> > +
> > +	if (data->agent_type_mask & AGENT_TYPE_CORE)
> > +		length += sysfs_emit_at(buf, length, "core ");
> > +
> > +	if (data->agent_type_mask & AGENT_TYPE_CACHE)
> > +		length += sysfs_emit_at(buf, length, "cache ");
> > +
> > +	if (data->agent_type_mask & AGENT_TYPE_MEMORY)
> > +		length += sysfs_emit_at(buf, length, "memory ");
> > +
> > +	if (data->agent_type_mask & AGENT_TYPE_IO)
> > +		length += sysfs_emit_at(buf, length, "io ");
> 
> Is this set going to get expanded soon?

Unlikely, as this list is adding every frequency scaled subsystem.

But I will try to change to loop. No issue.

Thanks,
Srinivas


>  It would feel more future proof to
> do this mapping using a loop and array. You also chose the quick and
> dirty 
> approach wrt. trailing spaces as getting rid of the extra space is a
> bit 
> tedious when open coding the mapping like that ;-).
>