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

Stefan Roese sr at denx.de
Thu Apr 21 10:45:12 CEST 2022


Hi Hajo,

On 3/24/22 01:48, Tony Dinh wrote:
> 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.

Correct. I'm handling the open MVEBU / Kirkwood patches now and this
one does not build any more. Please wait a few days until I've pushed
the currently open and ready patches to master. And then re-submit v4
with CONFIG_RESET_PHY_R removed as described by Tony above.

Thanks,
Stefan

> All the best,
> Tony
> 
> On Thu, Mar 17, 2022 at 7:20 PM Tony Dinh <mibodhi at gmail.com> wrote:
>>
>> Hi Stefan & Hajo,
>>
>> Please see my comment below.
>>
>> On Wed, Mar 9, 2022 at 2:44 PM Tony Dinh <mibodhi at gmail.com> wrote:
>>>
>>> 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.
>>
>> 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-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
>>>>>>

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de


More information about the U-Boot mailing list