[PATCH 0/2] coresight: ete: Always save state on power down

[PATCH 0/2] coresight: ete: Always save state on power down

[James Clark]

Fix PM save on ETE, which is an issue that showed up on the FVP when
booted with ACPI and the newly enabled idle states. Then refactor to
clarify and avoid any probe order issues.

Signed-off-by: James Clark <james.clark@linaro.org>
---
James Clark (2):
      coresight: ete: Always save state on power down
      coresight: etm4x: Refactor pm_save_enable handling

 drivers/hwtracing/coresight/coresight-etm4x-core.c | 55 ++++++++++++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  1 +
 2 files changed, 42 insertions(+), 14 deletions(-)
---
base-commit: 971f3474f8898ae8bbab19a9b547819a5e6fbcf1
change-id: 20260420-james-cs-ete-pm_save_enable-4e994e35cdac

Best regards,
-- 
James Clark <james.clark@linaro.org>

[PATCH 1/2] coresight: ete: Always save state on power down

[James Clark]

ETE registers are always system registers so it's highly unlikely there
will be an implementation that preserves them on CPU power down. Also
the ETE DT binding never documented
"arm,coresight-loses-context-with-cpu" so nobody would have legitimately
been able to use that binding to fix it.

Fix it by hard coding the setting for ETE and add a warning if the user
tried to use the module parameter. Don't add a warning if
loses-context-with-cpu is present in the DT as it's not a documented
binding anyway. etm4_init_pm_save() needs to happen after drvdata is
initialised so etm4x_is_ete() can be called.

This fixes the following error when using Coresight with ACPI on the FVP
which supports CPU PM:

  coresight ete0: External agent took claim tag
  WARNING: drivers/hwtracing/coresight/coresight-core.c:248 at coresight_disclaim_device_unlocked+0xe0/0xe8, CPU#0: perf/117

Fixes: 35e1c9163e02 ("coresight: ete: Add support for ETE tracing")
Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 +++++++++++++++-------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index d565a73f0042..a7fb680dd383 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -56,10 +56,11 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on boot");
 #define PARAM_PM_SAVE_NEVER	  1 /* never save any state */
 #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */
 
+/* Save option for ETM4. ETE ignores this option and always saves */
 static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE;
 module_param(pm_save_enable, int, 0444);
 MODULE_PARM_DESC(pm_save_enable,
-	"Save/restore state on power down: 1 = never, 2 = self-hosted");
+	"Save/restore state on power down: 1 = never, 2 = self-hosted. ETM4 only.");
 
 static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
 static void etm4_set_default_config(struct etmv4_config *config);
@@ -1365,6 +1366,30 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_drvdata *drvdata)
 	}
 }
 
+static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata *drvdata)
+{
+	if (etm4x_is_ete(drvdata)) {
+		/*
+		 * Always do PM save for ETE. It always uses system registers
+		 * which will be lost on CPU power down.
+		 */
+		pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
+	} else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) {
+		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
+			PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
+	}
+
+	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
+		drvdata->save_state = devm_kmalloc(dev,
+						   sizeof(struct etmv4_save_state),
+						   GFP_KERNEL);
+		if (!drvdata->save_state)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
 static void etm4_init_arch_data(void *info)
 {
 	u32 etmidr0;
@@ -2247,6 +2272,9 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
 		return -ENOMEM;
 
 	etm4_set_default(&drvdata->config);
+	ret = etm4_init_pm_save(dev, drvdata);
+	if (ret)
+		return ret;
 
 	pdata = coresight_get_platform_data(dev);
 	if (IS_ERR(pdata))
@@ -2305,17 +2333,6 @@ static int etm4_probe(struct device *dev)
 	if (ret)
 		return ret;
 
-	if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE)
-		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
-			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
-
-	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
-		drvdata->save_state = devm_kmalloc(dev,
-				sizeof(struct etmv4_save_state), GFP_KERNEL);
-		if (!drvdata->save_state)
-			return -ENOMEM;
-	}
-
 	raw_spin_lock_init(&drvdata->spinlock);
 
 	drvdata->cpu = coresight_get_cpu(dev);

-- 
2.34.1

[PATCH 2/2] coresight: etm4x: Refactor pm_save_enable handling

[James Clark]

pm_save_enable is a global module parameter used as a user input, but at
the same time is modified on probe. There are some theoretical scenarios
where the DT for one device says to save and another says not to which
result in one device not allocating save state memory but later trying
to save if probing happens in a certain order. We also couldn't warn if
the module parameter is used for ETE because the input value is
overwritten, then for the next device it looks like the user had set
a value triggering a warning.

Fix it by treating the module parameter strictly as user input and
adding a new per device pm_save boolean, and then add the warning that
couldn't be added before.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 22 ++++++++++++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index a7fb680dd383..915f348620d8 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1366,20 +1366,30 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_drvdata *drvdata)
 	}
 }
 
+/*
+ * Take the global pm_save_enable module argument and setup the CPU PM save
+ * setting for this device.
+ */
 static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata *drvdata)
 {
 	if (etm4x_is_ete(drvdata)) {
+		if (pm_save_enable)
+			dev_warn_once(dev, "pm_save_enable module option is only for ETM4\n");
+
 		/*
 		 * Always do PM save for ETE. It always uses system registers
 		 * which will be lost on CPU power down.
 		 */
-		pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
+		drvdata->pm_save = true;
 	} else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) {
-		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
-			PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
+		drvdata->pm_save = coresight_loses_context_with_cpu(dev);
+	} else if (pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED) {
+		drvdata->pm_save = true;
+	} else {
+		drvdata->pm_save = false;
 	}
 
-	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
+	if (drvdata->pm_save) {
 		drvdata->save_state = devm_kmalloc(dev,
 						   sizeof(struct etmv4_save_state),
 						   GFP_KERNEL);
@@ -2037,7 +2047,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int ret = 0;
 
-	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+	if (!drvdata->pm_save)
 		return 0;
 
 	/*
@@ -2152,7 +2162,7 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 
 static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 {
-	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+	if (!drvdata->pm_save)
 		return;
 
 	if (coresight_get_mode(drvdata->csdev))
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 89d81ce4e04e..6472677f3098 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -1078,6 +1078,7 @@ struct etmv4_drvdata {
 	bool				lpoverride : 1;
 	bool				skip_power_up : 1;
 	bool				paused : 1;
+	bool				pm_save : 1;
 	u64				trfcr;
 	struct etmv4_config		config;
 	struct etmv4_save_state		*save_state;

-- 
2.34.1

Re: [PATCH 1/2] coresight: ete: Always save state on power down

[Leo Yan]

On Tue, Apr 28, 2026 at 01:18:11PM +0100, James Clark wrote:
> ETE registers are always system registers so it's highly unlikely there
> will be an implementation that preserves them on CPU power down. Also
> the ETE DT binding never documented
> "arm,coresight-loses-context-with-cpu" so nobody would have legitimately
> been able to use that binding to fix it.
> 
> Fix it by hard coding the setting for ETE and add a warning if the user
> tried to use the module parameter. Don't add a warning if
> loses-context-with-cpu is present in the DT as it's not a documented
> binding anyway. etm4_init_pm_save() needs to happen after drvdata is
> initialised so etm4x_is_ete() can be called.
> 
> This fixes the following error when using Coresight with ACPI on the FVP
> which supports CPU PM:
> 
>   coresight ete0: External agent took claim tag
>   WARNING: drivers/hwtracing/coresight/coresight-core.c:248 at coresight_disclaim_device_unlocked+0xe0/0xe8, CPU#0: perf/117
> 
> Fixes: 35e1c9163e02 ("coresight: ete: Add support for ETE tracing")
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Leo Yan <leo.yan@arm.com>

Re: [PATCH 2/2] coresight: etm4x: Refactor pm_save_enable handling

[Leo Yan]

On Tue, Apr 28, 2026 at 01:18:12PM +0100, James Clark wrote:

[...]

>  static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata *drvdata)
>  {
>  	if (etm4x_is_ete(drvdata)) {
> +		if (pm_save_enable)
> +			dev_warn_once(dev, "pm_save_enable module option is only for ETM4\n");
> +
>  		/*
>  		 * Always do PM save for ETE. It always uses system registers
>  		 * which will be lost on CPU power down.
>  		 */
> -		pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
> +		drvdata->pm_save = true;
>  	} else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) {
> -		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
> -			PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
> +		drvdata->pm_save = coresight_loses_context_with_cpu(dev);
> +	} else if (pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED) {
> +		drvdata->pm_save = true;
> +	} else {
> +		drvdata->pm_save = false;
>  	}

Could we separate ETE and ETM a bit?

        int cpu_pm;

        cpu_pm = coresight_loses_context_with_cpu(dev) ?
                PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;

        if (etm4x_is_ete(drvdata))
                goto alloc_out;

        /* Now this is only for ETM case */

        /* Do consistency check */
        if ((cpu_fw_pm == PARAM_PM_SAVE_SELF_HOSTED &&
             pm_save_enable == PARAM_PM_SAVE_NEVER) ||
            (cpu_fw_pm == PARAM_PM_SAVE_NEVER &&
             pm_save_enable == PARAM_PM_SAVE_SELF_HOSTED))
                dev_warn(dev, "inconsistent setting ...\n");

        /* Force to use the global setting */
        if (pm_save_enable != PARAM_PM_SAVE_FIRMWARE)
            cpu_pm = pm_save_enable;
>  
> -	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
> +	if (drvdata->pm_save) {

would be:

        if (cpu_pm == PARAM_PM_SAVE_SELF_HOSTED)

>  		drvdata->save_state = devm_kmalloc(dev,
>  						   sizeof(struct etmv4_save_state),
>  						   GFP_KERNEL);
> @@ -2037,7 +2047,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  {
>  	int ret = 0;
>  
> -	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
> +	if (!drvdata->pm_save)

Drop "pm_save" and directly use drvdata->save_state ?

We don't need "pm_save" anyway after we apply the patch [1].

[1] http://listhost.cambridge.arm.com/pipermail/linux-eng/2026-April/030289.html

Re: [PATCH 1/2] coresight: ete: Always save state on power down

[Suzuki K Poulose]

On 28/04/2026 13:18, James Clark wrote:
> ETE registers are always system registers so it's highly unlikely there
> will be an implementation that preserves them on CPU power down. Also
> the ETE DT binding never documented
> "arm,coresight-loses-context-with-cpu" so nobody would have legitimately
> been able to use that binding to fix it.
> 
> Fix it by hard coding the setting for ETE and add a warning if the user
> tried to use the module parameter. Don't add a warning if
> loses-context-with-cpu is present in the DT as it's not a documented
> binding anyway. etm4_init_pm_save() needs to happen after drvdata is
> initialised so etm4x_is_ete() can be called.
> 
> This fixes the following error when using Coresight with ACPI on the FVP
> which supports CPU PM:
> 
>    coresight ete0: External agent took claim tag
>    WARNING: drivers/hwtracing/coresight/coresight-core.c:248 at coresight_disclaim_device_unlocked+0xe0/0xe8, CPU#0: perf/117
> 
> Fixes: 35e1c9163e02 ("coresight: ete: Add support for ETE tracing")
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 +++++++++++++++-------
>   1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index d565a73f0042..a7fb680dd383 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -56,10 +56,11 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on boot");
>   #define PARAM_PM_SAVE_NEVER	  1 /* never save any state */
>   #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */
>   
> +/* Save option for ETM4. ETE ignores this option and always saves */
>   static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE;
>   module_param(pm_save_enable, int, 0444);
>   MODULE_PARM_DESC(pm_save_enable,
> -	"Save/restore state on power down: 1 = never, 2 = self-hosted");
> +	"Save/restore state on power down: 1 = never, 2 = self-hosted. ETM4 only.");
>   
>   static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
>   static void etm4_set_default_config(struct etmv4_config *config);
> @@ -1365,6 +1366,30 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_drvdata *drvdata)
>   	}
>   }
>   
> +static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata *drvdata)
> +{
> +	if (etm4x_is_ete(drvdata)) {
> +		/*
> +		 * Always do PM save for ETE. It always uses system registers
> +		 * which will be lost on CPU power down.
> +		 */
> +		pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;

Should we do this instead based on if the ETM/ETE is accessed via sys 
instructions ? That would cover all implementations?


Suzuki

> +	} else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) {
> +		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
> +			PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
> +	}
> +
> +	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
> +		drvdata->save_state = devm_kmalloc(dev,
> +						   sizeof(struct etmv4_save_state),
> +						   GFP_KERNEL);
> +		if (!drvdata->save_state)
> +			return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
>   static void etm4_init_arch_data(void *info)
>   {
>   	u32 etmidr0;
> @@ -2247,6 +2272,9 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
>   		return -ENOMEM;
>   
>   	etm4_set_default(&drvdata->config);
> +	ret = etm4_init_pm_save(dev, drvdata);
> +	if (ret)
> +		return ret;
>   
>   	pdata = coresight_get_platform_data(dev);
>   	if (IS_ERR(pdata))
> @@ -2305,17 +2333,6 @@ static int etm4_probe(struct device *dev)
>   	if (ret)
>   		return ret;
>   
> -	if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE)
> -		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
> -			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
> -
> -	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
> -		drvdata->save_state = devm_kmalloc(dev,
> -				sizeof(struct etmv4_save_state), GFP_KERNEL);
> -		if (!drvdata->save_state)
> -			return -ENOMEM;
> -	}
> -
>   	raw_spin_lock_init(&drvdata->spinlock);
>   
>   	drvdata->cpu = coresight_get_cpu(dev);
> 

Re: [PATCH 1/2] coresight: ete: Always save state on power down

[James Clark]

On 01/05/2026 9:24 am, Suzuki K Poulose wrote:
> On 28/04/2026 13:18, James Clark wrote:
>> ETE registers are always system registers so it's highly unlikely there
>> will be an implementation that preserves them on CPU power down. Also
>> the ETE DT binding never documented
>> "arm,coresight-loses-context-with-cpu" so nobody would have legitimately
>> been able to use that binding to fix it.
>>
>> Fix it by hard coding the setting for ETE and add a warning if the user
>> tried to use the module parameter. Don't add a warning if
>> loses-context-with-cpu is present in the DT as it's not a documented
>> binding anyway. etm4_init_pm_save() needs to happen after drvdata is
>> initialised so etm4x_is_ete() can be called.
>>
>> This fixes the following error when using Coresight with ACPI on the FVP
>> which supports CPU PM:
>>
>>    coresight ete0: External agent took claim tag
>>    WARNING: drivers/hwtracing/coresight/coresight-core.c:248 at 
>> coresight_disclaim_device_unlocked+0xe0/0xe8, CPU#0: perf/117
>>
>> Fixes: 35e1c9163e02 ("coresight: ete: Add support for ETE tracing")
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 ++++++++++++ 
>> +++-------
>>   1 file changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/ 
>> drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index d565a73f0042..a7fb680dd383 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -56,10 +56,11 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on 
>> boot");
>>   #define PARAM_PM_SAVE_NEVER      1 /* never save any state */
>>   #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */
>> +/* Save option for ETM4. ETE ignores this option and always saves */
>>   static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE;
>>   module_param(pm_save_enable, int, 0444);
>>   MODULE_PARM_DESC(pm_save_enable,
>> -    "Save/restore state on power down: 1 = never, 2 = self-hosted");
>> +    "Save/restore state on power down: 1 = never, 2 = self-hosted. 
>> ETM4 only.");
>>   static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
>>   static void etm4_set_default_config(struct etmv4_config *config);
>> @@ -1365,6 +1366,30 @@ static void etm4_fixup_wrong_ccitmin(struct 
>> etmv4_drvdata *drvdata)
>>       }
>>   }
>> +static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata 
>> *drvdata)
>> +{
>> +    if (etm4x_is_ete(drvdata)) {
>> +        /*
>> +         * Always do PM save for ETE. It always uses system registers
>> +         * which will be lost on CPU power down.
>> +         */
>> +        pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
> 
> Should we do this instead based on if the ETM/ETE is accessed via sys 
> instructions ? That would cover all implementations?
> 
> 
> Suzuki
> 

I did discuss that with Leo but we thought it might be a riskier change 
as ETM is older and nobody has reported any issues, and there is already 
the DT option to fix it that way. Turning it on by default could cause 
some performance regressions. Although it's only if the ETM is in use, 
so the impact is minimal.

In reality it probably does make sense as it's highly unlikely that 
sysreg ETM wouldn't need saving, so it might make some platforms with 
misconfigured DTs more stable.

I can send another version if you think it makes sense, what do you 
think Leo?

>> +    } else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) {
>> +        pm_save_enable = coresight_loses_context_with_cpu(dev) ?
>> +            PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
>> +    }
>> +
>> +    if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
>> +        drvdata->save_state = devm_kmalloc(dev,
>> +                           sizeof(struct etmv4_save_state),
>> +                           GFP_KERNEL);
>> +        if (!drvdata->save_state)
>> +            return -ENOMEM;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static void etm4_init_arch_data(void *info)
>>   {
>>       u32 etmidr0;
>> @@ -2247,6 +2272,9 @@ static int etm4_add_coresight_dev(struct 
>> etm4_init_arg *init_arg)
>>           return -ENOMEM;
>>       etm4_set_default(&drvdata->config);
>> +    ret = etm4_init_pm_save(dev, drvdata);
>> +    if (ret)
>> +        return ret;
>>       pdata = coresight_get_platform_data(dev);
>>       if (IS_ERR(pdata))
>> @@ -2305,17 +2333,6 @@ static int etm4_probe(struct device *dev)
>>       if (ret)
>>           return ret;
>> -    if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE)
>> -        pm_save_enable = coresight_loses_context_with_cpu(dev) ?
>> -                   PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
>> -
>> -    if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
>> -        drvdata->save_state = devm_kmalloc(dev,
>> -                sizeof(struct etmv4_save_state), GFP_KERNEL);
>> -        if (!drvdata->save_state)
>> -            return -ENOMEM;
>> -    }
>> -
>>       raw_spin_lock_init(&drvdata->spinlock);
>>       drvdata->cpu = coresight_get_cpu(dev);
>>
> 

Re: [PATCH 1/2] coresight: ete: Always save state on power down

[Leo Yan]

On Fri, May 01, 2026 at 10:43:09AM +0100, James Clark wrote:

[...]

> > > +static int etm4_init_pm_save(struct device *dev, struct
> > > etmv4_drvdata *drvdata)
> > > +{
> > > +    if (etm4x_is_ete(drvdata)) {
> > > +        /*
> > > +         * Always do PM save for ETE. It always uses system registers
> > > +         * which will be lost on CPU power down.
> > > +         */
> > > +        pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
> > 
> > Should we do this instead based on if the ETM/ETE is accessed via sys
> > instructions ? That would cover all implementations?
> > 
> > 
> > Suzuki
> > 
> 
> I did discuss that with Leo but we thought it might be a riskier change as
> ETM is older and nobody has reported any issues, and there is already the DT
> option to fix it that way. Turning it on by default could cause some
> performance regressions. Although it's only if the ETM is in use, so the
> impact is minimal.
> 
> In reality it probably does make sense as it's highly unlikely that sysreg
> ETM wouldn't need saving, so it might make some platforms with misconfigured
> DTs more stable.
> 
> I can send another version if you think it makes sense, what do you think
> Leo?

If extend a bit for ACPI context, it is fine for save / restore based on
sys reg implemenation. This is most nature way for support ACPI?

We can limit "arm,coresight-loses-context-with-cpu" for only ETM +
MMIO mode.

BTW, when respin, could you consider the approach for not changing
pm_save_enable. We simply take it as a global parameter, and calculate PM
mode for each CPU. But it is not necessary to write the PM mode back to
pm_save_enable.

Thanks,
Leo

Re: [PATCH 1/2] coresight: ete: Always save state on power down

[James Clark]

On 01/05/2026 2:50 pm, Leo Yan wrote:
> On Fri, May 01, 2026 at 10:43:09AM +0100, James Clark wrote:
> 
> [...]
> 
>>>> +static int etm4_init_pm_save(struct device *dev, struct
>>>> etmv4_drvdata *drvdata)
>>>> +{
>>>> +    if (etm4x_is_ete(drvdata)) {
>>>> +        /*
>>>> +         * Always do PM save for ETE. It always uses system registers
>>>> +         * which will be lost on CPU power down.
>>>> +         */
>>>> +        pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
>>>
>>> Should we do this instead based on if the ETM/ETE is accessed via sys
>>> instructions ? That would cover all implementations?
>>>
>>>
>>> Suzuki
>>>
>>
>> I did discuss that with Leo but we thought it might be a riskier change as
>> ETM is older and nobody has reported any issues, and there is already the DT
>> option to fix it that way. Turning it on by default could cause some
>> performance regressions. Although it's only if the ETM is in use, so the
>> impact is minimal.
>>
>> In reality it probably does make sense as it's highly unlikely that sysreg
>> ETM wouldn't need saving, so it might make some platforms with misconfigured
>> DTs more stable.
>>
>> I can send another version if you think it makes sense, what do you think
>> Leo?
> 
> If extend a bit for ACPI context, it is fine for save / restore based on
> sys reg implemenation. This is most nature way for support ACPI?
> 
> We can limit "arm,coresight-loses-context-with-cpu" for only ETM +
> MMIO mode.
> 

Does that end up looking something like this? ETE is always !MMIO so we 
don't need to check for it explicitly:

   /*
    * Force PM save for ACPI as it has no way to enable it, and for
    * sysreg based ETMs (and ETE which is always sysreg) because they are
    * likely to lose state.
    */
   if (!init_arg->csa->io_mem || is_acpi_device_node(fwnode))
      pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;

> BTW, when respin, could you consider the approach for not changing
> pm_save_enable. We simply take it as a global parameter, and calculate PM
> mode for each CPU. But it is not necessary to write the PM mode back to
> pm_save_enable.
> 
> Thanks,
> Leo

Yep will do.