Re: [PATCH 2/5] omap4: l2x0: Construct the AUXCTRL value using defines

[Kevin Hilman <khilman@deeprootsystems.com>]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> This patch removes the hardcoded value of auxctrl value and
> construct it using bitfields
>
> Bit 25 is reserved and is always set to 1. Same value
> of this bit is retained in this patch

Is this OMAP specific, or is this ARM generic?

> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Tested-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/omap4-common.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c
> index 2f89555..c814604 100644
> --- a/arch/arm/mach-omap2/omap4-common.c
> +++ b/arch/arm/mach-omap2/omap4-common.c
> @@ -53,6 +53,8 @@ static void omap4_l2x0_disable(void)
>  
>  static int __init omap_l2_cache_init(void)
>  {
> +	u32 aux_ctrl = 0;
> +
>  	/*
>  	 * To avoid code running on other OMAPs in
>  	 * multi-omap builds
> @@ -72,10 +74,17 @@ static int __init omap_l2_cache_init(void)
>  	 * Way size - 32KB (es1.0)
>  	 * Way size - 64KB (es2.0 +)
>  	 */
> +	aux_ctrl = ((1 << L2X0_AUX_CTRL_ASSOCIATIVITY_SHIFT) |
> +			(0x1 << 25) |

it doesn't look right to have a single hard-coded constant here among
the others.   Either give this a name (RESERVED_ALWAYS_1, or something)
or add a comment to the code like is done in the changelog.

> +			(0x1 << L2X0_AUX_CTRL_NS_LOCKDOWN_SHIFT) |
> +			(0x1 << L2X0_AUX_CTRL_NS_INT_CTRL_SHIFT));
> +
>  	if (omap_rev() == OMAP4430_REV_ES1_0)
> -		l2x0_init(l2cache_base, 0x0e050000, 0xc0000fff);
> +		aux_ctrl |= 0x2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT;
>  	else
> -		l2x0_init(l2cache_base, 0x0e070000, 0xc0000fff);
> +		aux_ctrl |= 0x3 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT;
> +
> +	l2x0_init(l2cache_base, aux_ctrl, L2X0_AUX_CTRL_MASK);
>  
>  	/*
>  	 * Override default outer_cache.disable with a OMAP4

Kevin