[U-Boot] [PATCH v4 2/3] ARM: add support for Network Space v2 Lite and Mini

[Prafulla Wadaskar]

> -----Original Message-----
> From: Simon Guinot [mailto:simon.guinot at sequanux.org]
> Sent: 07 September 2012 02:28
> To: Prafulla Wadaskar
> Cc: Albert ARIBAUD; u-boot at lists.denx.de
> Subject: Re: [PATCH v4 2/3] ARM: add support for Network Space v2 Lite
> and Mini
> 
> On Thu, Sep 06, 2012 at 04:19:16PM -0700, Prafulla Wadaskar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Simon Guinot [mailto:simon.guinot at sequanux.org]
> > > Sent: 06 September 2012 13:52
> > > To: Prafulla Wadaskar; Albert ARIBAUD
> > > Cc: u-boot at lists.denx.de; Simon Guinot
> > > Subject: [PATCH v4 2/3] ARM: add support for Network Space v2 Lite
> and
> > > Mini
> > >
> > > This patch adds support for the LaCie boards Network Space v2
> (Lite
> > > and
> > > Mini). This two boards are derived from the Network Space v2 and a
> lot
> > > of hardware caracteristics are shared.
> > >
> > > - CPU: Marvell 88F6192 800Mhz
> > > - SDRAM memory: 128MB DDR2 200Mhz
> > > - 1 SATA port: internal
> > > - Gigabit ethernet: PHY Marvell 88E1318
> > > - Flash memory: SPI NOR 512KB (Macronix MX25L4005A)
> > > - i2c EEPROM: 512 bytes (24C04 type)
> > > - 2 USB2 ports (Lite only): host and host/device
> > > - 1 push button
> > > - 1 SATA LED (bi-color, blue and red)
> > >
> > > Signed-off-by: Simon Guinot <simon.guinot at sequanux.org>
> > > ---
> > > Changes for v4:
> > >  - Include missing MACH_TYPE_ in configs/lacie_kw.h.
> > >
> > > No changes for v3.
> > >
> > > Changes for v2:
> > >  - Move mach-types update into a separate patch.
> > >
> > >  board/LaCie/common/common.c               |   36 ++++++-
> > >  board/LaCie/common/common.h               |    1 +
> > >  board/LaCie/netspace_v2/kwbimage-ns2l.cfg |  162
> > > +++++++++++++++++++++++++++++
> >
> > Hi Simon,
> >
> > You have added one more cfg file in this patch
> > Now you have three cfg files in this folder that just diff from each
> other for DRAM configuration.
> >
> > Whereas you can delete two of them, use maximum dram setting in cfg
> file, and then in board specific file you can tune the configuration
> for required size.
> >
> > You may look for this optimization.
> > FYI: pls see the captured log of diff
> 
> OK I will look at this but are you sure it is safe to update the DDR
> settings at run time (even only the size) ?

I think so, size should not have any issues

> 
> >
> > [prafulla at pe-dt061 u-boot-marvell.git (master)]$ diff
> board/LaCie/netspace_v2/kwbimage-ns2l.cfg
> board/LaCie/netspace_v2/kwbimage.cfg
> > 44c44
> > < DATA 0xFFD01404 0x34143000    # DDR Controller Control Low
> > ---
> > > DATA 0xFFD01404 0x35143000    # DDR Controller Control Low
> > 72c72
> > < DATA 0xFFD01410 0x0000DDDD    #  DDR Address Control
> > ---
> > > DATA 0xFFD01410 0x0000000C    #  DDR Address Control
> > 74c74
> 
> For example, here we have a different SDRAM width configuration
> configuration for CS0: x8 against x16. It looks like it can't be
> merged.

Okay

> 
> > < # bit3-2:   10, Cs0size=512Mb
> > ---
> > > # bit3-2:   11, Cs0size=1Gb
> > 133c133
> > < DATA 0xFFD01504 0x07FFFFF1    # CS[0]n Size
> > ---
> > > DATA 0xFFD01504 0x0FFFFFF1    # CS[0]n Size
> > [prafulla at pe-dt061 u-boot-marvell.git (master)]$ diff
> board/LaCie/netspace_v2/kwbimage-ns2l.cfg
> board/LaCie/netspace_v2/kwbimage
> > kwbimage.cfg       kwbimage-is2.cfg   kwbimage-ns2l.cfg
> > [prafulla at pe-dt061 u-boot-marvell.git (master)]$ diff
> board/LaCie/netspace_v2/kwbimage-ns2l.cfg
> board/LaCie/netspace_v2/kwbimage-is2.cfg
> > 44c44
> > < DATA 0xFFD01404 0x34143000    # DDR Controller Control Low
> > ---
> > > DATA 0xFFD01404 0x35143000    # DDR Controller Control Low
> > 72c72
> > < DATA 0xFFD01410 0x0000DDDD    #  DDR Address Control
> > ---
> > > DATA 0xFFD01410 0x00000008    #  DDR Address Control
> 
> Same comment as before.
> 
> > [prafulla at pe-dt061 u-boot-marvell.git (master)]$ diff
> board/LaCie/netspace_v2/kwbimage-is2.cfg
> board/LaCie/netspace_v2/kwbimage.cfg
> > 72c72
> > < DATA 0xFFD01410 0x00000008    #  DDR Address Control
> > ---
> > > DATA 0xFFD01410 0x0000000C    #  DDR Address Control
> > 74c74
> > < # bit3-2:   10, Cs0size=512Mb
> > ---
> > > # bit3-2:   11, Cs0size=1Gb
> > 133c133
> > < DATA 0xFFD01504 0x07FFFFF1    # CS[0]n Size
> > ---
> > > DATA 0xFFD01504 0x0FFFFFF1    # CS[0]n Size
> 
> Ok. Here, the only difference is the size. So, this two configurations
> could probably be merged. Thanks for pointing this out.
> 
> Considering this merge is rather optimisation than fix, is that good
> enough for you if I schedule this task for the next U-Boot release ?

Okay, you can take it for next release.

Regards...
Prafulla . . .