[U-Boot] [PATCH v4 2/3] Add TI816X support

[TENART Antoine]

> [snip]
>> +/* WDT related */
>> +#define WDT_WDSC		(WDT_BASE + 0x010)
>> +#define WDT_WDST		(WDT_BASE + 0x014)
>> +#define WDT_WISR		(WDT_BASE + 0x018)
>> +#define WDT_WIER		(WDT_BASE + 0x01C)
>> +#define WDT_WWER		(WDT_BASE + 0x020)
>> +#define WDT_WCLR		(WDT_BASE + 0x024)
>> +#define WDT_WCRR		(WDT_BASE + 0x028)
>> +#define WDT_WLDR		(WDT_BASE + 0x02C)
>> +#define WDT_WTGR		(WDT_BASE + 0x030)
>> +#define WDT_WWPS		(WDT_BASE + 0x034)
>> +#define WDT_WDLY		(WDT_BASE + 0x044)
>> +#define WDT_WSPR		(WDT_BASE + 0x048)
>> +#define WDT_WIRQEOI		(WDT_BASE + 0x050)
>> +#define WDT_WIRQSTATRAW		(WDT_BASE + 0x054)
>> +#define WDT_WIRQSTAT		(WDT_BASE + 0x058)
>> +#define WDT_WIRQENSET		(WDT_BASE + 0x05C)
>> +#define WDT_WIRQENCLR		(WDT_BASE + 0x060)
>> +#define WDT_UNFREEZE		(CTRL_BASE + 0x100)
>
> This should be using a struct like the other platforms do for wdt.

I'll do that and use the wd_timer structure.

>
> [snip]
>> +/* needed by config_dmm() */
>> +void enable_dmm_clocks(void) {}
>
> #ifndef the caller?
>

Yes.

>> +void ddr_pll_config(unsigned int pll)
>> +{
>> +	writel(0x5,&(ddr_reg[pll])->cm0config);
>
> Magic value (0x5).

I'll fix it.

>
> [snip]
>> +	main_pll_ctrl&= 0xFFFFFFFB;
>> +	main_pll_ctrl |= 4;
>
> More magic.

This one is already explained in the beginning of the function. I'll put 
a per instruction comment instead.

>
> [snip]
>> +static void peripheral_enable(void)
>
> Lots more magic values in here.
>

I'll remove the magic.

-- 
Antoine