The claim register is set twice in the restore flow; remove the
duplicate operation.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 5c20ed4cf4ed..228317991ec2 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1958,8 +1958,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	if (drvdata->numvmidc > 4)
 		etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
 
-	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
-
 	if (!drvdata->skip_power_up)
 		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
 
-- 
2.34.1

On 01/05/2025 11:42 am, Leo Yan wrote:
> The claim register is set twice in the restore flow; remove the
> duplicate operation.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 5c20ed4cf4ed..228317991ec2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1958,8 +1958,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>   	if (drvdata->numvmidc > 4)
>   		etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
>   
> -	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
> -
>   	if (!drvdata->skip_power_up)
>   		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
>   

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

Semi-related to this patch but probably not worth fixing: There's 
technically a race condition with an external debugger here. It would be 
better to use coresight_claim_device_unlocked() and report the error 
that's returned instead of manually writing to the whole claim register 
without checking it before and after like the claim function does.

On Thu, May 01, 2025 at 11:54:58AM +0100, James Clark wrote:
> 
> On 01/05/2025 11:42 am, Leo Yan wrote:
> > The claim register is set twice in the restore flow; remove the
> > duplicate operation.
> > 
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > ---
> >   drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 --
> >   1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index 5c20ed4cf4ed..228317991ec2 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -1958,8 +1958,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
> >   	if (drvdata->numvmidc > 4)
> >   		etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
> > -	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
> > -
> >   	if (!drvdata->skip_power_up)
> >   		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
> 
> Reviewed-by: James Clark <james.clark@linaro.org>
> 
> Semi-related to this patch but probably not worth fixing: There's
> technically a race condition with an external debugger here. It would be
> better to use coresight_claim_device_unlocked() and report the error that's
> returned instead of manually writing to the whole claim register without
> checking it before and after like the claim function does.

I thought again and conclude the current patch would be fine (at least
not bad ;)

The reason is the ETM driver has acquired the OS lock during idle
states, this prevents the race condition with external debuggers.

And if a CPU detects pending interrupts and abort a low power flow,
the CoreSight self claim tag is still set.  In this case,
coresight_claim_device_unlocked() will report unnecessary warnings.

Thanks,
Leo