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

[James Clark <james.clark@linaro.org>]

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.