Hi, On 2/11/23 07:32, Srinivas Pandruvada wrote: > To support S3/S4 with TPMI interface add suspend/resume callbacks. > Here HW state is stored in suspend callback and restored during > resume callback. > > The hardware state which needs to be stored/restored: > - CLOS configuration > - CLOS Association > - SST-CP enable/disable status > - SST-PP perf level setting > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > --- > .../x86/intel/speed_select_if/isst_tpmi.c | 21 ++++++++ > .../intel/speed_select_if/isst_tpmi_core.c | 49 +++++++++++++++++++ > .../intel/speed_select_if/isst_tpmi_core.h | 2 + > 3 files changed, 72 insertions(+) > > diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c > index 7b4bdeefb8bc..ef39870c9829 100644 > --- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c > +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi.c > @@ -34,6 +34,24 @@ static void intel_sst_remove(struct auxiliary_device *auxdev) > tpmi_sst_exit(); > } > > +static int __maybe_unused intel_sst_suspend(struct device *dev) > +{ > + tpmi_sst_dev_suspend(to_auxiliary_dev(dev)); > + > + return 0; > +} > + > +static int __maybe_unused intel_sst_resume(struct device *dev) > +{ > + tpmi_sst_dev_resume(to_auxiliary_dev(dev)); > + > + return 0; > +} Please drop the __maybe_unused here; and > + > +static const struct dev_pm_ops intel_sst_pm = { > + SET_SYSTEM_SLEEP_PM_OPS(intel_sst_suspend, intel_sst_resume) > +}; > + Replace this with: static DEFINE_SIMPLE_DEV_PM_OPS(intel_sst_pm, intel_sst_suspend, intel_sst_resume); > static const struct auxiliary_device_id intel_sst_id_table[] = { > { .name = "intel_vsec.tpmi-sst" }, > {} > @@ -44,6 +62,9 @@ static struct auxiliary_driver intel_sst_aux_driver = { > .id_table = intel_sst_id_table, > .remove = intel_sst_remove, > .probe = intel_sst_probe, > + .driver = { > + .pm = &intel_sst_pm, And use: .pm = pm_sleep_ptr(&intel_sst_pm), Here, this is the new #ifdef and __maybe_unused free way of dealing with pm_ops. Regards, Hans > + }, > }; > > module_auxiliary_driver(intel_sst_aux_driver); > 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 9eaff90bb649..e173167085ea 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 > @@ -229,6 +229,10 @@ struct perf_level { > * @status_offset: Store the status offset for each PP-level > * @sst_base: Mapped SST base IO memory > * @auxdev: Auxiliary device instance enumerated this instance > + * @saved_sst_cp_control: Save SST-CP control configuration to store restore for suspend/resume > + * @saved_clos_configs: Save SST-CP CLOS configuration to store restore for suspend/resume > + * @saved_clos_assocs: Save SST-CP CLOS association to store restore for suspend/resume > + * @saved_pp_control: Save SST-PP control information to store restore for suspend/resume > * > * This structure is used store complete SST information for a power_domain. This information > * is used to read/write request for any SST IOCTL. Each physical CPU package can have multiple > @@ -250,6 +254,10 @@ struct tpmi_per_power_domain_info { > struct pp_status_offset status_offset; > void __iomem *sst_base; > struct auxiliary_device *auxdev; > + u64 saved_sst_cp_control; > + u64 saved_clos_configs[4]; > + u64 saved_clos_assocs[4]; > + u64 saved_pp_control; > }; > > /** > @@ -1333,6 +1341,47 @@ void tpmi_sst_dev_remove(struct auxiliary_device *auxdev) > } > EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_remove, INTEL_TPMI_SST); > > +void tpmi_sst_dev_suspend(struct auxiliary_device *auxdev) > +{ > + struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev); > + struct tpmi_per_power_domain_info *power_domain_info = tpmi_sst->power_domain_info; > + void __iomem *cp_base; > + > + cp_base = power_domain_info->sst_base + power_domain_info->sst_header.cp_offset; > + power_domain_info->saved_sst_cp_control = readq(cp_base + SST_CP_CONTROL_OFFSET); > + > + memcpy_fromio(power_domain_info->saved_clos_configs, cp_base + SST_CLOS_CONFIG_0_OFFSET, > + sizeof(power_domain_info->saved_clos_configs)); > + > + memcpy_fromio(power_domain_info->saved_clos_assocs, cp_base + SST_CLOS_ASSOC_0_OFFSET, > + sizeof(power_domain_info->saved_clos_assocs)); > + > + power_domain_info->saved_pp_control = readq(power_domain_info->sst_base + > + power_domain_info->sst_header.pp_offset + > + SST_PP_CONTROL_OFFSET); > +} > +EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_suspend, INTEL_TPMI_SST); > + > +void tpmi_sst_dev_resume(struct auxiliary_device *auxdev) > +{ > + struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev); > + struct tpmi_per_power_domain_info *power_domain_info = tpmi_sst->power_domain_info; > + void __iomem *cp_base; > + > + cp_base = power_domain_info->sst_base + power_domain_info->sst_header.cp_offset; > + writeq(power_domain_info->saved_sst_cp_control, cp_base + SST_CP_CONTROL_OFFSET); > + > + memcpy_toio(cp_base + SST_CLOS_CONFIG_0_OFFSET, power_domain_info->saved_clos_configs, > + sizeof(power_domain_info->saved_clos_configs)); > + > + memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET, power_domain_info->saved_clos_assocs, > + sizeof(power_domain_info->saved_clos_assocs)); > + > + writeq(power_domain_info->saved_pp_control, power_domain_info->sst_base + > + power_domain_info->sst_header.pp_offset + SST_PP_CONTROL_OFFSET); > +} > +EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_resume, INTEL_TPMI_SST); > + > #define ISST_TPMI_API_VERSION 0x02 > > int tpmi_sst_init(void) > diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h > index 356cb02273b1..900b483703f9 100644 > --- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h > +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.h > @@ -13,4 +13,6 @@ int tpmi_sst_init(void); > void tpmi_sst_exit(void); > int tpmi_sst_dev_add(struct auxiliary_device *auxdev); > void tpmi_sst_dev_remove(struct auxiliary_device *auxdev); > +void tpmi_sst_dev_suspend(struct auxiliary_device *auxdev); > +void tpmi_sst_dev_resume(struct auxiliary_device *auxdev); > #endif