[13/14] sunxi: Transition from the MMC to a FAT-based environment

Message ID a8e504a71c510a6185f98e7d8360e66da50d540b.1511864667.git-series.maxime.ripard@free-electrons.com
State Superseded
Headers show
Series
  • env: Multiple env support and env transition for sunxi
Related show

Commit Message

Maxime Ripard Nov. 28, 2017, 10:24 a.m.
The current environment has been hardcoded to an offset that starts to be
an issue given the current size of our main U-Boot binary.

By implementing a custom environment location routine, we can always favor
the FAT-based environment, and fallback to the MMC if we don't find
something in the FAT partition. We also implement the same order when
saving the environment, so that hopefully we can slowly migrate the users
over to FAT-based environment and away from the raw MMC one.

Eventually, and hopefully before we reach that limit again, we will have
most of our users using that setup, and we'll be able to retire the raw
environment, and gain more room for the U-Boot binary.

Reviewed-by: Lukasz Majewski <lukma@denx.de>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 board/sunxi/board.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

André Przywara Dec. 5, 2017, 10:28 a.m. | #1
Hi,

On 28/11/17 10:24, Maxime Ripard wrote:
> The current environment has been hardcoded to an offset that starts to be
> an issue given the current size of our main U-Boot binary.
> 
> By implementing a custom environment location routine, we can always favor
> the FAT-based environment, and fallback to the MMC if we don't find
> something in the FAT partition. We also implement the same order when
> saving the environment, so that hopefully we can slowly migrate the users
> over to FAT-based environment and away from the raw MMC one.
> 
> Eventually, and hopefully before we reach that limit again, we will have
> most of our users using that setup, and we'll be able to retire the raw
> environment, and gain more room for the U-Boot binary.
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  board/sunxi/board.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index dcacdf3e626d..8891961dcc6b 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -173,6 +173,22 @@ void i2c_init_board(void)
>  #endif
>  }
>  
> +#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT)
> +enum env_location env_get_location(enum env_operation op, int prio)
> +{
> +	switch (prio) {
> +	case 0:
> +		return ENVL_FAT;
> +
> +	case 1:
> +		return ENVL_MMC;

So even though the actual u-boot.bin for 64-bit boards is still somewhat
below the limit (~480KB), adding the ATF image (~32KB) pushes it over
the edge. So since v2017.11 u-boot.itb is already too big for the
traditional MMC env location.
So shall this "case 1:" be guarded by #ifndef CONFIG_ARM64, to not even
consider MMC for sunxi64 anymore?

Cheers,
Andre.

> +
> +	default:
> +		return ENVL_UNKNOWN;
> +	}
> +}
> +#endif
> +
>  /* add board specific code here */
>  int board_init(void)
>  {
>
Maxime Ripard Dec. 8, 2017, 8:42 a.m. | #2
Hi, Andre,

On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> Hi,

> 

> On 28/11/17 10:24, Maxime Ripard wrote:

> > The current environment has been hardcoded to an offset that starts to be

> > an issue given the current size of our main U-Boot binary.

> > 

> > By implementing a custom environment location routine, we can always favor

> > the FAT-based environment, and fallback to the MMC if we don't find

> > something in the FAT partition. We also implement the same order when

> > saving the environment, so that hopefully we can slowly migrate the users

> > over to FAT-based environment and away from the raw MMC one.

> > 

> > Eventually, and hopefully before we reach that limit again, we will have

> > most of our users using that setup, and we'll be able to retire the raw

> > environment, and gain more room for the U-Boot binary.

> > 

> > Reviewed-by: Lukasz Majewski <lukma@denx.de>

> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

> > ---

> >  board/sunxi/board.c | 16 ++++++++++++++++

> >  1 file changed, 16 insertions(+)

> > 

> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c

> > index dcacdf3e626d..8891961dcc6b 100644

> > --- a/board/sunxi/board.c

> > +++ b/board/sunxi/board.c

> > @@ -173,6 +173,22 @@ void i2c_init_board(void)

> >  #endif

> >  }

> >  

> > +#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT)

> > +enum env_location env_get_location(enum env_operation op, int prio)

> > +{

> > +	switch (prio) {

> > +	case 0:

> > +		return ENVL_FAT;

> > +

> > +	case 1:

> > +		return ENVL_MMC;

> 

> So even though the actual u-boot.bin for 64-bit boards is still somewhat

> below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> the edge. So since v2017.11 u-boot.itb is already too big for the

> traditional MMC env location.


Even with the alignment fix you merged recently?

> So shall this "case 1:" be guarded by #ifndef CONFIG_ARM64, to not even

> consider MMC for sunxi64 anymore?


I guess it's even simpler: you just don't enable ENV_IS_IN_MMC if
ARM64 is set :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard Dec. 19, 2017, 1:12 p.m. | #3
On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> So even though the actual u-boot.bin for 64-bit boards is still somewhat

> below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> the edge. So since v2017.11 u-boot.itb is already too big for the

> traditional MMC env location.


So I've had a quick look about what could go possibly go away in our
current armv8 config (using the pine64+ defconfig). Let me know if
some are actually vitals:

 - FIT_ENABLE_SHA256_SUPPORT
 - CONSOLE_MUX
 - CMD_CRC32
 - CMD_LZMADEC
 - CMD_UNZIP
 - CMD_LOADB
 - CMD_LOADS
 - CMD_MISC (actually implementing the command sleep)
 - ISO_PARTITION (yes. For CDROMs.)
 - VIDEO_BPP8, VIDEO_BPP16
 - VIDEO_ANSI
 - SHA256
 - LZMA

Removing those options make the u-boot.itb binary size going from
516kB to 478kB, making it functional again *and* allowing us to enable
the DT overlays that seem way more important than any feature
mentionned above (and bumps the size to 483kB).

Let me know what you think,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Tom Rini Dec. 19, 2017, 1:15 p.m. | #4
On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:
> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > So even though the actual u-boot.bin for 64-bit boards is still somewhat

> > below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > the edge. So since v2017.11 u-boot.itb is already too big for the

> > traditional MMC env location.

> 

> So I've had a quick look about what could go possibly go away in our

> current armv8 config (using the pine64+ defconfig). Let me know if

> some are actually vitals:

> 

>  - FIT_ENABLE_SHA256_SUPPORT

>  - CONSOLE_MUX

>  - CMD_CRC32

>  - CMD_LZMADEC

>  - CMD_UNZIP

>  - CMD_LOADB

>  - CMD_LOADS

>  - CMD_MISC (actually implementing the command sleep)

>  - ISO_PARTITION (yes. For CDROMs.)

>  - VIDEO_BPP8, VIDEO_BPP16

>  - VIDEO_ANSI

>  - SHA256

>  - LZMA

> 

> Removing those options make the u-boot.itb binary size going from

> 516kB to 478kB, making it functional again *and* allowing us to enable

> the DT overlays that seem way more important than any feature

> mentionned above (and bumps the size to 483kB).


You want to keep sha256 support in FIT images, we only have dropping
that allowed for some very odd use-cases.  And while I don't have a
strong opinion about unzip, lzma is one of the valid/likelu compressions
for an Image (and aside, I, or someone, needs to look into having
'booti' recognize various compression algorithms and automatically
decompress them) so I don't think we should get rid of that.

-- 
Tom
Maxime Ripard Dec. 19, 2017, 1:26 p.m. | #5
1;5002;0c
On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:
> On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:

> > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > > So even though the actual u-boot.bin for 64-bit boards is still somewhat

> > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > > the edge. So since v2017.11 u-boot.itb is already too big for the

> > > traditional MMC env location.

> > 

> > So I've had a quick look about what could go possibly go away in our

> > current armv8 config (using the pine64+ defconfig). Let me know if

> > some are actually vitals:

> > 

> >  - FIT_ENABLE_SHA256_SUPPORT

> >  - CONSOLE_MUX

> >  - CMD_CRC32

> >  - CMD_LZMADEC

> >  - CMD_UNZIP

> >  - CMD_LOADB

> >  - CMD_LOADS

> >  - CMD_MISC (actually implementing the command sleep)

> >  - ISO_PARTITION (yes. For CDROMs.)

> >  - VIDEO_BPP8, VIDEO_BPP16

> >  - VIDEO_ANSI

> >  - SHA256

> >  - LZMA

> > 

> > Removing those options make the u-boot.itb binary size going from

> > 516kB to 478kB, making it functional again *and* allowing us to enable

> > the DT overlays that seem way more important than any feature

> > mentionned above (and bumps the size to 483kB).

> 

> You want to keep sha256 support in FIT images, we only have dropping

> that allowed for some very odd use-cases.  And while I don't have a

> strong opinion about unzip, lzma is one of the valid/likelu compressions

> for an Image (and aside, I, or someone, needs to look into having

> 'booti' recognize various compression algorithms and automatically

> decompress them) so I don't think we should get rid of that.


So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at
499kB. Still tight, but it fits.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Alexander Graf Dec. 19, 2017, 1:28 p.m. | #6
On 12/19/2017 02:12 PM, Maxime Ripard wrote:
> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>> the edge. So since v2017.11 u-boot.itb is already too big for the
>> traditional MMC env location.
> So I've had a quick look about what could go possibly go away in our
> current armv8 config (using the pine64+ defconfig). Let me know if
> some are actually vitals:
>
>   - FIT_ENABLE_SHA256_SUPPORT
>   - CONSOLE_MUX
>   - CMD_CRC32

We use this in travis tests, but since no Allwinner systems are present 
there, I guess we don't care.

>   - CMD_LZMADEC
>   - CMD_UNZIP
>   - CMD_LOADB
>   - CMD_LOADS
>   - CMD_MISC (actually implementing the command sleep)
>   - ISO_PARTITION (yes. For CDROMs.)

This one is needed to boot openSUSE or SLES images, since they come as 
.iso with el torito (which then gets booted using distro + bootefi).

>   - VIDEO_BPP8, VIDEO_BPP16
>   - VIDEO_ANSI
>   - SHA256
>   - LZMA
>
> Removing those options make the u-boot.itb binary size going from
> 516kB to 478kB, making it functional again *and* allowing us to enable
> the DT overlays that seem way more important than any feature
> mentionned above (and bumps the size to 483kB).
>
> Let me know what you think,

Out of the ones above, only ISO_PARTITION is the one I would definitely 
care about; please don't remove that one ;)


Alex
Tom Rini Dec. 19, 2017, 1:30 p.m. | #7
On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:
> 1;5002;0c

> On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:

> > On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:

> > > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > > > So even though the actual u-boot.bin for 64-bit boards is still somewhat

> > > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > > > the edge. So since v2017.11 u-boot.itb is already too big for the

> > > > traditional MMC env location.

> > > 

> > > So I've had a quick look about what could go possibly go away in our

> > > current armv8 config (using the pine64+ defconfig). Let me know if

> > > some are actually vitals:

> > > 

> > >  - FIT_ENABLE_SHA256_SUPPORT

> > >  - CONSOLE_MUX

> > >  - CMD_CRC32

> > >  - CMD_LZMADEC

> > >  - CMD_UNZIP

> > >  - CMD_LOADB

> > >  - CMD_LOADS

> > >  - CMD_MISC (actually implementing the command sleep)

> > >  - ISO_PARTITION (yes. For CDROMs.)

> > >  - VIDEO_BPP8, VIDEO_BPP16

> > >  - VIDEO_ANSI

> > >  - SHA256

> > >  - LZMA

> > > 

> > > Removing those options make the u-boot.itb binary size going from

> > > 516kB to 478kB, making it functional again *and* allowing us to enable

> > > the DT overlays that seem way more important than any feature

> > > mentionned above (and bumps the size to 483kB).

> > 

> > You want to keep sha256 support in FIT images, we only have dropping

> > that allowed for some very odd use-cases.  And while I don't have a

> > strong opinion about unzip, lzma is one of the valid/likelu compressions

> > for an Image (and aside, I, or someone, needs to look into having

> > 'booti' recognize various compression algorithms and automatically

> > decompress them) so I don't think we should get rid of that.

> 

> So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at

> 499kB. Still tight, but it fits.


Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?

-- 
Tom
Tom Rini Dec. 19, 2017, 1:31 p.m. | #8
On Tue, Dec 19, 2017 at 02:28:03PM +0100, Alexander Graf wrote:
> On 12/19/2017 02:12 PM, Maxime Ripard wrote:

> >On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> >>So even though the actual u-boot.bin for 64-bit boards is still somewhat

> >>below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> >>the edge. So since v2017.11 u-boot.itb is already too big for the

> >>traditional MMC env location.

> >So I've had a quick look about what could go possibly go away in our

> >current armv8 config (using the pine64+ defconfig). Let me know if

> >some are actually vitals:

> >

> >  - FIT_ENABLE_SHA256_SUPPORT

> >  - CONSOLE_MUX

> >  - CMD_CRC32

> 

> We use this in travis tests, but since no Allwinner systems are present

> there, I guess we don't care.


Note that tests that require CMD_CRC32 should be marked as such too.

-- 
Tom
André Przywara Dec. 19, 2017, 1:38 p.m. | #9
Hi Maxime,

thanks for having a look!

On 19/12/17 13:12, Maxime Ripard wrote:
> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>> the edge. So since v2017.11 u-boot.itb is already too big for the
>> traditional MMC env location.
> 
> So I've had a quick look about what could go possibly go away in our
> current armv8 config (using the pine64+ defconfig). Let me know if
> some are actually vitals:
> 
>  - FIT_ENABLE_SHA256_SUPPORT
>  - CONSOLE_MUX
>  - CMD_CRC32
>  - CMD_LZMADEC
>  - CMD_UNZIP
>  - CMD_LOADB
>  - CMD_LOADS
>  - CMD_MISC (actually implementing the command sleep)
>  - ISO_PARTITION (yes. For CDROMs.)

As Alex mentioned, this is needed for some installer images, which come
as ISOs. So if possible, we should keep this in.

>  - VIDEO_BPP8, VIDEO_BPP16
>  - VIDEO_ANSI
>  - SHA256
>  - LZMA

From just looking at the names I am fine with the rest gone. But let me
test tonight if there are any side effects.

Some of them seem useful, but I would leave enabling them to the actual
users. If someone needs it, they can enable them and loose the raw MMC
environment. I think this is a fair trade-off.

> Removing those options make the u-boot.itb binary size going from
> 516kB to 478kB, making it functional again *and* allowing us to enable
> the DT overlays that seem way more important than any feature
> mentionned above (and bumps the size to 483kB).

How important is the raw MMC environment for the ARM64 boards, actually?
Most of the rationale for the 32-bit side seemed to apply to legacy use
cases only. Do we have reports/complaints from 64-bit users?

Cheers,
Andre.
Mark Kettenis Dec. 19, 2017, 1:41 p.m. | #10
> Date: Tue, 19 Dec 2017 14:12:02 +0100
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> > So even though the actual u-boot.bin for 64-bit boards is still somewhat
> > below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> > the edge. So since v2017.11 u-boot.itb is already too big for the
> > traditional MMC env location.
> 
> So I've had a quick look about what could go possibly go away in our
> current armv8 config (using the pine64+ defconfig). Let me know if
> some are actually vitals:
> 
>  - FIT_ENABLE_SHA256_SUPPORT
>  - CONSOLE_MUX
>  - CMD_CRC32
>  - CMD_LZMADEC
>  - CMD_UNZIP
>  - CMD_LOADB
>  - CMD_LOADS
>  - CMD_MISC (actually implementing the command sleep)
>  - ISO_PARTITION (yes. For CDROMs.)
>  - VIDEO_BPP8, VIDEO_BPP16
>  - VIDEO_ANSI
>  - SHA256
>  - LZMA
> 
> Removing those options make the u-boot.itb binary size going from
> 516kB to 478kB, making it functional again *and* allowing us to enable
> the DT overlays that seem way more important than any feature
> mentionned above (and bumps the size to 483kB).

So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial
console and usb keyboard are present?  Is the usb keyboard used when
it is plugged in but serial otherwise?

In any case, this would be just a stopgap measure for 2018.01 wouldn't
it?  We really have to move to the FAT-based environment or just bite
the bullet and move the location of the environment.  Personally I
don't cutsomize my environment beyond setting boot_targets.  So losing
my old settings upon upgrade is not a big issue for me.
Mark Kettenis Dec. 19, 2017, 1:51 p.m. | #11
> From: Andre Przywara <andre.przywara@arm.com>
> Date: Tue, 19 Dec 2017 13:38:59 +0000
> 
> Hi Maxime,
> 
> thanks for having a look!
> 
> On 19/12/17 13:12, Maxime Ripard wrote:
> > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> >> So even though the actual u-boot.bin for 64-bit boards is still somewhat
> >> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> >> the edge. So since v2017.11 u-boot.itb is already too big for the
> >> traditional MMC env location.
> > 
> > So I've had a quick look about what could go possibly go away in our
> > current armv8 config (using the pine64+ defconfig). Let me know if
> > some are actually vitals:
> > 
> >  - FIT_ENABLE_SHA256_SUPPORT
> >  - CONSOLE_MUX
> >  - CMD_CRC32
> >  - CMD_LZMADEC
> >  - CMD_UNZIP
> >  - CMD_LOADB
> >  - CMD_LOADS
> >  - CMD_MISC (actually implementing the command sleep)
> >  - ISO_PARTITION (yes. For CDROMs.)
> 
> As Alex mentioned, this is needed for some installer images, which come
> as ISOs. So if possible, we should keep this in.
> 
> >  - VIDEO_BPP8, VIDEO_BPP16
> >  - VIDEO_ANSI
> >  - SHA256
> >  - LZMA
> 
> From just looking at the names I am fine with the rest gone. But let me
> test tonight if there are any side effects.
> 
> Some of them seem useful, but I would leave enabling them to the actual
> users. If someone needs it, they can enable them and loose the raw MMC
> environment. I think this is a fair trade-off.
> 
> > Removing those options make the u-boot.itb binary size going from
> > 516kB to 478kB, making it functional again *and* allowing us to enable
> > the DT overlays that seem way more important than any feature
> > mentionned above (and bumps the size to 483kB).
> 
> How important is the raw MMC environment for the ARM64 boards, actually?
> Most of the rationale for the 32-bit side seemed to apply to legacy use
> cases only. Do we have reports/complaints from 64-bit users?

For me/us (OpenBSD) the environment is still important.  I have many
setups where U-Boot lives on a uSD card but the installed OS lives on
a USB device.  In that scenario I set boot_targets to boot the EFI
bootloader and OS off the USB disk.  This is very helpfull for testing
new versions of U-Boot as I can simply swap the uSD card.  But for
some setups this is essential as OpenBSD doesn't support the SD/MCC
controller on all ARM hardware yet (but we do support it on
Allwinner).

Cheers,

Mark
Maxime Ripard Dec. 19, 2017, 2:09 p.m. | #12
On Tue, Dec 19, 2017 at 08:30:31AM -0500, Tom Rini wrote:
> On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:
> > 1;5002;0c
> > On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:
> > > On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:
> > > > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> > > > > So even though the actual u-boot.bin for 64-bit boards is still somewhat
> > > > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> > > > > the edge. So since v2017.11 u-boot.itb is already too big for the
> > > > > traditional MMC env location.
> > > > 
> > > > So I've had a quick look about what could go possibly go away in our
> > > > current armv8 config (using the pine64+ defconfig). Let me know if
> > > > some are actually vitals:
> > > > 
> > > >  - FIT_ENABLE_SHA256_SUPPORT
> > > >  - CONSOLE_MUX
> > > >  - CMD_CRC32
> > > >  - CMD_LZMADEC
> > > >  - CMD_UNZIP
> > > >  - CMD_LOADB
> > > >  - CMD_LOADS
> > > >  - CMD_MISC (actually implementing the command sleep)
> > > >  - ISO_PARTITION (yes. For CDROMs.)
> > > >  - VIDEO_BPP8, VIDEO_BPP16
> > > >  - VIDEO_ANSI
> > > >  - SHA256
> > > >  - LZMA
> > > > 
> > > > Removing those options make the u-boot.itb binary size going from
> > > > 516kB to 478kB, making it functional again *and* allowing us to enable
> > > > the DT overlays that seem way more important than any feature
> > > > mentionned above (and bumps the size to 483kB).
> > > 
> > > You want to keep sha256 support in FIT images, we only have dropping
> > > that allowed for some very odd use-cases.  And while I don't have a
> > > strong opinion about unzip, lzma is one of the valid/likelu compressions
> > > for an Image (and aside, I, or someone, needs to look into having
> > > 'booti' recognize various compression algorithms and automatically
> > > decompress them) so I don't think we should get rid of that.
> > 
> > So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at
> > 499kB. Still tight, but it fits.
> 
> Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?

And we're at 498kB now! :)

Is CMD_ELF useful as well?

Maxime
Maxime Ripard Dec. 19, 2017, 2:13 p.m. | #13
On Tue, Dec 19, 2017 at 02:28:03PM +0100, Alexander Graf wrote:
> On 12/19/2017 02:12 PM, Maxime Ripard wrote:
> >   - VIDEO_BPP8, VIDEO_BPP16
> >   - VIDEO_ANSI
> >   - SHA256
> >   - LZMA
> > 
> > Removing those options make the u-boot.itb binary size going from
> > 516kB to 478kB, making it functional again *and* allowing us to enable
> > the DT overlays that seem way more important than any feature
> > mentionned above (and bumps the size to 483kB).
> > 
> > Let me know what you think,
> 
> Out of the ones above, only ISO_PARTITION is the one I would definitely care
> about; please don't remove that one ;)

Well, I guess you're never done with the 90s ;)

And who knows, maybe it'll be hype again in a few years :)

Maxime
Emmanuel Vadot Dec. 19, 2017, 2:16 p.m. | #14
On Tue, 19 Dec 2017 15:09:52 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Tue, Dec 19, 2017 at 08:30:31AM -0500, Tom Rini wrote:
> > On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:
> > > 1;5002;0c
> > > On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:
> > > > On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:
> > > > > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> > > > > > So even though the actual u-boot.bin for 64-bit boards is still somewhat
> > > > > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> > > > > > the edge. So since v2017.11 u-boot.itb is already too big for the
> > > > > > traditional MMC env location.
> > > > > 
> > > > > So I've had a quick look about what could go possibly go away in our
> > > > > current armv8 config (using the pine64+ defconfig). Let me know if
> > > > > some are actually vitals:
> > > > > 
> > > > >  - FIT_ENABLE_SHA256_SUPPORT
> > > > >  - CONSOLE_MUX
> > > > >  - CMD_CRC32
> > > > >  - CMD_LZMADEC
> > > > >  - CMD_UNZIP
> > > > >  - CMD_LOADB
> > > > >  - CMD_LOADS
> > > > >  - CMD_MISC (actually implementing the command sleep)
> > > > >  - ISO_PARTITION (yes. For CDROMs.)
> > > > >  - VIDEO_BPP8, VIDEO_BPP16
> > > > >  - VIDEO_ANSI
> > > > >  - SHA256
> > > > >  - LZMA
> > > > > 
> > > > > Removing those options make the u-boot.itb binary size going from
> > > > > 516kB to 478kB, making it functional again *and* allowing us to enable
> > > > > the DT overlays that seem way more important than any feature
> > > > > mentionned above (and bumps the size to 483kB).
> > > > 
> > > > You want to keep sha256 support in FIT images, we only have dropping
> > > > that allowed for some very odd use-cases.  And while I don't have a
> > > > strong opinion about unzip, lzma is one of the valid/likelu compressions
> > > > for an Image (and aside, I, or someone, needs to look into having
> > > > 'booti' recognize various compression algorithms and automatically
> > > > decompress them) so I don't think we should get rid of that.
> > > 
> > > So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at
> > > 499kB. Still tight, but it fits.
> > 
> > Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?
> 
> And we're at 498kB now! :)
> 
> Is CMD_ELF useful as well?

 Speaking for the BSDs, FreeBSD and OpenBSD are using EFI, NetBSD is
using uImage, CMD_ELF <might> be useful for test but people who run
test on u-boot should know how to configure it.

> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
André Przywara Dec. 19, 2017, 2:17 p.m. | #15
Hi,

On 19/12/17 13:51, Mark Kettenis wrote:
>> From: Andre Przywara <andre.przywara@arm.com>
>> Date: Tue, 19 Dec 2017 13:38:59 +0000
>>
>> Hi Maxime,
>>
>> thanks for having a look!
>>
>> On 19/12/17 13:12, Maxime Ripard wrote:
>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>> traditional MMC env location.
>>>
>>> So I've had a quick look about what could go possibly go away in our
>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>> some are actually vitals:
>>>
>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>  - CONSOLE_MUX
>>>  - CMD_CRC32
>>>  - CMD_LZMADEC
>>>  - CMD_UNZIP
>>>  - CMD_LOADB
>>>  - CMD_LOADS
>>>  - CMD_MISC (actually implementing the command sleep)
>>>  - ISO_PARTITION (yes. For CDROMs.)
>>
>> As Alex mentioned, this is needed for some installer images, which come
>> as ISOs. So if possible, we should keep this in.
>>
>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>  - VIDEO_ANSI
>>>  - SHA256
>>>  - LZMA
>>
>> From just looking at the names I am fine with the rest gone. But let me
>> test tonight if there are any side effects.
>>
>> Some of them seem useful, but I would leave enabling them to the actual
>> users. If someone needs it, they can enable them and loose the raw MMC
>> environment. I think this is a fair trade-off.
>>
>>> Removing those options make the u-boot.itb binary size going from
>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>> the DT overlays that seem way more important than any feature
>>> mentionned above (and bumps the size to 483kB).
>>
>> How important is the raw MMC environment for the ARM64 boards, actually?
>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>> cases only. Do we have reports/complaints from 64-bit users?
> 
> For me/us (OpenBSD) the environment is still important.  I have many
> setups where U-Boot lives on a uSD card but the installed OS lives on
> a USB device.  In that scenario I set boot_targets to boot the EFI
> bootloader and OS off the USB disk.  This is very helpfull for testing
> new versions of U-Boot as I can simply swap the uSD card.  But for
> some setups this is essential as OpenBSD doesn't support the SD/MCC
> controller on all ARM hardware yet (but we do support it on
> Allwinner).

I see, but I wasn't arguing for dropping the environment altogether,
more for supporting FAT environments *only*.
So how important is preserving existing environments over a firmware
update in your scenario? I think this is the killer question here, isn't
it? I'm inclined to just drop raw MMC environment support from sunxi64
boards and then enjoy the ~450KB more worth of code, until we hit the
first MB boundary.
I have builds with all (DDR3) A64 board DTs in the binary [1], which
would be larger than 504K anyway.

Cheers,
Andre.

[1] https://github.com/apritzel/pine64/commit/ee12bea43
Maxime Ripard Dec. 19, 2017, 2:20 p.m. | #16
On Tue, Dec 19, 2017 at 01:38:59PM +0000, Andre Przywara wrote:
> Hi Maxime,
> 
> thanks for having a look!
> 
> On 19/12/17 13:12, Maxime Ripard wrote:
> > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> >> So even though the actual u-boot.bin for 64-bit boards is still somewhat
> >> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> >> the edge. So since v2017.11 u-boot.itb is already too big for the
> >> traditional MMC env location.
> > 
> > So I've had a quick look about what could go possibly go away in our
> > current armv8 config (using the pine64+ defconfig). Let me know if
> > some are actually vitals:
> > 
> >  - FIT_ENABLE_SHA256_SUPPORT
> >  - CONSOLE_MUX
> >  - CMD_CRC32
> >  - CMD_LZMADEC
> >  - CMD_UNZIP
> >  - CMD_LOADB
> >  - CMD_LOADS
> >  - CMD_MISC (actually implementing the command sleep)
> >  - ISO_PARTITION (yes. For CDROMs.)
> 
> As Alex mentioned, this is needed for some installer images, which come
> as ISOs. So if possible, we should keep this in.

So, with FIT_ENABLE_SHA256_SUPPORT, LZMADEC, ISO_PARTITION and the
overlay support, we're at 500kB.

Again, tight, but under the limit.

> >  - VIDEO_BPP8, VIDEO_BPP16
> >  - VIDEO_ANSI
> >  - SHA256
> >  - LZMA
> 
> From just looking at the names I am fine with the rest gone. But let me
> test tonight if there are any side effects.
> 
> Some of them seem useful, but I would leave enabling them to the actual
> users. If someone needs it, they can enable them and loose the raw MMC
> environment. I think this is a fair trade-off.

Yes, that's my view too.

> > Removing those options make the u-boot.itb binary size going from
> > 516kB to 478kB, making it functional again *and* allowing us to enable
> > the DT overlays that seem way more important than any feature
> > mentionned above (and bumps the size to 483kB).
> 
> How important is the raw MMC environment for the ARM64 boards, actually?
> Most of the rationale for the 32-bit side seemed to apply to legacy use
> cases only. Do we have reports/complaints from 64-bit users?

Pretty much as important as it is on arm I guess. We just have less
history, but the same use cases.

I'd really like to give at least one release for transition, which
would mean having a schedule like this:

  - in 2018.01, merge those config removals so that we have least have
    something that works quite fast

  - in 2018.03, merge the multiple environment patches. We seem to
    have reached a consensus here, so I'm not really concerned that we
    will have it merged.

  - in 2018.05, if needed, remove the raw MMC options and complete the
    transition to FAT.

Maxime
Tom Rini Dec. 19, 2017, 2:20 p.m. | #17
On Tue, Dec 19, 2017 at 03:09:52PM +0100, Maxime Ripard wrote:
> On Tue, Dec 19, 2017 at 08:30:31AM -0500, Tom Rini wrote:

> > On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:

> > > 1;5002;0c

> > > On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:

> > > > On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:

> > > > > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > > > > > So even though the actual u-boot.bin for 64-bit boards is still somewhat

> > > > > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > > > > > the edge. So since v2017.11 u-boot.itb is already too big for the

> > > > > > traditional MMC env location.

> > > > > 

> > > > > So I've had a quick look about what could go possibly go away in our

> > > > > current armv8 config (using the pine64+ defconfig). Let me know if

> > > > > some are actually vitals:

> > > > > 

> > > > >  - FIT_ENABLE_SHA256_SUPPORT

> > > > >  - CONSOLE_MUX

> > > > >  - CMD_CRC32

> > > > >  - CMD_LZMADEC

> > > > >  - CMD_UNZIP

> > > > >  - CMD_LOADB

> > > > >  - CMD_LOADS

> > > > >  - CMD_MISC (actually implementing the command sleep)

> > > > >  - ISO_PARTITION (yes. For CDROMs.)

> > > > >  - VIDEO_BPP8, VIDEO_BPP16

> > > > >  - VIDEO_ANSI

> > > > >  - SHA256

> > > > >  - LZMA

> > > > > 

> > > > > Removing those options make the u-boot.itb binary size going from

> > > > > 516kB to 478kB, making it functional again *and* allowing us to enable

> > > > > the DT overlays that seem way more important than any feature

> > > > > mentionned above (and bumps the size to 483kB).

> > > > 

> > > > You want to keep sha256 support in FIT images, we only have dropping

> > > > that allowed for some very odd use-cases.  And while I don't have a

> > > > strong opinion about unzip, lzma is one of the valid/likelu compressions

> > > > for an Image (and aside, I, or someone, needs to look into having

> > > > 'booti' recognize various compression algorithms and automatically

> > > > decompress them) so I don't think we should get rid of that.

> > > 

> > > So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at

> > > 499kB. Still tight, but it fits.

> > 

> > Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?

> 

> And we're at 498kB now! :)

> 

> Is CMD_ELF useful as well?


That's a good question.  In 32bit land, that's how you're going to
launch FreeRTOS and proprietary apps and so forth.  I don't know if for
aarch64 people will still be doing ELF applications or UEFI applications
for all of this.

-- 
Tom
André Przywara Dec. 19, 2017, 2:22 p.m. | #18
Hi,

On 19/12/17 14:20, Tom Rini wrote:
> On Tue, Dec 19, 2017 at 03:09:52PM +0100, Maxime Ripard wrote:
>> On Tue, Dec 19, 2017 at 08:30:31AM -0500, Tom Rini wrote:
>>> On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:
>>>> 1;5002;0c
>>>> On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:
>>>>> On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:
>>>>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>>>>> traditional MMC env location.
>>>>>>
>>>>>> So I've had a quick look about what could go possibly go away in our
>>>>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>>>>> some are actually vitals:
>>>>>>
>>>>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>>>>  - CONSOLE_MUX
>>>>>>  - CMD_CRC32
>>>>>>  - CMD_LZMADEC
>>>>>>  - CMD_UNZIP
>>>>>>  - CMD_LOADB
>>>>>>  - CMD_LOADS
>>>>>>  - CMD_MISC (actually implementing the command sleep)
>>>>>>  - ISO_PARTITION (yes. For CDROMs.)
>>>>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>>>>  - VIDEO_ANSI
>>>>>>  - SHA256
>>>>>>  - LZMA
>>>>>>
>>>>>> Removing those options make the u-boot.itb binary size going from
>>>>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>>>>> the DT overlays that seem way more important than any feature
>>>>>> mentionned above (and bumps the size to 483kB).
>>>>>
>>>>> You want to keep sha256 support in FIT images, we only have dropping
>>>>> that allowed for some very odd use-cases.  And while I don't have a
>>>>> strong opinion about unzip, lzma is one of the valid/likelu compressions
>>>>> for an Image (and aside, I, or someone, needs to look into having
>>>>> 'booti' recognize various compression algorithms and automatically
>>>>> decompress them) so I don't think we should get rid of that.
>>>>
>>>> So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at
>>>> 499kB. Still tight, but it fits.
>>>
>>> Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?
>>
>> And we're at 498kB now! :)
>>
>> Is CMD_ELF useful as well?
> 
> That's a good question.  In 32bit land, that's how you're going to
> launch FreeRTOS and proprietary apps and so forth.  I don't know if for
> aarch64 people will still be doing ELF applications or UEFI applications
> for all of this.

Eventually we will be, but for now (and the *defconfig*) I think it's
fine to drop it. If we stick to Maxime's plan, we can get it back in 6
months or so.

Cheers,
Andre.
>
Tom Rini Dec. 19, 2017, 2:24 p.m. | #19
On Tue, Dec 19, 2017 at 02:22:59PM +0000, Andre Przywara wrote:
> Hi,

> 

> On 19/12/17 14:20, Tom Rini wrote:

> > On Tue, Dec 19, 2017 at 03:09:52PM +0100, Maxime Ripard wrote:

> >> On Tue, Dec 19, 2017 at 08:30:31AM -0500, Tom Rini wrote:

> >>> On Tue, Dec 19, 2017 at 02:26:40PM +0100, Maxime Ripard wrote:

> >>>> 1;5002;0c

> >>>> On Tue, Dec 19, 2017 at 08:15:31AM -0500, Tom Rini wrote:

> >>>>> On Tue, Dec 19, 2017 at 02:12:02PM +0100, Maxime Ripard wrote:

> >>>>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> >>>>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat

> >>>>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> >>>>>>> the edge. So since v2017.11 u-boot.itb is already too big for the

> >>>>>>> traditional MMC env location.

> >>>>>>

> >>>>>> So I've had a quick look about what could go possibly go away in our

> >>>>>> current armv8 config (using the pine64+ defconfig). Let me know if

> >>>>>> some are actually vitals:

> >>>>>>

> >>>>>>  - FIT_ENABLE_SHA256_SUPPORT

> >>>>>>  - CONSOLE_MUX

> >>>>>>  - CMD_CRC32

> >>>>>>  - CMD_LZMADEC

> >>>>>>  - CMD_UNZIP

> >>>>>>  - CMD_LOADB

> >>>>>>  - CMD_LOADS

> >>>>>>  - CMD_MISC (actually implementing the command sleep)

> >>>>>>  - ISO_PARTITION (yes. For CDROMs.)

> >>>>>>  - VIDEO_BPP8, VIDEO_BPP16

> >>>>>>  - VIDEO_ANSI

> >>>>>>  - SHA256

> >>>>>>  - LZMA

> >>>>>>

> >>>>>> Removing those options make the u-boot.itb binary size going from

> >>>>>> 516kB to 478kB, making it functional again *and* allowing us to enable

> >>>>>> the DT overlays that seem way more important than any feature

> >>>>>> mentionned above (and bumps the size to 483kB).

> >>>>>

> >>>>> You want to keep sha256 support in FIT images, we only have dropping

> >>>>> that allowed for some very odd use-cases.  And while I don't have a

> >>>>> strong opinion about unzip, lzma is one of the valid/likelu compressions

> >>>>> for an Image (and aside, I, or someone, needs to look into having

> >>>>> 'booti' recognize various compression algorithms and automatically

> >>>>> decompress them) so I don't think we should get rid of that.

> >>>>

> >>>> So, adding back CMD_LZMADEC and FIT_ENABLE_SHA256_SUPPORT, we're at

> >>>> 499kB. Still tight, but it fits.

> >>>

> >>> Looking over myself, perhaps drop CMD_BOOTD and move LOGLEVEL to 3?

> >>

> >> And we're at 498kB now! :)

> >>

> >> Is CMD_ELF useful as well?

> > 

> > That's a good question.  In 32bit land, that's how you're going to

> > launch FreeRTOS and proprietary apps and so forth.  I don't know if for

> > aarch64 people will still be doing ELF applications or UEFI applications

> > for all of this.

> 

> Eventually we will be, but for now (and the *defconfig*) I think it's

> fine to drop it. If we stick to Maxime's plan, we can get it back in 6

> months or so.


If we have to, we have to.  But I am weary of dropping and then
re-adding functionality like that as you never know what version some
company/vendor/etc is going to pick up and run with.  If we can't keep
the limit without dropping ELF, well, then we can't.

-- 
Tom
André Przywara Dec. 19, 2017, 2:27 p.m. | #20
Hi Maxime,

On 19/12/17 14:20, Maxime Ripard wrote:
> On Tue, Dec 19, 2017 at 01:38:59PM +0000, Andre Przywara wrote:
>> Hi Maxime,
>>
>> thanks for having a look!
>>
>> On 19/12/17 13:12, Maxime Ripard wrote:
>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>> traditional MMC env location.
>>>
>>> So I've had a quick look about what could go possibly go away in our
>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>> some are actually vitals:
>>>
>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>  - CONSOLE_MUX
>>>  - CMD_CRC32
>>>  - CMD_LZMADEC
>>>  - CMD_UNZIP
>>>  - CMD_LOADB
>>>  - CMD_LOADS
>>>  - CMD_MISC (actually implementing the command sleep)
>>>  - ISO_PARTITION (yes. For CDROMs.)
>>
>> As Alex mentioned, this is needed for some installer images, which come
>> as ISOs. So if possible, we should keep this in.
> 
> So, with FIT_ENABLE_SHA256_SUPPORT, LZMADEC, ISO_PARTITION and the
> overlay support, we're at 500kB.
> 
> Again, tight, but under the limit.

Phew! ;-)

> 
>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>  - VIDEO_ANSI
>>>  - SHA256
>>>  - LZMA
>>
>> From just looking at the names I am fine with the rest gone. But let me
>> test tonight if there are any side effects.
>>
>> Some of them seem useful, but I would leave enabling them to the actual
>> users. If someone needs it, they can enable them and loose the raw MMC
>> environment. I think this is a fair trade-off.
> 
> Yes, that's my view too.
> 
>>> Removing those options make the u-boot.itb binary size going from
>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>> the DT overlays that seem way more important than any feature
>>> mentionned above (and bumps the size to 483kB).
>>
>> How important is the raw MMC environment for the ARM64 boards, actually?
>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>> cases only. Do we have reports/complaints from 64-bit users?
> 
> Pretty much as important as it is on arm I guess. We just have less
> history, but the same use cases.
> 
> I'd really like to give at least one release for transition, which
> would mean having a schedule like this:
> 
>   - in 2018.01, merge those config removals so that we have least have
>     something that works quite fast
> 
>   - in 2018.03, merge the multiple environment patches. We seem to
>     have reached a consensus here, so I'm not really concerned that we
>     will have it merged.
> 
>   - in 2018.05, if needed, remove the raw MMC options and complete the
>     transition to FAT.

That sounds very reasonable to me, thanks for writing this down!

This gives us an only intermediate shortage of features for defconfig.

We should encourage everyone who builds (and ships) firmware right now
to drop MMC env support already, if they tinker with the .config anyway.
That is, if there is no legacy usage to be concerned about.

Cheers,
Andre.
Jagan Teki Dec. 19, 2017, 2:38 p.m. | #21
On Tue, Dec 19, 2017 at 7:57 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Maxime,
>
> On 19/12/17 14:20, Maxime Ripard wrote:
>> On Tue, Dec 19, 2017 at 01:38:59PM +0000, Andre Przywara wrote:
>>> Hi Maxime,
>>>
>>> thanks for having a look!
>>>
>>> On 19/12/17 13:12, Maxime Ripard wrote:
>>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>>> traditional MMC env location.
>>>>
>>>> So I've had a quick look about what could go possibly go away in our
>>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>>> some are actually vitals:
>>>>
>>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>>  - CONSOLE_MUX
>>>>  - CMD_CRC32
>>>>  - CMD_LZMADEC
>>>>  - CMD_UNZIP
>>>>  - CMD_LOADB
>>>>  - CMD_LOADS
>>>>  - CMD_MISC (actually implementing the command sleep)
>>>>  - ISO_PARTITION (yes. For CDROMs.)
>>>
>>> As Alex mentioned, this is needed for some installer images, which come
>>> as ISOs. So if possible, we should keep this in.
>>
>> So, with FIT_ENABLE_SHA256_SUPPORT, LZMADEC, ISO_PARTITION and the
>> overlay support, we're at 500kB.
>>
>> Again, tight, but under the limit.
>
> Phew! ;-)
>
>>
>>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>>  - VIDEO_ANSI
>>>>  - SHA256
>>>>  - LZMA
>>>
>>> From just looking at the names I am fine with the rest gone. But let me
>>> test tonight if there are any side effects.
>>>
>>> Some of them seem useful, but I would leave enabling them to the actual
>>> users. If someone needs it, they can enable them and loose the raw MMC
>>> environment. I think this is a fair trade-off.
>>
>> Yes, that's my view too.
>>
>>>> Removing those options make the u-boot.itb binary size going from
>>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>>> the DT overlays that seem way more important than any feature
>>>> mentionned above (and bumps the size to 483kB).
>>>
>>> How important is the raw MMC environment for the ARM64 boards, actually?
>>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>>> cases only. Do we have reports/complaints from 64-bit users?
>>
>> Pretty much as important as it is on arm I guess. We just have less
>> history, but the same use cases.
>>
>> I'd really like to give at least one release for transition, which
>> would mean having a schedule like this:
>>
>>   - in 2018.01, merge those config removals so that we have least have
>>     something that works quite fast
>>
>>   - in 2018.03, merge the multiple environment patches. We seem to
>>     have reached a consensus here, so I'm not really concerned that we
>>     will have it merged.
>>
>>   - in 2018.05, if needed, remove the raw MMC options and complete the
>>     transition to FAT.
>
> That sounds very reasonable to me, thanks for writing this down!
>
> This gives us an only intermediate shortage of features for defconfig.
>
> We should encourage everyone who builds (and ships) firmware right now
> to drop MMC env support already, if they tinker with the .config anyway.
> That is, if there is no legacy usage to be concerned about.

All these trimming(if it fits) seems to be nice for now, but what if
once driver-model MMC, reset, pinctrl, clk, regulator are IN?
of-course we can't do anything with SPL size because of SRAM
constraints, but U-Boot proper size? why can't we increase the u-boot
partition size?

thanks!
Chen-Yu Tsai Dec. 19, 2017, 2:41 p.m. | #22
On Tue, Dec 19, 2017 at 10:38 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Tue, Dec 19, 2017 at 7:57 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi Maxime,
>>
>> On 19/12/17 14:20, Maxime Ripard wrote:
>>> On Tue, Dec 19, 2017 at 01:38:59PM +0000, Andre Przywara wrote:
>>>> Hi Maxime,
>>>>
>>>> thanks for having a look!
>>>>
>>>> On 19/12/17 13:12, Maxime Ripard wrote:
>>>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>>>> traditional MMC env location.
>>>>>
>>>>> So I've had a quick look about what could go possibly go away in our
>>>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>>>> some are actually vitals:
>>>>>
>>>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>>>  - CONSOLE_MUX
>>>>>  - CMD_CRC32
>>>>>  - CMD_LZMADEC
>>>>>  - CMD_UNZIP
>>>>>  - CMD_LOADB
>>>>>  - CMD_LOADS
>>>>>  - CMD_MISC (actually implementing the command sleep)
>>>>>  - ISO_PARTITION (yes. For CDROMs.)
>>>>
>>>> As Alex mentioned, this is needed for some installer images, which come
>>>> as ISOs. So if possible, we should keep this in.
>>>
>>> So, with FIT_ENABLE_SHA256_SUPPORT, LZMADEC, ISO_PARTITION and the
>>> overlay support, we're at 500kB.
>>>
>>> Again, tight, but under the limit.
>>
>> Phew! ;-)
>>
>>>
>>>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>>>  - VIDEO_ANSI
>>>>>  - SHA256
>>>>>  - LZMA
>>>>
>>>> From just looking at the names I am fine with the rest gone. But let me
>>>> test tonight if there are any side effects.
>>>>
>>>> Some of them seem useful, but I would leave enabling them to the actual
>>>> users. If someone needs it, they can enable them and loose the raw MMC
>>>> environment. I think this is a fair trade-off.
>>>
>>> Yes, that's my view too.
>>>
>>>>> Removing those options make the u-boot.itb binary size going from
>>>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>>>> the DT overlays that seem way more important than any feature
>>>>> mentionned above (and bumps the size to 483kB).
>>>>
>>>> How important is the raw MMC environment for the ARM64 boards, actually?
>>>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>>>> cases only. Do we have reports/complaints from 64-bit users?
>>>
>>> Pretty much as important as it is on arm I guess. We just have less
>>> history, but the same use cases.
>>>
>>> I'd really like to give at least one release for transition, which
>>> would mean having a schedule like this:
>>>
>>>   - in 2018.01, merge those config removals so that we have least have
>>>     something that works quite fast
>>>
>>>   - in 2018.03, merge the multiple environment patches. We seem to
>>>     have reached a consensus here, so I'm not really concerned that we
>>>     will have it merged.
>>>
>>>   - in 2018.05, if needed, remove the raw MMC options and complete the
>>>     transition to FAT.
>>
>> That sounds very reasonable to me, thanks for writing this down!
>>
>> This gives us an only intermediate shortage of features for defconfig.
>>
>> We should encourage everyone who builds (and ships) firmware right now
>> to drop MMC env support already, if they tinker with the .config anyway.
>> That is, if there is no legacy usage to be concerned about.
>
> All these trimming(if it fits) seems to be nice for now, but what if
> once driver-model MMC, reset, pinctrl, clk, regulator are IN?
> of-course we can't do anything with SPL size because of SRAM
> constraints, but U-Boot proper size? why can't we increase the u-boot
> partition size?

That is a really big if, given no one is actively working on it.
I reckon we deal with it when someone starts having patches merged. :)

ChenYu
André Przywara Dec. 19, 2017, 2:41 p.m. | #23
Hi,

On 19/12/17 14:38, Jagan Teki wrote:
> On Tue, Dec 19, 2017 at 7:57 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi Maxime,
>>
>> On 19/12/17 14:20, Maxime Ripard wrote:
>>> On Tue, Dec 19, 2017 at 01:38:59PM +0000, Andre Przywara wrote:
>>>> Hi Maxime,
>>>>
>>>> thanks for having a look!
>>>>
>>>> On 19/12/17 13:12, Maxime Ripard wrote:
>>>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>>>> traditional MMC env location.
>>>>>
>>>>> So I've had a quick look about what could go possibly go away in our
>>>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>>>> some are actually vitals:
>>>>>
>>>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>>>  - CONSOLE_MUX
>>>>>  - CMD_CRC32
>>>>>  - CMD_LZMADEC
>>>>>  - CMD_UNZIP
>>>>>  - CMD_LOADB
>>>>>  - CMD_LOADS
>>>>>  - CMD_MISC (actually implementing the command sleep)
>>>>>  - ISO_PARTITION (yes. For CDROMs.)
>>>>
>>>> As Alex mentioned, this is needed for some installer images, which come
>>>> as ISOs. So if possible, we should keep this in.
>>>
>>> So, with FIT_ENABLE_SHA256_SUPPORT, LZMADEC, ISO_PARTITION and the
>>> overlay support, we're at 500kB.
>>>
>>> Again, tight, but under the limit.
>>
>> Phew! ;-)
>>
>>>
>>>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>>>  - VIDEO_ANSI
>>>>>  - SHA256
>>>>>  - LZMA
>>>>
>>>> From just looking at the names I am fine with the rest gone. But let me
>>>> test tonight if there are any side effects.
>>>>
>>>> Some of them seem useful, but I would leave enabling them to the actual
>>>> users. If someone needs it, they can enable them and loose the raw MMC
>>>> environment. I think this is a fair trade-off.
>>>
>>> Yes, that's my view too.
>>>
>>>>> Removing those options make the u-boot.itb binary size going from
>>>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>>>> the DT overlays that seem way more important than any feature
>>>>> mentionned above (and bumps the size to 483kB).
>>>>
>>>> How important is the raw MMC environment for the ARM64 boards, actually?
>>>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>>>> cases only. Do we have reports/complaints from 64-bit users?
>>>
>>> Pretty much as important as it is on arm I guess. We just have less
>>> history, but the same use cases.
>>>
>>> I'd really like to give at least one release for transition, which
>>> would mean having a schedule like this:
>>>
>>>   - in 2018.01, merge those config removals so that we have least have
>>>     something that works quite fast
>>>
>>>   - in 2018.03, merge the multiple environment patches. We seem to
>>>     have reached a consensus here, so I'm not really concerned that we
>>>     will have it merged.
>>>
>>>   - in 2018.05, if needed, remove the raw MMC options and complete the
>>>     transition to FAT.
>>
>> That sounds very reasonable to me, thanks for writing this down!
>>
>> This gives us an only intermediate shortage of features for defconfig.
>>
>> We should encourage everyone who builds (and ships) firmware right now
>> to drop MMC env support already, if they tinker with the .config anyway.
>> That is, if there is no legacy usage to be concerned about.
> 
> All these trimming(if it fits) seems to be nice for now, but what if
> once driver-model MMC, reset, pinctrl, clk, regulator are IN?

But that is what all of this is about? At the moment we can't go beyond
504K because the raw MMC U-Boot environment lives there on the SD card.
But with Maxime's plan for 2018.05, this will be gone and we should have
about 984KB (1MB - 40K).

Cheers,
Andre.

> of-course we can't do anything with SPL size because of SRAM
> constraints, but U-Boot proper size? why can't we increase the u-boot
> partition size?
> 
> thanks!
>
Maxime Ripard Dec. 19, 2017, 2:50 p.m. | #24
On Tue, Dec 19, 2017 at 10:41:23PM +0800, Chen-Yu Tsai wrote:
> > All these trimming(if it fits) seems to be nice for now, but what if
> > once driver-model MMC, reset, pinctrl, clk, regulator are IN?

I guess a better question would be: what are we doing to fix an issue
we had in a release already and will destroy the binary as soon as
someone does a saveenv?

> > of-course we can't do anything with SPL size because of SRAM
> > constraints, but U-Boot proper size? why can't we increase the u-boot
> > partition size?
> 
> That is a really big if, given no one is actively working on it.
> I reckon we deal with it when someone starts having patches merged. :)

+1.

Maxime
Maxime Ripard Dec. 19, 2017, 3:24 p.m. | #25
On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:
> > Date: Tue, 19 Dec 2017 14:12:02 +0100

> > From: Maxime Ripard <maxime.ripard@free-electrons.com>

> > 

> > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > > So even though the actual u-boot.bin for 64-bit boards is still somewhat

> > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > > the edge. So since v2017.11 u-boot.itb is already too big for the

> > > traditional MMC env location.

> > 

> > So I've had a quick look about what could go possibly go away in our

> > current armv8 config (using the pine64+ defconfig). Let me know if

> > some are actually vitals:

> > 

> >  - FIT_ENABLE_SHA256_SUPPORT

> >  - CONSOLE_MUX

> >  - CMD_CRC32

> >  - CMD_LZMADEC

> >  - CMD_UNZIP

> >  - CMD_LOADB

> >  - CMD_LOADS

> >  - CMD_MISC (actually implementing the command sleep)

> >  - ISO_PARTITION (yes. For CDROMs.)

> >  - VIDEO_BPP8, VIDEO_BPP16

> >  - VIDEO_ANSI

> >  - SHA256

> >  - LZMA

> > 

> > Removing those options make the u-boot.itb binary size going from

> > 516kB to 478kB, making it functional again *and* allowing us to enable

> > the DT overlays that seem way more important than any feature

> > mentionned above (and bumps the size to 483kB).

> 

> So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial

> console and usb keyboard are present?  Is the usb keyboard used when

> it is plugged in but serial otherwise?


That's actually a very good question, and I don't know, I never used a
USB keyboard with U-Boot.

This is enabled on 7 Allwinner boards, and no one complained so far,
so I would expect to work without that option activated. However, it
doesn't take that much space either, so it's not like we really need
to disable it either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard Dec. 19, 2017, 3:36 p.m. | #26
On Tue, Dec 19, 2017 at 02:27:57PM +0000, Andre Przywara wrote:
> >>> Removing those options make the u-boot.itb binary size going from

> >>> 516kB to 478kB, making it functional again *and* allowing us to enable

> >>> the DT overlays that seem way more important than any feature

> >>> mentionned above (and bumps the size to 483kB).

> >>

> >> How important is the raw MMC environment for the ARM64 boards, actually?

> >> Most of the rationale for the 32-bit side seemed to apply to legacy use

> >> cases only. Do we have reports/complaints from 64-bit users?

> > 

> > Pretty much as important as it is on arm I guess. We just have less

> > history, but the same use cases.

> > 

> > I'd really like to give at least one release for transition, which

> > would mean having a schedule like this:

> > 

> >   - in 2018.01, merge those config removals so that we have least have

> >     something that works quite fast


So, given the various feedback, the current diff for the pine64 is:
http://code.bulix.org/t7icrw-243561

With that, We're at 500kB with the ATF.

Does it work for everyone here?
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Tom Rini Dec. 19, 2017, 3:44 p.m. | #27
On Tue, Dec 19, 2017 at 04:36:13PM +0100, Maxime Ripard wrote:
> On Tue, Dec 19, 2017 at 02:27:57PM +0000, Andre Przywara wrote:

> > >>> Removing those options make the u-boot.itb binary size going from

> > >>> 516kB to 478kB, making it functional again *and* allowing us to enable

> > >>> the DT overlays that seem way more important than any feature

> > >>> mentionned above (and bumps the size to 483kB).

> > >>

> > >> How important is the raw MMC environment for the ARM64 boards, actually?

> > >> Most of the rationale for the 32-bit side seemed to apply to legacy use

> > >> cases only. Do we have reports/complaints from 64-bit users?

> > > 

> > > Pretty much as important as it is on arm I guess. We just have less

> > > history, but the same use cases.

> > > 

> > > I'd really like to give at least one release for transition, which

> > > would mean having a schedule like this:

> > > 

> > >   - in 2018.01, merge those config removals so that we have least have

> > >     something that works quite fast

> 

> So, given the various feedback, the current diff for the pine64 is:

> http://code.bulix.org/t7icrw-243561

> 

> With that, We're at 500kB with the ATF.

> 

> Does it work for everyone here?


Works for me, thanks!

-- 
Tom
Jagan Teki Dec. 19, 2017, 3:50 p.m. | #28
On Tue, Dec 19, 2017 at 9:06 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Dec 19, 2017 at 02:27:57PM +0000, Andre Przywara wrote:
>> >>> Removing those options make the u-boot.itb binary size going from
>> >>> 516kB to 478kB, making it functional again *and* allowing us to enable
>> >>> the DT overlays that seem way more important than any feature
>> >>> mentionned above (and bumps the size to 483kB).
>> >>
>> >> How important is the raw MMC environment for the ARM64 boards, actually?
>> >> Most of the rationale for the 32-bit side seemed to apply to legacy use
>> >> cases only. Do we have reports/complaints from 64-bit users?
>> >
>> > Pretty much as important as it is on arm I guess. We just have less
>> > history, but the same use cases.
>> >
>> > I'd really like to give at least one release for transition, which
>> > would mean having a schedule like this:
>> >
>> >   - in 2018.01, merge those config removals so that we have least have
>> >     something that works quite fast
>
> So, given the various feedback, the current diff for the pine64 is:
> http://code.bulix.org/t7icrw-243561
>
> With that, We're at 500kB with the ATF.
>
> Does it work for everyone here?

496KB for a64-olinuxino, nanopi-a64
Mark Kettenis Dec. 19, 2017, 10:35 p.m. | #29
> Date: Tue, 19 Dec 2017 16:24:59 +0100
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:
> > > Date: Tue, 19 Dec 2017 14:12:02 +0100
> > > From: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > 
> > > On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> > > > So even though the actual u-boot.bin for 64-bit boards is still somewhat
> > > > below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> > > > the edge. So since v2017.11 u-boot.itb is already too big for the
> > > > traditional MMC env location.
> > > 
> > > So I've had a quick look about what could go possibly go away in our
> > > current armv8 config (using the pine64+ defconfig). Let me know if
> > > some are actually vitals:
> > > 
> > >  - FIT_ENABLE_SHA256_SUPPORT
> > >  - CONSOLE_MUX
> > >  - CMD_CRC32
> > >  - CMD_LZMADEC
> > >  - CMD_UNZIP
> > >  - CMD_LOADB
> > >  - CMD_LOADS
> > >  - CMD_MISC (actually implementing the command sleep)
> > >  - ISO_PARTITION (yes. For CDROMs.)
> > >  - VIDEO_BPP8, VIDEO_BPP16
> > >  - VIDEO_ANSI
> > >  - SHA256
> > >  - LZMA
> > > 
> > > Removing those options make the u-boot.itb binary size going from
> > > 516kB to 478kB, making it functional again *and* allowing us to enable
> > > the DT overlays that seem way more important than any feature
> > > mentionned above (and bumps the size to 483kB).
> > 
> > So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial
> > console and usb keyboard are present?  Is the usb keyboard used when
> > it is plugged in but serial otherwise?
> 
> That's actually a very good question, and I don't know, I never used a
> USB keyboard with U-Boot.
> 
> This is enabled on 7 Allwinner boards, and no one complained so far,
> so I would expect to work without that option activated. However, it
> doesn't take that much space either, so it's not like we really need
> to disable it either.

I've used it on several board including my Orange Pi PC2.  With this
option enabled, all output from U-Boot and the EFI bootloader is
visible on both serial and hdmi, and I can interact with U-Boot and
the EFI bootloader over serial and using a USB keyboard.

That allows users to interact with U-Boot and/or the EFI bootloader
and configure whether the kernel sould use serial or "glass" console.
André Przywara Dec. 20, 2017, 1:55 a.m. | #30
On 19/12/17 15:24, Maxime Ripard wrote:
> On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:
>>> Date: Tue, 19 Dec 2017 14:12:02 +0100
>>> From: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>
>>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
>>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
>>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
>>>> the edge. So since v2017.11 u-boot.itb is already too big for the
>>>> traditional MMC env location.
>>>
>>> So I've had a quick look about what could go possibly go away in our
>>> current armv8 config (using the pine64+ defconfig). Let me know if
>>> some are actually vitals:
>>>
>>>  - FIT_ENABLE_SHA256_SUPPORT
>>>  - CONSOLE_MUX
>>>  - CMD_CRC32
>>>  - CMD_LZMADEC
>>>  - CMD_UNZIP
>>>  - CMD_LOADB
>>>  - CMD_LOADS
>>>  - CMD_MISC (actually implementing the command sleep)
>>>  - ISO_PARTITION (yes. For CDROMs.)
>>>  - VIDEO_BPP8, VIDEO_BPP16
>>>  - VIDEO_ANSI
>>>  - SHA256
>>>  - LZMA
>>>
>>> Removing those options make the u-boot.itb binary size going from
>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>> the DT overlays that seem way more important than any feature
>>> mentionned above (and bumps the size to 483kB).
>>
>> So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial
>> console and usb keyboard are present?  Is the usb keyboard used when
>> it is plugged in but serial otherwise?
> 
> That's actually a very good question, and I don't know, I never used a
> USB keyboard with U-Boot.
> 
> This is enabled on 7 Allwinner boards, and no one complained so far,
> so I would expect to work without that option activated. However, it
> doesn't take that much space either, so it's not like we really need
> to disable it either.

Just tested it: without CONSOLE_MUX we don't use video and USB keyboard
automatically. It seems to default to serial, "setenv stdout vidconsole"
switches over (immediately). This is quite weird behaviour, and probably
quite unpleasant for the casual user.
With CONSOLE_MUX defined we get what we want: always serial console,
plus USB keyboard and HDMI, if connected. Output appears on both, input
is accepted from both.
So I would very much like to keep it in.

Cheers,
Andre.
André Przywara Dec. 20, 2017, 2:02 a.m. | #31
On 19/12/17 15:36, Maxime Ripard wrote:
> On Tue, Dec 19, 2017 at 02:27:57PM +0000, Andre Przywara wrote:
>>>>> Removing those options make the u-boot.itb binary size going from
>>>>> 516kB to 478kB, making it functional again *and* allowing us to enable
>>>>> the DT overlays that seem way more important than any feature
>>>>> mentionned above (and bumps the size to 483kB).
>>>>
>>>> How important is the raw MMC environment for the ARM64 boards, actually?
>>>> Most of the rationale for the 32-bit side seemed to apply to legacy use
>>>> cases only. Do we have reports/complaints from 64-bit users?
>>>
>>> Pretty much as important as it is on arm I guess. We just have less
>>> history, but the same use cases.
>>>
>>> I'd really like to give at least one release for transition, which
>>> would mean having a schedule like this:
>>>
>>>   - in 2018.01, merge those config removals so that we have least have
>>>     something that works quite fast
> 
> So, given the various feedback, the current diff for the pine64 is:
> http://code.bulix.org/t7icrw-243561
> 
> With that, We're at 500kB with the ATF.
> 
> Does it work for everyone here?

Yes, briefly tested with HDMI, USB keyboard, also UEFI boot (just grub).

The only downside is the missing "boot" command. Not a big deal (as one
can always type "run bootmcd"), but quite inconvenient for the casual
user. If one (accidentally) aborts the timeout, just typing "boot" does
the right thing. This add ~450 bytes on my setup, so I wonder if we can
keep it in?

Cheers,
Andre.
Maxime Ripard Dec. 20, 2017, 7:15 a.m. | #32
On Wed, Dec 20, 2017 at 01:55:51AM +0000, André Przywara wrote:
> On 19/12/17 15:24, Maxime Ripard wrote:

> > On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:

> >>> Date: Tue, 19 Dec 2017 14:12:02 +0100

> >>> From: Maxime Ripard <maxime.ripard@free-electrons.com>

> >>>

> >>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> >>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat

> >>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> >>>> the edge. So since v2017.11 u-boot.itb is already too big for the

> >>>> traditional MMC env location.

> >>>

> >>> So I've had a quick look about what could go possibly go away in our

> >>> current armv8 config (using the pine64+ defconfig). Let me know if

> >>> some are actually vitals:

> >>>

> >>>  - FIT_ENABLE_SHA256_SUPPORT

> >>>  - CONSOLE_MUX

> >>>  - CMD_CRC32

> >>>  - CMD_LZMADEC

> >>>  - CMD_UNZIP

> >>>  - CMD_LOADB

> >>>  - CMD_LOADS

> >>>  - CMD_MISC (actually implementing the command sleep)

> >>>  - ISO_PARTITION (yes. For CDROMs.)

> >>>  - VIDEO_BPP8, VIDEO_BPP16

> >>>  - VIDEO_ANSI

> >>>  - SHA256

> >>>  - LZMA

> >>>

> >>> Removing those options make the u-boot.itb binary size going from

> >>> 516kB to 478kB, making it functional again *and* allowing us to enable

> >>> the DT overlays that seem way more important than any feature

> >>> mentionned above (and bumps the size to 483kB).

> >>

> >> So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial

> >> console and usb keyboard are present?  Is the usb keyboard used when

> >> it is plugged in but serial otherwise?

> > 

> > That's actually a very good question, and I don't know, I never used a

> > USB keyboard with U-Boot.

> > 

> > This is enabled on 7 Allwinner boards, and no one complained so far,

> > so I would expect to work without that option activated. However, it

> > doesn't take that much space either, so it's not like we really need

> > to disable it either.

> 

> Just tested it: without CONSOLE_MUX we don't use video and USB keyboard

> automatically. It seems to default to serial, "setenv stdout vidconsole"

> switches over (immediately). This is quite weird behaviour, and probably

> quite unpleasant for the casual user.

> With CONSOLE_MUX defined we get what we want: always serial console,

> plus USB keyboard and HDMI, if connected. Output appears on both, input

> is accepted from both.

> So I would very much like to keep it in.


Sounds like we should even enable it for everyone then.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Mark Kettenis Dec. 20, 2017, 7:42 a.m. | #33
> Date: Wed, 20 Dec 2017 08:15:46 +0100
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> On Wed, Dec 20, 2017 at 01:55:51AM +0000, Andr=E9 Przywara wrote:
> > On 19/12/17 15:24, Maxime Ripard wrote:
> > > On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:
> > >>> Date: Tue, 19 Dec 2017 14:12:02 +0100
> > >>> From: Maxime Ripard <maxime.ripard@free-electrons.com>
> > >>>
> > >>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> > >>>> So even though the actual u-boot.bin for 64-bit boards is still some=
> what
> > >>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> > >>>> the edge. So since v2017.11 u-boot.itb is already too big for the
> > >>>> traditional MMC env location.
> > >>>
> > >>> So I've had a quick look about what could go possibly go away in our
> > >>> current armv8 config (using the pine64+ defconfig). Let me know if
> > >>> some are actually vitals:
> > >>>
> > >>>  - FIT_ENABLE_SHA256_SUPPORT
> > >>>  - CONSOLE_MUX
> > >>>  - CMD_CRC32
> > >>>  - CMD_LZMADEC
> > >>>  - CMD_UNZIP
> > >>>  - CMD_LOADB
> > >>>  - CMD_LOADS
> > >>>  - CMD_MISC (actually implementing the command sleep)
> > >>>  - ISO_PARTITION (yes. For CDROMs.)
> > >>>  - VIDEO_BPP8, VIDEO_BPP16
> > >>>  - VIDEO_ANSI
> > >>>  - SHA256
> > >>>  - LZMA
> > >>>
> > >>> Removing those options make the u-boot.itb binary size going from
> > >>> 516kB to 478kB, making it functional again *and* allowing us to enable
> > >>> the DT overlays that seem way more important than any feature
> > >>> mentionned above (and bumps the size to 483kB).
> > >>
> > >> So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial
> > >> console and usb keyboard are present?  Is the usb keyboard used when
> > >> it is plugged in but serial otherwise?
> > >=20
> > > That's actually a very good question, and I don't know, I never used a
> > > USB keyboard with U-Boot.
> > >=20
> > > This is enabled on 7 Allwinner boards, and no one complained so far,
> > > so I would expect to work without that option activated. However, it
> > > doesn't take that much space either, so it's not like we really need
> > > to disable it either.
> >=20
> > Just tested it: without CONSOLE_MUX we don't use video and USB keyboard
> > automatically. It seems to default to serial, "setenv stdout vidconsole"
> > switches over (immediately). This is quite weird behaviour, and probably
> > quite unpleasant for the casual user.
> > With CONSOLE_MUX defined we get what we want: always serial console,
> > plus USB keyboard and HDMI, if connected. Output appears on both, input
> > is accepted from both.
> > So I would very much like to keep it in.
> 
> Sounds like we should even enable it for everyone then.

common/Kconfig has:

config CONSOLE_MUX
        bool "Enable console multiplexing"
        default y if DM_VIDEO || VIDEO || LCD

so it is already enabled in the cases where it matters.
Maxime Ripard Dec. 20, 2017, 9:20 a.m. | #34
On Wed, Dec 20, 2017 at 08:42:22AM +0100, Mark Kettenis wrote:
> > Date: Wed, 20 Dec 2017 08:15:46 +0100

> > From: Maxime Ripard <maxime.ripard@free-electrons.com>

> > 

> > On Wed, Dec 20, 2017 at 01:55:51AM +0000, Andr=E9 Przywara wrote:

> > > On 19/12/17 15:24, Maxime Ripard wrote:

> > > > On Tue, Dec 19, 2017 at 02:41:17PM +0100, Mark Kettenis wrote:

> > > >>> Date: Tue, 19 Dec 2017 14:12:02 +0100

> > > >>> From: Maxime Ripard <maxime.ripard@free-electrons.com>

> > > >>>

> > > >>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:

> > > >>>> So even though the actual u-boot.bin for 64-bit boards is still some=

> > what

> > > >>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over

> > > >>>> the edge. So since v2017.11 u-boot.itb is already too big for the

> > > >>>> traditional MMC env location.

> > > >>>

> > > >>> So I've had a quick look about what could go possibly go away in our

> > > >>> current armv8 config (using the pine64+ defconfig). Let me know if

> > > >>> some are actually vitals:

> > > >>>

> > > >>>  - FIT_ENABLE_SHA256_SUPPORT

> > > >>>  - CONSOLE_MUX

> > > >>>  - CMD_CRC32

> > > >>>  - CMD_LZMADEC

> > > >>>  - CMD_UNZIP

> > > >>>  - CMD_LOADB

> > > >>>  - CMD_LOADS

> > > >>>  - CMD_MISC (actually implementing the command sleep)

> > > >>>  - ISO_PARTITION (yes. For CDROMs.)

> > > >>>  - VIDEO_BPP8, VIDEO_BPP16

> > > >>>  - VIDEO_ANSI

> > > >>>  - SHA256

> > > >>>  - LZMA

> > > >>>

> > > >>> Removing those options make the u-boot.itb binary size going from

> > > >>> 516kB to 478kB, making it functional again *and* allowing us to enable

> > > >>> the DT overlays that seem way more important than any feature

> > > >>> mentionned above (and bumps the size to 483kB).

> > > >>

> > > >> So without CONFIG_CONSOLE_MUX, what is the behaviour when both serial

> > > >> console and usb keyboard are present?  Is the usb keyboard used when

> > > >> it is plugged in but serial otherwise?

> > > >=20

> > > > That's actually a very good question, and I don't know, I never used a

> > > > USB keyboard with U-Boot.

> > > >=20

> > > > This is enabled on 7 Allwinner boards, and no one complained so far,

> > > > so I would expect to work without that option activated. However, it

> > > > doesn't take that much space either, so it's not like we really need

> > > > to disable it either.

> > >=20

> > > Just tested it: without CONSOLE_MUX we don't use video and USB keyboard

> > > automatically. It seems to default to serial, "setenv stdout vidconsole"

> > > switches over (immediately). This is quite weird behaviour, and probably

> > > quite unpleasant for the casual user.

> > > With CONSOLE_MUX defined we get what we want: always serial console,

> > > plus USB keyboard and HDMI, if connected. Output appears on both, input

> > > is accepted from both.

> > > So I would very much like to keep it in.

> > 

> > Sounds like we should even enable it for everyone then.

> 

> common/Kconfig has:

> 

> config CONSOLE_MUX

>         bool "Enable console multiplexing"

>         default y if DM_VIDEO || VIDEO || LCD

> 

> so it is already enabled in the cases where it matters.


Ok, then the next question is why a few of our boards are actually
setting it in their defconfig instead of just using that default :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Mark Kettenis Dec. 20, 2017, 9:31 a.m. | #35
> From: Andre Przywara <andre.przywara@arm.com>
> Date: Tue, 19 Dec 2017 14:17:37 +0000
> 
> Hi,
> 
> On 19/12/17 13:51, Mark Kettenis wrote:
> >> From: Andre Przywara <andre.przywara@arm.com>
> >> Date: Tue, 19 Dec 2017 13:38:59 +0000
> >>
> >> Hi Maxime,
> >>
> >> thanks for having a look!
> >>
> >> On 19/12/17 13:12, Maxime Ripard wrote:
> >>> On Tue, Dec 05, 2017 at 10:28:20AM +0000, Andre Przywara wrote:
> >>>> So even though the actual u-boot.bin for 64-bit boards is still somewhat
> >>>> below the limit (~480KB), adding the ATF image (~32KB) pushes it over
> >>>> the edge. So since v2017.11 u-boot.itb is already too big for the
> >>>> traditional MMC env location.
> >>>
> >>> So I've had a quick look about what could go possibly go away in our
> >>> current armv8 config (using the pine64+ defconfig). Let me know if
> >>> some are actually vitals:
> >>>
> >>>  - FIT_ENABLE_SHA256_SUPPORT
> >>>  - CONSOLE_MUX
> >>>  - CMD_CRC32
> >>>  - CMD_LZMADEC
> >>>  - CMD_UNZIP
> >>>  - CMD_LOADB
> >>>  - CMD_LOADS
> >>>  - CMD_MISC (actually implementing the command sleep)
> >>>  - ISO_PARTITION (yes. For CDROMs.)
> >>
> >> As Alex mentioned, this is needed for some installer images, which come
> >> as ISOs. So if possible, we should keep this in.
> >>
> >>>  - VIDEO_BPP8, VIDEO_BPP16
> >>>  - VIDEO_ANSI
> >>>  - SHA256
> >>>  - LZMA
> >>
> >> From just looking at the names I am fine with the rest gone. But let me
> >> test tonight if there are any side effects.
> >>
> >> Some of them seem useful, but I would leave enabling them to the actual
> >> users. If someone needs it, they can enable them and loose the raw MMC
> >> environment. I think this is a fair trade-off.
> >>
> >>> Removing those options make the u-boot.itb binary size going from
> >>> 516kB to 478kB, making it functional again *and* allowing us to enable
> >>> the DT overlays that seem way more important than any feature
> >>> mentionned above (and bumps the size to 483kB).
> >>
> >> How important is the raw MMC environment for the ARM64 boards, actually?
> >> Most of the rationale for the 32-bit side seemed to apply to legacy use
> >> cases only. Do we have reports/complaints from 64-bit users?
> > 
> > For me/us (OpenBSD) the environment is still important.  I have many
> > setups where U-Boot lives on a uSD card but the installed OS lives on
> > a USB device.  In that scenario I set boot_targets to boot the EFI
> > bootloader and OS off the USB disk.  This is very helpfull for testing
> > new versions of U-Boot as I can simply swap the uSD card.  But for
> > some setups this is essential as OpenBSD doesn't support the SD/MCC
> > controller on all ARM hardware yet (but we do support it on
> > Allwinner).
> 
> I see, but I wasn't arguing for dropping the environment altogether,
> more for supporting FAT environments *only*.
> So how important is preserving existing environments over a firmware
> update in your scenario? I think this is the killer question here, isn't
> it? I'm inclined to just drop raw MMC environment support from sunxi64
> boards and then enjoy the ~450KB more worth of code, until we hit the
> first MB boundary.

I wouldn't consider preserving the environment as crucial.

> I have builds with all (DDR3) A64 board DTs in the binary [1], which
> would be larger than 504K anyway.

Oh, that's some cool work on the ATF side.  Need to check that out.

> Cheers,
> Andre.
> 
> [1] https://github.com/apritzel/pine64/commit/ee12bea43

Patch

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index dcacdf3e626d..8891961dcc6b 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -173,6 +173,22 @@  void i2c_init_board(void)
 #endif
 }
 
+#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT)
+enum env_location env_get_location(enum env_operation op, int prio)
+{
+	switch (prio) {
+	case 0:
+		return ENVL_FAT;
+
+	case 1:
+		return ENVL_MMC;
+
+	default:
+		return ENVL_UNKNOWN;
+	}
+}
+#endif
+
 /* add board specific code here */
 int board_init(void)
 {