[v1] platform/x86: ISST: Fix for write lock and suspend/resume

[PATCH 2/2] platform/x86: ISST: Store and restore all domains data

Posted by Srinivas Pandruvada 1 month, 1 week ago

The suspend/resume callbacks currently only store and restore the
configuration for power domain 0. However, other power domains may also
have modified configurations that need to be preserved across suspend/
resume cycles.

Extend the store/restore functionality to handle all power domains.

Fixes: 91576acab020 ("platform/x86: ISST: Add suspend/resume callbacks")
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
CC: stable@vger.kernel.org
---
 .../intel/speed_select_if/isst_tpmi_core.c    | 53 ++++++++++++-------
 1 file changed, 33 insertions(+), 20 deletions(-)

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 f587709ddd47..47026bb3e1af 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
@@ -1723,55 +1723,68 @@ 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;
+	struct tpmi_per_power_domain_info *power_domain_info, *pd_info;
 	struct oobmsm_plat_info *plat_info;
 	void __iomem *cp_base;
+	int num_resources, i;
 
 	plat_info = tpmi_get_platform_data(auxdev);
 	if (!plat_info)
 		return;
 
 	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
+	num_resources = tpmi_sst->number_of_power_domains[plat_info->partition];
 
-	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);
+	for (i = 0; i < num_resources; i++) {
+		pd_info = &power_domain_info[i];
+		if (!pd_info || !pd_info->sst_base)
+			continue;
 
-	memcpy_fromio(power_domain_info->saved_clos_configs, cp_base + SST_CLOS_CONFIG_0_OFFSET,
-		      sizeof(power_domain_info->saved_clos_configs));
+		cp_base = pd_info->sst_base + pd_info->sst_header.cp_offset;
 
-	memcpy_fromio(power_domain_info->saved_clos_assocs, cp_base + SST_CLOS_ASSOC_0_OFFSET,
-		      sizeof(power_domain_info->saved_clos_assocs));
+		pd_info->saved_sst_cp_control = readq(cp_base + SST_CP_CONTROL_OFFSET);
+		memcpy_fromio(pd_info->saved_clos_configs, cp_base + SST_CLOS_CONFIG_0_OFFSET,
+			      sizeof(pd_info->saved_clos_configs));
+		memcpy_fromio(pd_info->saved_clos_assocs, cp_base + SST_CLOS_ASSOC_0_OFFSET,
+			      sizeof(pd_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);
+		pd_info->saved_pp_control = readq(pd_info->sst_base +
+						  pd_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;
+	struct tpmi_per_power_domain_info *power_domain_info, *pd_info;
 	struct oobmsm_plat_info *plat_info;
 	void __iomem *cp_base;
+	int num_resources, i;
 
 	plat_info = tpmi_get_platform_data(auxdev);
 	if (!plat_info)
 		return;
 
 	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
+	num_resources = tpmi_sst->number_of_power_domains[plat_info->partition];
 
-	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));
+	for (i = 0; i < num_resources; i++) {
+		pd_info = &power_domain_info[i];
+		if (!pd_info || !pd_info->sst_base)
+			continue;
 
-	memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET, power_domain_info->saved_clos_assocs,
-		    sizeof(power_domain_info->saved_clos_assocs));
+		cp_base = pd_info->sst_base + pd_info->sst_header.cp_offset;
+		writeq(pd_info->saved_sst_cp_control, cp_base + SST_CP_CONTROL_OFFSET);
+		memcpy_toio(cp_base + SST_CLOS_CONFIG_0_OFFSET, pd_info->saved_clos_configs,
+			    sizeof(pd_info->saved_clos_configs));
+		memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET, pd_info->saved_clos_assocs,
+			    sizeof(pd_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);
+		writeq(pd_info->saved_pp_control, power_domain_info->sst_base +
+		       pd_info->sst_header.pp_offset + SST_PP_CONTROL_OFFSET);
+	}
 }
 EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_resume, "INTEL_TPMI_SST");
 
-- 
2.52.0

Re: [PATCH 2/2] platform/x86: ISST: Store and restore all domains data

Posted by Ilpo Järvinen 1 month ago

On Mon, 29 Dec 2025, Srinivas Pandruvada wrote:

> The suspend/resume callbacks currently only store and restore the
> configuration for power domain 0. However, other power domains may also
> have modified configurations that need to be preserved across suspend/
> resume cycles.
> 
> Extend the store/restore functionality to handle all power domains.
> 
> Fixes: 91576acab020 ("platform/x86: ISST: Add suspend/resume callbacks")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> CC: stable@vger.kernel.org
> ---
>  .../intel/speed_select_if/isst_tpmi_core.c    | 53 ++++++++++++-------
>  1 file changed, 33 insertions(+), 20 deletions(-)
> 
> 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 f587709ddd47..47026bb3e1af 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
> @@ -1723,55 +1723,68 @@ 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;
> +	struct tpmi_per_power_domain_info *power_domain_info, *pd_info;
>  	struct oobmsm_plat_info *plat_info;
>  	void __iomem *cp_base;
> +	int num_resources, i;
>  
>  	plat_info = tpmi_get_platform_data(auxdev);
>  	if (!plat_info)
>  		return;
>  
>  	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
> +	num_resources = tpmi_sst->number_of_power_domains[plat_info->partition];
>  
> -	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);
> +	for (i = 0; i < num_resources; i++) {
> +		pd_info = &power_domain_info[i];
> +		if (!pd_info || !pd_info->sst_base)
> +			continue;
>  
> -	memcpy_fromio(power_domain_info->saved_clos_configs, cp_base + SST_CLOS_CONFIG_0_OFFSET,
> -		      sizeof(power_domain_info->saved_clos_configs));
> +		cp_base = pd_info->sst_base + pd_info->sst_header.cp_offset;
>  
> -	memcpy_fromio(power_domain_info->saved_clos_assocs, cp_base + SST_CLOS_ASSOC_0_OFFSET,
> -		      sizeof(power_domain_info->saved_clos_assocs));
> +		pd_info->saved_sst_cp_control = readq(cp_base + SST_CP_CONTROL_OFFSET);
> +		memcpy_fromio(pd_info->saved_clos_configs, cp_base + SST_CLOS_CONFIG_0_OFFSET,
> +			      sizeof(pd_info->saved_clos_configs));
> +		memcpy_fromio(pd_info->saved_clos_assocs, cp_base + SST_CLOS_ASSOC_0_OFFSET,
> +			      sizeof(pd_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);
> +		pd_info->saved_pp_control = readq(pd_info->sst_base +
> +						  pd_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;
> +	struct tpmi_per_power_domain_info *power_domain_info, *pd_info;
>  	struct oobmsm_plat_info *plat_info;
>  	void __iomem *cp_base;
> +	int num_resources, i;
>  
>  	plat_info = tpmi_get_platform_data(auxdev);
>  	if (!plat_info)
>  		return;
>  
>  	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
> +	num_resources = tpmi_sst->number_of_power_domains[plat_info->partition];
>  
> -	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));
> +	for (i = 0; i < num_resources; i++) {
> +		pd_info = &power_domain_info[i];
> +		if (!pd_info || !pd_info->sst_base)
> +			continue;
>  
> -	memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET, power_domain_info->saved_clos_assocs,
> -		    sizeof(power_domain_info->saved_clos_assocs));
> +		cp_base = pd_info->sst_base + pd_info->sst_header.cp_offset;
> +		writeq(pd_info->saved_sst_cp_control, cp_base + SST_CP_CONTROL_OFFSET);
> +		memcpy_toio(cp_base + SST_CLOS_CONFIG_0_OFFSET, pd_info->saved_clos_configs,
> +			    sizeof(pd_info->saved_clos_configs));
> +		memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET, pd_info->saved_clos_assocs,
> +			    sizeof(pd_info->saved_clos_assocs));

Why is the use of empty lines inconsistent between suspend and resume?

> -	writeq(power_domain_info->saved_pp_control, power_domain_info->sst_base +
> -				power_domain_info->sst_header.pp_offset + SST_PP_CONTROL_OFFSET);
> +		writeq(pd_info->saved_pp_control, power_domain_info->sst_base +
> +		       pd_info->sst_header.pp_offset + SST_PP_CONTROL_OFFSET);
> +	}
>  }
>  EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_resume, "INTEL_TPMI_SST");
>  
> 

-- 
 i.

Re: [PATCH 2/2] platform/x86: ISST: Store and restore all domains data

Posted by srinivas pandruvada 1 month ago

On Tue, 2026-01-06 at 11:44 +0200, Ilpo Järvinen wrote:
> On Mon, 29 Dec 2025, Srinivas Pandruvada wrote:
> 
> > The suspend/resume callbacks currently only store and restore the
> > configuration for power domain 0. However, other power domains may
> > also
> > have modified configurations that need to be preserved across
> > suspend/
> > resume cycles.
> > 
> > Extend the store/restore functionality to handle all power domains.
> > 
> > Fixes: 91576acab020 ("platform/x86: ISST: Add suspend/resume
> > callbacks")
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > CC: stable@vger.kernel.org
> > ---
> >  .../intel/speed_select_if/isst_tpmi_core.c    | 53 ++++++++++++---
> > ----
> >  1 file changed, 33 insertions(+), 20 deletions(-)
> > 
> > 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 f587709ddd47..47026bb3e1af 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
> > @@ -1723,55 +1723,68 @@ 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;
> > +	struct tpmi_per_power_domain_info *power_domain_info,
> > *pd_info;
> >  	struct oobmsm_plat_info *plat_info;
> >  	void __iomem *cp_base;
> > +	int num_resources, i;
> >  
> >  	plat_info = tpmi_get_platform_data(auxdev);
> >  	if (!plat_info)
> >  		return;
> >  
> >  	power_domain_info = tpmi_sst->power_domain_info[plat_info-
> > >partition];
> > +	num_resources = tpmi_sst-
> > >number_of_power_domains[plat_info->partition];
> >  
> > -	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);
> > +	for (i = 0; i < num_resources; i++) {
> > +		pd_info = &power_domain_info[i];
> > +		if (!pd_info || !pd_info->sst_base)
> > +			continue;
> >  
> > -	memcpy_fromio(power_domain_info->saved_clos_configs,
> > cp_base + SST_CLOS_CONFIG_0_OFFSET,
> > -		      sizeof(power_domain_info-
> > >saved_clos_configs));
> > +		cp_base = pd_info->sst_base + pd_info-
> > >sst_header.cp_offset;
> >  
> > -	memcpy_fromio(power_domain_info->saved_clos_assocs,
> > cp_base + SST_CLOS_ASSOC_0_OFFSET,
> > -		      sizeof(power_domain_info-
> > >saved_clos_assocs));
> > +		pd_info->saved_sst_cp_control = readq(cp_base +
> > SST_CP_CONTROL_OFFSET);
> > +		memcpy_fromio(pd_info->saved_clos_configs, cp_base
> > + SST_CLOS_CONFIG_0_OFFSET,
> > +			      sizeof(pd_info-
> > >saved_clos_configs));
> > +		memcpy_fromio(pd_info->saved_clos_assocs, cp_base
> > + SST_CLOS_ASSOC_0_OFFSET,
> > +			      sizeof(pd_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);
> > +		pd_info->saved_pp_control = readq(pd_info-
> > >sst_base +
> > +						  pd_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;
> > +	struct tpmi_per_power_domain_info *power_domain_info,
> > *pd_info;
> >  	struct oobmsm_plat_info *plat_info;
> >  	void __iomem *cp_base;
> > +	int num_resources, i;
> >  
> >  	plat_info = tpmi_get_platform_data(auxdev);
> >  	if (!plat_info)
> >  		return;
> >  
> >  	power_domain_info = tpmi_sst->power_domain_info[plat_info-
> > >partition];
> > +	num_resources = tpmi_sst-
> > >number_of_power_domains[plat_info->partition];
> >  
> > -	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));
> > +	for (i = 0; i < num_resources; i++) {
> > +		pd_info = &power_domain_info[i];
> > +		if (!pd_info || !pd_info->sst_base)
> > +			continue;
> >  
> > -	memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET,
> > power_domain_info->saved_clos_assocs,
> > -		    sizeof(power_domain_info->saved_clos_assocs));
> > +		cp_base = pd_info->sst_base + pd_info-
> > >sst_header.cp_offset;
> > +		writeq(pd_info->saved_sst_cp_control, cp_base +
> > SST_CP_CONTROL_OFFSET);
> > +		memcpy_toio(cp_base + SST_CLOS_CONFIG_0_OFFSET,
> > pd_info->saved_clos_configs,
> > +			    sizeof(pd_info->saved_clos_configs));
> > +		memcpy_toio(cp_base + SST_CLOS_ASSOC_0_OFFSET,
> > pd_info->saved_clos_assocs,
> > +			    sizeof(pd_info->saved_clos_assocs));
> 
> Why is the use of empty lines inconsistent between suspend and
> resume?

I will fix that.

Thanks,
Srinivas

> 
> > -	writeq(power_domain_info->saved_pp_control,
> > power_domain_info->sst_base +
> > -				power_domain_info-
> > >sst_header.pp_offset + SST_PP_CONTROL_OFFSET);
> > +		writeq(pd_info->saved_pp_control,
> > power_domain_info->sst_base +
> > +		       pd_info->sst_header.pp_offset +
> > SST_PP_CONTROL_OFFSET);
> > +	}
> >  }
> >  EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_resume, "INTEL_TPMI_SST");
> >  
> >

[PATCH 1/2] platform/x86: ISST: Add missing write block check
[PATCH 2/2] platform/x86: ISST: Store and restore all domains data