| Message ID | 20240615220654.10648-1-mibodhi@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v2] arm: mvebu: Enable bootstd and other modernization for Synology DS414 (Armada XP) board | expand |
Hi Tony, On Sat, Jun 15, 2024 at 03:06:54PM -0700, Tony Dinh wrote: [...] > 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; > -} Could you please leave misc_init_r() in place (along with MISC_INIT_R in defconfig)? A closer look at doc/README.enetaddr suggests that implementing this board-specific function which acts if the environment does not have ethaddr defined already is exactly the right thing to do. Also, it doesn't conflict with NET_RANDOM_ETHADDR: If do_syno_populate() succeeds, no random MAC addresses are generated. If I manually remove the call, random MACs come into play. So having this option enabled serves as a fallback for boxes which lack the data in flash. > -int checkboard(void) > -{ > - puts("Board: DS414\n"); > - > + /* Do late init to ensure successful enumeration of XHCI devices */ > + pci_init(); > return 0; > } FWIW, booting from rear USB port worked fine for me! > diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig > index ecf9501ba5..501ed51129 100644 > --- a/configs/ds414_defconfig > +++ b/configs/ds414_defconfig [...] > @@ -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}" So these should not be necessary anymore with BOOTSTD. I wonder if it is possible to provide a static bootmeth (or bootflow?) with low priority which boots the legacy OS from flash. It could hold all the details instead of the *_legacy env vars introduced below. A pending issue for me is inability to 'saveenv' - the flash seems read-only in that spot. Does it work for you? > # 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> " Can we detect whether it is 414 or 214, BTW? bootargs_legacy contains 414-specific info as well. [...] > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > index 9446acba79..89e55bf0d4 100644 > --- a/include/configs/ds414.h > +++ b/include/configs/ds414.h [...] > @@ -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 */ AIUI, this is a limitation of my local NIC, not the DS414 one. It just took too long for link detection and autoneg when directly connected. A switch should have helped. Or do you have more details? Cheers, Phil
Hi Phil, On Wed, Jun 26, 2024 at 3:31 AM Phil Sutter <phil@nwl.cc> wrote: > > Hi Tony, > > On Sat, Jun 15, 2024 at 03:06:54PM -0700, Tony Dinh wrote: > [...] > > 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; > > -} > > Could you please leave misc_init_r() in place (along with MISC_INIT_R in > defconfig)? A closer look at doc/README.enetaddr suggests that > implementing this board-specific function which acts if the environment > does not have ethaddr defined already is exactly the right thing to do. > Also, it doesn't conflict with NET_RANDOM_ETHADDR: If do_syno_populate() > succeeds, no random MAC addresses are generated. If I manually remove > the call, random MACs come into play. So having this option enabled > serves as a fallback for boxes which lack the data in flash. Sure I will do that. I was mistaken in assuming that network random addresses were already generated before misc_init_r(). Thanks! > > > -int checkboard(void) > > -{ > > - puts("Board: DS414\n"); > > - > > + /* Do late init to ensure successful enumeration of XHCI devices */ > > + pci_init(); > > return 0; > > } > > FWIW, booting from rear USB port worked fine for me! Cool! That was the main thing I was interested in for sending in this patch. One thing led to another... :) > > > diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig > > index ecf9501ba5..501ed51129 100644 > > --- a/configs/ds414_defconfig > > +++ b/configs/ds414_defconfig > [...] > > @@ -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}" > > So these should not be necessary anymore with BOOTSTD. I wonder if it is > possible to provide a static bootmeth (or bootflow?) with low priority > which boots the legacy OS from flash. It could hold all the details > instead of the *_legacy env vars introduced below. I recall from bootstd documentation that it is possible to have a bootflow on flash. But it seems a bit too much for this patch. I think we'll have to repartition or find some space on the flash to store that bootflow script. > > A pending issue for me is inability to 'saveenv' - the flash seems > read-only in that spot. Does it work for you? Yes it does work for me. With the latest u-boot SPI flash driver, as I recall, the entire flash is automatically unprotected at initialization. But we need to be careful using saveenv running stock FW, because it could overwrite the rootfs on flash. It is common for Synology NAS that the envs location is at the same place as the stock rootfs. Since I don't care about stock FW, I saved the envs and used the location for u-boot envs in Linux /etc/fw_env.config. I'm not sure if my DS214+ box can boot stock FW anymore :) > > > # 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> " > > Can we detect whether it is 414 or 214, BTW? bootargs_legacy contains > 414-specific info as well. It's a bit hard to do without creating another defconfig. AFAICT, the only difference between DS414 and DS214+ is the number of SATA bays. But currently u-boot SATA does not work for this box. U-Boot sata_mv driver is a subset of Linux sata_mv, and we don't have support for the right PCIe SATA chip in u-boot sata_mv. > > [...] > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > index 9446acba79..89e55bf0d4 100644 > > --- a/include/configs/ds414.h > > +++ b/include/configs/ds414.h > [...] > > @@ -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 */ > > AIUI, this is a limitation of my local NIC, not the DS414 one. It just > took too long for link detection and autoneg when directly connected. A > switch should have helped. Or do you have more details? Ah! The typical PHY_ANEG_TIMEOUT on other Armada 38x boards is 8000. I thought the 16000 was for Armada XP when I saw that code. I think it's OK to use a larger timeout. It really does not hurt? All the best, Tony > Cheers, Phil
Hi Phil, On Fri, Jun 28, 2024 at 3:04 PM Tony Dinh <mibodhi@gmail.com> wrote: > > Hi Phil, > > On Wed, Jun 26, 2024 at 3:31 AM Phil Sutter <phil@nwl.cc> wrote: > > > > Hi Tony, > > > > On Sat, Jun 15, 2024 at 03:06:54PM -0700, Tony Dinh wrote: > > [...] > > > 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; > > > -} > > > > Could you please leave misc_init_r() in place (along with MISC_INIT_R in > > defconfig)? A closer look at doc/README.enetaddr suggests that > > implementing this board-specific function which acts if the environment > > does not have ethaddr defined already is exactly the right thing to do. > > Also, it doesn't conflict with NET_RANDOM_ETHADDR: If do_syno_populate() > > succeeds, no random MAC addresses are generated. If I manually remove > > the call, random MACs come into play. So having this option enabled > > serves as a fallback for boxes which lack the data in flash. > > Sure I will do that. I was mistaken in assuming that network random > addresses were already generated before misc_init_r(). Thanks! > > > > > > -int checkboard(void) > > > -{ > > > - puts("Board: DS414\n"); > > > - > > > + /* Do late init to ensure successful enumeration of XHCI devices */ > > > + pci_init(); > > > return 0; > > > } > > > > FWIW, booting from rear USB port worked fine for me! > > Cool! That was the main thing I was interested in for sending in this > patch. One thing led to another... :) > > > > > > diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig > > > index ecf9501ba5..501ed51129 100644 > > > --- a/configs/ds414_defconfig > > > +++ b/configs/ds414_defconfig > > [...] > > > @@ -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}" > > > > So these should not be necessary anymore with BOOTSTD. I wonder if it is > > possible to provide a static bootmeth (or bootflow?) with low priority > > which boots the legacy OS from flash. It could hold all the details > > instead of the *_legacy env vars introduced below. > > I recall from bootstd documentation that it is possible to have a > bootflow on flash. But it seems a bit too much for this patch. I think > we'll have to repartition or find some space on the flash to store > that bootflow script. > Sorry I've misread your comment "it is possible to provide a static bootmeth (or bootflow?) ". I think by "static bootflow" you must have meant we would define it in the board code, and convince bootstd to use it as the last boot method (without hunting for it on flash). I'm not sure. Perhaps "global bootflow" is something that can be defined and used here. I'll have to dig into bootstd documentation a bit more. All the best, Tony > > > > A pending issue for me is inability to 'saveenv' - the flash seems > > read-only in that spot. Does it work for you? > > Yes it does work for me. With the latest u-boot SPI flash driver, as I > recall, the entire flash is automatically unprotected at > initialization. But we need to be careful using saveenv running stock > FW, because it could overwrite the rootfs on flash. It is common for > Synology NAS that the envs location is at the same place as the stock > rootfs. Since I don't care about stock FW, I saved the envs and used > the location for u-boot envs in Linux /etc/fw_env.config. I'm not sure > if my DS214+ box can boot stock FW anymore :) > > > > > > # 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> " > > > > Can we detect whether it is 414 or 214, BTW? bootargs_legacy contains > > 414-specific info as well. > > It's a bit hard to do without creating another defconfig. AFAICT, the > only difference between DS414 and DS214+ is the number of SATA bays. > But currently u-boot SATA does not work for this box. U-Boot sata_mv > driver is a subset of Linux sata_mv, and we don't have support for the > right PCIe SATA chip in u-boot sata_mv. > > > > > [...] > > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > > index 9446acba79..89e55bf0d4 100644 > > > --- a/include/configs/ds414.h > > > +++ b/include/configs/ds414.h > > [...] > > > @@ -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 */ > > > > AIUI, this is a limitation of my local NIC, not the DS414 one. It just > > took too long for link detection and autoneg when directly connected. A > > switch should have helped. Or do you have more details? > > Ah! The typical PHY_ANEG_TIMEOUT on other Armada 38x boards is 8000. I > thought the 16000 was for Armada XP when I saw that code. I think it's > OK to use a larger timeout. It really does not hurt? > > All the best, > Tony > > > Cheers, Phil
Hi Tony, On Fri, Jun 28, 2024 at 03:44:01PM -0700, Tony Dinh wrote: > On Fri, Jun 28, 2024 at 3:04 PM Tony Dinh <mibodhi@gmail.com> wrote: > > On Wed, Jun 26, 2024 at 3:31 AM Phil Sutter <phil@nwl.cc> wrote: > > > On Sat, Jun 15, 2024 at 03:06:54PM -0700, Tony Dinh wrote: > > > [...] > > > > 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; > > > > -} > > > > > > Could you please leave misc_init_r() in place (along with MISC_INIT_R in > > > defconfig)? A closer look at doc/README.enetaddr suggests that > > > implementing this board-specific function which acts if the environment > > > does not have ethaddr defined already is exactly the right thing to do. > > > Also, it doesn't conflict with NET_RANDOM_ETHADDR: If do_syno_populate() > > > succeeds, no random MAC addresses are generated. If I manually remove > > > the call, random MACs come into play. So having this option enabled > > > serves as a fallback for boxes which lack the data in flash. > > > > Sure I will do that. I was mistaken in assuming that network random > > addresses were already generated before misc_init_r(). Thanks! It's nice that we may keep both so there's a fallback if data extraction from flash fails. > > > > -int checkboard(void) > > > > -{ > > > > - puts("Board: DS414\n"); > > > > - > > > > + /* Do late init to ensure successful enumeration of XHCI devices */ > > > > + pci_init(); > > > > return 0; > > > > } > > > > > > FWIW, booting from rear USB port worked fine for me! > > > > Cool! That was the main thing I was interested in for sending in this > > patch. One thing led to another... :) Which is a bit odd TBH: Scrolling through git history, I found commit c57f920504297 ("arm: mvebu: configs: ds414: Enable XHCI_PCI by default") in which I claimed the rear USB ports to be functional after enabling XHCI_PCI. Took me only three years to forget all the details. :( > > > > diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig > > > > index ecf9501ba5..501ed51129 100644 > > > > --- a/configs/ds414_defconfig > > > > +++ b/configs/ds414_defconfig > > > [...] > > > > @@ -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}" > > > > > > So these should not be necessary anymore with BOOTSTD. I wonder if it is > > > possible to provide a static bootmeth (or bootflow?) with low priority > > > which boots the legacy OS from flash. It could hold all the details > > > instead of the *_legacy env vars introduced below. > > > > I recall from bootstd documentation that it is possible to have a > > bootflow on flash. But it seems a bit too much for this patch. I think > > we'll have to repartition or find some space on the flash to store > > that bootflow script. > > > > Sorry I've misread your comment "it is possible to provide a static > bootmeth (or bootflow?) ". I think by "static bootflow" you must have > meant we would define it in the board code, and convince bootstd to > use it as the last boot method (without hunting for it on flash). I'm > not sure. Perhaps "global bootflow" is something that can be defined > and used here. I'll have to dig into bootstd documentation a bit more. Yes, that would be great. Booting legacy should be fairly simple by calling 'run bootcmd_legacy', though automatically falling back to it is even better. BTW: $bootcmd_legacy contains 'run bootargs_legacy' instruction. Are you sure this is going to work? Shouldn't this be 'setenv bootargs $bootargs_legacy' instead? > > > A pending issue for me is inability to 'saveenv' - the flash seems > > > read-only in that spot. Does it work for you? > > > > Yes it does work for me. With the latest u-boot SPI flash driver, as I > > recall, the entire flash is automatically unprotected at > > initialization. But we need to be careful using saveenv running stock > > FW, because it could overwrite the rootfs on flash. It is common for > > Synology NAS that the envs location is at the same place as the stock > > rootfs. Since I don't care about stock FW, I saved the envs and used > > the location for u-boot envs in Linux /etc/fw_env.config. I'm not sure > > if my DS214+ box can boot stock FW anymore :) The saveenv problem exists only with stock u-boot as Synology apparently didn't care to adjust CONFIG_ENV_OFFSET (or they failed). If I got that right, it should be set to 0x100000 and thus points into "zImage" area. Upstream we have ENV_OFFSET at 0x7E0000, i.e. the start of "RedBoot config" (which is unused by stock firmware). What's odd though, is that I fail to write to SPI flash using u-boot built from current HEAD. Trying legacy boot had failed with "Ramdisk image is corrupt or invalid" message, so I tried flashing a dump of rd.gz and ended with "flash area is locked" error message. Despite calling 'sf protect unlock ...' in beforehand. I wonder why you don't have these issues. What does 'sf probe' print on your box? > > > > # 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> " > > > > > > Can we detect whether it is 414 or 214, BTW? bootargs_legacy contains > > > 414-specific info as well. > > > > It's a bit hard to do without creating another defconfig. AFAICT, the > > only difference between DS414 and DS214+ is the number of SATA bays. > > But currently u-boot SATA does not work for this box. U-Boot sata_mv > > driver is a subset of Linux sata_mv, and we don't have support for the > > right PCIe SATA chip in u-boot sata_mv. Back in 2015, u-boot's sata_mv didn't have PCI support to begin with. I see you spent much more time with sata_mv than I did. What's missing to support the Marvell 88SX7042 in u-boot? > > > [...] > > > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > > > index 9446acba79..89e55bf0d4 100644 > > > > --- a/include/configs/ds414.h > > > > +++ b/include/configs/ds414.h > > > [...] > > > > @@ -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 */ > > > > > > AIUI, this is a limitation of my local NIC, not the DS414 one. It just > > > took too long for link detection and autoneg when directly connected. A > > > switch should have helped. Or do you have more details? > > > > Ah! The typical PHY_ANEG_TIMEOUT on other Armada 38x boards is 8000. I > > thought the 16000 was for Armada XP when I saw that code. I think it's > > OK to use a larger timeout. It really does not hurt? It does not - this change merely makes u-boot wait for longer until giving up waiting for link. My setup involves a direct link between the DS414 and a Realtek NIC, so it is rather surprising 16s timeout is sufficient. ;) Ah, one more thing: You mentioned mtdparts to define a flash map which should simplify flashing u-boot into the right spot. I haven't had any luck doing so for SPI flash, do you? Cheers, Phil
Hi Phil, On Thu, Jul 4, 2024 at 4:27 AM Phil Sutter <phil@nwl.cc> wrote: > > Hi Tony, > > On Fri, Jun 28, 2024 at 03:44:01PM -0700, Tony Dinh wrote: > > On Fri, Jun 28, 2024 at 3:04 PM Tony Dinh <mibodhi@gmail.com> wrote: > > > On Wed, Jun 26, 2024 at 3:31 AM Phil Sutter <phil@nwl.cc> wrote: > > > > On Sat, Jun 15, 2024 at 03:06:54PM -0700, Tony Dinh wrote: > > > > [...] > > > > > 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; > > > > > -} > > > > > > > > Could you please leave misc_init_r() in place (along with MISC_INIT_R in > > > > defconfig)? A closer look at doc/README.enetaddr suggests that > > > > implementing this board-specific function which acts if the environment > > > > does not have ethaddr defined already is exactly the right thing to do. > > > > Also, it doesn't conflict with NET_RANDOM_ETHADDR: If do_syno_populate() > > > > succeeds, no random MAC addresses are generated. If I manually remove > > > > the call, random MACs come into play. So having this option enabled > > > > serves as a fallback for boxes which lack the data in flash. > > > > > > Sure I will do that. I was mistaken in assuming that network random > > > addresses were already generated before misc_init_r(). Thanks! > > It's nice that we may keep both so there's a fallback if data extraction > from flash fails. > > > > > > -int checkboard(void) > > > > > -{ > > > > > - puts("Board: DS414\n"); > > > > > - > > > > > + /* Do late init to ensure successful enumeration of XHCI devices */ > > > > > + pci_init(); > > > > > return 0; > > > > > } > > > > > > > > FWIW, booting from rear USB port worked fine for me! > > > > > > Cool! That was the main thing I was interested in for sending in this > > > patch. One thing led to another... :) > > Which is a bit odd TBH: Scrolling through git history, I found commit > c57f920504297 ("arm: mvebu: configs: ds414: Enable XHCI_PCI by default") > in which I claimed the rear USB ports to be functional after enabling > XHCI_PCI. Took me only three years to forget all the details. :( > Happens to me quite often, our old memories need to be filed away in some brain cabinet :) ... Perhaps the switch to Driver Model was when it started behaving like it's not working, but actually not broken. The "pci init" is needed to enumerate the XHCI controller first, before "usb start" can work normally as before. > > > > > diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig > > > > > index ecf9501ba5..501ed51129 100644 > > > > > --- a/configs/ds414_defconfig > > > > > +++ b/configs/ds414_defconfig > > > > [...] > > > > > @@ -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}" > > > > > > > > So these should not be necessary anymore with BOOTSTD. I wonder if it is > > > > possible to provide a static bootmeth (or bootflow?) with low priority > > > > which boots the legacy OS from flash. It could hold all the details > > > > instead of the *_legacy env vars introduced below. > > > > > > I recall from bootstd documentation that it is possible to have a > > > bootflow on flash. But it seems a bit too much for this patch. I think > > > we'll have to repartition or find some space on the flash to store > > > that bootflow script. > > > > > > > Sorry I've misread your comment "it is possible to provide a static > > bootmeth (or bootflow?) ". I think by "static bootflow" you must have > > meant we would define it in the board code, and convince bootstd to > > use it as the last boot method (without hunting for it on flash). I'm > > not sure. Perhaps "global bootflow" is something that can be defined > > and used here. I'll have to dig into bootstd documentation a bit more. > > Yes, that would be great. Booting legacy should be fairly simple by > calling 'run bootcmd_legacy', though automatically falling back to it is > even better. > > BTW: $bootcmd_legacy contains 'run bootargs_legacy' instruction. Are you > sure this is going to work? Shouldn't this be 'setenv bootargs > $bootargs_legacy' instead? Yes, my bad. Should be 'setenv bootargs $bootargs_legacy'. Thanks! > > > > A pending issue for me is inability to 'saveenv' - the flash seems > > > > read-only in that spot. Does it work for you? > > > > > > Yes it does work for me. With the latest u-boot SPI flash driver, as I > > > recall, the entire flash is automatically unprotected at > > > initialization. But we need to be careful using saveenv running stock > > > FW, because it could overwrite the rootfs on flash. It is common for > > > Synology NAS that the envs location is at the same place as the stock > > > rootfs. Since I don't care about stock FW, I saved the envs and used > > > the location for u-boot envs in Linux /etc/fw_env.config. I'm not sure > > > if my DS214+ box can boot stock FW anymore :) > > The saveenv problem exists only with stock u-boot as Synology apparently > didn't care to adjust CONFIG_ENV_OFFSET (or they failed). If I got that > right, it should be set to 0x100000 and thus points into "zImage" area. I think they did not care. Their u-boot only uses internal envs. > > Upstream we have ENV_OFFSET at 0x7E0000, i.e. the start of "RedBoot > config" (which is unused by stock firmware). > > What's odd though, is that I fail to write to SPI flash using u-boot > built from current HEAD. Trying legacy boot had failed with "Ramdisk > image is corrupt or invalid" message, so I tried flashing a dump of > rd.gz and ended with "flash area is locked" error message. Despite > calling 'sf protect unlock ...' in beforehand. I wonder why you don't > have these issues. What does 'sf probe' print on your box? DS414> sf probe SF: Detected n25q064 with page size 256 Bytes, erase size 4 KiB, total 8 MiB I have some stashed SPI flash patches that I need to look at again. I vaguely recall that some SPI flash chips were not automatically unlocked, even u-boot intention is to unlock all SPI flash like Linux kernel has been doing. > > > > > > # 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> " > > > > > > > > Can we detect whether it is 414 or 214, BTW? bootargs_legacy contains > > > > 414-specific info as well. > > > > > > It's a bit hard to do without creating another defconfig. AFAICT, the > > > only difference between DS414 and DS214+ is the number of SATA bays. > > > But currently u-boot SATA does not work for this box. U-Boot sata_mv > > > driver is a subset of Linux sata_mv, and we don't have support for the > > > right PCIe SATA chip in u-boot sata_mv. > > Back in 2015, u-boot's sata_mv didn't have PCI support to begin with. I > see you spent much more time with sata_mv than I did. What's missing to > support the Marvell 88SX7042 in u-boot? Linux sata_mv driver (123K code) has a chip type for 88SX7042 and parameters associated with it. U-Boot sata_mv (29K code) does not have any chip type specifically called out. Apparently, it was ported from Linux too long ago. I did not try very hard to get it working after I saw the difference, though. I'll make another attempt when I have some free time. > > > > > [...] > > > > > diff --git a/include/configs/ds414.h b/include/configs/ds414.h > > > > > index 9446acba79..89e55bf0d4 100644 > > > > > --- a/include/configs/ds414.h > > > > > +++ b/include/configs/ds414.h > > > > [...] > > > > > @@ -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 */ > > > > > > > > AIUI, this is a limitation of my local NIC, not the DS414 one. It just > > > > took too long for link detection and autoneg when directly connected. A > > > > switch should have helped. Or do you have more details? > > > > > > Ah! The typical PHY_ANEG_TIMEOUT on other Armada 38x boards is 8000. I > > > thought the 16000 was for Armada XP when I saw that code. I think it's > > > OK to use a larger timeout. It really does not hurt? > > It does not - this change merely makes u-boot wait for longer until > giving up waiting for link. My setup involves a direct link between the > DS414 and a Realtek NIC, so it is rather surprising 16s timeout is > sufficient. ;) > > Ah, one more thing: You mentioned mtdparts to define a flash map which > should simplify flashing u-boot into the right spot. I haven't had any > luck doing so for SPI flash, do you? I used this definition for booting with modern distro only. U-boot takes the 1st MB, and u-boot envs at 0x7E0000. So I can flash u-boot and u-boot envs images in Linux. mtdparts=spi0.0:1m(u-boot),7040k(kernel),64k(u-boot-env),-(data) All the best, Tony > > 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..89e55bf0d4 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,54 @@ * 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" -/* Default Environment */ +/* + * mv-common.h should be defined after CMD configs since it used them + * to enable certain macros + */ +#include "mv-common.h" -#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; " \ +#ifndef CONFIG_SPL_BUILD + +#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 EXTRA_ENV_SETTINGS_LEGACY \ + "bootargs_legacy=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\0" \ + "bootcmd_legacy=sf probe; sf read ${loadaddr} 0xd0000 0x2d0000; " \ + "sf read ${ramdisk_addr_r} 0x3a0000 0x430000; " \ + "run bootargs_legacy; " \ + "bootm ${loadaddr} ${ramdisk_addr_r}\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 \ + EXTRA_ENV_SETTINGS_LEGACY \ + "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> --- Changes in v2: - Define EXTRA_ENV_SETTINGS_LEGACY to restore default envs support for booting legacy FW. board/Synology/ds414/ds414.c | 15 ++------- configs/ds414_defconfig | 20 +++++------ include/configs/ds414.h | 65 +++++++++++++++++++++++++----------- 3 files changed, 57 insertions(+), 43 deletions(-)