On Thu, 2023-11-30 at 14:20 +0200, Ilpo Järvinen wrote: > On Tue, 28 Nov 2023, Srinivas Pandruvada wrote: > > > When a feature is read blocked, don't continue to read SST > > information > > and register with SST core. > > > > When the feature is write blocked, continue to offer read interface > > for > > SST parameters, but don't allow any operation to change state. A > > state > > change results from SST level change, feature change or class of > > service > > change. > > > > Signed-off-by: Srinivas Pandruvada > > <srinivas.pandruvada@xxxxxxxxxxxxxxx> > > --- > > .../intel/speed_select_if/isst_tpmi_core.c | 25 > > +++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git > > a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c > > b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c > > index 0b6d2c864437..ed3a04d6c99c 100644 > > --- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c > > +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c > > @@ -514,6 +516,9 @@ static long isst_if_clos_param(void __user > > *argp) > > if (!power_domain_info) > > return -EINVAL; > > > > + if (power_domain_info->write_blocked) > > + return -EPERM; > > + > > I don't understand this, doesn't this now -EPERM both > _write_cp_info() AND > _read_cp_info()??? Does _read_cp_info() also change state?? You have a point here. Unlike other SST features, CP access is useful for OS as it know workloads and priorities. But I will change for consistency. Thanks, Srinivas > > > if (clos_param.get_set) { > > _write_cp_info("clos.min_freq", > > clos_param.min_freq_mhz, > > (SST_CLOS_CONFIG_0_OFFSET + > > clos_param.clos * SST_REG_SIZE), > > @@ -602,6 +607,9 @@ static long isst_if_clos_assoc(void __user > > *argp) > > > > power_domain_info = &sst_inst- > > >power_domain_info[punit_id]; > > > > + if (power_domain_info->write_blocked) > > + return -EPERM; > > Same here, this blocks also the get path? > >