[U-Boot] [PATH v7 2/2] Add support for Network Space v2

Prafulla Wadaskar prafulla at marvell.com
Thu May 12 14:04:43 CEST 2011



> -----Original Message-----
> From: Simon Guinot [mailto:simon at sequanux.org]
> Sent: Thursday, May 12, 2011 5:17 PM
> To: Prafulla Wadaskar
> Cc: Simon Guinot; Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare;
> Prabhanjan Sarnaik
> Subject: Re: [PATH v7 2/2] Add support for Network Space v2
> 
> Hi Prafulla,
> 
> On Thu, May 12, 2011 at 03:36:48AM -0700, Prafulla Wadaskar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Simon Guinot [mailto:simon.guinot at sequanux.org]
> > > Sent: Thursday, May 12, 2011 3:19 PM
> > > To: Prafulla Wadaskar
> > > Cc: Albert ARIBAUD; u-boot at lists.denx.de; Simon Guinot
> > > Subject: [PATH v7 2/2] Add support for Network Space v2
> > >
> > > This patch add support for the Network Space v2 board and parents,
> based
> > > on the Marvell Kirkwood 6281 SoC. This include Network Space (Max)
> v2
> > > and Internet Space v2.
> > >
> > > Additional information is available at:
> > > http://lacie-nas.org/doku.php?id=network_space_v2
> > >
> > > Signed-off-by: Simon Guinot <sguinot at lacie.com>
> > > ---
> > > Changes for v2:
> > >   - add entries to MAINTAINERS file
> > >   - move boards from root Makefile to boards.cfg
> > >   - move MACH_TYPE definition into netspace_v2.h
> > >   - remove CONFIG_SYS_HZ redefinition
> > >   - turn PHY initialization message into debug()
> > >
> > > Changes for v3: none
> > >
> > > Changes for v4:
> > >   - enhance commit message: add SoC information
> > >
> > > Changes for v5: none
> > >
> > > Changes for v6:
> > >   - enable device tree support
> > >   - clean some "#define" in netspace_v2.h
> > >   - enhance commit message: provide description URL and mention SoC
> > >     family
> > >   - rebase against u-boot-{arm,marvell}/master branches
> > >
> > > Changes for v7:
> > >   - rebase against u-boot-marvell/master branch
> > >
> > >  MAINTAINERS                           |    6 +
> > >  board/LaCie/netspace_v2/Makefile      |   49 ++++++++++
> > >  board/LaCie/netspace_v2/kwbimage.cfg  |  162
> > > +++++++++++++++++++++++++++++++++
> > >  board/LaCie/netspace_v2/netspace_v2.c |  144
> > > +++++++++++++++++++++++++++++
> > >  board/LaCie/netspace_v2/netspace_v2.h |   39 ++++++++
> > >  boards.cfg                            |    3 +
> > >  include/configs/netspace_v2.h         |  153
> > > +++++++++++++++++++++++++++++++
> > >  7 files changed, 556 insertions(+), 0 deletions(-)
> > >  create mode 100644 board/LaCie/netspace_v2/Makefile
> > >  create mode 100644 board/LaCie/netspace_v2/kwbimage.cfg
> > >  create mode 100644 board/LaCie/netspace_v2/netspace_v2.c
> > >  create mode 100644 board/LaCie/netspace_v2/netspace_v2.h
> > >  create mode 100644 include/configs/netspace_v2.h
> > >
...snip..
> > > +
> > > +/* Configure and initialize PHY */
> > > +void reset_phy(void)
> > > +{
> > > +     mv_phy_88e1116_init("egiga0");
> > > +}
> > > +
> > > +#define NETSPACE_V2_GPIO_BUTTON              32
> >
> > How about defining this in header file ?
> 
> I don't know. The GPIO button value is only useful within this file.
> Visualise both the define and the usage on the same screen is rather
> comfortable. As an alternative, I could drop the define...
> 
> What is your opinion ?

Do not drop it, I will recommend to put it in board/LaCie/netspace_v2/netspace_v2.h.

...snip...

> > > +#endif
> >
> > There should be #elif for third board and #error for #else part
> 
> I have noticed this #error in the other board include files. Is it
> really needed ? This file is only included if one machine is selected.
> 

No it is not really needed, choice is yours :-)

> >
> > > +
> > > +/*
> > > + * High Level Configuration Options (easy to change)
> > > + */
> > > +#define CONFIG_FEROCEON_88FR131              /* CPU Core subversion
> */
> > > +#define CONFIG_KIRKWOOD                      /* SOC Family Name */
> > > +#define CONFIG_KW88F6281             /* SOC Name */
> > > +#define CONFIG_SKIP_LOWLEVEL_INIT    /* disable board lowlevel_init
> */
> > > +
> > > +/*
> > > + * Commands configuration
> > > + */
> > > +#define CONFIG_SYS_NO_FLASH          /* Declare no flash (NOR/SPI)
> */
> > > +#include <config_cmd_default.h>
> > > +#define CONFIG_CMD_ENV
> > > +#define CONFIG_CMD_DHCP
> > > +#define CONFIG_CMD_PING
> > > +#define CONFIG_CMD_SF
> > > +#define CONFIG_CMD_I2C
> > > +#define CONFIG_CMD_IDE
> > > +#define CONFIG_CMD_USB
> > > +
> > > +/*
> > > + * Core clock definition.
> > > + */
> > > +#define CONFIG_SYS_TCLK                      166000000 /* 166MHz */
> >
> > This is not needed, defined in ..arch-kirkwood/kw88f6xxx.h
> 
> Actually it is. The core clock value is not standard for this boards.
> For a 6281 SoC, TCLK is defined to 200MHz and Network Space v2 TCLK is
> 166MHz.
> 
> This define has been introduced with a previous patch (already merged):
> 18c4cb0f: Kirkwood: allow to override CONFIG_SYS_TCLK
> 

Okay I take my words back, I was referring kw88f6162.h :-(

> >
> > > +
> > > +/*
> > > + * mv-common.h should be defined after CMD configs since it used
> them
> > > + * to enable certain macros
> > > + */
> > > +#define CONFIG_NR_DRAM_BANKS         2
> > > +#include "mv-common.h"
> > > +
> > > +/* Remove or override few declarations from mv-common.h */
> > > +#undef CONFIG_RBTREE
> > > +#undef CONFIG_ENV_SPI_MAX_HZ
> > > +#undef CONFIG_SYS_IDE_MAXBUS
> > > +#undef CONFIG_SYS_IDE_MAXDEVICE
> > > +#undef CONFIG_SYS_PROMPT
> > > +#define CONFIG_ENV_SPI_MAX_HZ           20000000 /* 20Mhz */
> > > +#define CONFIG_SYS_IDE_MAXBUS           1
> > > +#define CONFIG_SYS_IDE_MAXDEVICE        1
> > > +#define CONFIG_SYS_PROMPT            "ns2> "
> > > +
> > > +/*
> > > + * Ethernet Driver configuration
> > > + */
> > > +#ifdef CONFIG_CMD_NET
> > > +#define CONFIG_MVGBE_PORTS           {1, 0} /* enable port 0 only
> */
> > > +#define CONFIG_NETCONSOLE
> > > +#endif
> > > +
> > > +/*
> > > + * SATA Driver configuration
> > > + */
> > > +#ifdef CONFIG_MVSATA_IDE
> > > +#define CONFIG_SYS_ATA_IDE0_OFFSET      MV_SATA_PORT0_OFFSET
> > > +/* Network Space Max v2 use 2 SATA ports */
> > > +#ifdef CONFIG_NETSPACE_MAX_V2
> > > +#define CONFIG_SYS_ATA_IDE1_OFFSET      MV_SATA_PORT1_OFFSET
> > > +#endif
> > > +#endif
> > > +
> > > +/*
> > > + * Enable GPI0 support
> > > + */
> > > +#define CONFIG_KIRKWOOD_GPIO
> > > +
> > > +/*
> > > + * File systems support
> > > + */
> > > +#define CONFIG_CMD_EXT2
> > > +#define CONFIG_CMD_FAT
> >
> > Get rid of this, use CONFIG_SYS_MVFS
> 
> Outch. Please no :)
> 
> CONFIG_SYS_MVFS enable support for mtd, jffs2 and ubifs.
> It is completely useless for a Network Space v2 board where the SPI
> flash size is 512KB. The whole space is for U-Boot.

Okay..

> 
> >
> > > +
> > > +/*
> > > + * Use the HUSH parser
> > > + */
> > > +#define CONFIG_SYS_HUSH_PARSER
> > > +#define CONFIG_SYS_PROMPT_HUSH_PS2   "> "
> > > +
> > > +/*
> > > + * Console configuration
> > > + */
> > > +#define CONFIG_CONSOLE_MUX
> > > +#define CONFIG_SYS_CONSOLE_IS_IN_ENV
> > > +
> > > +/*
> > > + * Enable device tree support
> > > + */
> > > +#define CONFIG_OF_LIBFDT
> > > +
> > > +/*
> > > + * Environment variables configurations
> > > + */
> > > +#define CONFIG_ENV_IS_IN_SPI_FLASH
> > > +#define CONFIG_ENV_SECT_SIZE         0x10000 /* 64KB */
> > > +#define CONFIG_ENV_SIZE                      0x1000  /* 4KB */
> > > +#define CONFIG_ENV_ADDR                      0x70000
> > > +#define CONFIG_ENV_OFFSET            0x70000 /* env starts here */
> > > +
> > > +/*
> > > + * Default environment variables
> > > + */
> > > +#define CONFIG_BOOTARGS "console=ttyS0,115200"
> > > +
> > > +#define CONFIG_BOOTCOMMAND                           \
> > > +     "if run usbload || run diskload; then bootm 0x800000; fi"
> > > +
> > > +#define CONFIG_EXTRA_ENV_SETTINGS                    \
> > > +     "stdin=serial,nc\0"                             \
> > > +     "stdout=serial,nc\0"                            \
> > > +     "stderr=serial,nc\0"                            \
> > > +     "ipaddr=192.168.1.111\0"                        \
> >
> > NAK for ipaddr, no ip address should be defined by default.
> 
> I understand, but I need a known IP address.
> 
> I want people to be able to update the stock U-Boot using netconsole
> (without a serial link which require to open the case). At restart,
> right after the update, a user must be able to figure out the board IP.
> 
> An alternative could be using a DHCP configuration.

I wish to know Wolfgang's or Ben's comments on this.
Because I remember strategically hard coding ipaddress is not recommend.

Regards..
Prafulla . .


More information about the U-Boot mailing list