Re: [PATCH v5 07/11] coresight: etm4x: Remove the redundant DSB

[Yeoreum Yun <yeoreum.yun@arm.com>]

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>

> As recommended in section 4.3.7 "Synchronization when using the
> memory-mapped interface" of ARM IHI0064H.b:
>
>   When using the memory-mapped interface to program the trace unit, the
>   trace analyzer must ensure that writes have completed, to ensure that
>   the trace unit is fully programmed and either enabled or disabled.
>
>   To ensure writes have completed, the trace analyzer can do ...
>
>   If the memory marked is as Device-nGnRE or stronger, read back the
>   value of any register in the trace unit. This relies on peripheral
>   coherence order defined in the Arm architecture.
>
> Polling TRCSTATR ensures the previous write has completed. Therefore,
> removes the redundant DSB barrier in the enabling flow.
>
> Update the comment in the disable flow for consistency.
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 23 +++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index fdda924a2c7117a6828e5325d78586b09e75febd..bdf5ab85b2213dfd601391fdcf50960667fa7cea 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -475,10 +475,16 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
>  	}
>
>  	/*
> -	 * As recommended by section 4.3.7 ("Synchronization when using the
> -	 * memory-mapped interface") of ARM IHI 0064D
> +	 * As recommended in section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 *
> +	 * For the memory-mapped interface, the registers are mapped as Device
> +	 * type (Device-nGnRE). Reading back the value of any register in the
> +	 * trace unit ensures that all writes have completed. Therefore, polling
> +	 * on TRCSTATR guarantees that the writing TRCPRGCTLR is complete, and
> +	 * no explicit dsb() is required at here.
>  	 */
> -	dsb(sy);
>  	isb();
>
>  	return 0;
> @@ -974,8 +980,15 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
>  		dev_err(etm_dev,
>  			"timeout while waiting for PM stable Trace Status\n");
>  	/*
> -	 * As recommended by section 4.3.7 (Synchronization of register updates)
> -	 * of ARM IHI 0064H.b.
> +	 * As recommended in section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 *
> +	 * For the memory-mapped interface, the registers are mapped as Device
> +	 * type (Device-nGnRE). Reading back the value of any register in the
> +	 * trace unit ensures that all writes have completed. Therefore, polling
> +	 * on TRCSTATR guarantees that the writing TRCPRGCTLR is complete, and
> +	 * no explicit dsb() is required at here.
>  	 */
>  	isb();
>  }
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun