[U-Boot] [PATCH 2/5] arm: rmobile: Add support Renesas SH73A0

Wolfgang Denk wd at denx.de
Thu Aug 9 20:40:08 CEST 2012


Dear Nobuhiro Iwamatsu,

In message <1341471662-1157-2-git-send-email-nobuhiro.iwamatsu.yj at renesas.com> you wrote:
> Renesas SH73A0 is CPU with Cortex-A9.
> This supports the basic register definition and GPIO.

> --- a/arch/arm/cpu/armv7/rmobile/Makefile
> +++ b/arch/arm/cpu/armv7/rmobile/Makefile
> @@ -25,11 +25,13 @@ include $(TOPDIR)/config.mk
>  
>  LIB	= $(obj)lib$(SOC).o
>  
> -COBJS += cpu_info.o
> -COBJS += timer.o
> +SOBJS = lowlevel_init.o
> +COBJS += cpu_info.o board.o timer.o
> +COBJS-$(CONFIG_SH73A0) += pfc-sh73a0.o cpu_info-sh73a0.o

Please make this one object per line, and sort the list.


> diff --git a/arch/arm/include/asm/arch-rmobile/sh73a0.h b/arch/arm/include/asm/arch-rmobile/sh73a0.h
> new file mode 100644
> index 0000000..605dd44
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-rmobile/sh73a0.h
> @@ -0,0 +1,282 @@
...
> +#define __ASM_ARCH_RMOBILE_SH73A0_H
> +
> +/* Global Timer */
> +#define GLOBAL_TIMER_BASE_ADDR	(0xF0000200)
> +#define MERAM_BASE	(0xE5580000)
> +
> +/* GIC */
> +#define GIC_BASE	(0xF0000100)
> +#define ICCICR		(GIC_BASE + 0x000)
> +#define ICCPMR		(GIC_BASE + 0x004)

NAK.  Please use C structs to describe the peripoherals, here and
everywhere else.

> +/* RWDT */
> +struct sh73a0_rwdt {
> +	volatile u16 rwtcnt0;	/* 0x00 */
> +	volatile u16 rwtcsra0;	/* 0x04 */
> +	volatile u16 rwtcsrb0;	/* 0x08 */
> +};

All these volatiles here are wrong; please remove.  Checkpatch says:

WARNING: Use of volatile is usually wrong: see
Documentation/volatile-considered-harmful.txt


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
Eeeek!
'eval' on strings should have been named 'evil'.    -- Tom Phoenix in
        <Pine.GSO.3.96.980526121813.27437N-100000 at user2.teleport.com>


More information about the U-Boot mailing list