On 05.03.21 21:03, Phil Sutter wrote:

With the recent fixes in pci_mvebu and xhci-pci drivers, the two rear USB3 ports are finally usable and accessing them no longer hangs the system. Moreover, if Linux is booted without a prior call to 'pci enum' and 'usb start', the HCD is detected but attached devices are not usable:

| xhci_hcd 0000:02:00.0: xHCI Host Controller | xhci_hcd 0000:02:00.0: new USB bus registered, assigned bus number 2 | xhci_hcd 0000:02:00.0: hcc params 0x040040a5 hci version 0x100 quirks 0x0000000000080490 | usb usb2: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 5.04 | usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 | usb usb2: Product: xHCI Host Controller | usb usb2: Manufacturer: Linux 5.4.92-1 xhci-hcd | usb usb2: SerialNumber: 0000:02:00.0 | hub 2-0:1.0: USB hub found | ata1: SATA link down (SStatus 0 SControl 300) | hub 2-0:1.0: 2 ports detected | xhci_hcd 0000:02:00.0: xHCI Host Controller | xhci_hcd 0000:02:00.0: new USB bus registered, assigned bus number 3 | xhci_hcd 0000:02:00.0: Host supports USB 3.0 SuperSpeed | usb usb3: We don't know the algorithms for LPM for this host, disabling LPM. | usb usb3: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 5.04 | usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1 | usb usb3: Product: xHCI Host Controller | usb usb3: Manufacturer: Linux 5.4.92-1 xhci-hcd | usb usb3: SerialNumber: 0000:02:00.0 | hub 3-0:1.0: USB hub found | hub 3-0:1.0: 2 ports detected [...] | xhci_hcd 0000:02:00.0: Error while assigning device slot ID | xhci_hcd 0000:02:00.0: Max number of devices this xHCI host supports is 64. | usb usb2-port2: couldn't allocate usb_device

To avoid this problem, enumerate PCI (and USB) from PREBOOT.

Signed-off-by: Phil Sutter phil@nwl.cc

Changes since v1:

• Have to enable XHCI_HCD as well to fulfill Kconfig dependency.
• Explicitly disable XHCI_MARVELL which defaults to enabled.
• Prefix PREBOOT with 'pci enum'.
• Update commit message accordingly.

Hmmm, in general is should not be necessary to configure / setup any of the devices in the bootloader so that it works correctly in Linux. It's best practice, that Linux does not rely on any bootloader setup. If a device, like PCI and/or USB does not work in Linux without this U-Boot setup, then it should be fixed in Linux instead.

Especially calling "usb start" adds a quite big delay to the bootup time. I would really like to not add such changes. Perhaps you could check with the maintainer(s) of the Linux PCI driver and/or the USB PCI controller driver about this issue instead?

Thanks, Stefan

---

configs/ds414_defconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index 3e6dcec3edde3..24466d81dffdb 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -22,7 +22,7 @@ CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200" CONFIG_USE_PREBOOT=y -CONFIG_PREBOOT="usb start; sf probe" +CONFIG_PREBOOT="pci enum; usb start; sf probe" # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_SPL_I2C_SUPPORT=y @@ -65,5 +65,8 @@ CONFIG_SYS_NS16550=y CONFIG_KIRKWOOD_SPI=y CONFIG_USB=y CONFIG_DM_USB=y +CONFIG_USB_XHCI_HCD=y +# CONFIG_USB_XHCI_MVEBU is not set +CONFIG_USB_XHCI_PCI=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_STORAGE=y

Viele Grüße, Stefan

On 03.03.21 01:55, Phil Sutter wrote:

Move the relevant bits from ds109.{c,h} into common/ and adjust the code to fit both DS109 and DS414. Moreover:

• Introduce syno_board_id() which translates CONFIG_MACH_TYPE into the expected board ID tag value.

• Properly initialize isusbhost, mac and mtu fields from env variables.

• Set the right bootargs/bootcmd to correctly boot legacy kernel out of the (DS414) box. Getting the ramdisk location right is a bit tedious.

Cc: Walter Schweizer swwa@users.sourceforge.net Signed-off-by: Phil Sutter phil@nwl.cc

---

board/Synology/common/Makefile | 5 +++ board/Synology/common/legacy.c | 75 ++++++++++++++++++++++++++++++++++ board/Synology/common/legacy.h | 33 +++++++++++++++ board/Synology/ds109/ds109.c | 32 --------------- board/Synology/ds109/ds109.h | 17 -------- configs/ds414_defconfig | 2 +- include/configs/ds109.h | 3 +- include/configs/ds414.h | 15 ++++++- 8 files changed, 130 insertions(+), 52 deletions(-) create mode 100644 board/Synology/common/Makefile create mode 100644 board/Synology/common/legacy.c create mode 100644 board/Synology/common/legacy.h

diff --git a/board/Synology/common/Makefile b/board/Synology/common/Makefile new file mode 100644 index 0000000000000..62354cc2e82e6 --- /dev/null +++ b/board/Synology/common/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 Phil Sutter phil@nwl.cc

+obj-y += legacy.o diff --git a/board/Synology/common/legacy.c b/board/Synology/common/legacy.c new file mode 100644 index 0000000000000..678e7b6809168 --- /dev/null +++ b/board/Synology/common/legacy.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2021
• • Walter Schweizer swwa@users.sourceforge.net
• • Phil Sutter phil@nwl.cc
• */

+#include <common.h>

Please don't include "common.h" any more. There is ongoing work to depricate this common header.

Other than this:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

+#include <env.h> +#include <net.h> +#include <asm/setup.h>

+#include "legacy.h"

+static unsigned int syno_board_id(void) +{

• switch (CONFIG_MACH_TYPE) {
• case 527:

• return SYNO_DS109_ID;
  

• case 3036:

• return SYNO_AXP_4BAY_2BAY;
  

• default:

• return 0;
  

• }

+}

+static unsigned int usb_port_modes(void) +{

• unsigned int i, ret = 0;
• char var[32], *val;
• for (i = 0; i < USBPORT_MAX; i++) {

• snprintf(var, 32, "usb%dMode", i);
  

• val = env_get(var);
  

• if (!val || strcasecmp(val, "host"))
  

• 	continue;
  

• ret |= 1 << i;
  

• }
• return ret;

+}

+/* Support old kernels */ +void setup_board_tags(struct tag **in_params) +{

• struct tag_mv_uboot *t;
• struct tag *params;
• int i;
• debug("Synology board tags...\n");
• params = *in_params;
• t = (struct tag_mv_uboot *)&params->u;
• t->uboot_version = VER_NUM | syno_board_id();
• t->tclk = CONFIG_SYS_TCLK;
• t->sysclk = CONFIG_SYS_TCLK * 2;
• t->isusbhost = usb_port_modes();
• for (i = 0; i < ETHADDR_MAX; i++) {

• char addrvar[16], mtuvar[16];
  

• sprintf(addrvar, i ? "eth%daddr" : "ethaddr", i);
  

• sprintf(mtuvar, i ? "eth%dmtu" : "ethmtu", i);
  

• eth_env_get_enetaddr(addrvar, t->macaddr[i]);
  

• t->mtu[i] = env_get_ulong(mtuvar, 10, 0);
  

• }
• params->hdr.tag = ATAG_MV_UBOOT;
• params->hdr.size = tag_size(tag_mv_uboot);
• params = tag_next(params);
• *in_params = params;

+} diff --git a/board/Synology/common/legacy.h b/board/Synology/common/legacy.h new file mode 100644 index 0000000000000..595e57f09eae0 --- /dev/null +++ b/board/Synology/common/legacy.h @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2021
• • Walter Schweizer swwa@users.sourceforge.net
• • Phil Sutter phil@nwl.cc
• */

+#ifndef __SYNO_LEGACY_H +#define __SYNO_LEGACY_H

+/* Marvell uboot parameters */ +#define ATAG_MV_UBOOT 0x41000403 +#define VER_NUM 0x03040400 /* 3.4.4 */

+#define BOARD_ID_BASE 0x0 +#define SYNO_DS109_ID (BOARD_ID_BASE+0x15) +#define SYNO_AXP_4BAY_2BAY (0xf + 1)

+#define ETHADDR_MAX 4 +#define USBPORT_MAX 3

+struct tag_mv_uboot {

• u32 uboot_version;
• u32 tclk;
• u32 sysclk;
• u32 isusbhost;
• u8 macaddr[ETHADDR_MAX][ETH_ALEN];
• u16 mtu[ETHADDR_MAX];
• u32 fw_image_base;
• u32 fw_image_size;

+};

+#endif /* __SYNO_LEGACY_H */ diff --git a/board/Synology/ds109/ds109.c b/board/Synology/ds109/ds109.c index eaac95460c6e1..3914faaf37bb4 100644 --- a/board/Synology/ds109/ds109.c +++ b/board/Synology/ds109/ds109.c @@ -114,38 +114,6 @@ void reset_misc(void) SOFTWARE_REBOOT); }

-/* Support old kernels */ -void setup_board_tags(struct tag **in_params) -{

• unsigned int boardId;
• struct tag *params;
• struct tag_mv_uboot *t;
• int i;
• printf("Synology board tags...");
• params = *in_params;
• t = (struct tag_mv_uboot *)&params->u;
• t->uboot_version = VER_NUM;
• boardId = SYNO_DS109_ID;
• t->uboot_version |= boardId;
• t->tclk = CONFIG_SYS_TCLK;
• t->sysclk = CONFIG_SYS_TCLK*2;
• t->isusbhost = 1;
• for (i = 0; i < 4; i++) {

• memset(t->macaddr[i], 0, sizeof(t->macaddr[i]));
  

• t->mtu[i] = 0;
  

• }
• params->hdr.tag = ATAG_MV_UBOOT;
• params->hdr.size = tag_size(tag_mv_uboot);
• params = tag_next(params);
• *in_params = params;

-}

• #ifdef CONFIG_RESET_PHY_R /* Configure and enable MV88E1116 PHY */ void reset_phy(void)

diff --git a/board/Synology/ds109/ds109.h b/board/Synology/ds109/ds109.h index cc6ef991f3973..0cf05257c8d35 100644 --- a/board/Synology/ds109/ds109.h +++ b/board/Synology/ds109/ds109.h @@ -23,21 +23,4 @@ #define MV88E1116_RGMII_TXTM_CTRL (1 << 4) #define MV88E1116_RGMII_RXTM_CTRL (1 << 5)

-/* Marvell uboot parameters */ -#define ATAG_MV_UBOOT 0x41000403 -#define VER_NUM 0x03040400 /* 3.4.4 */ -#define BOARD_ID_BASE 0x0 -#define SYNO_DS109_ID (BOARD_ID_BASE+0x15)

-struct tag_mv_uboot {

• u32 uboot_version;
• u32 tclk;
• u32 sysclk;
• u32 isusbhost;
• char macaddr[4][6];
• u16 mtu[4];
• u32 fw_image_base;
• u32 fw_image_size;

-};

• #endif /* __DS109_H */

diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index 254e42f84d2e9..fa9366778748c 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -20,7 +20,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-synology-ds414" CONFIG_DEBUG_UART=y CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y -CONFIG_BOOTARGS="console=ttyS0,115200" +CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1" CONFIG_USE_PREBOOT=y CONFIG_PREBOOT="usb start; sf probe" # CONFIG_DISPLAY_BOARDINFO is not set diff --git a/include/configs/ds109.h b/include/configs/ds109.h index 1f033ababf6ee..35d85361b4542 100644 --- a/include/configs/ds109.h +++ b/include/configs/ds109.h @@ -44,7 +44,8 @@ "x_bootcmd_kernel=fatload usb 0 0x6400000 uImage\0" \ "x_bootargs=console=ttyS0,115200\0" \ "x_bootargs_root=root=/dev/sda2 rootdelay=10\0" \

• "ipaddr=192.168.1.5\0"

• "ipaddr=192.168.1.5\0" \

• "usb0Mode=host\0"

  /*

  • Ethernet Driver configuration

diff --git a/include/configs/ds414.h b/include/configs/ds414.h index 8aa2d47bec68d..a2248cf75ad72 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -6,6 +6,9 @@ #ifndef _CONFIG_SYNOLOGY_DS414_H #define _CONFIG_SYNOLOGY_DS414_H

+/* Vendor kernel expects this MACH_TYPE */ +#define CONFIG_MACH_TYPE 3036

• /*
  • High Level Configuration Options (easy to change)
  */

@@ -74,8 +77,18 @@ #define CONFIG_DDR_32BIT

/* Default Environment */ -#define CONFIG_BOOTCOMMAND "sf read ${loadaddr} 0xd0000 0x700000; bootm" #define CONFIG_LOADADDR 0x80000 +#define CONFIG_BOOTCOMMAND \

• "sf probe; " \
• "sf read ${loadaddr} 0xd0000 0x2d0000; " \
• "sf read ${ramdisk_addr_r} 0x3a0000 0x430000; " \
• "bootm ${loadaddr} ${ramdisk_addr_r}"

+#define CONFIG_EXTRA_ENV_SETTINGS \

• "initrd_high=0xffffffff\0" \

• "ramdisk_addr_r=0x8000000\0" \

• "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \

• "ethmtu=1500\0eth1mtu=1500\0"

  /* increase autoneg timeout, my NIC sucks */ #define PHY_ANEG_TIMEOUT 16000

Viele Grüße, Stefan

Move the relevant bits from ds109.{c,h} into common/ and adjust the code to fit both DS109 and DS414. Moreover:

* Introduce syno_board_id() which translates CONFIG_MACH_TYPE into the expected board ID tag value.

* Properly initialize isusbhost, mac and mtu fields from env variables.

* Set the right bootargs/bootcmd to correctly boot legacy kernel out of the (DS414) box. Getting the ramdisk location right is a bit tedious.

Cc: Walter Schweizer swwa@users.sourceforge.net Signed-off-by: Phil Sutter phil@nwl.cc -- Changes since v1: - Avoid deprecated common.h header include. - Remove 'sf probe' call from DS414 PREBOOT, bootcmd contains it now. - Use proper comment-style for SPDX tag in introduced legacy.h. --- board/Synology/common/Makefile | 5 +++ board/Synology/common/legacy.c | 76 ++++++++++++++++++++++++++++++++++ board/Synology/common/legacy.h | 33 +++++++++++++++ board/Synology/ds109/ds109.c | 32 -------------- board/Synology/ds109/ds109.h | 17 -------- configs/ds414_defconfig | 4 +- include/configs/ds109.h | 3 +- include/configs/ds414.h | 15 ++++++- 8 files changed, 132 insertions(+), 53 deletions(-) create mode 100644 board/Synology/common/Makefile create mode 100644 board/Synology/common/legacy.c create mode 100644 board/Synology/common/legacy.h

diff --git a/board/Synology/common/Makefile b/board/Synology/common/Makefile new file mode 100644 index 0000000000000..62354cc2e82e6 --- /dev/null +++ b/board/Synology/common/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 Phil Sutter phil@nwl.cc + +obj-y += legacy.o diff --git a/board/Synology/common/legacy.c b/board/Synology/common/legacy.c new file mode 100644 index 0000000000000..3c89e92ae7382 --- /dev/null +++ b/board/Synology/common/legacy.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 + * Walter Schweizer swwa@users.sourceforge.net + * Phil Sutter phil@nwl.cc + */ + +#include <config.h> +#include <vsprintf.h> +#include <env.h> +#include <net.h> +#include <asm/setup.h> + +#include "legacy.h" + +static unsigned int syno_board_id(void) +{ + switch (CONFIG_MACH_TYPE) { + case 527: + return SYNO_DS109_ID; + case 3036: + return SYNO_AXP_4BAY_2BAY; + default: + return 0; + } +} + +static unsigned int usb_port_modes(void) +{ + unsigned int i, ret = 0; + char var[32], *val; + + for (i = 0; i < USBPORT_MAX; i++) { + snprintf(var, 32, "usb%dMode", i); + val = env_get(var); + + if (!val || strcasecmp(val, "host")) + continue; + + ret |= 1 << i; + } + return ret; +} + +/* Support old kernels */ +void setup_board_tags(struct tag **in_params) +{ + struct tag_mv_uboot *t; + struct tag *params; + int i; + + debug("Synology board tags...\n"); + + params = *in_params; + t = (struct tag_mv_uboot *)&params->u; + + t->uboot_version = VER_NUM | syno_board_id(); + t->tclk = CONFIG_SYS_TCLK; + t->sysclk = CONFIG_SYS_TCLK * 2; + t->isusbhost = usb_port_modes(); + + for (i = 0; i < ETHADDR_MAX; i++) { + char addrvar[16], mtuvar[16]; + + sprintf(addrvar, i ? "eth%daddr" : "ethaddr", i); + sprintf(mtuvar, i ? "eth%dmtu" : "ethmtu", i); + + eth_env_get_enetaddr(addrvar, t->macaddr[i]); + t->mtu[i] = env_get_ulong(mtuvar, 10, 0); + } + + params->hdr.tag = ATAG_MV_UBOOT; + params->hdr.size = tag_size(tag_mv_uboot); + params = tag_next(params); + *in_params = params; +} diff --git a/board/Synology/common/legacy.h b/board/Synology/common/legacy.h new file mode 100644 index 0000000000000..0a814324d0977 --- /dev/null +++ b/board/Synology/common/legacy.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2021 + * Walter Schweizer swwa@users.sourceforge.net + * Phil Sutter phil@nwl.cc + */ + +#ifndef __SYNO_LEGACY_H +#define __SYNO_LEGACY_H + +/* Marvell uboot parameters */ +#define ATAG_MV_UBOOT 0x41000403 +#define VER_NUM 0x03040400 /* 3.4.4 */ + +#define BOARD_ID_BASE 0x0 +#define SYNO_DS109_ID (BOARD_ID_BASE + 0x15) +#define SYNO_AXP_4BAY_2BAY (0xf + 1) + +#define ETHADDR_MAX 4 +#define USBPORT_MAX 3 + +struct tag_mv_uboot { + u32 uboot_version; + u32 tclk; + u32 sysclk; + u32 isusbhost; + u8 macaddr[ETHADDR_MAX][ETH_ALEN]; + u16 mtu[ETHADDR_MAX]; + u32 fw_image_base; + u32 fw_image_size; +}; + +#endif /* __SYNO_LEGACY_H */ diff --git a/board/Synology/ds109/ds109.c b/board/Synology/ds109/ds109.c index eaac95460c6e1..3914faaf37bb4 100644 --- a/board/Synology/ds109/ds109.c +++ b/board/Synology/ds109/ds109.c @@ -114,38 +114,6 @@ void reset_misc(void) SOFTWARE_REBOOT); }

-/* Support old kernels */ -void setup_board_tags(struct tag **in_params) -{ - unsigned int boardId; - struct tag *params; - struct tag_mv_uboot *t; - int i; - - printf("Synology board tags..."); - params = *in_params; - t = (struct tag_mv_uboot *)&params->u; - - t->uboot_version = VER_NUM; - - boardId = SYNO_DS109_ID; - t->uboot_version |= boardId; - - t->tclk = CONFIG_SYS_TCLK; - t->sysclk = CONFIG_SYS_TCLK*2; - - t->isusbhost = 1; - for (i = 0; i < 4; i++) { - memset(t->macaddr[i], 0, sizeof(t->macaddr[i])); - t->mtu[i] = 0; - } - - params->hdr.tag = ATAG_MV_UBOOT; - params->hdr.size = tag_size(tag_mv_uboot); - params = tag_next(params); - *in_params = params; -} - #ifdef CONFIG_RESET_PHY_R /* Configure and enable MV88E1116 PHY */ void reset_phy(void) diff --git a/board/Synology/ds109/ds109.h b/board/Synology/ds109/ds109.h index cc6ef991f3973..0cf05257c8d35 100644 --- a/board/Synology/ds109/ds109.h +++ b/board/Synology/ds109/ds109.h @@ -23,21 +23,4 @@ #define MV88E1116_RGMII_TXTM_CTRL (1 << 4) #define MV88E1116_RGMII_RXTM_CTRL (1 << 5)

-/* Marvell uboot parameters */ -#define ATAG_MV_UBOOT 0x41000403 -#define VER_NUM 0x03040400 /* 3.4.4 */ -#define BOARD_ID_BASE 0x0 -#define SYNO_DS109_ID (BOARD_ID_BASE+0x15) - -struct tag_mv_uboot { - u32 uboot_version; - u32 tclk; - u32 sysclk; - u32 isusbhost; - char macaddr[4][6]; - u16 mtu[4]; - u32 fw_image_base; - u32 fw_image_size; -}; - #endif /* __DS109_H */ diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index 24466d81dffdb..4fb0abee81c58 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -20,9 +20,9 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-synology-ds414" CONFIG_DEBUG_UART=y CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y -CONFIG_BOOTARGS="console=ttyS0,115200" +CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1" CONFIG_USE_PREBOOT=y -CONFIG_PREBOOT="pci enum; usb start; sf probe" +CONFIG_PREBOOT="pci enum; usb start" # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_SPL_I2C_SUPPORT=y diff --git a/include/configs/ds109.h b/include/configs/ds109.h index 1f033ababf6ee..35d85361b4542 100644 --- a/include/configs/ds109.h +++ b/include/configs/ds109.h @@ -44,7 +44,8 @@ "x_bootcmd_kernel=fatload usb 0 0x6400000 uImage\0" \ "x_bootargs=console=ttyS0,115200\0" \ "x_bootargs_root=root=/dev/sda2 rootdelay=10\0" \ - "ipaddr=192.168.1.5\0" + "ipaddr=192.168.1.5\0" \ + "usb0Mode=host\0"

/* * Ethernet Driver configuration diff --git a/include/configs/ds414.h b/include/configs/ds414.h index 8aa2d47bec68d..a2248cf75ad72 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -6,6 +6,9 @@ #ifndef _CONFIG_SYNOLOGY_DS414_H #define _CONFIG_SYNOLOGY_DS414_H

+/* Vendor kernel expects this MACH_TYPE */ +#define CONFIG_MACH_TYPE 3036 + /* * High Level Configuration Options (easy to change) */ @@ -74,8 +77,18 @@ #define CONFIG_DDR_32BIT

/* Default Environment */ -#define CONFIG_BOOTCOMMAND "sf read ${loadaddr} 0xd0000 0x700000; bootm" #define CONFIG_LOADADDR 0x80000 +#define CONFIG_BOOTCOMMAND \ + "sf probe; " \ + "sf read ${loadaddr} 0xd0000 0x2d0000; " \ + "sf read ${ramdisk_addr_r} 0x3a0000 0x430000; " \ + "bootm ${loadaddr} ${ramdisk_addr_r}" + +#define CONFIG_EXTRA_ENV_SETTINGS \ + "initrd_high=0xffffffff\0" \ + "ramdisk_addr_r=0x8000000\0" \ + "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ + "ethmtu=1500\0eth1mtu=1500\0"

/* increase autoneg timeout, my NIC sucks */ #define PHY_ANEG_TIMEOUT 16000

On 07.03.21 22:22, Phil Sutter wrote:

Move the relevant bits from ds109.{c,h} into common/ and adjust the code to fit both DS109 and DS414. Moreover:

• Introduce syno_board_id() which translates CONFIG_MACH_TYPE into the expected board ID tag value.

• Properly initialize isusbhost, mac and mtu fields from env variables.

• Set the right bootargs/bootcmd to correctly boot legacy kernel out of the (DS414) box. Getting the ramdisk location right is a bit tedious.

Cc: Walter Schweizer swwa@users.sourceforge.net Signed-off-by: Phil Sutter phil@nwl.cc

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

-- Changes since v2:

• Drop PREBOOT entirely, 'usb init' is not needed.

Changes since v1:

• Avoid deprecated common.h header include.
• Remove 'sf probe' call from DS414 PREBOOT, bootcmd contains it now.

---

board/Synology/common/Makefile | 5 +++ board/Synology/common/legacy.c | 76 ++++++++++++++++++++++++++++++++++ board/Synology/common/legacy.h | 33 +++++++++++++++ board/Synology/ds109/ds109.c | 32 -------------- board/Synology/ds109/ds109.h | 17 -------- configs/ds414_defconfig | 5 +-- include/configs/ds109.h | 3 +- include/configs/ds414.h | 15 ++++++- 8 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 board/Synology/common/Makefile create mode 100644 board/Synology/common/legacy.c create mode 100644 board/Synology/common/legacy.h

diff --git a/board/Synology/common/Makefile b/board/Synology/common/Makefile new file mode 100644 index 0000000000000..62354cc2e82e6 --- /dev/null +++ b/board/Synology/common/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 Phil Sutter phil@nwl.cc

+obj-y += legacy.o diff --git a/board/Synology/common/legacy.c b/board/Synology/common/legacy.c new file mode 100644 index 0000000000000..3c89e92ae7382 --- /dev/null +++ b/board/Synology/common/legacy.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2021
• • Walter Schweizer swwa@users.sourceforge.net
• • Phil Sutter phil@nwl.cc
• */

+#include <config.h> +#include <vsprintf.h> +#include <env.h> +#include <net.h> +#include <asm/setup.h>

+#include "legacy.h"

+static unsigned int syno_board_id(void) +{

• switch (CONFIG_MACH_TYPE) {
• case 527:

• return SYNO_DS109_ID;
  

• case 3036:

• return SYNO_AXP_4BAY_2BAY;
  

• default:

• return 0;
  

• }

+}

+static unsigned int usb_port_modes(void) +{

• unsigned int i, ret = 0;
• char var[32], *val;
• for (i = 0; i < USBPORT_MAX; i++) {

• snprintf(var, 32, "usb%dMode", i);
  

• val = env_get(var);
  

• if (!val || strcasecmp(val, "host"))
  

• 	continue;
  

• ret |= 1 << i;
  

• }
• return ret;

+}

+/* Support old kernels */ +void setup_board_tags(struct tag **in_params) +{

• struct tag_mv_uboot *t;
• struct tag *params;
• int i;
• debug("Synology board tags...\n");
• params = *in_params;
• t = (struct tag_mv_uboot *)&params->u;
• t->uboot_version = VER_NUM | syno_board_id();
• t->tclk = CONFIG_SYS_TCLK;
• t->sysclk = CONFIG_SYS_TCLK * 2;
• t->isusbhost = usb_port_modes();
• for (i = 0; i < ETHADDR_MAX; i++) {

• char addrvar[16], mtuvar[16];
  

• sprintf(addrvar, i ? "eth%daddr" : "ethaddr", i);
  

• sprintf(mtuvar, i ? "eth%dmtu" : "ethmtu", i);
  

• eth_env_get_enetaddr(addrvar, t->macaddr[i]);
  

• t->mtu[i] = env_get_ulong(mtuvar, 10, 0);
  

• }
• params->hdr.tag = ATAG_MV_UBOOT;
• params->hdr.size = tag_size(tag_mv_uboot);
• params = tag_next(params);
• *in_params = params;

+} diff --git a/board/Synology/common/legacy.h b/board/Synology/common/legacy.h new file mode 100644 index 0000000000000..0a814324d0977 --- /dev/null +++ b/board/Synology/common/legacy.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (C) 2021
• • Walter Schweizer swwa@users.sourceforge.net
• • Phil Sutter phil@nwl.cc
• */

+#ifndef __SYNO_LEGACY_H +#define __SYNO_LEGACY_H

+/* Marvell uboot parameters */ +#define ATAG_MV_UBOOT 0x41000403 +#define VER_NUM 0x03040400 /* 3.4.4 */

+#define BOARD_ID_BASE 0x0 +#define SYNO_DS109_ID (BOARD_ID_BASE + 0x15) +#define SYNO_AXP_4BAY_2BAY (0xf + 1)

+#define ETHADDR_MAX 4 +#define USBPORT_MAX 3

+struct tag_mv_uboot {

• u32 uboot_version;
• u32 tclk;
• u32 sysclk;
• u32 isusbhost;
• u8 macaddr[ETHADDR_MAX][ETH_ALEN];
• u16 mtu[ETHADDR_MAX];
• u32 fw_image_base;
• u32 fw_image_size;

+};

+#endif /* __SYNO_LEGACY_H */ diff --git a/board/Synology/ds109/ds109.c b/board/Synology/ds109/ds109.c index eaac95460c6e1..3914faaf37bb4 100644 --- a/board/Synology/ds109/ds109.c +++ b/board/Synology/ds109/ds109.c @@ -114,38 +114,6 @@ void reset_misc(void) SOFTWARE_REBOOT); }

-/* Support old kernels */ -void setup_board_tags(struct tag **in_params) -{

• unsigned int boardId;
• struct tag *params;
• struct tag_mv_uboot *t;
• int i;
• printf("Synology board tags...");
• params = *in_params;
• t = (struct tag_mv_uboot *)&params->u;
• t->uboot_version = VER_NUM;
• boardId = SYNO_DS109_ID;
• t->uboot_version |= boardId;
• t->tclk = CONFIG_SYS_TCLK;
• t->sysclk = CONFIG_SYS_TCLK*2;
• t->isusbhost = 1;
• for (i = 0; i < 4; i++) {

• memset(t->macaddr[i], 0, sizeof(t->macaddr[i]));
  

• t->mtu[i] = 0;
  

• }
• params->hdr.tag = ATAG_MV_UBOOT;
• params->hdr.size = tag_size(tag_mv_uboot);
• params = tag_next(params);
• *in_params = params;

-}

• #ifdef CONFIG_RESET_PHY_R /* Configure and enable MV88E1116 PHY */ void reset_phy(void)

diff --git a/board/Synology/ds109/ds109.h b/board/Synology/ds109/ds109.h index cc6ef991f3973..0cf05257c8d35 100644 --- a/board/Synology/ds109/ds109.h +++ b/board/Synology/ds109/ds109.h @@ -23,21 +23,4 @@ #define MV88E1116_RGMII_TXTM_CTRL (1 << 4) #define MV88E1116_RGMII_RXTM_CTRL (1 << 5)

-/* Marvell uboot parameters */ -#define ATAG_MV_UBOOT 0x41000403 -#define VER_NUM 0x03040400 /* 3.4.4 */ -#define BOARD_ID_BASE 0x0 -#define SYNO_DS109_ID (BOARD_ID_BASE+0x15)

-struct tag_mv_uboot {

• u32 uboot_version;
• u32 tclk;
• u32 sysclk;
• u32 isusbhost;
• char macaddr[4][6];
• u16 mtu[4];
• u32 fw_image_base;
• u32 fw_image_size;

-};

• #endif /* __DS109_H */

diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index 412559256e6ca..8ef2e79dc91b0 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -20,9 +20,8 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-synology-ds414" CONFIG_DEBUG_UART=y CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y -CONFIG_BOOTARGS="console=ttyS0,115200" -CONFIG_USE_PREBOOT=y -CONFIG_PREBOOT="usb start; sf probe" +CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1" +# CONFIG_USE_PREBOOT is not set # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_SPL_I2C_SUPPORT=y diff --git a/include/configs/ds109.h b/include/configs/ds109.h index 1f033ababf6ee..35d85361b4542 100644 --- a/include/configs/ds109.h +++ b/include/configs/ds109.h @@ -44,7 +44,8 @@ "x_bootcmd_kernel=fatload usb 0 0x6400000 uImage\0" \ "x_bootargs=console=ttyS0,115200\0" \ "x_bootargs_root=root=/dev/sda2 rootdelay=10\0" \

• "ipaddr=192.168.1.5\0"

• "ipaddr=192.168.1.5\0" \

• "usb0Mode=host\0"

  /*

  • Ethernet Driver configuration

diff --git a/include/configs/ds414.h b/include/configs/ds414.h index 8aa2d47bec68d..a2248cf75ad72 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -6,6 +6,9 @@ #ifndef _CONFIG_SYNOLOGY_DS414_H #define _CONFIG_SYNOLOGY_DS414_H

+/* Vendor kernel expects this MACH_TYPE */ +#define CONFIG_MACH_TYPE 3036

• /*
  • High Level Configuration Options (easy to change)
  */

@@ -74,8 +77,18 @@ #define CONFIG_DDR_32BIT

/* Default Environment */ -#define CONFIG_BOOTCOMMAND "sf read ${loadaddr} 0xd0000 0x700000; bootm" #define CONFIG_LOADADDR 0x80000 +#define CONFIG_BOOTCOMMAND \

• "sf probe; " \
• "sf read ${loadaddr} 0xd0000 0x2d0000; " \
• "sf read ${ramdisk_addr_r} 0x3a0000 0x430000; " \
• "bootm ${loadaddr} ${ramdisk_addr_r}"

+#define CONFIG_EXTRA_ENV_SETTINGS \

• "initrd_high=0xffffffff\0" \

• "ramdisk_addr_r=0x8000000\0" \

• "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \

• "ethmtu=1500\0eth1mtu=1500\0"

  /* increase autoneg timeout, my NIC sucks */ #define PHY_ANEG_TIMEOUT 16000

Viele Grüße, Stefan

On 03.03.21 01:55, Phil Sutter wrote:

Define a misc_init_r() which calls "syno populate_env" if the environment seems incomplete (or default), indicated by missing "ethaddr" variable. With this in place, no random MAC address fallback is needed anymore.

Signed-off-by: Phil Sutter phil@nwl.cc

board/Synology/ds414/cmd_syno.c | 2 +- board/Synology/ds414/ds414.c | 13 +++++++++++++ configs/ds414_defconfig | 4 +--- 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c index a120c3123ffb3..07bb395da3acc 100644 --- a/board/Synology/ds414/cmd_syno.c +++ b/board/Synology/ds414/cmd_syno.c @@ -22,7 +22,7 @@ #define SYNO_CHKSUM_TAG "CHK="

-static int do_syno_populate(int argc, char *const argv[]) +int do_syno_populate(int argc, char *const argv[]) { unsigned int bus = CONFIG_SF_DEFAULT_BUS; unsigned int cs = CONFIG_SF_DEFAULT_CS; diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c index 9c4ce670ddfbd..c2469d6665255 100644 --- a/board/Synology/ds414/ds414.c +++ b/board/Synology/ds414/ds414.c @@ -179,6 +179,19 @@ int board_init(void) return 0; }

+#ifndef CONFIG_SPL_BUILD +int do_syno_populate(int argc, char *const argv[]);

I suspect that this prototype in a C file will trigger at least a checkpatch warning?

Other than that:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

+int misc_init_r(void) +{

• if (!env_get("ethaddr")) {

• puts("Incomplete environment, populating from SPI flash\n");
  

• do_syno_populate(0, NULL);
  

• }
• return 0;

+} +#endif

• int checkboard(void) { puts("Board: DS414\n");

diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index fa9366778748c..948b22f3d1f66 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -21,10 +21,9 @@ CONFIG_DEBUG_UART=y CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1" -CONFIG_USE_PREBOOT=y -CONFIG_PREBOOT="usb start; sf probe" # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_MISC_INIT_R=y CONFIG_SPL_I2C_SUPPORT=y # CONFIG_CMD_FLASH is not set CONFIG_CMD_I2C=y @@ -47,7 +46,6 @@ CONFIG_ENV_OVERWRITE=y CONFIG_USE_ENV_SPI_MAX_HZ=y CONFIG_ENV_SPI_MAX_HZ=50000000 CONFIG_SYS_RELOC_GD_ENV_ADDR=y -CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_OF_TRANSLATE=y CONFIG_BLK=y # CONFIG_MMC is not set

Viele Grüße, Stefan

On 04.03.21 14:20, Phil Sutter wrote:

On Thu, Mar 04, 2021 at 02:06:01PM +0100, Stefan Roese wrote:

...

On 03.03.21 01:55, Phil Sutter wrote:

...

Define a misc_init_r() which calls "syno populate_env" if the environment seems incomplete (or default), indicated by missing "ethaddr" variable. With this in place, no random MAC address fallback is needed anymore.

Signed-off-by: Phil Sutter phil@nwl.cc

board/Synology/ds414/cmd_syno.c | 2 +- board/Synology/ds414/ds414.c | 13 +++++++++++++ configs/ds414_defconfig | 4 +--- 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c index a120c3123ffb3..07bb395da3acc 100644 --- a/board/Synology/ds414/cmd_syno.c +++ b/board/Synology/ds414/cmd_syno.c @@ -22,7 +22,7 @@ #define SYNO_CHKSUM_TAG "CHK="

-static int do_syno_populate(int argc, char *const argv[]) +int do_syno_populate(int argc, char *const argv[]) { unsigned int bus = CONFIG_SF_DEFAULT_BUS; unsigned int cs = CONFIG_SF_DEFAULT_CS; diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c index 9c4ce670ddfbd..c2469d6665255 100644 --- a/board/Synology/ds414/ds414.c +++ b/board/Synology/ds414/ds414.c @@ -179,6 +179,19 @@ int board_init(void) return 0; }

+#ifndef CONFIG_SPL_BUILD +int do_syno_populate(int argc, char *const argv[]);

I suspect that this prototype in a C file will trigger at least a checkpatch warning?

What is checkpatch? ;)

Provided with Linux and your favorite bootloader: ;)

./scripts/checkpatch.pl

Mea culpa, sometimes past me's dirty hacks slip my review. I'll go find a checkpatch-compliant way.

NP. Thanks for working on this.

Thanks, Stefan

On Thu, Mar 04, 2021 at 02:09:34PM +0100, Stefan Roese wrote:

On 03.03.21 01:55, Phil Sutter wrote:

...

Signed-off-by: Phil Sutter phil@nwl.cc

It's common practice to add some minimal text in the commit text, even for simple patches.

I should learn to put less meaningful subjects in those cases so I have an easier time finding something to write about in the description. Then again, your question indicates I just didn't think hard enough.

[...]

...

diff --git a/include/configs/ds414.h b/include/configs/ds414.h index a2248cf75ad72..c8b45066cc75a 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -88,7 +88,12 @@ "initrd_high=0xffffffff\0" \ "ramdisk_addr_r=0x8000000\0" \ "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \

• "ethmtu=1500\0eth1mtu=1500\0"

• "ethmtu=1500\0eth1mtu=1500\0" \
• "update_uboot=sf probe; dhcp; " \

• "mw.b ${loadaddr} 0x0 0xd0000; "		\
  

• "tftpboot ${loadaddr} u-boot-spl.kwb; "		\
  

• "sf update ${loadaddr} 0x0 0xd0000\0"
  

Wouldn't it be better to use ${filesize} instead of 0xd0000 here?

U-Boot's "partition" size is fixed and hard-coded in the DTB at least. Putting the destination size as a limit at least avoids the unlikely chance of overwriting data past that partition.

BTW: I spent a while trying to make the DTB-defined partitions available in sf command, but eventually gave up. Seems I need to have this SPI->MTD mapping and still define partitions in environment. Not worth spending cycles on though, I was just curious because 'help sf' mentions partitions as offset alternatives.

Cheers, Phil