| Message ID | 20240606234437.24549-1-mibodhi@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board | expand |
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 <mibodhi@gmail.com> > --- > > 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=0x10000000\0" \ > + "initrd_high=0x10000000\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(0x1000000) > +#define FDT_ADDR_R __stringify(0x2000000) > +#define RAMDISK_ADDR_R __stringify(0x2200000) > +#define SCRIPT_ADDR_R __stringify(0x1800000) > +#define PXEFILE_ADDR_R __stringify(0x1900000) > > -#define CFG_EXTRA_ENV_SETTINGS \ > - "initrd_high=0xffffffff\0" \ > - "ramdisk_addr_r=0x8000000\0" \ > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > - "ethmtu=1500\0eth1mtu=1500\0" \ > - "update_uboot=sf probe; dhcp; " \ > - "mw.b ${loadaddr} 0x0 0xd0000; " \ > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ > - "sf update ${loadaddr} 0x0 0xd0000\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
Hi Phil, On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter <phil@nwl.cc> 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 <mibodhi@gmail.com> > > --- > > > > 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=0x10000000\0" \ > > + "initrd_high=0x10000000\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(0x1000000) > > +#define FDT_ADDR_R __stringify(0x2000000) > > +#define RAMDISK_ADDR_R __stringify(0x2200000) > > +#define SCRIPT_ADDR_R __stringify(0x1800000) > > +#define PXEFILE_ADDR_R __stringify(0x1900000) > > > > -#define CFG_EXTRA_ENV_SETTINGS \ > > - "initrd_high=0xffffffff\0" \ > > - "ramdisk_addr_r=0x8000000\0" \ > > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > > - "ethmtu=1500\0eth1mtu=1500\0" \ > > - "update_uboot=sf probe; dhcp; " \ > > - "mw.b ${loadaddr} 0x0 0xd0000; " \ > > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ > > - "sf update ${loadaddr} 0x0 0xd0000\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 not come across their usage anywhere for a long time. The USB driver model seems to handle everything pretty well. > that update_uboot macro at least helps preventing people from writing > into wrong spots. What's the cause for dropping it? I think we usually use mtdparts to define where u-boot is. So users know where the right place to flash is. But you are right, it only helps to describe exactly how to flash a new u-boot. So I'll put that back in the V2 patch. Thanks for taking time to review the patch. And I'm glad to see you are still active with the u-boot community! All the best, Tony > > Thanks, Phil
On Fri, Jun 07, 2024 at 04:14:26PM -0700, Tony Dinh wrote: > On Fri, Jun 7, 2024 at 6:45 AM Phil Sutter <phil@nwl.cc> 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 <mibodhi@gmail.com> > > > --- > > > > > > 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=0x10000000\0" \ > > > + "initrd_high=0x10000000\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(0x1000000) > > > +#define FDT_ADDR_R __stringify(0x2000000) > > > +#define RAMDISK_ADDR_R __stringify(0x2200000) > > > +#define SCRIPT_ADDR_R __stringify(0x1800000) > > > +#define PXEFILE_ADDR_R __stringify(0x1900000) > > > > > > -#define CFG_EXTRA_ENV_SETTINGS \ > > > - "initrd_high=0xffffffff\0" \ > > > - "ramdisk_addr_r=0x8000000\0" \ > > > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > > > - "ethmtu=1500\0eth1mtu=1500\0" \ > > > - "update_uboot=sf probe; dhcp; " \ > > > - "mw.b ${loadaddr} 0x0 0xd0000; " \ > > > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ > > > - "sf update ${loadaddr} 0x0 0xd0000\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 not come across their usage anywhere for a long time. > The USB driver model seems to handle everything pretty well. I think both are required for stock Synology Linux, see setup_board_tags() in board/Synology/common/legacy.c. One could just hard-code sane fallback values into that routine though. > > that update_uboot macro at least helps preventing people from writing > > into wrong spots. What's the cause for dropping it? > > I think we usually use mtdparts to define where u-boot is. So users > know where the right place to flash is. But you are right, it only > helps to describe exactly how to flash a new u-boot. So I'll put that > back in the V2 patch. Cool, thanks! > Thanks for taking time to review the patch. And I'm glad to see you > are still active with the u-boot community! My DS414 is still alive (and running in production most of the time), so I benefit from improved u-boot support as well! :) Cheers, Phil
Hi Phil, On Sat, Jun 8, 2024 at 4:32 PM Phil Sutter <phil@nwl.cc> 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 <phil@nwl.cc> 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 <mibodhi@gmail.com> > > > > --- > > > > > > > > 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. Are you still using the stock Linux kernel at all? > > > > [...] > > > > 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=0x10000000\0" \ > > > > + "initrd_high=0x10000000\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(0x1000000) > > > > +#define FDT_ADDR_R __stringify(0x2000000) > > > > +#define RAMDISK_ADDR_R __stringify(0x2200000) > > > > +#define SCRIPT_ADDR_R __stringify(0x1800000) > > > > +#define PXEFILE_ADDR_R __stringify(0x1900000) > > > > > > > > -#define CFG_EXTRA_ENV_SETTINGS \ > > > > - "initrd_high=0xffffffff\0" \ > > > > - "ramdisk_addr_r=0x8000000\0" \ > > > > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > > > > - "ethmtu=1500\0eth1mtu=1500\0" \ > > > > - "update_uboot=sf probe; dhcp; " \ > > > > - "mw.b ${loadaddr} 0x0 0xd0000; " \ > > > > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ > > > > - "sf update ${loadaddr} 0x0 0xd0000\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 not come across their usage anywhere for a long time. > > The USB driver model seems to handle everything pretty well. > > I think both are required for stock Synology Linux, see > setup_board_tags() in board/Synology/common/legacy.c. One could just > hard-code sane fallback values into that routine though. I see. Now I realized it's my bad that I did not consider stock OS at all! The purpose of this patch is to get u-boot to use bootstd to boot modern distros using a distro boot script. Should I put back these envs "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" "ethmtu=1500\0eth1mtu=1500\0" so we can support stock FW? > > > > that update_uboot macro at least helps preventing people from writing > > > into wrong spots. What's the cause for dropping it? > > > > I think we usually use mtdparts to define where u-boot is. So users > > know where the right place to flash is. But you are right, it only > > helps to describe exactly how to flash a new u-boot. So I'll put that > > back in the V2 patch. > > Cool, thanks! > > > Thanks for taking time to review the patch. And I'm glad to see you > > are still active with the u-boot community! > > My DS414 is still alive (and running in production most of the time), so > I benefit from improved u-boot support as well! :) Cool! All the best, Tony > > Cheers, Phil
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 <phil@nwl.cc> 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 <phil@nwl.cc> 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 <mibodhi@gmail.com> > > > > > --- > > > > > > > > > > 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. > > Are you still using the stock Linux kernel at all? No, but I still have it on flash. When working on vanilla u-boot, being able to boot into stock OS and having it appear to work fine was a criteria for me. Also it is nice if other users are able to replace the crappy Synology u-boot without being forced to also boot a custom OS. > > > > [...] > > > > > 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=0x10000000\0" \ > > > > > + "initrd_high=0x10000000\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(0x1000000) > > > > > +#define FDT_ADDR_R __stringify(0x2000000) > > > > > +#define RAMDISK_ADDR_R __stringify(0x2200000) > > > > > +#define SCRIPT_ADDR_R __stringify(0x1800000) > > > > > +#define PXEFILE_ADDR_R __stringify(0x1900000) > > > > > > > > > > -#define CFG_EXTRA_ENV_SETTINGS \ > > > > > - "initrd_high=0xffffffff\0" \ > > > > > - "ramdisk_addr_r=0x8000000\0" \ > > > > > - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ > > > > > - "ethmtu=1500\0eth1mtu=1500\0" \ > > > > > - "update_uboot=sf probe; dhcp; " \ > > > > > - "mw.b ${loadaddr} 0x0 0xd0000; " \ > > > > > - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ > > > > > - "sf update ${loadaddr} 0x0 0xd0000\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 not come across their usage anywhere for a long time. > > > The USB driver model seems to handle everything pretty well. > > > > I think both are required for stock Synology Linux, see > > setup_board_tags() in board/Synology/common/legacy.c. One could just > > hard-code sane fallback values into that routine though. > > I see. Now I realized it's my bad that I did not consider stock OS at > all! The purpose of this patch is to get u-boot to use bootstd to boot > modern distros using a distro boot script. Should I put back these > envs "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" > "ethmtu=1500\0eth1mtu=1500\0" so we can support stock FW? Yes, please. It's the simplest way to retain that functionality. If CONFIG_NET_RANDOM_ETHADDR populates eth*addr env vars, we may even keep that (and fetching "the real" addresses becomes merely a bonus). Cheers, Phil
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; -} - -int checkboard(void) -{ - puts("Board: DS414\n"); - + /* Do late init to ensure successful enumeration of XHCI devices */ + pci_init(); return 0; } diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index ecf9501ba5..501ed51129 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -1,5 +1,6 @@ CONFIG_ARM=y CONFIG_ARCH_CPU_INIT=y +CONFIG_SYS_THUMB_BUILD=y CONFIG_ARCH_MVEBU=y CONFIG_SUPPORT_PASSING_ATAGS=y CONFIG_CMDLINE_TAG=y @@ -25,44 +26,40 @@ CONFIG_SPL_BSS_MAX_SIZE=0x4000 CONFIG_SPL=y CONFIG_DEBUG_UART_BASE=0xf1012000 CONFIG_DEBUG_UART_CLOCK=250000000 +CONFIG_IDENT_STRING="\nSynology DS214+/DS414 2/4-Bay Diskstation" CONFIG_SYS_LOAD_ADDR=0x800000 CONFIG_PCI=y CONFIG_DEBUG_UART=y +CONFIG_BOOTSTD_FULL=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_BOOTCOMMAND=y -CONFIG_BOOTCOMMAND="sf probe; sf read ${loadaddr} 0xd0000 0x2d0000; sf read ${ramdisk_addr_r} 0x3a0000 0x430000; bootm ${loadaddr} ${ramdisk_addr_r}" # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y -CONFIG_MISC_INIT_R=y +CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_MAX_SIZE=0x1bfd0 CONFIG_SPL_SYS_MALLOC_SIMPLE=y # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set CONFIG_SPL_I2C=y +CONFIG_SYS_PROMPT="DS414> " CONFIG_SYS_MAXARGS=32 CONFIG_CMD_I2C=y CONFIG_CMD_PCI=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y -# CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_DHCP=y CONFIG_CMD_TFTPPUT=y -CONFIG_CMD_MII=y -CONFIG_CMD_PING=y CONFIG_CMD_TIME=y -CONFIG_CMD_EXT2=y -CONFIG_CMD_FAT=y CONFIG_CMD_JFFS2=y CONFIG_CMD_MTDPARTS=y CONFIG_CMD_UBI=y -CONFIG_ISO_PARTITION=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_SPI_MAX_HZ=50000000 CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_ARP_TIMEOUT=200 CONFIG_NET_RETRY_COUNT=50 +CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_OF_TRANSLATE=y +CONFIG_LBA48=y +CONFIG_SYS_64BIT_LBA=y CONFIG_DM_I2C=y CONFIG_SYS_I2C_MVTWSI=y # CONFIG_MMC is not set @@ -83,4 +80,3 @@ 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 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 @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* + * Copyright (C) 2024 Tony Dinh <mibodhi@gmail.com> * Copyright (C) 2014 Stefan Roese <sr@denx.de> */ @@ -16,14 +17,9 @@ * U-Boot into it. */ -/* I2C */ #define CFG_I2C_MVTWSI_BASE0 MVEBU_TWSI_BASE -/* - * mv-common.h should be defined after CMD configs since it used them - * to enable certain macros - */ -#include "mv-common.h" +#define PHY_ANEG_TIMEOUT 16000 /* PHY needs a longer aneg time */ /* * Memory layout while starting into the bin_hdr via the @@ -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=0x10000000\0" \ + "initrd_high=0x10000000\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(0x1000000) +#define FDT_ADDR_R __stringify(0x2000000) +#define RAMDISK_ADDR_R __stringify(0x2200000) +#define SCRIPT_ADDR_R __stringify(0x1800000) +#define PXEFILE_ADDR_R __stringify(0x1900000) -#define CFG_EXTRA_ENV_SETTINGS \ - "initrd_high=0xffffffff\0" \ - "ramdisk_addr_r=0x8000000\0" \ - "usb0Mode=host\0usb1Mode=host\0usb2Mode=device\0" \ - "ethmtu=1500\0eth1mtu=1500\0" \ - "update_uboot=sf probe; dhcp; " \ - "mw.b ${loadaddr} 0x0 0xd0000; " \ - "tftpboot ${loadaddr} u-boot-with-spl.kwb; " \ - "sf update ${loadaddr} 0x0 0xd0000\0" +#define LOAD_ADDRESS_ENV_SETTINGS \ + "kernel_addr_r=" KERNEL_ADDR_R "\0" \ + "fdt_addr_r=" FDT_ADDR_R "\0" \ + "ramdisk_addr_r=" RAMDISK_ADDR_R "\0" \ + "scriptaddr=" SCRIPT_ADDR_R "\0" \ + "pxefile_addr_r=" PXEFILE_ADDR_R "\0" +#define CFG_EXTRA_ENV_SETTINGS \ + RELOCATION_LIMITS_ENV_SETTINGS \ + LOAD_ADDRESS_ENV_SETTINGS \ + "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \ + "console=ttyS0,115200\0" -/* increase autoneg timeout, my NIC sucks */ -#define PHY_ANEG_TIMEOUT 16000 +#endif /* CONFIG_SPL_BUILD */ #endif /* _CONFIG_SYNOLOGY_DS414_H */
- Switch to standard boot (in include/configs/ds414.h and configs/ds414_defconfig) - Implement board_late_init() to ensure successful enumeration of USB3 devices - Remove unnecessary misc_init_r(), since NET_RANDOM_ETHADDR is now configured in - 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 <mibodhi@gmail.com> --- board/Synology/ds414/ds414.c | 15 +++-------- configs/ds414_defconfig | 20 ++++++-------- include/configs/ds414.h | 51 ++++++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 44 deletions(-)