[PATCH v3] arm: kirkwood: nas220: Add DM Ethernet, SATA, GPIO
Stefan Roese
sr at denx.de
Mon Mar 21 07:09:09 CET 2022
Hi Tony,
On 3/18/22 03:20, Tony Dinh 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.
Fine with me:
Reviewed-by: Stefan Roese <sr at denx.de>
Thanks,
Stefan
> 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 |= (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