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