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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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);
>