> -----Original Message-----
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: 26 June 2012 18:15
> To: Prafulla Wadaskar
> Cc: u-boot@lists.denx.de; Wolfgang Denk
> Subject: Re: [PATCH 3/3] Kirkwood: Add support for Ka-Ro TK71
> 
> Dear Prafulla Wadaskar,
> 
> > > -----Original Message-----
> > > From: Marek Vasut [mailto:ma...@denx.de]
> > > Sent: 26 June 2012 17:45
> > > To: u-boot@lists.denx.de
> > > Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
> > > Subject: [PATCH 3/3] Kirkwood: Add support for Ka-Ro TK71
> > >
> > > Signed-off-by: Marek Vasut <ma...@denx.de>
> > > Cc: Prafulla Wadaskar <prafu...@marvell.com>
> > > Cc: Wolfgang Denk <w...@denx.de>
> > > ---
> > >
> > >  board/karo/tk71/Makefile     |   45 +++++++++++
> > >  board/karo/tk71/kwbimage.cfg |  174
...snip...
> > > +int board_init(void)
> > > +{
> > > + /*
> > > +  * arch number of board
> > > +  */
> > > + gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
> > > +
> > > + /* adress of boot parameters */
> > > + gd->bd->bi_boot_params = kw_sdram_bar(0) + 0x100;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void kw_adjust_dram(void)
> > > +{
> > > + unsigned long size = get_ram_size(PHYS_SDRAM_1,
> > > PHYS_SDRAM_1_SIZE);
> > > +
> > > + /* 256MB module, adjust BAR register */
> > > + if (size == 256 * 1024 * 1024)
> > > +         writel(0x0ffffff1, KW_REG_CPUCS_WIN_SZ(0));
> >
> > No hard coding please, please use register structures (preferred)
> 
> Do we have any such structures defined ?

There are few structures in place but not for DRAM, let's do it, this was 
missing part in early patches.

> 
> > > +
> > > + gd->bd->bi_dram[0].size = size;
> > > + gd->ram_size = size;
> > > +}
> >
> > You have created kwbimage.cfg in this, it makes more sense to set
> 256MB
> > size by default in kwbimage.cfg. Or preferred use any existing
> > kwbimage.cfg (preferred)
> 
> I don't think I follow. I always believe these are for setting up the
> platform,
> so at least one per platform is necessary.

Ideally YES, but if we have similar platforms and delta is very small like 
tuning DRAM size, then for such things we can reuse existing one by supporting 
additional post configuration using board_early_init_f()

This is something positive to avoid code duplication in a cleaner way.

> 
> > The main idea of having such weak functions is to avoid code
> duplication
> > (kwbimage.cfg) for small manageable changes.
> 
> But then, isn't this a bit out of scope of this patchset? Unifying
> kwbimages I

Yes, so please keep you board support patch separate (standalone)
>From DRAM patches.

Regards..
Prafulla . . .
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to