Re: [PATCH] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board
Hi, On Sun, Jun 09, 2024 at 01:51:34PM -0700, Tony Dinh wrote: > On Sat, Jun 8, 2024 at 4:32 PM Phil Sutter wrote: > > On Fri, Jun 07, 2024 at 04:14:26PM -0700, Tony Dinh wrote: > > > On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter wrote: > > > > On Thu, Jun 06, 2024 at 04:44:36PM -0700, Tony Dinh wrote: > > > > > - Switch to standard boot (in include/configs/ds414.h and > > > > > configs/ds414_defconfig) > > > > > - Implement board_late_init() to ensure successful enumeration > > > > > of USB3 devices > > > > > > > > Oh, this makes the rear USB3 ports usable? I had failed to get there > > > > when working on DS414 support. > > > > > > Yes! There have been a lot of updates in the PCIe MVEBU area. So it > > > could have been a different reason why it did not work when you added > > > support for this board. > > > > Nice! > > > > > Still, without this board_late_init(), we would need to run "pci enum" > > > and then "usb start" or "usb reset" to bring up the USB3 ports. The > > > USB3 controller is on the PCIe bus, and MVEBU PCIe drivers don't > > > initialize everything when it comes up. > > > > Yeah, at some point I still had > > | CONFIG_PREBOOT="usb start; sf probe" > > in ds414_defconfig. Looking at my initial DS414 support patch from 2015, > > at least 'usb start' seemed to be necessary for USB to work in Linux. > > This might have changed later as I dropped the config symbol six years > > later in e471ddf0f3472 ("arm: mvebu: board/Synology: Unify legacy kernel > > support"). > > > > > > > - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now > > > > > configured in > > > > > > > > So u-boot will assign random MAC addresses to the NICs instead of the > > > > real ones? Won't this potentially break PXE boot setups? > > > > > > I did not see any problem using a random MAC address in my test using > > > bootstd. Is there a setup where this board MAC address must be known > > > when we boot with PXE? my impression is that only ipaddr and serverip > > > are needed, and the MAC address could be whatever we define for the > > > board. > > > > I haven't played with PXE for a long time, but I assumed the DHCP daemon > > might send different options to different hosts identified by their MAC > > address. The latter being random obviously defeats such setups. > > > > Does the random MAC persist in Linux, BTW? I don't recall Linux > > extracting data from the vendor-specific flash storage. Unless I'm > > mistaken, it should be u-boot's responsibility to setup MAC addresses in > > the NICs (to the vendor-assigned ones). > > Yes, it persists. Whatever was assigned to ethaddr and eth1addr are > passed to the Linux kernel. So the random local MAC address is > recognized as the hardware MAC address. Thanks for confirming. I'll try board_eth_init() in one of the next evenings - my DS414 needs a revamp anyway, the rootfs on that bloody USB stick I boot from keeps disintegrating and the box is FUBAR ATM. > As an aside, there is a bug in bootstd that "bootflow scan -l" will go > into a seemingly infinite loop during PXE hunting if eth1addr is not > defined. I've not figured out how to fix this error recovery. I might > just give up and send the bug report to Simon :) Fixing the bug as well as making sure eth1addr is properly set should not hurt. :) > > > > > - Remove unnecessary checkboard() > > > > > - Updated IDENT_STRING to indicate this u-boot supports both Synology > > > > > DS414 and DS214+ boards > > > > > - Add SYS_THUMB_BUILD to reduce binary size > > > > > - Add NET_RANDOM_ETHADDR > > > > > - Add CONFIG_LBA48 and CONFIG_SYS_64BIT_LBA to support >2TB HDD/SDD > > > > > > > > > > Signed-off-by: Tony Dinh > > > > > --- > > > > > > > > > > board/Synology/ds414/ds414.c | 15 +++ > > > > > configs/ds414_defconfig | 20 ++ > > > > > include/configs/ds414.h | 51 > > > > > ++-- > > > > > 3 files changed, 42 insertions(+), 44 deletions(-) > > > > > > > > > > diff --git a/board/Synology/ds414/ds414.c > > > > > b/board/Synology/ds414/ds414.c > > > > > index abe6f9eb5e..f0b55fa095 100644 > > > > > --- a/board/Synology/ds414/ds414.c > > > > > +++ b/board/Synology/ds414/ds414.c > > > > > @@ -181,18 +181,9 @@ int board_init(void) > > > > > return 0; > > > > > } > > > > > > > > > > -int misc_init_r(void) > > > > > +int board_late_init(void) > > > > > { > > > > > - if (!env_get("ethaddr")) { > > > > > - puts("Incomplete environment, populating from SPI > > > > > flash\n"); > > > > > - do_syno_populate(0, NULL); > > > > > - } > > > > > - return 0; > > > > > -} > > > > > > > > This is useful for first boot of flashed vanilla u-boot. Can this be > > > > retained somehow? > > > > > > We could, but with NET_RANDOM_ETHADDR configured in, this routine will > > > do nothing. Perhaps the fact that the envs will show CRC errors in the > > > 1st boot should be an indicator that it needs to be populated? > > > > Mayb
Re: [PATCH] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board
Hi Phil, On Sat, Jun 8, 2024 at 4:32 PM Phil Sutter wrote: > > On Fri, Jun 07, 2024 at 04:14:26PM -0700, Tony Dinh wrote: > > On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter wrote: > > > On Thu, Jun 06, 2024 at 04:44:36PM -0700, Tony Dinh wrote: > > > > - Switch to standard boot (in include/configs/ds414.h and > > > > configs/ds414_defconfig) > > > > - Implement board_late_init() to ensure successful enumeration > > > > of USB3 devices > > > > > > Oh, this makes the rear USB3 ports usable? I had failed to get there > > > when working on DS414 support. > > > > Yes! There have been a lot of updates in the PCIe MVEBU area. So it > > could have been a different reason why it did not work when you added > > support for this board. > > Nice! > > > Still, without this board_late_init(), we would need to run "pci enum" > > and then "usb start" or "usb reset" to bring up the USB3 ports. The > > USB3 controller is on the PCIe bus, and MVEBU PCIe drivers don't > > initialize everything when it comes up. > > Yeah, at some point I still had > | CONFIG_PREBOOT="usb start; sf probe" > in ds414_defconfig. Looking at my initial DS414 support patch from 2015, > at least 'usb start' seemed to be necessary for USB to work in Linux. > This might have changed later as I dropped the config symbol six years > later in e471ddf0f3472 ("arm: mvebu: board/Synology: Unify legacy kernel > support"). > > > > > - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now > > > > configured in > > > > > > So u-boot will assign random MAC addresses to the NICs instead of the > > > real ones? Won't this potentially break PXE boot setups? > > > > I did not see any problem using a random MAC address in my test using > > bootstd. Is there a setup where this board MAC address must be known > > when we boot with PXE? my impression is that only ipaddr and serverip > > are needed, and the MAC address could be whatever we define for the > > board. > > I haven't played with PXE for a long time, but I assumed the DHCP daemon > might send different options to different hosts identified by their MAC > address. The latter being random obviously defeats such setups. > > Does the random MAC persist in Linux, BTW? I don't recall Linux > extracting data from the vendor-specific flash storage. Unless I'm > mistaken, it should be u-boot's responsibility to setup MAC addresses in > the NICs (to the vendor-assigned ones). Yes, it persists. Whatever was assigned to ethaddr and eth1addr are passed to the Linux kernel. So the random local MAC address is recognized as the hardware MAC address. As an aside, there is a bug in bootstd that "bootflow scan -l" will go into a seemingly infinite loop during PXE hunting if eth1addr is not defined. I've not figured out how to fix this error recovery. I might just give up and send the bug report to Simon :) > > > > > - Remove unnecessary checkboard() > > > > - Updated IDENT_STRING to indicate this u-boot supports both Synology > > > > DS414 and DS214+ boards > > > > - Add SYS_THUMB_BUILD to reduce binary size > > > > - Add NET_RANDOM_ETHADDR > > > > - Add CONFIG_LBA48 and CONFIG_SYS_64BIT_LBA to support >2TB HDD/SDD > > > > > > > > Signed-off-by: Tony Dinh > > > > --- > > > > > > > > board/Synology/ds414/ds414.c | 15 +++ > > > > configs/ds414_defconfig | 20 ++ > > > > include/configs/ds414.h | 51 ++-- > > > > 3 files changed, 42 insertions(+), 44 deletions(-) > > > > > > > > diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c > > > > index abe6f9eb5e..f0b55fa095 100644 > > > > --- a/board/Synology/ds414/ds414.c > > > > +++ b/board/Synology/ds414/ds414.c > > > > @@ -181,18 +181,9 @@ int board_init(void) > > > > return 0; > > > > } > > > > > > > > -int misc_init_r(void) > > > > +int board_late_init(void) > > > > { > > > > - if (!env_get("ethaddr")) { > > > > - puts("Incomplete environment, populating from SPI > > > > flash\n"); > > > > - do_syno_populate(0, NULL); > > > > - } > > > > - return 0; > > > > -} > > > > > > This is useful for first boot of flashed vanilla u-boot. Can this be > > > retained somehow? > > > > We could, but with NET_RANDOM_ETHADDR configured in, this routine will > > do nothing. Perhaps the fact that the envs will show CRC errors in the > > 1st boot should be an indicator that it needs to be populated? > > Maybe this env population approach is not the right one to begin with. > Reading doc/README.enetaddr, it seems we want something like step 1 in > "Correct flow of setting up the MAC address", though I'm not sure which > initialize() function this is about (board_eth_init() maybe?). Also we > need a working spi flash driver at that point. > > > With standard boot enabled in this patch, the default envs can be > > scripted in /boot/boot.scr or /boot.scr on the rootfs disk storage, or > > on some flash location, or on the network . So we don't even need the > > full
Re: [PATCH] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board
On Fri, Jun 07, 2024 at 04:14:26PM -0700, Tony Dinh wrote: > On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter wrote: > > On Thu, Jun 06, 2024 at 04:44:36PM -0700, Tony Dinh wrote: > > > - Switch to standard boot (in include/configs/ds414.h and > > > configs/ds414_defconfig) > > > - Implement board_late_init() to ensure successful enumeration > > > of USB3 devices > > > > Oh, this makes the rear USB3 ports usable? I had failed to get there > > when working on DS414 support. > > Yes! There have been a lot of updates in the PCIe MVEBU area. So it > could have been a different reason why it did not work when you added > support for this board. Nice! > Still, without this board_late_init(), we would need to run "pci enum" > and then "usb start" or "usb reset" to bring up the USB3 ports. The > USB3 controller is on the PCIe bus, and MVEBU PCIe drivers don't > initialize everything when it comes up. Yeah, at some point I still had | CONFIG_PREBOOT="usb start; sf probe" in ds414_defconfig. Looking at my initial DS414 support patch from 2015, at least 'usb start' seemed to be necessary for USB to work in Linux. This might have changed later as I dropped the config symbol six years later in e471ddf0f3472 ("arm: mvebu: board/Synology: Unify legacy kernel support"). > > > - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now > > > configured in > > > > So u-boot will assign random MAC addresses to the NICs instead of the > > real ones? Won't this potentially break PXE boot setups? > > I did not see any problem using a random MAC address in my test using > bootstd. Is there a setup where this board MAC address must be known > when we boot with PXE? my impression is that only ipaddr and serverip > are needed, and the MAC address could be whatever we define for the > board. I haven't played with PXE for a long time, but I assumed the DHCP daemon might send different options to different hosts identified by their MAC address. The latter being random obviously defeats such setups. Does the random MAC persist in Linux, BTW? I don't recall Linux extracting data from the vendor-specific flash storage. Unless I'm mistaken, it should be u-boot's responsibility to setup MAC addresses in the NICs (to the vendor-assigned ones). > > > - Remove unnecessary checkboard() > > > - Updated IDENT_STRING to indicate this u-boot supports both Synology > > > DS414 and DS214+ boards > > > - Add SYS_THUMB_BUILD to reduce binary size > > > - Add NET_RANDOM_ETHADDR > > > - Add CONFIG_LBA48 and CONFIG_SYS_64BIT_LBA to support >2TB HDD/SDD > > > > > > Signed-off-by: Tony Dinh > > > --- > > > > > > board/Synology/ds414/ds414.c | 15 +++ > > > configs/ds414_defconfig | 20 ++ > > > include/configs/ds414.h | 51 ++-- > > > 3 files changed, 42 insertions(+), 44 deletions(-) > > > > > > diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c > > > index abe6f9eb5e..f0b55fa095 100644 > > > --- a/board/Synology/ds414/ds414.c > > > +++ b/board/Synology/ds414/ds414.c > > > @@ -181,18 +181,9 @@ int board_init(void) > > > return 0; > > > } > > > > > > -int misc_init_r(void) > > > +int board_late_init(void) > > > { > > > - if (!env_get("ethaddr")) { > > > - puts("Incomplete environment, populating from SPI flash\n"); > > > - do_syno_populate(0, NULL); > > > - } > > > - return 0; > > > -} > > > > This is useful for first boot of flashed vanilla u-boot. Can this be > > retained somehow? > > We could, but with NET_RANDOM_ETHADDR configured in, this routine will > do nothing. Perhaps the fact that the envs will show CRC errors in the > 1st boot should be an indicator that it needs to be populated? Maybe this env population approach is not the right one to begin with. Reading doc/README.enetaddr, it seems we want something like step 1 in "Correct flow of setting up the MAC address", though I'm not sure which initialize() function this is about (board_eth_init() maybe?). Also we need a working spi flash driver at that point. > With standard boot enabled in this patch, the default envs can be > scripted in /boot/boot.scr or /boot.scr on the rootfs disk storage, or > on some flash location, or on the network . So we don't even need the > full envs on SPI flash like we used to. It might still be useful but > not necessary. Maybe one could implement a translator which presents the 'vendor' partition contents as such default boot env. > > [...] > > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > > index 9446acba79..bbefe632e0 100644 > > > --- a/include/configs/ds414.h > > > +++ b/include/configs/ds414.h > > [...] > > > @@ -38,23 +34,38 @@ > > > * L2 cache thus cannot be used. > > > */ > > > > > > -/* SPL */ > > > -/* Defines for SPL */ > > > +/* Keep device tree and initrd in lower memory so the kernel can access > > > them */ > > > +#define RELOCATION_LIMITS_ENV_SETTINGS \ > > > + "fdt_high=0x
Re: [PATCH] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board
Hi Phil, On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter wrote: > > Hi Tony, > > On Thu, Jun 06, 2024 at 04:44:36PM -0700, Tony Dinh wrote: > > - Switch to standard boot (in include/configs/ds414.h and > > configs/ds414_defconfig) > > - Implement board_late_init() to ensure successful enumeration > > of USB3 devices > > Oh, this makes the rear USB3 ports usable? I had failed to get there > when working on DS414 support. Yes! There have been a lot of updates in the PCIe MVEBU area. So it could have been a different reason why it did not work when you added support for this board. Still, without this board_late_init(), we would need to run "pci enum" and then "usb start" or "usb reset" to bring up the USB3 ports. The USB3 controller is on the PCIe bus, and MVEBU PCIe drivers don't initialize everything when it comes up. > > > - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now > > configured in > > So u-boot will assign random MAC addresses to the NICs instead of the > real ones? Won't this potentially break PXE boot setups? I did not see any problem using a random MAC address in my test using bootstd. Is there a setup where this board MAC address must be known when we boot with PXE? my impression is that only ipaddr and serverip are needed, and the MAC address could be whatever we define for the board. > > > - Remove unnecessary checkboard() > > - Updated IDENT_STRING to indicate this u-boot supports both Synology > > DS414 and DS214+ boards > > - Add SYS_THUMB_BUILD to reduce binary size > > - Add NET_RANDOM_ETHADDR > > - Add CONFIG_LBA48 and CONFIG_SYS_64BIT_LBA to support >2TB HDD/SDD > > > > Signed-off-by: Tony Dinh > > --- > > > > board/Synology/ds414/ds414.c | 15 +++ > > configs/ds414_defconfig | 20 ++ > > include/configs/ds414.h | 51 ++-- > > 3 files changed, 42 insertions(+), 44 deletions(-) > > > > diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c > > index abe6f9eb5e..f0b55fa095 100644 > > --- a/board/Synology/ds414/ds414.c > > +++ b/board/Synology/ds414/ds414.c > > @@ -181,18 +181,9 @@ int board_init(void) > > return 0; > > } > > > > -int misc_init_r(void) > > +int board_late_init(void) > > { > > - if (!env_get("ethaddr")) { > > - puts("Incomplete environment, populating from SPI flash\n"); > > - do_syno_populate(0, NULL); > > - } > > - return 0; > > -} > > This is useful for first boot of flashed vanilla u-boot. Can this be > retained somehow? We could, but with NET_RANDOM_ETHADDR configured in, this routine will do nothing. Perhaps the fact that the envs will show CRC errors in the 1st boot should be an indicator that it needs to be populated? With standard boot enabled in this patch, the default envs can be scripted in /boot/boot.scr or /boot.scr on the rootfs disk storage, or on some flash location, or on the network . So we don't even need the full envs on SPI flash like we used to. It might still be useful but not necessary. > > [...] > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > index 9446acba79..bbefe632e0 100644 > > --- a/include/configs/ds414.h > > +++ b/include/configs/ds414.h > [...] > > @@ -38,23 +34,38 @@ > > * L2 cache thus cannot be used. > > */ > > > > -/* SPL */ > > -/* Defines for SPL */ > > +/* Keep device tree and initrd in lower memory so the kernel can access > > them */ > > +#define RELOCATION_LIMITS_ENV_SETTINGS \ > > + "fdt_high=0x1000\0" \ > > + "initrd_high=0x1000\0" > > + > > +/* > > + * mv-common.h should be defined after CMD configs since it used them > > + * to enable certain macros > > + */ > > +#include "mv-common.h" > > + > > +#ifndef CONFIG_SPL_BUILD > > > > -/* Default Environment */ > > +#define KERNEL_ADDR_R__stringify(0x100) > > +#define FDT_ADDR_R __stringify(0x200) > > +#define RAMDISK_ADDR_R __stringify(0x220) > > +#define SCRIPT_ADDR_R__stringify(0x180) > > +#define PXEFILE_ADDR_R __stringify(0x190) > > > > -#define CFG_EXTRA_ENV_SETTINGS \ > > - "initrd_high=0x\0" \ > > - "ramdisk_addr_r=0x800\0"\ > > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > > - "ethmtu=1500\0eth1mtu=1500\0" \ > > - "update_uboot=sf probe; dhcp; " \ > > - "mw.b ${loadaddr} 0x0 0xd; "\ > > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; "\ > > - "sf update ${loadaddr} 0x0 0xd\0" > > Are these dropped or am I missing a generic spot which adds them? IIRC, > the usb*mode and eth*mtu settings are required, is that wrong? Lastly, MTU is default to 1500 in mvneta.c. So the eth*mtu env is no longer necessary. Also, I believe usb*mode is no longer necessary with the USB driver model. I have
Re: [PATCH] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board
Hi Tony, On Thu, Jun 06, 2024 at 04:44:36PM -0700, Tony Dinh wrote: > - Switch to standard boot (in include/configs/ds414.h and > configs/ds414_defconfig) > - Implement board_late_init() to ensure successful enumeration > of USB3 devices Oh, this makes the rear USB3 ports usable? I had failed to get there when working on DS414 support. > - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now > configured in So u-boot will assign random MAC addresses to the NICs instead of the real ones? Won't this potentially break PXE boot setups? > - Remove unnecessary checkboard() > - Updated IDENT_STRING to indicate this u-boot supports both Synology > DS414 and DS214+ boards > - Add SYS_THUMB_BUILD to reduce binary size > - Add NET_RANDOM_ETHADDR > - Add CONFIG_LBA48 and CONFIG_SYS_64BIT_LBA to support >2TB HDD/SDD > > Signed-off-by: Tony Dinh > --- > > board/Synology/ds414/ds414.c | 15 +++ > configs/ds414_defconfig | 20 ++ > include/configs/ds414.h | 51 ++-- > 3 files changed, 42 insertions(+), 44 deletions(-) > > diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c > index abe6f9eb5e..f0b55fa095 100644 > --- a/board/Synology/ds414/ds414.c > +++ b/board/Synology/ds414/ds414.c > @@ -181,18 +181,9 @@ int board_init(void) > return 0; > } > > -int misc_init_r(void) > +int board_late_init(void) > { > - if (!env_get("ethaddr")) { > - puts("Incomplete environment, populating from SPI flash\n"); > - do_syno_populate(0, NULL); > - } > - return 0; > -} This is useful for first boot of flashed vanilla u-boot. Can this be retained somehow? [...] > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > index 9446acba79..bbefe632e0 100644 > --- a/include/configs/ds414.h > +++ b/include/configs/ds414.h [...] > @@ -38,23 +34,38 @@ > * L2 cache thus cannot be used. > */ > > -/* SPL */ > -/* Defines for SPL */ > +/* Keep device tree and initrd in lower memory so the kernel can access them > */ > +#define RELOCATION_LIMITS_ENV_SETTINGS \ > + "fdt_high=0x1000\0" \ > + "initrd_high=0x1000\0" > + > +/* > + * mv-common.h should be defined after CMD configs since it used them > + * to enable certain macros > + */ > +#include "mv-common.h" > + > +#ifndef CONFIG_SPL_BUILD > > -/* Default Environment */ > +#define KERNEL_ADDR_R__stringify(0x100) > +#define FDT_ADDR_R __stringify(0x200) > +#define RAMDISK_ADDR_R __stringify(0x220) > +#define SCRIPT_ADDR_R__stringify(0x180) > +#define PXEFILE_ADDR_R __stringify(0x190) > > -#define CFG_EXTRA_ENV_SETTINGS \ > - "initrd_high=0x\0" \ > - "ramdisk_addr_r=0x800\0"\ > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > - "ethmtu=1500\0eth1mtu=1500\0" \ > - "update_uboot=sf probe; dhcp; " \ > - "mw.b ${loadaddr} 0x0 0xd; "\ > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; "\ > - "sf update ${loadaddr} 0x0 0xd\0" Are these dropped or am I missing a generic spot which adds them? IIRC, the usb*mode and eth*mtu settings are required, is that wrong? Lastly, that update_uboot macro at least helps preventing people from writing into wrong spots. What's the cause for dropping it? Thanks, Phil