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

[Nobuhiro Iwamatsu]

Hi,

Thanks for your review.

On Fri, Aug 10, 2012 at 3:40 AM, Wolfgang Denk <wd at denx.de> wrote:
> 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.
>

OK, I'll fix.

>
>> 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.

oh, sorry, I'll fix.

>
>> +/* 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
>

This already fix in http://patchwork.ozlabs.org/patch/172103/.

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu