Hi Phil,
On Wed, Jun 26, 2024 at 3:31 AM Phil Sutter <p...@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
