diff mbox series

[4/5] ds414: Auto-populate env if appropriate

Message ID 20210303005526.15056-5-phil@nwl.cc
State Superseded
Delegated to: Stefan Roese
Headers show
Series Synology DS414 integration mini-review | expand

Commit Message

Phil Sutter March 3, 2021, 12:55 a.m. UTC
Define a misc_init_r() which calls "syno populate_env" if the
environment seems incomplete (or default), indicated by missing
"ethaddr" variable. With this in place, no random MAC address fallback
is needed anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 board/Synology/ds414/cmd_syno.c |  2 +-
 board/Synology/ds414/ds414.c    | 13 +++++++++++++
 configs/ds414_defconfig         |  4 +---
 3 files changed, 15 insertions(+), 4 deletions(-)

Comments

Stefan Roese March 4, 2021, 1:06 p.m. UTC | #1
On 03.03.21 01:55, Phil Sutter wrote:
> Define a misc_init_r() which calls "syno populate_env" if the
> environment seems incomplete (or default), indicated by missing
> "ethaddr" variable. With this in place, no random MAC address fallback
> is needed anymore.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>   board/Synology/ds414/cmd_syno.c |  2 +-
>   board/Synology/ds414/ds414.c    | 13 +++++++++++++
>   configs/ds414_defconfig         |  4 +---
>   3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c
> index a120c3123ffb3..07bb395da3acc 100644
> --- a/board/Synology/ds414/cmd_syno.c
> +++ b/board/Synology/ds414/cmd_syno.c
> @@ -22,7 +22,7 @@
>   #define SYNO_CHKSUM_TAG		"CHK="
>   
>   
> -static int do_syno_populate(int argc, char *const argv[])
> +int do_syno_populate(int argc, char *const argv[])
>   {
>   	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
>   	unsigned int cs = CONFIG_SF_DEFAULT_CS;
> diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c
> index 9c4ce670ddfbd..c2469d6665255 100644
> --- a/board/Synology/ds414/ds414.c
> +++ b/board/Synology/ds414/ds414.c
> @@ -179,6 +179,19 @@ int board_init(void)
>   	return 0;
>   }
>   
> +#ifndef CONFIG_SPL_BUILD
> +int do_syno_populate(int argc, char *const argv[]);

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

Other than that:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan


> +
> +int misc_init_r(void)
> +{
> +	if (!env_get("ethaddr")) {
> +		puts("Incomplete environment, populating from SPI flash\n");
> +		do_syno_populate(0, NULL);
> +	}
> +	return 0;
> +}
> +#endif
> +
>   int checkboard(void)
>   {
>   	puts("Board: DS414\n");
> diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
> index fa9366778748c..948b22f3d1f66 100644
> --- a/configs/ds414_defconfig
> +++ b/configs/ds414_defconfig
> @@ -21,10 +21,9 @@ CONFIG_DEBUG_UART=y
>   CONFIG_BOOTDELAY=3
>   CONFIG_USE_BOOTARGS=y
>   CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1"
> -CONFIG_USE_PREBOOT=y
> -CONFIG_PREBOOT="usb start; sf probe"
>   # CONFIG_DISPLAY_BOARDINFO is not set
>   CONFIG_DISPLAY_BOARDINFO_LATE=y
> +CONFIG_MISC_INIT_R=y
>   CONFIG_SPL_I2C_SUPPORT=y
>   # CONFIG_CMD_FLASH is not set
>   CONFIG_CMD_I2C=y
> @@ -47,7 +46,6 @@ CONFIG_ENV_OVERWRITE=y
>   CONFIG_USE_ENV_SPI_MAX_HZ=y
>   CONFIG_ENV_SPI_MAX_HZ=50000000
>   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> -CONFIG_NET_RANDOM_ETHADDR=y
>   CONFIG_SPL_OF_TRANSLATE=y
>   CONFIG_BLK=y
>   # CONFIG_MMC is not set
> 


Viele Grüße,
Stefan
Phil Sutter March 4, 2021, 1:20 p.m. UTC | #2
On Thu, Mar 04, 2021 at 02:06:01PM +0100, Stefan Roese wrote:
> On 03.03.21 01:55, Phil Sutter wrote:
> > Define a misc_init_r() which calls "syno populate_env" if the
> > environment seems incomplete (or default), indicated by missing
> > "ethaddr" variable. With this in place, no random MAC address fallback
> > is needed anymore.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >   board/Synology/ds414/cmd_syno.c |  2 +-
> >   board/Synology/ds414/ds414.c    | 13 +++++++++++++
> >   configs/ds414_defconfig         |  4 +---
> >   3 files changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c
> > index a120c3123ffb3..07bb395da3acc 100644
> > --- a/board/Synology/ds414/cmd_syno.c
> > +++ b/board/Synology/ds414/cmd_syno.c
> > @@ -22,7 +22,7 @@
> >   #define SYNO_CHKSUM_TAG		"CHK="
> >   
> >   
> > -static int do_syno_populate(int argc, char *const argv[])
> > +int do_syno_populate(int argc, char *const argv[])
> >   {
> >   	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
> >   	unsigned int cs = CONFIG_SF_DEFAULT_CS;
> > diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c
> > index 9c4ce670ddfbd..c2469d6665255 100644
> > --- a/board/Synology/ds414/ds414.c
> > +++ b/board/Synology/ds414/ds414.c
> > @@ -179,6 +179,19 @@ int board_init(void)
> >   	return 0;
> >   }
> >   
> > +#ifndef CONFIG_SPL_BUILD
> > +int do_syno_populate(int argc, char *const argv[]);
> 
> I suspect that this prototype in a C file will trigger at least a
> checkpatch warning?

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

Thanks, Phil
Stefan Roese March 4, 2021, 1:22 p.m. UTC | #3
On 04.03.21 14:20, Phil Sutter wrote:
> On Thu, Mar 04, 2021 at 02:06:01PM +0100, Stefan Roese wrote:
>> On 03.03.21 01:55, Phil Sutter wrote:
>>> Define a misc_init_r() which calls "syno populate_env" if the
>>> environment seems incomplete (or default), indicated by missing
>>> "ethaddr" variable. With this in place, no random MAC address fallback
>>> is needed anymore.
>>>
>>> Signed-off-by: Phil Sutter <phil@nwl.cc>
>>> ---
>>>    board/Synology/ds414/cmd_syno.c |  2 +-
>>>    board/Synology/ds414/ds414.c    | 13 +++++++++++++
>>>    configs/ds414_defconfig         |  4 +---
>>>    3 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c
>>> index a120c3123ffb3..07bb395da3acc 100644
>>> --- a/board/Synology/ds414/cmd_syno.c
>>> +++ b/board/Synology/ds414/cmd_syno.c
>>> @@ -22,7 +22,7 @@
>>>    #define SYNO_CHKSUM_TAG		"CHK="
>>>    
>>>    
>>> -static int do_syno_populate(int argc, char *const argv[])
>>> +int do_syno_populate(int argc, char *const argv[])
>>>    {
>>>    	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
>>>    	unsigned int cs = CONFIG_SF_DEFAULT_CS;
>>> diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c
>>> index 9c4ce670ddfbd..c2469d6665255 100644
>>> --- a/board/Synology/ds414/ds414.c
>>> +++ b/board/Synology/ds414/ds414.c
>>> @@ -179,6 +179,19 @@ int board_init(void)
>>>    	return 0;
>>>    }
>>>    
>>> +#ifndef CONFIG_SPL_BUILD
>>> +int do_syno_populate(int argc, char *const argv[]);
>>
>> I suspect that this prototype in a C file will trigger at least a
>> checkpatch warning?
> 
> What is checkpatch? ;)

Provided with Linux and your favorite bootloader: ;)

./scripts/checkpatch.pl

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

NP. Thanks for working on this.

Thanks,
Stefan
diff mbox series

Patch

diff --git a/board/Synology/ds414/cmd_syno.c b/board/Synology/ds414/cmd_syno.c
index a120c3123ffb3..07bb395da3acc 100644
--- a/board/Synology/ds414/cmd_syno.c
+++ b/board/Synology/ds414/cmd_syno.c
@@ -22,7 +22,7 @@ 
 #define SYNO_CHKSUM_TAG		"CHK="
 
 
-static int do_syno_populate(int argc, char *const argv[])
+int do_syno_populate(int argc, char *const argv[])
 {
 	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
 	unsigned int cs = CONFIG_SF_DEFAULT_CS;
diff --git a/board/Synology/ds414/ds414.c b/board/Synology/ds414/ds414.c
index 9c4ce670ddfbd..c2469d6665255 100644
--- a/board/Synology/ds414/ds414.c
+++ b/board/Synology/ds414/ds414.c
@@ -179,6 +179,19 @@  int board_init(void)
 	return 0;
 }
 
+#ifndef CONFIG_SPL_BUILD
+int do_syno_populate(int argc, char *const argv[]);
+
+int misc_init_r(void)
+{
+	if (!env_get("ethaddr")) {
+		puts("Incomplete environment, populating from SPI flash\n");
+		do_syno_populate(0, NULL);
+	}
+	return 0;
+}
+#endif
+
 int checkboard(void)
 {
 	puts("Board: DS414\n");
diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
index fa9366778748c..948b22f3d1f66 100644
--- a/configs/ds414_defconfig
+++ b/configs/ds414_defconfig
@@ -21,10 +21,9 @@  CONFIG_DEBUG_UART=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 ip=off initrd=0x8000040,8M root=/dev/md0 rw syno_hw_version=DS414r1 ihd_num=4 netif_num=2 flash_size=8 SataLedSpecial=1 HddHotplug=1"
-CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="usb start; sf probe"
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_MISC_INIT_R=y
 CONFIG_SPL_I2C_SUPPORT=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_I2C=y
@@ -47,7 +46,6 @@  CONFIG_ENV_OVERWRITE=y
 CONFIG_USE_ENV_SPI_MAX_HZ=y
 CONFIG_ENV_SPI_MAX_HZ=50000000
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
-CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_BLK=y
 # CONFIG_MMC is not set