[PATCH] board: tbs2910: Add support for generic distro configuration

[PATCH] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

This keeps the compatibility with the old bootcmd.

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
---
 include/configs/tbs2910.h | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
index b598fca1ec..8867918f37 100644
--- a/include/configs/tbs2910.h
+++ b/include/configs/tbs2910.h
@@ -8,6 +8,26 @@
 #ifndef __TBS2910_CONFIG_H
 #define __TBS2910_CONFIG_H
 
+#define CONFIG_BOOTCOMMAND \
+	"mmc rescan; " \
+	"if run bootcmd_up1; then " \
+		"run bootcmd_up2; " \
+	"else " \
+		"run bootcmd_mmc || run distro_bootcmd; " \
+	"fi"
+
+#ifndef CONFIG_SPL_BUILD
+#define BOOT_TARGET_DEVICES(func) \
+	func(MMC, mmc, 0) \
+	func(MMC, mmc, 1) \
+	func(MMC, mmc, 2) \
+	func(SATA, sata, 0) \
+	func(USB, usb, 0) \
+	func(PXE, pxe, na) \
+	func(DHCP, dhcp, na)
+#include <config_distro_bootcmd.h>
+#endif
+
 #include "mx6_common.h"
 
 /* General configuration */
@@ -80,6 +100,13 @@
 #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
 
 #define CONFIG_EXTRA_ENV_SETTINGS \
+	"fdt_addr=0x13000000\0" \
+	"fdt_addr_r=0x13000000\0" \
+	"initrd_high=0xffffffff\0" \
+	"kernel_addr_r=0x10008000\0" \
+	"pxefile_addr_r=0x10008000\0" \
+	"ramdisk_addr_r=0x18000000\0" \
+	"scriptaddr=0x14000000\0" \
 	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
 	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
 			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
@@ -102,14 +129,8 @@
 			"setenv stderr serial,vga\0" \
 	"stderr=serial,vga\0" \
 	"stdin=serial,usbkbd\0" \
-	"stdout=serial,vga\0"
-
-#define CONFIG_BOOTCOMMAND \
-	"mmc rescan; " \
-	"if run bootcmd_up1; then " \
-		"run bootcmd_up2; " \
-	"else " \
-		"run bootcmd_mmc; " \
-	"fi"
+	"stdout=serial,vga\0" \
+	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
+	BOOTENV
 
 #endif			       /* __TBS2910_CONFIG_H * */
-- 
2.24.1

[PATCH] board: tbs2910: Add support for generic distro configuration

[Tom Rini]

On Sat, Jan 25, 2020 at 02:42:40AM +0100, Denis 'GNUtoo' Carikli wrote:

> This keeps the compatibility with the old bootcmd.
> 
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>

First, I have concerns about this increasing the size of the board.

> ---
>  include/configs/tbs2910.h | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> index b598fca1ec..8867918f37 100644
> --- a/include/configs/tbs2910.h
> +++ b/include/configs/tbs2910.h
> @@ -8,6 +8,26 @@
>  #ifndef __TBS2910_CONFIG_H
>  #define __TBS2910_CONFIG_H
>  
> +#define CONFIG_BOOTCOMMAND \
> +	"mmc rescan; " \
> +	"if run bootcmd_up1; then " \
> +		"run bootcmd_up2; " \
> +	"else " \
> +		"run bootcmd_mmc || run distro_bootcmd; " \
> +	"fi"
> +
> +#ifndef CONFIG_SPL_BUILD
> +#define BOOT_TARGET_DEVICES(func) \
> +	func(MMC, mmc, 0) \
> +	func(MMC, mmc, 1) \
> +	func(MMC, mmc, 2) \
> +	func(SATA, sata, 0) \
> +	func(USB, usb, 0) \
> +	func(PXE, pxe, na) \
> +	func(DHCP, dhcp, na)
> +#include <config_distro_bootcmd.h>
> +#endif
> +
>  #include "mx6_common.h"
>  
>  /* General configuration */
> @@ -80,6 +100,13 @@
>  #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
>  
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> +	"fdt_addr=0x13000000\0" \
> +	"fdt_addr_r=0x13000000\0" \
> +	"initrd_high=0xffffffff\0" \
> +	"kernel_addr_r=0x10008000\0" \
> +	"pxefile_addr_r=0x10008000\0" \
> +	"ramdisk_addr_r=0x18000000\0" \
> +	"scriptaddr=0x14000000\0" \

Second, why are you disabling initrd relocation?  And we should set
bootm_size so that all relocations are done within the appropriate
memory window.

>  	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
>  	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
>  			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
> @@ -102,14 +129,8 @@
>  			"setenv stderr serial,vga\0" \
>  	"stderr=serial,vga\0" \
>  	"stdin=serial,usbkbd\0" \
> -	"stdout=serial,vga\0"
> -
> -#define CONFIG_BOOTCOMMAND \
> -	"mmc rescan; " \
> -	"if run bootcmd_up1; then " \
> -		"run bootcmd_up2; " \
> -	"else " \
> -		"run bootcmd_mmc; " \
> -	"fi"
> +	"stdout=serial,vga\0" \
> +	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
> +	BOOTENV
>  
>  #endif			       /* __TBS2910_CONFIG_H * */
> -- 
> 2.24.1
> 

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200125/86e4e324/attachment.sig>

[PATCH] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

Hi Denis,

thanks for your patch. In general I think it could be a good idea to
support distroboot on this board, especially if we can maintain
compatibility with the existing boot procedure. However, since this
board repeatedly has size problems with the u-boot binary, we carefully
need to check binary size.
More questions inline.

On 25.01.20 02:42, Denis 'GNUtoo' Carikli wrote:
> This keeps the compatibility with the old bootcmd.
>
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
> ---
>  include/configs/tbs2910.h | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> index b598fca1ec..8867918f37 100644
> --- a/include/configs/tbs2910.h
> +++ b/include/configs/tbs2910.h
> @@ -8,6 +8,26 @@
>  #ifndef __TBS2910_CONFIG_H
>  #define __TBS2910_CONFIG_H
>
> +#define CONFIG_BOOTCOMMAND \
> +	"mmc rescan; " \
> +	"if run bootcmd_up1; then " \
> +		"run bootcmd_up2; " \
> +	"else " \
> +		"run bootcmd_mmc || run distro_bootcmd; " \
> +	"fi"
> +
Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
existing one?
> +#ifndef CONFIG_SPL_BUILD
There in no SPL for tbs2910. So this is not required.
> +#define BOOT_TARGET_DEVICES(func) \
> +	func(MMC, mmc, 0) \
> +	func(MMC, mmc, 1) \
> +	func(MMC, mmc, 2) \
> +	func(SATA, sata, 0) \
> +	func(USB, usb, 0) \
> +	func(PXE, pxe, na) \
> +	func(DHCP, dhcp, na)
> +#include <config_distro_bootcmd.h>
> +#endif
> +
>  #include "mx6_common.h"
>
>  /* General configuration */
> @@ -80,6 +100,13 @@
>  #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
>
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> +	"fdt_addr=0x13000000\0" \
> +	"fdt_addr_r=0x13000000\0" \
> +	"initrd_high=0xffffffff\0" \
This should not be defined. Usually we want to relocate the initrd.

Regards,
Soeren
> +	"kernel_addr_r=0x10008000\0" \
> +	"pxefile_addr_r=0x10008000\0" \
> +	"ramdisk_addr_r=0x18000000\0" \
> +	"scriptaddr=0x14000000\0" \
>  	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
>  	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
>  			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
> @@ -102,14 +129,8 @@
>  			"setenv stderr serial,vga\0" \
>  	"stderr=serial,vga\0" \
>  	"stdin=serial,usbkbd\0" \
> -	"stdout=serial,vga\0"
> -
> -#define CONFIG_BOOTCOMMAND \
> -	"mmc rescan; " \
> -	"if run bootcmd_up1; then " \
> -		"run bootcmd_up2; " \
> -	"else " \
> -		"run bootcmd_mmc; " \
> -	"fi"
> +	"stdout=serial,vga\0" \
> +	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
> +	BOOTENV
>
>  #endif			       /* __TBS2910_CONFIG_H * */

[PATCH] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 25.01.20 17:24, Tom Rini wrote:
> On Sat, Jan 25, 2020 at 02:42:40AM +0100, Denis 'GNUtoo' Carikli wrote:
>
>> This keeps the compatibility with the old bootcmd.
>>
>> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
> First, I have concerns about this increasing the size of the board.
>
>> ---
>>  include/configs/tbs2910.h | 39 ++++++++++++++++++++++++++++++---------
>>  1 file changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
>> index b598fca1ec..8867918f37 100644
>> --- a/include/configs/tbs2910.h
>> +++ b/include/configs/tbs2910.h
>> @@ -8,6 +8,26 @@
>>  #ifndef __TBS2910_CONFIG_H
>>  #define __TBS2910_CONFIG_H
>>
>> +#define CONFIG_BOOTCOMMAND \
>> +	"mmc rescan; " \
>> +	"if run bootcmd_up1; then " \
>> +		"run bootcmd_up2; " \
>> +	"else " \
>> +		"run bootcmd_mmc || run distro_bootcmd; " \
>> +	"fi"
>> +
>> +#ifndef CONFIG_SPL_BUILD
>> +#define BOOT_TARGET_DEVICES(func) \
>> +	func(MMC, mmc, 0) \
>> +	func(MMC, mmc, 1) \
>> +	func(MMC, mmc, 2) \
>> +	func(SATA, sata, 0) \
>> +	func(USB, usb, 0) \
>> +	func(PXE, pxe, na) \
>> +	func(DHCP, dhcp, na)
>> +#include <config_distro_bootcmd.h>
>> +#endif
>> +
>>  #include "mx6_common.h"
>>
>>  /* General configuration */
>> @@ -80,6 +100,13 @@
>>  #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
>>
>>  #define CONFIG_EXTRA_ENV_SETTINGS \
>> +	"fdt_addr=0x13000000\0" \
>> +	"fdt_addr_r=0x13000000\0" \
>> +	"initrd_high=0xffffffff\0" \
>> +	"kernel_addr_r=0x10008000\0" \
>> +	"pxefile_addr_r=0x10008000\0" \
>> +	"ramdisk_addr_r=0x18000000\0" \
>> +	"scriptaddr=0x14000000\0" \
> Second, why are you disabling initrd relocation?  And we should set
> bootm_size so that all relocations are done within the appropriate
> memory window.
We use CONFIG_SYS_BOOTMAPSZ so far to define the available save low
memory. Is bootm_size required for distroboot, or is this a better
alternative?

Regards,
Soeren
>
>>  	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
>>  	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
>>  			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
>> @@ -102,14 +129,8 @@
>>  			"setenv stderr serial,vga\0" \
>>  	"stderr=serial,vga\0" \
>>  	"stdin=serial,usbkbd\0" \
>> -	"stdout=serial,vga\0"
>> -
>> -#define CONFIG_BOOTCOMMAND \
>> -	"mmc rescan; " \
>> -	"if run bootcmd_up1; then " \
>> -		"run bootcmd_up2; " \
>> -	"else " \
>> -		"run bootcmd_mmc; " \
>> -	"fi"
>> +	"stdout=serial,vga\0" \
>> +	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
>> +	BOOTENV
>>
>>  #endif			       /* __TBS2910_CONFIG_H * */
>> --
>> 2.24.1
>>

[PATCH] board: tbs2910: Add support for generic distro configuration

[Tom Rini]

On Sat, Jan 25, 2020 at 08:56:12PM +0100, Soeren Moch wrote:
> On 25.01.20 17:24, Tom Rini wrote:
> > On Sat, Jan 25, 2020 at 02:42:40AM +0100, Denis 'GNUtoo' Carikli wrote:
> >
> >> This keeps the compatibility with the old bootcmd.
> >>
> >> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
> > First, I have concerns about this increasing the size of the board.
> >
> >> ---
> >>  include/configs/tbs2910.h | 39 ++++++++++++++++++++++++++++++---------
> >>  1 file changed, 30 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> >> index b598fca1ec..8867918f37 100644
> >> --- a/include/configs/tbs2910.h
> >> +++ b/include/configs/tbs2910.h
> >> @@ -8,6 +8,26 @@
> >>  #ifndef __TBS2910_CONFIG_H
> >>  #define __TBS2910_CONFIG_H
> >>
> >> +#define CONFIG_BOOTCOMMAND \
> >> +	"mmc rescan; " \
> >> +	"if run bootcmd_up1; then " \
> >> +		"run bootcmd_up2; " \
> >> +	"else " \
> >> +		"run bootcmd_mmc || run distro_bootcmd; " \
> >> +	"fi"
> >> +
> >> +#ifndef CONFIG_SPL_BUILD
> >> +#define BOOT_TARGET_DEVICES(func) \
> >> +	func(MMC, mmc, 0) \
> >> +	func(MMC, mmc, 1) \
> >> +	func(MMC, mmc, 2) \
> >> +	func(SATA, sata, 0) \
> >> +	func(USB, usb, 0) \
> >> +	func(PXE, pxe, na) \
> >> +	func(DHCP, dhcp, na)
> >> +#include <config_distro_bootcmd.h>
> >> +#endif
> >> +
> >>  #include "mx6_common.h"
> >>
> >>  /* General configuration */
> >> @@ -80,6 +100,13 @@
> >>  #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
> >>
> >>  #define CONFIG_EXTRA_ENV_SETTINGS \
> >> +	"fdt_addr=0x13000000\0" \
> >> +	"fdt_addr_r=0x13000000\0" \
> >> +	"initrd_high=0xffffffff\0" \
> >> +	"kernel_addr_r=0x10008000\0" \
> >> +	"pxefile_addr_r=0x10008000\0" \
> >> +	"ramdisk_addr_r=0x18000000\0" \
> >> +	"scriptaddr=0x14000000\0" \
> > Second, why are you disabling initrd relocation?  And we should set
> > bootm_size so that all relocations are done within the appropriate
> > memory window.
>
> We use CONFIG_SYS_BOOTMAPSZ so far to define the available save low
> memory. Is bootm_size required for distroboot, or is this a better
> alternative?

We have many ways of achieving the same ends.  Per the README (I had to
double check), if SYS_BOOTMAPSZ is not set then if bootm_size is set in
the environment we use that.  So in the end, same thing.  And we just
drop initrd_high here.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200125/200e5418/attachment.sig>

[PATCH] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

On Sat, 25 Jan 2020 20:52:36 +0100
Soeren Moch <smoch@web.de> wrote:

> Hi Denis,
Hi,

> thanks for your patch. In general I think it could be a good idea to
> support distroboot on this board, especially if we can maintain
> compatibility with the existing boot procedure. However, since this
> board repeatedly has size problems with the u-boot binary, we
> carefully need to check binary size.
I saw that.

I also experienced size issues with the stock tbs2910_defconfig, and
for now I just locally removed support for things that eats up too much
space like PCIe support.

> There in no SPL for tbs2910. So this is not required.
Ahh ok, now I understand. That probably explains the repeated size
issues.

For my patch, I could guard the code addition with some ifdefs for
CONFIG_DISTRO_DEFAULTS if necessary. Note that CONFIG_DISTRO_DEFAULTS is
not set in the tbs2910_defconfig, so if done correctly it should not
make things worse.

In the long run it's probably worth looking into adding SPL support.
For instance the Wandboard has it. I'll try to find the time to look
into it but I can't guarantee anything.

As for the rest of the questions:
> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
> existing one?
> > +#define CONFIG_BOOTCOMMAND \
> > +	"mmc rescan; " \
> > +	"if run bootcmd_up1; then " \
> > +		"run bootcmd_up2; " \
> > +	"else " \
> > +		"run bootcmd_mmc || run distro_bootcmd; " \
> > +	"fi"
> > +  
> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
> existing one?
I'm a bit confused with it: There seem to be many way to do the same
thing and I'm not sure which one is the best.

Because of that, I just kept it in the code as it was (instead of
moving it to tbs2910_defconfig).

I'm also not sure if it's best to run distro_bootcmd before or
after the bootcmd_up1/bootcmd_up2/bootcmd_mmc commands, which are
probably used to boot some distribution like LibreElec.

Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200126/fedd9124/attachment.sig>

[PATCH] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 26.01.20 00:15, Denis 'GNUtoo' Carikli wrote:
> On Sat, 25 Jan 2020 20:52:36 +0100
> Soeren Moch <smoch@web.de> wrote:
>
>> Hi Denis,
> Hi,
>
>> thanks for your patch. In general I think it could be a good idea to
>> support distroboot on this board, especially if we can maintain
>> compatibility with the existing boot procedure. However, since this
>> board repeatedly has size problems with the u-boot binary, we
>> carefully need to check binary size.
> I saw that.
>
> I also experienced size issues with the stock tbs2910_defconfig, and
> for now I just locally removed support for things that eats up too much
> space like PCIe support.
>
>> There in no SPL for tbs2910. So this is not required.
> Ahh ok, now I understand. That probably explains the repeated size
> issues.
Why? With SPL+u-boot proper it would be even worse, since then there is
a gap between SPL and u-boot, u-boot starts at higher address in eMMC,
and it would not be smaller. And this 2-stage boot would break the
documented u-boot update procedure, since we have to flash 2 files then.
>
> For my patch, I could guard the code addition with some ifdefs for
> CONFIG_DISTRO_DEFAULTS if necessary. Note that CONFIG_DISTRO_DEFAULTS is 
> not set in the tbs2910_defconfig, so if done correctly it should not
> make things worse.
That means, without this change in defconfig the whole distroboot does
not work?
I will only take this patch if distroboot really works with defconfig then.
>
> In the long run it's probably worth looking into adding SPL support.
> For instance the Wandboard has it. I'll try to find the time to look
> into it but I can't guarantee anything.
SPL only is required to detect i.MX6 flavour and SDRAM size and
location. This is not necessary for tbs2910, since this board is only
available with i.MX6Q.
>
> As for the rest of the questions:
>> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
>> existing one?
>>> +#define CONFIG_BOOTCOMMAND \
>>> +	"mmc rescan; " \
>>> +	"if run bootcmd_up1; then " \
>>> +		"run bootcmd_up2; " \
>>> +	"else " \
>>> +		"run bootcmd_mmc || run distro_bootcmd; " \
>>> +	"fi"
>>> +  
>> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
>> existing one?
> I'm a bit confused with it: There seem to be many way to do the same
> thing and I'm not sure which one is the best.
>
> Because of that, I just kept it in the code as it was (instead of
> moving it to tbs2910_defconfig).
This is ok. What I mean: You moved it in the file, and therefore you
unnecessarily changed a lot of lines without actually changing most of
it's contents.
>
> I'm also not sure if it's best to run distro_bootcmd before or
> after the bootcmd_up1/bootcmd_up2/bootcmd_mmc commands, which are
> probably used to boot some distribution like LibreElec.
>
You did it right, as last boot option. Otherwise you would break
compatibility to the existing boot flow.

One additional comment: currently we want to strip down the embedded dtb
to reduce size. This means we cannot pass this dtb to linux, and always
need to load a different one (together with kernel and initrd). This is
no problem for extlinux.conf, but maybe we can omit to define fdtfile
just to make sure we have to load a different one. I'm not sure,
however, if distroboot scripts work without this default dtb.

Soeren

[PATCH] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

On Sun, 26 Jan 2020 01:40:13 +0100
Soeren Moch <smoch@web.de> wrote:
> > Ahh ok, now I understand. That probably explains the repeated size
> > issues.
> Why? With SPL+u-boot proper it would be even worse, since then there
> is a gap between SPL and u-boot, u-boot starts at higher address in
> eMMC, and it would not be smaller. And this 2-stage boot would break
> the documented u-boot update procedure, since we have to flash 2
> files then.
I assumed that in some conditions, the bootrom could load only the SPL
in sram. Once loaded, the SPL would initialize the RAM, detect the boot
media, and fetch u-boot.img from it, and execute it.

I also hoped the the SPL would have been significantly smaller than the
current u-boot.imx image.

In the meantime, I'll send a v2 with some additional patches to reduce
the size of the resulting u-boot.imx.

Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200128/0905fdb2/attachment.sig>

[PATCH v2][ 1/3] tbs2910: disable fuse command

[Denis 'GNUtoo' Carikli]

The fuse command is not needed for booting or during usual
users interactions with u-boot.

As that the resulting u-boot.imx image is already very
close to the size limit, removing the fuse command shouldn't
hurt.

With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
GNU/Linux distribution, it shrinks the image from 392192 to
388096 bytes.

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
---
 configs/tbs2910_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index 61d4c74324..0f12b94257 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -24,6 +24,7 @@ CONFIG_CMD_BOOTZ=y
 # CONFIG_BOOTM_VXWORKS is not set
 # CONFIG_CMD_FDT is not set
 CONFIG_CMD_MEMTEST=y
+# CONFIG_CMD_FUSE is not set
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
-- 
2.24.1

[PATCH v2][ 2/3] tbs2910: disable loadb and loads commands

[Denis 'GNUtoo' Carikli]

The loadb and loads commands are not needed for booting.

There are also more reliable and faster alternatives to
loadb and loads that can be used with the current configuration.

As that the resulting u-boot.imx image is already very close
to the size limit, removing the loadb and loads commands
shouldn't hurt.

With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
GNU/Linux distribution, it shrinks the image from 388096 to
384000 bytes.

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
---
 configs/tbs2910_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index 0f12b94257..0e91eeffd4 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -27,6 +27,8 @@ CONFIG_CMD_MEMTEST=y
 # CONFIG_CMD_FUSE is not set
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
+# CONFIG_CMD_LOADB is not set
+# CONFIG_CMD_LOADS is not set
 CONFIG_CMD_MMC=y
 CONFIG_CMD_PART=y
 CONFIG_CMD_PCI=y
-- 
2.24.1

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

This keeps the compatibility with the old bootcmd.

The fdtfile environment variable also needed to be set to
imx6q-tbs2910.dtb to enable booting mainline kernels
otherwise with extlinux.conf it tries to load
mx6-tbs2910.dtb instead.

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
---
 configs/tbs2910_defconfig | 17 ++++-------------
 include/configs/tbs2910.h | 37 ++++++++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index 0e91eeffd4..0d9e11bd20 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -9,16 +9,16 @@ CONFIG_NR_DRAM_BANKS=1
 CONFIG_PRE_CON_BUF_ADDR=0x7c000000
 CONFIG_CMD_HDMIDETECT=y
 CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
 CONFIG_BOOTDELAY=3
+# CONFIG_USE_BOOTCOMMAND is not set
 CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="echo PCI:; pci enum; pci 1; usb start; if hdmidet; then run set_con_hdmi; else run set_con_serial; fi"
 CONFIG_PRE_CONSOLE_BUFFER=y
-CONFIG_SUPPORT_RAW_INITRD=y
+CONFIG_DEFAULT_FDT_FILE="imx6q-tbs2910.dtb"
 CONFIG_BOUNCE_BUFFER=y
 CONFIG_BOARD_EARLY_INIT_F=y
-CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="Matrix U-Boot> "
-CONFIG_CMD_BOOTZ=y
 # CONFIG_BOOTM_PLAN9 is not set
 # CONFIG_BOOTM_RTEMS is not set
 # CONFIG_BOOTM_VXWORKS is not set
@@ -30,22 +30,14 @@ CONFIG_CMD_I2C=y
 # CONFIG_CMD_LOADB is not set
 # CONFIG_CMD_LOADS is not set
 CONFIG_CMD_MMC=y
-CONFIG_CMD_PART=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SATA=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_USB_MASS_STORAGE=y
-CONFIG_CMD_DHCP=y
-CONFIG_CMD_MII=y
-CONFIG_CMD_PING=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
-CONFIG_CMD_EXT2=y
-CONFIG_CMD_EXT4=y
 CONFIG_CMD_EXT4_WRITE=y
-CONFIG_CMD_FAT=y
-CONFIG_CMD_FS_GENERIC=y
-CONFIG_EFI_PARTITION=y
+# CONFIG_ISO_PARTITION is not set
 CONFIG_OF_CONTROL=y
 CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="imx6q-tbs2910"
@@ -75,7 +67,6 @@ CONFIG_RTC_DS1307=y
 CONFIG_DM_THERMAL=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
-CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y
 CONFIG_USB_GADGET=y
diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
index b598fca1ec..abeca16555 100644
--- a/include/configs/tbs2910.h
+++ b/include/configs/tbs2910.h
@@ -8,6 +8,24 @@
 #ifndef __TBS2910_CONFIG_H
 #define __TBS2910_CONFIG_H
 
+#define CONFIG_BOOTCOMMAND \
+	"mmc rescan; " \
+	"if run bootcmd_up1; then " \
+		"run bootcmd_up2; " \
+	"else " \
+		"run bootcmd_mmc || run distro_bootcmd; " \
+	"fi"
+
+#define BOOT_TARGET_DEVICES(func) \
+	func(MMC, mmc, 0) \
+	func(MMC, mmc, 1) \
+	func(MMC, mmc, 2) \
+	func(SATA, sata, 0) \
+	func(USB, usb, 0) \
+	func(PXE, pxe, na) \
+	func(DHCP, dhcp, na)
+#include <config_distro_bootcmd.h>
+
 #include "mx6_common.h"
 
 /* General configuration */
@@ -80,6 +98,13 @@
 #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
 
 #define CONFIG_EXTRA_ENV_SETTINGS \
+	"bootm_size=0x80000000\0" \
+	"fdt_addr=0x13000000\0" \
+	"fdt_addr_r=0x13000000\0" \
+	"kernel_addr_r=0x10008000\0" \
+	"pxefile_addr_r=0x10008000\0" \
+	"ramdisk_addr_r=0x18000000\0" \
+	"scriptaddr=0x14000000\0" \
 	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
 	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
 			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
@@ -102,14 +127,8 @@
 			"setenv stderr serial,vga\0" \
 	"stderr=serial,vga\0" \
 	"stdin=serial,usbkbd\0" \
-	"stdout=serial,vga\0"
-
-#define CONFIG_BOOTCOMMAND \
-	"mmc rescan; " \
-	"if run bootcmd_up1; then " \
-		"run bootcmd_up2; " \
-	"else " \
-		"run bootcmd_mmc; " \
-	"fi"
+	"stdout=serial,vga\0" \
+	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
+	BOOTENV
 
 #endif			       /* __TBS2910_CONFIG_H * */
-- 
2.24.1

[PATCH v2][ 1/3] tbs2910: disable fuse command

[Fabio Estevam]

On Tue, Jan 28, 2020 at 2:04 PM Denis 'GNUtoo' Carikli
<GNUtoo@cyberdimension.org> wrote:
>
> The fuse command is not needed for booting or during usual
> users interactions with u-boot.
>
> As that the resulting u-boot.imx image is already very
> close to the size limit, removing the fuse command shouldn't
> hurt.
>
> With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
> GNU/Linux distribution, it shrinks the image from 392192 to
> 388096 bytes.

I think it would be more readable if you put the delta value instead
of initial versus final.

[PATCH v2][ 1/3] tbs2910: disable fuse command

[Soeren Moch]

Sorry, sent with wrong sender address. Please only use this address here.

Soeren

On 28.01.20 18:13, Soeren Moch wrote:
> On 28.01.20 18:07, Fabio Estevam wrote:
>> On Tue, Jan 28, 2020 at 2:04 PM Denis 'GNUtoo' Carikli
>> <GNUtoo@cyberdimension.org> wrote:
>>> The fuse command is not needed for booting or during usual
>>> users interactions with u-boot.
>>>
>>> As that the resulting u-boot.imx image is already very
>>> close to the size limit, removing the fuse command shouldn't
>>> hurt.
>>>
>>> With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
>>> GNU/Linux distribution, it shrinks the image from 392192 to
>>> 388096 bytes.
>> I think it would be more readable if you put the delta value instead
>> of initial versus final.
> Which is 4k, surprise, surprise, the alignment of imx files. Actually
> you only shrink the binary by a few bytes, which is not worth the pain
> it you need fuses.
>
> Tom today merged a patch with much bigger size reduction (mentioned
> earlier), so this should not be required.
>
> Soeren

[PATCH] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 28.01.20 18:02, Denis 'GNUtoo' Carikli wrote:
> On Sun, 26 Jan 2020 01:40:13 +0100
> Soeren Moch <smoch@web.de> wrote:
>>> Ahh ok, now I understand. That probably explains the repeated size
>>> issues.
>> Why? With SPL+u-boot proper it would be even worse, since then there
>> is a gap between SPL and u-boot, u-boot starts at higher address in
>> eMMC, and it would not be smaller. And this 2-stage boot would break
>> the documented u-boot update procedure, since we have to flash 2
>> files then.
> I assumed that in some conditions, the bootrom could load only the SPL
> in sram. Once loaded, the SPL would initialize the RAM, detect the boot
> media, and fetch u-boot.img from it, and execute it.
It is different here. i.MX6Q boot rom loads a imx file. This initializes
the DDR controller first, then loads the executable payload, u-boot.bin
in this case. So there is no restriction from SRAM size, as long as the
DDR settings are fixed, what is the case for tbs2910.
>
> I also hoped the the SPL would have been significantly smaller than the
> current u-boot.imx image.
It would be. But then you need the u-boot.bin in addition to the SPL.imx
, with all the problems explained earlier.
>
> In the meantime, I'll send a v2 with some additional patches to reduce
> the size of the resulting u-boot.imx.
As already explained, this is not necessary now.

Soeren
>
> Denis.

[PATCH v2][ 1/3] tbs2910: disable fuse command

[Sören Moch]

On 28.01.20 18:04, Denis 'GNUtoo' Carikli wrote:
> The fuse command is not needed for booting or during usual
> users interactions with u-boot.
>
> As that the resulting u-boot.imx image is already very
> close to the size limit, removing the fuse command shouldn't
> hurt.
>
> With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
> GNU/Linux distribution, it shrinks the image from 392192 to
> 388096 bytes.
>
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
The fuse command is useful for tbs2910, especially for 1.x board revisions.
So

NAK.

Soeren

[PATCH v2][ 2/3] tbs2910: disable loadb and loads commands

[Soeren Moch]

On 28.01.20 18:04, Denis 'GNUtoo' Carikli wrote:
> The loadb and loads commands are not needed for booting.
>
> There are also more reliable and faster alternatives to
> loadb and loads that can be used with the current configuration.
>
> As that the resulting u-boot.imx image is already very close
> to the size limit, removing the loadb and loads commands
> shouldn't hurt.
>
> With arm-linux-gnueabi-gcc 9.2.0-1 from the Parabola
> GNU/Linux distribution, it shrinks the image from 388096 to
> 384000 bytes.
>
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>

I don't know any use case of these commands on tbs2910, so

Acked-by: Soeren Moch <smoch@web.de>

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 28.01.20 18:04, Denis 'GNUtoo' Carikli wrote:
> This keeps the compatibility with the old bootcmd.
>
> The fdtfile environment variable also needed to be set to
> imx6q-tbs2910.dtb to enable booting mainline kernels
> otherwise with extlinux.conf it tries to load
> mx6-tbs2910.dtb instead.
>
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
There are a lot of unrelated/unexplained changes in tbs2910_defconfig.
This probably should not be part of this patch.

As already discussed, the bootm_size environment variable is not
necessary, otherwise  somewhat dangerous with this value.

The requested changes for CONFIG_BOOTCOMMAND are not addressed in this v2.

Soeren

> ---
>  configs/tbs2910_defconfig | 17 ++++-------------
>  include/configs/tbs2910.h | 37 ++++++++++++++++++++++++++++---------
>  2 files changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
> index 0e91eeffd4..0d9e11bd20 100644
> --- a/configs/tbs2910_defconfig
> +++ b/configs/tbs2910_defconfig
> @@ -9,16 +9,16 @@ CONFIG_NR_DRAM_BANKS=1
>  CONFIG_PRE_CON_BUF_ADDR=0x7c000000
>  CONFIG_CMD_HDMIDETECT=y
>  CONFIG_AHCI=y
> +CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=3
> +# CONFIG_USE_BOOTCOMMAND is not set
>  CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="echo PCI:; pci enum; pci 1; usb start; if hdmidet; then run set_con_hdmi; else run set_con_serial; fi"
>  CONFIG_PRE_CONSOLE_BUFFER=y
> -CONFIG_SUPPORT_RAW_INITRD=y
> +CONFIG_DEFAULT_FDT_FILE="imx6q-tbs2910.dtb"
>  CONFIG_BOUNCE_BUFFER=y
>  CONFIG_BOARD_EARLY_INIT_F=y
> -CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="Matrix U-Boot> "
> -CONFIG_CMD_BOOTZ=y
>  # CONFIG_BOOTM_PLAN9 is not set
>  # CONFIG_BOOTM_RTEMS is not set
>  # CONFIG_BOOTM_VXWORKS is not set
> @@ -30,22 +30,14 @@ CONFIG_CMD_I2C=y
>  # CONFIG_CMD_LOADB is not set
>  # CONFIG_CMD_LOADS is not set
>  CONFIG_CMD_MMC=y
> -CONFIG_CMD_PART=y
>  CONFIG_CMD_PCI=y
>  CONFIG_CMD_SATA=y
>  CONFIG_CMD_USB=y
>  CONFIG_CMD_USB_MASS_STORAGE=y
> -CONFIG_CMD_DHCP=y
> -CONFIG_CMD_MII=y
> -CONFIG_CMD_PING=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
> -CONFIG_CMD_EXT2=y
> -CONFIG_CMD_EXT4=y
>  CONFIG_CMD_EXT4_WRITE=y
> -CONFIG_CMD_FAT=y
> -CONFIG_CMD_FS_GENERIC=y
> -CONFIG_EFI_PARTITION=y
> +# CONFIG_ISO_PARTITION is not set
>  CONFIG_OF_CONTROL=y
>  CONFIG_OF_EMBED=y
>  CONFIG_DEFAULT_DEVICE_TREE="imx6q-tbs2910"
> @@ -75,7 +67,6 @@ CONFIG_RTC_DS1307=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_USB=y
>  CONFIG_DM_USB=y
> -CONFIG_USB_STORAGE=y
>  CONFIG_USB_KEYBOARD=y
>  CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y
>  CONFIG_USB_GADGET=y
> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> index b598fca1ec..abeca16555 100644
> --- a/include/configs/tbs2910.h
> +++ b/include/configs/tbs2910.h
> @@ -8,6 +8,24 @@
>  #ifndef __TBS2910_CONFIG_H
>  #define __TBS2910_CONFIG_H
>  
> +#define CONFIG_BOOTCOMMAND \
> +	"mmc rescan; " \
> +	"if run bootcmd_up1; then " \
> +		"run bootcmd_up2; " \
> +	"else " \
> +		"run bootcmd_mmc || run distro_bootcmd; " \
> +	"fi"
> +
> +#define BOOT_TARGET_DEVICES(func) \
> +	func(MMC, mmc, 0) \
> +	func(MMC, mmc, 1) \
> +	func(MMC, mmc, 2) \
> +	func(SATA, sata, 0) \
> +	func(USB, usb, 0) \
> +	func(PXE, pxe, na) \
> +	func(DHCP, dhcp, na)
> +#include <config_distro_bootcmd.h>
> +
>  #include "mx6_common.h"
>  
>  /* General configuration */
> @@ -80,6 +98,13 @@
>  #define CONFIG_BOARD_SIZE_LIMIT		392192 /* (CONFIG_ENV_OFFSET - 1024) */
>  
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> +	"bootm_size=0x80000000\0" \
> +	"fdt_addr=0x13000000\0" \
> +	"fdt_addr_r=0x13000000\0" \
> +	"kernel_addr_r=0x10008000\0" \
> +	"pxefile_addr_r=0x10008000\0" \
> +	"ramdisk_addr_r=0x18000000\0" \
> +	"scriptaddr=0x14000000\0" \
>  	"bootargs_mmc1=console=ttymxc0,115200 di0_primary console=tty1\0" \
>  	"bootargs_mmc2=video=mxcfb0:dev=hdmi,1920x1080M at 60 " \
>  			"video=mxcfb1:off video=mxcfb2:off fbmem=28M\0" \
> @@ -102,14 +127,8 @@
>  			"setenv stderr serial,vga\0" \
>  	"stderr=serial,vga\0" \
>  	"stdin=serial,usbkbd\0" \
> -	"stdout=serial,vga\0"
> -
> -#define CONFIG_BOOTCOMMAND \
> -	"mmc rescan; " \
> -	"if run bootcmd_up1; then " \
> -		"run bootcmd_up2; " \
> -	"else " \
> -		"run bootcmd_mmc; " \
> -	"fi"
> +	"stdout=serial,vga\0" \
> +	"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
> +	BOOTENV
>  
>  #endif			       /* __TBS2910_CONFIG_H * */

[PATCH] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

On Sun, 26 Jan 2020 01:40:13 +0100
Soeren Moch <smoch@web.de> wrote:

> >> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the
> >> existing one?  
> > I'm a bit confused with it: There seem to be many way to do the same
> > thing and I'm not sure which one is the best.
> >
> > Because of that, I just kept it in the code as it was (instead of
> > moving it to tbs2910_defconfig).  
> This is ok. What I mean: You moved it in the file, and therefore you
> unnecessarily changed a lot of lines without actually changing most of
> it's contents.
If CONFIG_BOOTCOMMAND is not defined before, it will be defined in
config_distro_bootcmd.h

This means that the order in which defines and #include
<config_distro_bootcmd.h> is done matters.

Which lead me to do the inclusion of config_distro_bootcmd.h as early
as possible in the tbs2910.h.

This way if there are some unintended redefinition due to things having
changed in config_distro_bootcmd.h in the future, the compiler will at
warn or error about it.

Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200128/78b15063/attachment.sig>

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

On Tue, 28 Jan 2020 18:51:26 +0100
Soeren Moch <smoch@web.de> wrote:

> There are a lot of unrelated/unexplained changes in tbs2910_defconfig.
> This probably should not be part of this patch.
I changed only 2 things here:
- I added CONFIG_DISTRO_DEFAULTS=y
- I added CONFIG_DEFAULT_FDT_FILE="imx6q-tbs2910.dtb"

I double checked that change before sending the patch.

Here the big diff is due to savedefconfig minimizing the defconfig after
enabling CONFIG_DISTRO_DEFAULTS=y.

Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200128/773f545a/attachment.sig>

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Denis 'GNUtoo' Carikli]

On Tue, 28 Jan 2020 18:51:26 +0100
Soeren Moch <smoch@web.de> wrote:

> As already discussed, the bootm_size environment variable is not
> necessary, otherwise  somewhat dangerous with this value.
Sorry, for the mistake, I've fixed it now.

I'll send a v3 after some feedback from my response to your other
points.

Thanks a lot for the reviews.

Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200128/1801faee/attachment.sig>

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 28.01.20 22:27, Denis 'GNUtoo' Carikli wrote:
> On Tue, 28 Jan 2020 18:51:26 +0100
> Soeren Moch <smoch@web.de> wrote:
>
>> As already discussed, the bootm_size environment variable is not
>> necessary, otherwise  somewhat dangerous with this value.
> Sorry, for the mistake, I've fixed it now.
>
> I'll send a v3 after some feedback from my response to your other
> points.
Thanks.
Not all expected patches made it into v2020.04-rc1. I have to check what
happened here before I will come back to your patch. Unfortunately I
will ask you for additional changes.

Please do not split your response into several e-mails.

Thanks,
Soeren


>
> Thanks a lot for the reviews.
>
> Denis.

[PATCH v2][ 3/3] board: tbs2910: Add support for generic distro configuration

[Soeren Moch]

On 28.01.20 22:27, Denis 'GNUtoo' Carikli wrote:
> On Tue, 28 Jan 2020 18:51:26 +0100
> Soeren Moch <smoch@web.de> wrote:
>
>> As already discussed, the bootm_size environment variable is not
>> necessary, otherwise  somewhat dangerous with this value.
> Sorry, for the mistake, I've fixed it now.
>
> I'll send a v3 after some feedback from my response to your other
> points.
>
> Thanks a lot for the reviews.
For your v3 please drop patch 1/3, please disable CONFIG_GZIP instead.
If you want to indicate the size reduction, please do so for u-boot.bin
instead of u-boot.imx. And please only report the size reduction instead
of absolute size values, as suggested by Fabio.

For patch 2/3 please include my Acked-by, and update size reduction
description as suggested above.

Please split patch 3/3 into 3 separate patches:
- Move (the existing) CONFIG_BOOTCOMMAND to defconfig.
- Enable DISTRO_DEFAULTS in defconfig, in preparation of distro_boot
support. Also do the defconfig sync here.
- Enable distro_boot support. This includes the modified BOOTCOMMAND,
DEFAULT_FDT_FILE,BOOT_TARGET_DEVICES, and environment update.

For all patches please also Cc: Stefano and Fabio.

Thanks,
Soeren