Re: [PATCH v2 06/23] coresight: trbe: Drop redundant disable from enable path

[James Clark]

On 16/10/2026 17:55, Leo Yan wrote:
> The Arm ARM (ARM DDI 0487 M.c) rule RDJMDD says:
>
>   | When the following conditions apply, the PE might ignore a direct or
>   | external write to any of certain Trace Buffer Unit registers, other
>   | than a direct write or external write to TRBLIMITR_EL1 that clears
>   | the conditions:
>   | • TRBLIMITR_EL1.E is 1, and either FEAT_TRBE_EXT is not implemented
>   |   or the Trace Buffer Unit is using Self-hosted mode.

Doesn't the rule just say that the buffer must be disabled before doing 
writes to other registers? But the explanation for removing the extra 
disable is that it's already disabled at this point, so nothing changes.

The commit message would be clearer without this bit IMO. There's no 
architectural requirement to make this change, it's only a software one.

Otherwise:

Reviewed-by: James Clark <james.clark@xxxxxxxxxx>

> trbe_enable_hw() calls set_trbe_disabled() first to ensure TRBE has
> already been disabled before programming any other TRBE registers.
>
> However, TRBE is already expected to be disabled before trbe_enable_hw()
> runs:
>
>   1. CPU probe resets TRBLIMITR_EL1 via trbe_reset_local();
>   2. Interrupt handling disables the buffer before re-enabling it;
>   3. The normal disable path clears TRBLIMITR_EL1.E.
>
> Calling set_trbe_disabled() from the enable path is therefore redundant.
> Drop the call from trbe_enable_hw(), which only needs to program a
> known-disabled TRBE.
>
> Signed-off-by: Leo Yan <leo.yan@xxxxxxx>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index a7c1037a048f5236598f8f3a7cc4f47bff9d329c..979d0fa84c1c7b448c1ce5cd7a37dff8845795ef 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -652,7 +652,6 @@ static void trbe_enable_hw(struct trbe_buf *buf)
>   	WARN_ON(buf->trbe_hw_base < buf->trbe_base);
>   	WARN_ON(buf->trbe_write < buf->trbe_hw_base);
>   	WARN_ON(buf->trbe_write >= buf->trbe_limit);
> -	set_trbe_disabled(buf->cpudata);
>   	clr_trbe_status();
>   	set_trbe_base_pointer(buf->trbe_hw_base);
>   	set_trbe_write_pointer(buf->trbe_write);