Re: [PATCH v10 16/20] coresight: Add PM callbacks for sink device

[Jie Gan <jie.gan@oss.qualcomm.com>]

On 4/5/2026 11:02 PM, Leo Yan wrote:
> Unlike system level sinks, per-CPU sinks may lose power during CPU idle
> states.  Currently, this applies specifically to TRBE.  This commit
> invokes save and restore callbacks for the sink in the CPU PM notifier.
> 
> If the sink provides PM callbacks but the source does not, this is
> unsafe because the sink cannot be disabled safely unless the source
> can also be controlled, so veto low power entry to avoid lockups.
> 
> Tested-by: James Clark <james.clark@linaro.org>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Reviewed-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++++++++++--
>   1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c1e8debc76aba7eb5ecf7efe2a3b9b8b3e11b10c..a918bf6398a932de30fe9b4947020cc4c1cfb2f7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1736,14 +1736,15 @@ static void coresight_release_device_list(void)
>   /* Return: 1 if PM is required, 0 if skip, <0 on error */
>   static int coresight_pm_check(struct coresight_path *path)
>   {
> -	struct coresight_device *source;
> -	bool source_has_cb;
> +	struct coresight_device *source, *sink;
> +	bool source_has_cb, sink_has_cb;
>   
>   	if (!path)
>   		return 0;
>   
>   	source = coresight_get_source(path);
> -	if (!source)
> +	sink = coresight_get_sink(path);
> +	if (!source || !sink)
>   		return 0;
>   
>   	/* Don't save and restore if the source is inactive */
> @@ -1759,16 +1760,36 @@ static int coresight_pm_check(struct coresight_path *path)
>   	if (source_has_cb)
>   		return 1;
>   
> +	sink_has_cb = coresight_ops(sink)->pm_save_disable &&
> +		      coresight_ops(sink)->pm_restore_enable;
> +	/*
> +	 * It is not permitted that the source has no callbacks while the sink
> +	 * does, as the sink cannot be disabled without disabling the source,
> +	 * which may lead to lockups. Alternatively, the ETM driver should
> +	 * enable self-hosted PM mode at probe (see etm4_probe()).
> +	 */
> +	if (sink_has_cb) {
> +		pr_warn_once("coresight PM failed: source has no PM callbacks; "
> +			     "cannot safely control sink\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
>   static int coresight_pm_device_save(struct coresight_device *csdev)
>   {
> +	if (!csdev || !coresight_ops(csdev)->pm_save_disable)
> +		return 0;
> +
>   	return coresight_ops(csdev)->pm_save_disable(csdev);
>   }
>   
>   static void coresight_pm_device_restore(struct coresight_device *csdev)
>   {
> +	if (!csdev || !coresight_ops(csdev)->pm_restore_enable)
> +		return;
> +
>   	coresight_ops(csdev)->pm_restore_enable(csdev);
>   }
>   
> @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct coresight_path *path)
>   	to = list_prev_entry(coresight_path_last_node(path), link);
>   	coresight_disable_path_from_to(path, from, to);
>   
> +	ret = coresight_pm_device_save(coresight_get_sink(path));
> +	if (ret)
> +		goto sink_failed;
> +
>   	return 0;
> +
> +sink_failed:
> +	if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
> +					   from, to))
> +		coresight_pm_device_restore(source);

I have go through the history messages. I have a question about this 
point here:

how can we handle the scenario if coresight_enable_path_from_to failed? 
It means we are never calling coresight_pm_device_restore for the ETM 
and leaving the ETM with OS lock state until CPU reset?

Consider we are calling etm4_disable_hw with OS lock:
etm4_disable_hw -> etm4_disable_trace_unit -> etm4x_wait_status (may 
timeout here?)


Thanks,
Jie

> +
> +	pr_err("Failed in coresight PM save on CPU%d: %d\n",
> +	       smp_processor_id(), ret);
> +	this_cpu_write(percpu_pm_failed, true);
> +	return ret;
>   }
>   
>   static void coresight_pm_restore(struct coresight_path *path)
>   {
>   	struct coresight_device *source = coresight_get_source(path);
> +	struct coresight_device *sink = coresight_get_sink(path);
>   	struct coresight_node *from, *to;
>   	int ret;
>   
> +	coresight_pm_device_restore(sink);
> +
>   	from = coresight_path_first_node(path);
>   	/* Up to the node before sink to avoid latency */
>   	to = list_prev_entry(coresight_path_last_node(path), link);
> @@ -1808,6 +1846,8 @@ static void coresight_pm_restore(struct coresight_path *path)
>   	return;
>   
>   path_failed:
> +	coresight_pm_device_save(sink);
> +
>   	pr_err("Failed in coresight PM restore on CPU%d: %d\n",
>   	       smp_processor_id(), ret);
>   
>