[U-Boot] [RFC] [PATCH] at91: add support for PM9263 board

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:02 Thu 05 Feb     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <1233785700-31051-1-git-send-email-plagnioj at jcrosoft.com> you wrote:
> > From: Ilko Iliev <iliev at ronetix.at>
> > 
> > This patch adds support for the PM9263 board of Ronetix GmbH (www.ronetix.at)
> > With the necessary to boot the board from NOR
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> I cannot parse this. What do you want to express?
I've not finish to clean the patch. I've send it for the update done in the
at91 in order to implement boot from NOR
> 
> > index ec6ad5d..122e23a 100644
> > --- a/cpu/arm926ejs/at91/lowlevel_init.S
> > +++ b/board/ronetix/pm9263/led.c
> 
> NAK.
> 
> I will not accept yet another copy of this file. Please create common
> code that can be configured as needed.
> 
> > diff --git a/board/ronetix/pm9263/partition.c b/board/ronetix/pm9263/partition.c
> > new file mode 100644
> > index 0000000..95ac398
> > --- /dev/null
> > +++ b/board/ronetix/pm9263/partition.c
> 
> NAK.
> 
> I will not accept yet another copy of this file. We have already 7 of
> these, most of them identical, and the remaining differences minimal.
> Please create common code that can be configured as needed.
It's plan to create it but I'll not have the time for this release
and this board wait until December IIRC
> 
> 
> > diff --git a/board/ronetix/pm9263/pm9263.c b/board/ronetix/pm9263/pm9263.c
> > new file mode 100644
> > index 0000000..7bda1b8
> ...
> 
> > +static int pm9263_lcd_hw_psram_init(void)
> > +{
> ...
> > +	if ((readw(PHYS_PSRAM) != 0x1234) || (readw(PHYS_PSRAM+2) != 0x5678))
> > +	{
> 
> Incorrect brace style.
> 
> > +		writew(0x5678, PHYS_PSRAM+2);
> > +		if ((readw(PHYS_PSRAM) != 0x1234)
> > +		   || (readw(PHYS_PSRAM + 2) != 0x5678))
> > +			return 1;
> 
> I recommend to use curly braces because of the multi-line "if".
> 
> Also, please put the "||" at the end of line 1.
> 
> > +int dram_init(void)
> > +{
> > +	gd->bd->bi_dram[0].start = PHYS_SDRAM;
> > +	gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
> > +	return 0;
> > +}
> 
> You should not hard-wire the memory size. You should auto-size it.
> 
> > +#ifdef CONFIG_RESET_PHY_R
> > +void reset_phy(void)
> > +{
> > +#ifdef CONFIG_MACB
> > +	/*
> > +	 * Initialize ethernet HW addr prior to starting Linux,
> > +	 * needed for nfsroot
> > +	 */
> > +	eth_init(gd->bd);
> > +#endif
> 
> Be careful. You mustnot initialize the ehternet port unless a U-Boot
> network command is being used. And you must shut it down after that.
I known this will be fix in the final version
> 
> > diff --git a/cpu/arm926ejs/at91/lowlevel_init.S b/board/ronetix/pm9263/u-boot.lds
> > similarity index 54%
> > copy from cpu/arm926ejs/at91/lowlevel_init.S
> > copy to board/ronetix/pm9263/u-boot.lds
> > index ec6ad5d..443f180 100644
> > --- a/cpu/arm926ejs/at91/lowlevel_init.S
> > +++ b/board/ronetix/pm9263/u-boot.lds
> 
> That's a pretty funny diff, isn't it? lowlevel_init.S versus
> u-boot.lds ...
yes do not known why

Best Regards,
J.