Hi Hajo,

Please see this thread:

https://lists.denx.de/pipermail/u-boot/2022-March/478686.html

Due to a conflict with Tom's patch to move CONFIG_RESET_PHY_R to
Kconfig, this NAS220 patch and my Sheevaplug patch will need to be
rebased and resubmitted. We can do that after Tom's patch has been
merged to master.

All the best,
Tony

On Thu, Mar 17, 2022 at 7:20 PM Tony Dinh <mibo...@gmail.com> wrote:
>
> Hi Stefan & Hajo,
>
> Please see my comment below.
>
> On Wed, Mar 9, 2022 at 2:44 PM Tony Dinh <mibo...@gmail.com> wrote:
> >
> > Hi Hajo,
> >
> > Please see comments below.
> >
> > On Wed, Mar 9, 2022 at 5:42 AM Hajo Noerenberg <hajo-ub...@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-ub...@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.
>
> In general, we'd like to discourage the use of
> CONFIG_SUPPORT_PASSING_ATAGS for boxes that already have mainline
> support for device-tree.
>
> However, during off-list testing done by myself and Hajo, I realized
> that the Debian mainline kernel assumes that the box is running with
> older and non-FDT u-boot only. In order to upgrade Debian kernel, it
> will take quite some time get the patch in and get it booting with
> separate DTB (ie. no ATAGS). This is because the configuration of
> Debian kernel files are on flash partitions (i.e. not on disk
> storage).
>
> To make it easier for the transition of the Debian kernel to full FDT,
> I would agree to include CONFIG_SUPPORT_PASSING_ATAGS for this NAS220
> board. And the process of upgrading the Debian kernel can take its
> time to resolve.
>
> In the future, we will make sure to keep other Kirkwood boards in full
> FDT compliance and not relying on ATAGS.
>
> Reviewed-by: Tony Dinh <mibodhi at gmail.com>
>
> Thanks,
> Tony
>
> >
> > 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-ub...@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@0x0(uboot),0x010000@0xa0000(env),0x500000@0xc0000(uimage),0x1a40000@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@0x0(uboot),"\
> > > >> -       "0x010000@0xa0000(env),"\
> > > >> -       "0x500000@0xc0000(uimage),"\
> > > >> -       "0x1a40000@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
> > > >>

Reply via email to