[U-Boot] [PATCH 5/8] armv7: add PL310 support to u-boot

[Wolfgang Denk]

Dear Aneesh V,

In message <1293018898-13253-6-git-send-email-aneesh at ti.com> you wrote:
> Add support for some of the key maintenance operations
> 	- Invalidate all
> 	- Invalidate range
> 	- Flush(clean & invalidate) all
> 	- Flush range

Can you please use a more descriptive subject, and commit message?

I have no idea what a "PL310" might be - is this a new board? Or a new
SoC? or a new Ethernet controller?


And what exactly are "key maintenance operations"?  Looks as if you
were talking about basic cache operations?

> --- /dev/null
> +++ b/arch/arm/include/asm/pl310.h
...
> +/* Register offsets */
> +#define PL310_CACHE_TYPE		0x004
> +#define PL310_AUX_CTRL			0x104
> +
> +#define PL310_CACHE_SYNC		0x730
> +#define PL310_INVAL_LINE_PA		0x770
> +#define PL310_INVAL_WAY			0x77C
> +#define PL310_CLEAN_LINE_PA		0x7B0
> +#define PL310_CLEAN_INVAL_WAY		0x7FC
> +#define PL310_CLEAN_INVAL_LINE_PA	0x7F0

NAK.  Please use a C struct instead.


> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -42,6 +42,7 @@ COBJS-y	+= cache.o
>  ifndef CONFIG_SYS_NO_CP15_CACHE
>  COBJS-y	+= cache-cp15.o
>  endif
> +COBJS-$(CONFIG_SYS_USE_PL310) += cache-pl310.o
>  COBJS-y	+= interrupts.o
>  COBJS-y	+= reset.o

There is no documentation for CONFIG_SYS_USE_PL310, and there is no
use of this variable.

Also, it seems CONFIG_SYS_PL310 would be more appropriate.

...
> +static void pl310_cache_sync(void)
> +{
> +	__raw_writel(0, CONFIG_SYS_PL310_BASE + PL310_CACHE_SYNC);
> +}

Please use a proper C struct instead of base address plus offset.
Please fix globally.

...
> +	for (pa = start; pa < stop; pa = pa + line_size)
> +		__raw_writel(pa, CONFIG_SYS_PL310_BASE +
> +			     PL310_CLEAN_INVAL_LINE_PA);

Please use braces for multiline statements.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No one can guarantee the actions of another.
	-- Spock, "Day of the Dove", stardate unknown