coresight: trbe: Support trigger and circle buffer modes

[PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

Posted by Leo Yan 2 months, 1 week ago

set_trbe_disabled() should never appear in the enable flow, otherwise,
it may potentially hide bugs in the disable flow.

Remove set_trbe_disabled() from the enable path.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 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 f5597bd9b5fba9a8f5053d5823b03380fd468b5c..e426991e2c2c398a9d3982e9d0f7f542e404cbab 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -629,7 +629,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);

-- 
2.34.1

Re: [PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

Posted by Anshuman Khandual 2 months, 1 week ago

On 01/12/25 4:51 PM, Leo Yan wrote:
> set_trbe_disabled() should never appear in the enable flow, otherwise,
> it may potentially hide bugs in the disable flow.
> 
> Remove set_trbe_disabled() from the enable path.

IIRC without first disabling TRBLIMITR_EL1_E - TRBE registers or their
fields should not be fetched or interpreted. Without that none of the
subsequent HW operations should be performed inside trbe_enable_hw()
leading upto enabling it.

> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  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 f5597bd9b5fba9a8f5053d5823b03380fd468b5c..e426991e2c2c398a9d3982e9d0f7f542e404cbab 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -629,7 +629,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);
>

Re: [PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

Posted by Leo Yan 2 months, 1 week ago

On Thu, Dec 04, 2025 at 06:13:56PM +0530, Anshuman Khandual wrote:
> On 01/12/25 4:51 PM, Leo Yan wrote:
> > set_trbe_disabled() should never appear in the enable flow, otherwise,
> > it may potentially hide bugs in the disable flow.
> > 
> > Remove set_trbe_disabled() from the enable path.
> 
> IIRC without first disabling TRBLIMITR_EL1_E - TRBE registers or their
> fields should not be fetched or interpreted.

Yes. I think you are referring to the rule DJMDD in Arm ARM: "The PE
might ignore a direct or external write to any of certain Trace Buffer
Unit registers ... (when) TRBLIMITR_EL1.E is 1, and the Trace Buffer
Unit is using Self-hosted mode."

> Without that none of the
> subsequent HW operations should be performed inside trbe_enable_hw()
> leading upto enabling it.

Fair enough.

If we can conclude the trace unit has been disabled properly in below
cases, no reason to arbitrarily calling set_trbe_disabled() during each
enable.

  1) The SYS_TRBLIMITR_EL1 register is cleared in trbe_reset_local()
     during probe phase.
  2) The SYS_TRBLIMITR_EL1.E bit is cleared in arm_trbe_irq_handler()
     for interrupt handling.
  3) The SYS_TRBLIMITR_EL1.E bit is cleared in the disable flow.

Seems to me, this is not only for code cleanup, we need to promise a
sane logic in the flow.

Thanks,
Leo