[PATCH v3] arm: kirkwood: nas220: Add DM Ethernet, SATA, GPIO

Tony Dinh mibodhi at gmail.com
Wed Mar 9 23:44:20 CET 2022


Hi Hajo,

Please see comments below.

On Wed, Mar 9, 2022 at 5:42 AM Hajo Noerenberg <hajo-uboot at noerenberg.de> wrote:
>
> On 09.03.2022 at 12:26 Tony Dinh wrote:
> > Hi Hajo,
> >
>
> Hi Tony,
>
> > On Wed, Mar 9, 2022 at 1:27 AM Hajo Noerenberg <hajo-uboot at noerenberg.de> wrote:
> >>
> >> Bring the NAS220 board up to current standards. This is basically an adaptation of the changes Tony Dinh implemented for the Dockstar board.
> >>
> >> - Implement the changes to v2 as suggested by Stefan Roese
> >> - Add CONFIG_SUPPORT_PASSING_ATAGS et al, otherwise standard Debian flash-kernel pkg is unable to start kernel
> >
> > I believe CONFIG_SUPPORT_PASSING_ATAGS should no longer be needed.
> > Debian mainline kernels have been device-tree based for quite some
> > time. Perhaps it is the flash-kernel package that needs to be looked
> > at (maybe it failed to include the NAS220 DTB?). FWIW, I have been
> > running a custom Debian Kirkwood kernel (with other Kirkwood boards)
> > for many years with u-boots that don't have the ATAGS option enabled.
> >
>
> I personally am a big friend of distributions running without changes, i.e. no modified kernel or similar, and full update support for everything. So I vote to keep ATAGS support and change this later in line with the distribution.
>
> Note: My knowledge of ATAGS and so on is quite limited, so if in doubt I would follow your thoughts, but would like to consider the above.

Sure, I have no strong objection about including the
CONFIG_SUPPORT_PASSING_ATAGS! I thought I just pointed out that it
might not be necessary.

> For reference, this is the relevant part from the flash-kernel database, other Kirkwood devices (e.g. Dockstar) seem to have different settings:
>
> Machine: Seagate Blackarmor NAS220
> Kernel-Flavors: kirkwood marvell
> DTB-Id: kirkwood-blackarmor-nas220.dtb
> DTB-Append: yes
> Mtd-Kernel: uimage
> Mtd-Initrd: rootfs
> U-Boot-Kernel-Address: 0x00040000
> U-Boot-Initrd-Address: 0x00800000
> Required-Packages: u-boot-tools
>
> Machine: Seagate FreeAgent Dockstar
> Machine: Seagate FreeAgent DockStar
> Kernel-Flavors: kirkwood marvell
> DTB-Id: kirkwood-dockstar.dtb
> DTB-Append: yes
> U-Boot-Kernel-Address: 0x00008000
> U-Boot-Initrd-Address: 0x0
> Boot-Kernel-Path: /boot/uImage
> Boot-Initrd-Path: /boot/uInitrd
> Boot-DTB-Path: /boot/dtb
> Required-Packages: u-boot-tools

The flash-kernel configurations above are practically the same as how
the kernel will behave. Both have DTB appended in uImage. The
differences are the locations of uImage and initrd. So if you can boot
the same Kirkwood kernel on both Dockstar and NAS220, then both
u-boots should have the same configuration regarding device-tree and
ATAGS.

The Dockstar u-boot does not have ATAGS:

make dockstar_config
grep ATAGS .config
# CONFIG_SUPPORT_PASSING_ATAGS is not set

But I think we are about to go off topic! I'd be OK with including
CONFIG_SUPPORT_PASSING_ATAGS for now.

And perhaps we can take this discussion offline to figure out why you
need this config for NAS220 but not for Dockstar.

Thanks,
Tony



> Kind regards,
> Hajo
>
>
>
>
>
> >> - Add CONFIG_SYS_64BIT_LBA, basic read tests with a 4TB hdd succeed with my NAS220 hardware
> >>
> >> - Thanks to Stefan and Tony
> >>
> >>
> >> Signed-off-by: Hajo Noerenberg <hajo-uboot at noerenberg.de>
> >> ---
> >>  board/Seagate/nas220/MAINTAINERS |  1 +
> >>  board/Seagate/nas220/nas220.c    | 68 +++++++++++---------------------
> >>  configs/nas220_defconfig         | 16 +++++++-
> >>  include/configs/nas220.h         | 35 ++++++----------
> >>  4 files changed, 49 insertions(+), 71 deletions(-)
> >>
> >> diff --git a/board/Seagate/nas220/MAINTAINERS b/board/Seagate/nas220/MAINTAINERS
> >> index f2df7ea64f..6033f93cf4 100644
> >> --- a/board/Seagate/nas220/MAINTAINERS
> >> +++ b/board/Seagate/nas220/MAINTAINERS
> >> @@ -4,3 +4,4 @@ S:      Maintained
> >>  F:     board/Seagate/nas220/
> >>  F:     include/configs/nas220.h
> >>  F:     configs/nas220_defconfig
> >> +F:     arch/arm/dts/kirkwood-blackarmor-nas220.dts
> >> diff --git a/board/Seagate/nas220/nas220.c b/board/Seagate/nas220/nas220.c
> >> index cd2bbdad1c..fdbf321ff9 100644
> >> --- a/board/Seagate/nas220/nas220.c
> >> +++ b/board/Seagate/nas220/nas220.c
> >> @@ -10,17 +10,22 @@
> >>
> >>  #include <common.h>
> >>  #include <init.h>
> >> -#include <miiphy.h>
> >> -#include <net.h>
> >> -#include <asm/global_data.h>
> >> -#include <asm/mach-types.h>
> >> +#include <netdev.h>
> >> +#include <asm/arch/cpu.h>
> >>  #include <asm/arch/soc.h>
> >>  #include <asm/arch/mpp.h>
> >> -#include <asm/arch/cpu.h>
> >> -#include <asm/io.h>
> >> +#include <asm/global_data.h>
> >> +#include <asm/mach-types.h>
> >> +#include <linux/bitops.h>
> >>
> >>  DECLARE_GLOBAL_DATA_PTR;
> >>
> >> +/* blue power led, board power, sata0, sata1 */
> >> +#define NAS220_GE_OE_LOW (~(BIT(12) | BIT(14) | BIT(24) | BIT(28)))
> >> +#define NAS220_GE_OE_HIGH (~(0))
> >> +#define NAS220_GE_OE_VAL_LOW (BIT(12) | BIT(14) | BIT(24) | BIT(28))
> >> +#define NAS220_GE_OE_VAL_HIGH (0)
> >> +
> >>  int board_early_init_f(void)
> >>  {
> >>         /*
> >> @@ -43,9 +48,9 @@ int board_early_init_f(void)
> >>                 MPP9_TW_SCK,
> >>                 MPP10_UART0_TXD,
> >>                 MPP11_UART0_RXD,
> >> -               MPP12_GPO,
> >> +               MPP12_GPO,              /* blue power led */
> >>                 MPP13_GPIO,
> >> -               MPP14_GPIO,
> >> +               MPP14_GPIO,             /* board power */
> >>                 MPP15_SATA0_ACTn,
> >>                 MPP16_SATA1_ACTn,
> >>                 MPP17_SATA0_PRESENTn,
> >> @@ -55,12 +60,12 @@ int board_early_init_f(void)
> >>                 MPP21_GPIO,
> >>                 MPP22_GPIO,
> >>                 MPP23_GPIO,
> >> -               MPP24_GPIO,
> >> +               MPP24_GPIO,             /* sata0 power */
> >>                 MPP25_GPIO,
> >> -               MPP26_GPIO,
> >> +               MPP26_GPIO,             /* power button */
> >>                 MPP27_GPIO,
> >> -               MPP28_GPIO,
> >> -               MPP29_GPIO,
> >> +               MPP28_GPIO,             /* sata1 power */
> >> +               MPP29_GPIO,             /* reset button */
> >>                 MPP30_GPIO,
> >>                 MPP31_GPIO,
> >>                 MPP32_GPIO,
> >> @@ -73,6 +78,11 @@ int board_early_init_f(void)
> >>         return 0;
> >>  }
> >>
> >> +int board_eth_init(struct bd_info *bis)
> >> +{
> >> +       return cpu_eth_init(bis);
> >> +}
> >> +
> >>  int board_init(void)
> >>  {
> >>         /*
> >> @@ -85,37 +95,3 @@ int board_init(void)
> >>
> >>         return 0;
> >>  }
> >> -
> >> -#ifdef CONFIG_RESET_PHY_R
> >> -/* Configure and enable MV88E1116 PHY */
> >> -void reset_phy(void)
> >> -{
> >> -       u16 reg;
> >> -       u16 devadr;
> >> -       char *name = "egiga0";
> >> -
> >> -       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 */
> >> -       miiphy_reset(name, devadr);
> >> -
> >> -       printf("88E1116 Initialized on %s\n", name);
> >> -}
> >> -#endif /* CONFIG_RESET_PHY_R */
> >> diff --git a/configs/nas220_defconfig b/configs/nas220_defconfig
> >> index f6a1dcbee0..5bf1233273 100644
> >> --- a/configs/nas220_defconfig
> >> +++ b/configs/nas220_defconfig
> >> @@ -3,6 +3,9 @@ CONFIG_SKIP_LOWLEVEL_INIT=y
> >>  CONFIG_SYS_DCACHE_OFF=y
> >>  CONFIG_ARCH_CPU_INIT=y
> >>  CONFIG_ARCH_KIRKWOOD=y
> >> +CONFIG_SUPPORT_PASSING_ATAGS=y
> >> +CONFIG_CMDLINE_TAG=y
> >> +CONFIG_INITRD_TAG=y
> >>  CONFIG_SYS_KWD_CONFIG="board/Seagate/nas220/kwbimage.cfg"
> >>  CONFIG_SYS_TEXT_BASE=0x600000
> >>  CONFIG_NR_DRAM_BANKS=2
> >> @@ -20,7 +23,7 @@ CONFIG_USE_PREBOOT=y
> >>  CONFIG_HUSH_PARSER=y
> >>  CONFIG_SYS_PROMPT="nas220> "
> >>  # CONFIG_CMD_FLASH is not set
> >> -CONFIG_CMD_IDE=y
> >> +CONFIG_CMD_SATA=y
> >>  CONFIG_CMD_NAND=y
> >>  CONFIG_CMD_USB=y
> >>  # CONFIG_CMD_SETEXPR is not set
> >> @@ -32,6 +35,7 @@ CONFIG_CMD_EXT4=y
> >>  CONFIG_CMD_FAT=y
> >>  CONFIG_CMD_JFFS2=y
> >>  CONFIG_CMD_MTDPARTS=y
> >> +CONFIG_MTDPARTS_DEFAULT="mtdparts=orion_nand:0xa0000 at 0x0(uboot),0x010000 at 0xa0000(env),0x500000 at 0xc0000(uimage),0x1a40000 at 0x5c0000(rootfs)"
> >>  CONFIG_CMD_UBI=y
> >>  CONFIG_ISO_PARTITION=y
> >>  CONFIG_EFI_PARTITION=y
> >> @@ -47,11 +51,21 @@ CONFIG_SYS_ATA_DATA_OFFSET=0x100
> >>  CONFIG_SYS_ATA_REG_OFFSET=0x100
> >>  CONFIG_SYS_ATA_ALT_OFFSET=0x100
> >>  CONFIG_KIRKWOOD_GPIO=y
> >> +CONFIG_MVEBU_GPIO=y
> >> +CONFIG_GPIO_EXTRA_HEADER=y
> >> +CONFIG_DM_GPIO=y
> >> +CONFIG_CMD_GPIO=y
> >> +CONFIG_GPIO=y
> >> +CONFIG_DM_GPIO_LOOKUP_LABEL=y
> >>  # CONFIG_MMC is not set
> >>  CONFIG_MTD=y
> >>  CONFIG_MTD_RAW_NAND=y
> >> +CONFIG_PHY_MARVELL=y
> >> +CONFIG_DM_ETH=y
> >>  CONFIG_MVGBE=y
> >>  CONFIG_MII=y
> >> +CONFIG_SATA_MV=y
> >> +CONFIG_SYS_SATA_MAX_DEVICE=2
> >>  CONFIG_DM_RTC=y
> >>  CONFIG_RTC_MV=y
> >>  CONFIG_SYS_NS16550=y
> >> diff --git a/include/configs/nas220.h b/include/configs/nas220.h
> >> index 815f81f649..4c20245e5f 100644
> >> --- a/include/configs/nas220.h
> >> +++ b/include/configs/nas220.h
> >> @@ -11,50 +11,37 @@
> >>  #ifndef _CONFIG_NAS220_H
> >>  #define _CONFIG_NAS220_H
> >>
> >> -/* power-on led, regulator, sata0, sata1 */
> >> -#define NAS220_GE_OE_VAL_LOW ((1 << 12)|(1 << 14)|(1 << 24)|(1 << 28))
> >> -#define NAS220_GE_OE_VAL_HIGH (0)
> >> -#define NAS220_GE_OE_LOW (~((1 << 12)|(1 << 14)|(1 << 24)|(1 << 28)))
> >> -#define NAS220_GE_OE_HIGH (~(0))
> >> -
> >> -/* 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)
> >> -
> >> -#include "mv-common.h"
> >> -
> >>  /*
> >> - *  Environment variables configurations
> >> + * mv-common.h should be defined after CMD configs since it used them
> >> + * to enable certain macros
> >>   */
> >>
> >> +#include "mv-common.h"
> >> +
> >>  /*
> >>   * Default environment variables
> >>   */
> >>
> >>  #define CONFIG_EXTRA_ENV_SETTINGS \
> >>         "bootargs=console=ttyS0,115200\0" \
> >> -       "mtdparts=mtdparts=orion_nand:0xa0000 at 0x0(uboot),"\
> >> -       "0x010000 at 0xa0000(env),"\
> >> -       "0x500000 at 0xc0000(uimage),"\
> >> -       "0x1a40000 at 0x5c0000(rootfs)\0" \
> >>         "mtdids=nand0=orion_nand\0"\
> >> +       "mtdparts=" CONFIG_MTDPARTS_DEFAULT \
> >>         "autostart=no\0"\
> >>         "autoload=no\0"
> >>
> >>  /*
> >>   * Ethernet Driver configuration
> >>   */
> >> -#ifdef CONFIG_CMD_NET
> >>  #define CONFIG_MVGBE_PORTS {1, 0}      /* enable port 0 only */
> >>  #define CONFIG_PHY_BASE_ADR 8
> >> -#endif /* CONFIG_CMD_NET */
> >> +#ifdef CONFIG_RESET_PHY_R
> >> +#undef CONFIG_RESET_PHY_R              /* remove legacy reset_phy() */
> >> +#endif
> >>
> >>  /*
> >> - * EFI partition
> >> + * SATA driver configuration
> >>   */
> >> +#define CONFIG_LBA48
> >> +#define CONFIG_SYS_64BIT_LBA
> >>
> >>  #endif /* _CONFIG_NAS220_H */
> >> --
> >> 2.20.1
> >>


More information about the U-Boot mailing list