Re: [PATCH v3 17/18] arm64: Fix cache maintenance function comments

[Mark Rutland <mark.rutland@arm.com>]

On Thu, May 20, 2021 at 01:44:05PM +0100, Fuad Tabba wrote:
> Fix and expand comments for the cache maintenance functions in
> cacheflush.h. Adds comments to functions that weren't described
> before. Explains what the functions do using Arm Architecture
> Reference Manual terminology.
> 
> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/cacheflush.h | 43 +++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
> index 70b389a8dea5..4b91d3530013 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -30,31 +30,44 @@
>   *	the implementation assumes non-aliasing VIPT D-cache and (aliasing)
>   *	VIPT I-cache.
>   *
> - *	flush_icache_range(start, end)
> - *
> - *		Ensure coherency between the I-cache and the D-cache in the
> - *		region described by start, end.
> + *	All functions below apply to the region described by [start, end)
>   *		- start  - virtual start address
>   *		- end    - virtual end address

Could we please say:

| *	All functions below apply to the interval [start, end)
| *		- start  - virtual start address (inclusive)
| *		- end    - virtual end address (exclusive)

The "interval" wording makes it slightly clearer that we're using
interval notation for '[' and ')', and being explicit when describing
start/end makes that clear for those not familiar with interval
notation.

>   *
> - *	invalidate_icache_range(start, end)
> + *	__flush_icache_range(start, end)
>   *
> - *		Invalidate the I-cache in the region described by start, end.
> - *		- start  - virtual start address
> - *		- end    - virtual end address
> + *		Ensure coherency between the I-cache and the D-cache region to
> + *		the Point of Unification.
>   *
>   *	__flush_cache_user_range(start, end)
>   *
> - *		Ensure coherency between the I-cache and the D-cache in the
> - *		region described by start, end.
> - *		- start  - virtual start address
> - *		- end    - virtual end address
> + *		Ensure coherency between the I-cache and the D-cache region to
> + *		the Point of Unification.
> + *		Use only if the region might access user memory.
> + *
> + *	invalidate_icache_range(start, end)
> + *
> + *		Invalidate I-cache region to the Point of Unification.
>   *
>   *	__flush_dcache_area(start, end)
>   *
> - *		Ensure that the data held in page is written back.
> - *		- start  - virtual start address
> - *		- end    - virtual end address
> + *		Clean and invalidate D-cache region to the Point of Coherence.

For better or worse, the architecture calls this "Point of Coherency"
rather than "Point of Coherence", so we should fix this to match, along
with the two instances below.

With those nits addressed:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> + *
> + *	__inval_dcache_area(start, end)
> + *
> + *		Invalidate D-cache region to the Point of Coherence.
> + *
> + *	__clean_dcache_area_poc(start, end)
> + *
> + *		Clean D-cache region to the Point of Coherence.
> + *
> + *	__clean_dcache_area_pop(start, end)
> + *
> + *		Clean D-cache region to the Point of Persistence.
> + *
> + *	__clean_dcache_area_pou(start, end)
> + *
> + *		Clean D-cache region to the Point of Unification.
>   */
>  extern void __flush_icache_range(unsigned long start, unsigned long end);
>  extern void invalidate_icache_range(unsigned long start, unsigned long end);
> -- 
> 2.31.1.751.gd2f1c929bd-goog
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel