Re: [PATCH 1/2] platform/x86: ISST: Support SST-TF revision 2

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

On Tue, 2025-04-29 at 17:23 +0300, Ilpo Järvinen wrote:
> On Thu, 17 Apr 2025, Srinivas Pandruvada wrote:
> 
> > SST-TF revision 2 supports a higher number of cores per bucket, as
> > the
> > current limit of 256 cores may be insufficient. To accommodate
> > this, a
> > new offset, "SST_TF_INFO-8," is introduced, allowing for a higher
> > core
> > count. Utilize this offset instead of the current "SST_TF_INFO-1"
> > offset,
> > based on SST-TF revision 2 or higher, and if there is a non-zero
> > core
> > count in any bucket.
> > 
...

> > +	if (feature_rev >= 2) {
> > +		bool valid = false;
> > +
> > +		for (i = 0; i < SST_TF_INFO_8_BUCKETS; ++i) {
> > +			_read_tf_level_info("bucket_*_mod_count",
> > turbo_freq.bucket_core_counts[i],
> > +					    turbo_freq.level,
> > SST_TF_INFO_8_OFFSET,
> > +					    SST_TF_NUM_MOD_0_WIDTH
> > * i, SST_TF_NUM_MOD_0_WIDTH,
> > +					    SST_MUL_FACTOR_NONE)
> > +
> > +			if (!valid &&
> > turbo_freq.bucket_core_counts[i])
> 
> I'd just drop !valid from this check.
> 
> > +				valid = true;
> > +		}
> > +
> > +		if (valid)
> 
> 
> Should this be named instead to something like has_tf_info_8 ? (As
> this is 
> not really valid/invalid check but whether this new info exists or
> not.)
We can.

Thanks,
Srinivas

> 
> > +			goto done_core_count;
> > +	}
> > +
> >  	for (i = 0; i < TRL_MAX_BUCKETS; ++i)
> >  		_read_tf_level_info("bucket_*_core_count",
> > turbo_freq.bucket_core_counts[i],
> >  				    turbo_freq.level,
> > SST_TF_INFO_1_OFFSET,
> >  				    SST_TF_NUM_CORE_0_WIDTH * i,
> > SST_TF_NUM_CORE_0_WIDTH,
> >  				    SST_MUL_FACTOR_NONE)
> >  
> > +
> > +done_core_count:
> > +
> >  	if (copy_to_user(argp, &turbo_freq, sizeof(turbo_freq)))
> >  		return -EFAULT;
> >  
> > 
>