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

[Simon Guinot <simon@sequanux.org>]

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@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
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 24a55c2..47b724c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -645,6 +645,12 @@ Sedji Gaouaou<sedji.gaouaou@atmel.com>
> >       at91sam9g10ek           ARM926EJS (AT91SAM9G10 SoC)
> >       at91sam9m10g45ek        ARM926EJS (AT91SAM9G45 SoC)
> >
> > +Simon Guinot <simon.guinot@sequanux.org>
> > +
> > +     inetspace_v2    ARM926EJS (Kirkwood SoC)
> > +     netspace_v2     ARM926EJS (Kirkwood SoC)
> > +     netspace_max_v2 ARM926EJS (Kirkwood SoC)
> 
> You are adding three boards support, the subject does not say the same.

OK, I will enhance the subject description.

[ snip ]

> > diff --git a/board/LaCie/netspace_v2/netspace_v2.c
> > b/board/LaCie/netspace_v2/netspace_v2.c
> > new file mode 100644
> > index 0000000..9bc9940
> > --- /dev/null
> > +++ b/board/LaCie/netspace_v2/netspace_v2.c
> > @@ -0,0 +1,144 @@
> > +/*
> > + * Copyright (C) 2011 Simon Guinot <sguinot@lacie.com>
> > + *
> > + * Based on Kirkwood support:
> > + * (C) Copyright 2009
> > + * Marvell Semiconductor <www.marvell.com>
> > + * Written-by: Prafulla Wadaskar <prafulla@marvell.com>
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <common.h>
> > +#include <miiphy.h>
> > +#include <netdev.h>
> > +#include <command.h>
> > +#include <asm/arch/kirkwood.h>
> > +#include <asm/arch/mpp.h>
> > +#include <asm/arch/gpio.h>
> > +#include "netspace_v2.h"
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +int board_early_init_f(void)
> > +{
> > +     /* Gpio configuration */
> > +     kw_config_gpio(NETSPACE_V2_OE_VAL_LOW, NETSPACE_V2_OE_VAL_HIGH,
> > +                     NETSPACE_V2_OE_LOW, NETSPACE_V2_OE_HIGH);
> > +
> > +     /* Multi-Purpose Pins Functionality configuration */
> > +     u32 kwmpp_config[] = {
> > +             MPP0_SPI_SCn,
> > +             MPP1_SPI_MOSI,
> > +             MPP2_SPI_SCK,
> > +             MPP3_SPI_MISO,
> > +             MPP4_NF_IO6,
> > +             MPP5_NF_IO7,
> > +             MPP6_SYSRST_OUTn,
> > +             MPP7_GPO,               /* Fan speed (bit 1) */
> > +             MPP8_TW_SDA,
> > +             MPP9_TW_SCK,
> > +             MPP10_UART0_TXD,
> > +             MPP11_UART0_RXD,
> > +             MPP12_GPO,              /* Red led */
> > +             MPP14_GPIO,             /* USB fuse */
> > +             MPP16_GPIO,             /* SATA 0 power */
> > +             MPP17_GPIO,             /* SATA 1 power */
> > +             MPP18_NF_IO0,
> > +             MPP19_NF_IO1,
> > +             MPP20_SATA1_ACTn,
> > +             MPP21_SATA0_ACTn,
> > +             MPP22_GPIO,             /* Fan speed (bit 0) */
> > +             MPP23_GPIO,             /* Fan power */
> > +             MPP24_GPIO,             /* USB mode select */
> > +             MPP25_GPIO,             /* Fan rotation fail */
> > +             MPP26_GPIO,             /* USB vbus-in detection */
> > +             MPP28_GPIO,             /* USB enable vbus-out */
> > +             MPP29_GPIO,             /* Blue led (slow register) */
> > +             MPP30_GPIO,             /* Blue led (command register) */
> > +             MPP31_GPIO,             /* Board power off */
> > +             MPP32_GPIO,             /* Button (0 = Released, 1 = Pushed) */
> > +             MPP33_GPIO,             /* Fan speed (bit 2) */
> > +             0
> > +     };
> > +     kirkwood_mpp_conf(kwmpp_config);
> > +
> > +     return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +     /* Machine number */
> > +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
> > +
> > +     /* Boot parameters address */
> > +     gd->bd->bi_boot_params = kw_sdram_bar(0) + 0x100;
> > +
> > +     return 0;
> > +}
> > +
> > +void mv_phy_88e1116_init(char *name)
> > +{
> > +     u16 reg;
> > +     u16 devadr;
> > +
> > +     if (miiphy_set_current_dev(name))
> > +             return;
> > +
> > +     /* command to read PHY dev address */
> > +     if (miiphy_read(name, 0xEE, 0xEE, (u16 *) &devadr)) {
> > +             printf("Err..(%s) could not read PHY dev address\n", __func__);
> > +             return;
> > +     }
> > +
> > +     /*
> > +      * Enable RGMII delay on Tx and Rx for CPU port
> > +      * Ref: sec 4.7.2 of chip datasheet
> > +      */
> > +     miiphy_write(name, devadr, MV88E1116_PGADR_REG, 2);
> > +     miiphy_read(name, devadr, MV88E1116_MAC_CTRL_REG, &reg);
> > +     reg |= (MV88E1116_RGMII_RXTM_CTRL | MV88E1116_RGMII_TXTM_CTRL);
> > +     miiphy_write(name, devadr, MV88E1116_MAC_CTRL_REG, reg);
> > +     miiphy_write(name, devadr, MV88E1116_PGADR_REG, 0);
> > +
> > +     /* reset the phy */
> > +     if (miiphy_read(name, devadr, MII_BMCR, &reg) != 0) {
> > +             printf("Err..(%s) PHY status read failed\n", __func__);
> > +             return;
> > +     }
> > +     if (miiphy_write(name, devadr, MII_BMCR, reg | 0x8000) != 0) {
> > +             printf("Err..(%s) PHY reset failed\n", __func__);
> > +             return;
> > +     }
> > +
> > +     debug("88E1116 Initialized on %s\n", name);
> > +}
> > +
> > +/* 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 ?

> 
> > +
> > +/* Return GPIO button status */
> > +static int
> > +do_read_button(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > +{
> > +     return kw_gpio_get_value(NETSPACE_V2_GPIO_BUTTON);
> > +}
> > +
> > +U_BOOT_CMD(button, 1, 1, do_read_button,
> > +        "Return GPIO button status 0=off 1=on", "");
> > diff --git a/board/LaCie/netspace_v2/netspace_v2.h
> > b/board/LaCie/netspace_v2/netspace_v2.h
> > new file mode 100644
> > index 0000000..c26a6e0
> > --- /dev/null
> > +++ b/board/LaCie/netspace_v2/netspace_v2.h
> > @@ -0,0 +1,39 @@
> > +/*
> > + * Copyright (C) 2011 Simon Guinot <sguinot@lacie.com>
> > + *
> > + * Based on Kirkwood support:
> > + * (C) Copyright 2009
> > + * Marvell Semiconductor <www.marvell.com>
> > + * Written-by: Prafulla Wadaskar <prafulla@marvell.com>
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef NETSPACE_V2_H
> > +#define NETSPACE_V2_H
> > +
> > +#define NETSPACE_V2_OE_LOW           0x06004000
> > +#define NETSPACE_V2_OE_HIGH          0x00000031
> > +#define NETSPACE_V2_OE_VAL_LOW               0x10030000
> > +#define NETSPACE_V2_OE_VAL_HIGH              0x00000000
> > +
> > +/* PHY related */
> > +#define MV88E1116_LED_FCTRL_REG              10
> > +#define MV88E1116_CPRSP_CR3_REG              21
> > +#define MV88E1116_MAC_CTRL_REG               21
> > +#define MV88E1116_PGADR_REG          22
> > +#define MV88E1116_RGMII_TXTM_CTRL    (1 << 4)
> > +#define MV88E1116_RGMII_RXTM_CTRL    (1 << 5)
> > +
> > +#endif /* NETSPACE_V2_H */
> > diff --git a/boards.cfg b/boards.cfg
> > index bc193c6..5bd320e 100644
> > --- a/boards.cfg
> > +++ b/boards.cfg
> > @@ -104,6 +104,9 @@ davinci_schmoogie            arm         arm926ejs
> > schmoogie           davinci
> >  davinci_sffsdr               arm         arm926ejs   sffsdr
> > davinci        davinci
> >  davinci_sonata               arm         arm926ejs   sonata
> > davinci        davinci
> >  suen3                        arm         arm926ejs   km_arm
> > keymile        kirkwood
> > +inetspace_v2                 arm         arm926ejs   netspace_v2
> > LaCie          kirkwood    netspace_v2:INETSPACE_V2
> > +netspace_v2                  arm         arm926ejs   netspace_v2
> > LaCie          kirkwood    netspace_v2:NETSPACE_V2
> > +netspace_max_v2              arm         arm926ejs   netspace_v2
> > LaCie          kirkwood    netspace_v2:NETSPACE_MAX_V2
> >  guruplug                     arm         arm926ejs   -
> > Marvell        kirkwood
> >  mv88f6281gtw_ge              arm         arm926ejs   -
> > Marvell        kirkwood
> >  openrd_base                  arm         arm926ejs   openrd
> > Marvell        kirkwood        openrd:BOARD_IS_OPENRD_BASE
> > diff --git a/include/configs/netspace_v2.h
> > b/include/configs/netspace_v2.h
> > new file mode 100644
> > index 0000000..95b1e4b
> > --- /dev/null
> > +++ b/include/configs/netspace_v2.h
> > @@ -0,0 +1,153 @@
> > +/*
> > + * Copyright (C) 2011 Simon Guinot <sguinot@lacie.com>
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef _CONFIG_NETSPACE_V2_H
> > +#define _CONFIG_NETSPACE_V2_H
> > +
> > +/*
> > + * Machine number definition
> > + */
> > +#if defined(CONFIG_INETSPACE_V2)
> > +#define CONFIG_MACH_TYPE             MACH_TYPE_INETSPACE_V2
> > +#define CONFIG_IDENT_STRING          " LaCie Internet Space v2"
> 
> Ident strings defined for each board are very long, can you use short names? The console logs with such long ident looks ugly.

Ok, I could drop the LaCie prefix. Is it enough ?
Otherwise, I could use some alias but it could be confusing for board
identification.

> 
> > +#elif defined(CONFIG_NETSPACE_V2)
> > +#define CONFIG_MACH_TYPE             MACH_TYPE_NETSPACE_V2
> > +#define CONFIG_IDENT_STRING          " LaCie Network Space v2"
> > +#elif defined(CONFIG_NETSPACE_MAX_V2)
> > +#define CONFIG_MACH_TYPE             MACH_TYPE_NETSPACE_MAX_V2
> > +#define CONFIG_IDENT_STRING          " LaCie Network Space Max v2"
> > +#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.

> 
> > +
> > +/*
> > + * 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

> 
> > +
> > +/*
> > + * 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. 

> 
> > +
> > +/*
> > + * 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.

> 
> > +     "diskload=ide reset && "                        \
> > +     "ext2load ide 0:1 0x800000 /boot/uImage\0"      \
> > +     "usbload=usb start && "                         \
> > +     "fatload usb 0:1 0x800000 /boot/uImage\0"
> > +
> > +#endif /* _CONFIG_NETSPACE_V2_H */

Thanks for your comments.

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110512/a0fce3c1/attachment.pgp