[01/11] cmd: crc32: Disable by default on sunXi

Message ID 20171221124030.23721-2-maxime.ripard@free-electrons.com
State Superseded
Headers show
Series
  • sunxi: arm64 binary size fixes
Related show

Commit Message

Maxime Ripard Dec. 21, 2017, 12:40 p.m.
The sunXi arm64 build has overflown, leading to the main U-boot binary
overwriting the environment when flashing the new image, or even worse,
overwriting itself when we're calling saveenv.

Disable this command that is not critical until we can adress the issue
properly.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 cmd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

=?UTF-8?Q?Andr=c3=a9_Przywara?= Dec. 21, 2017, 2:50 p.m. | #1
Hi,

On 21/12/17 12:40, Maxime Ripard wrote:
> The sunXi arm64 build has overflown, leading to the main U-boot binary
> overwriting the environment when flashing the new image, or even worse,
> overwriting itself when we're calling saveenv.
> 
> Disable this command that is not critical until we can adress the issue
> properly.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  cmd/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index c0332235261f..7751001819d0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -397,6 +397,7 @@ menu "Memory commands"
>  config CMD_CRC32
>  	bool "crc32"
>  	select HASH
> +	default n if ARCH_SUNXI

Is that meant to solve issues with 32 bit boards as well? Or shall we
use "ARCH_SUNXI && ARM64" here?

In case including 32-bit is intentional, this is:

for [PATCH 01/11] - [PATCH 04/11]:
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

If no one shoot any of those options down, you might want to squash them
into one patch, since they all go into the same file.
Unless you need to meet some company patch count target before the end
of the year :-D

Cheers,
Andre.

P.S. Shall we mark those patches somehow, so that we can easily revert
them later? Those commands can be useful.

Cheers,
Andre

>  	default y
>  	help
>  	  Compute CRC32.
>
Jagan Teki Dec. 22, 2017, 9:16 a.m. | #2
On Thu, Dec 21, 2017 at 8:20 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 21/12/17 12:40, Maxime Ripard wrote:
>> The sunXi arm64 build has overflown, leading to the main U-boot binary
>> overwriting the environment when flashing the new image, or even worse,
>> overwriting itself when we're calling saveenv.
>>
>> Disable this command that is not critical until we can adress the issue
>> properly.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  cmd/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index c0332235261f..7751001819d0 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -397,6 +397,7 @@ menu "Memory commands"
>>  config CMD_CRC32
>>       bool "crc32"
>>       select HASH
>> +     default n if ARCH_SUNXI
>
> Is that meant to solve issues with 32 bit boards as well? Or shall we
> use "ARCH_SUNXI && ARM64" here?

Agreed this, and If we go further better to have A64 and H5 check directly.
Maxime Ripard Jan. 5, 2018, 9:54 a.m. | #3
Hi Andre,

On Thu, Dec 21, 2017 at 02:50:29PM +0000, Andre Przywara wrote:
> Hi,

> 

> On 21/12/17 12:40, Maxime Ripard wrote:

> > The sunXi arm64 build has overflown, leading to the main U-boot binary

> > overwriting the environment when flashing the new image, or even worse,

> > overwriting itself when we're calling saveenv.

> > 

> > Disable this command that is not critical until we can adress the issue

> > properly.

> > 

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

> > ---

> >  cmd/Kconfig | 1 +

> >  1 file changed, 1 insertion(+)

> > 

> > diff --git a/cmd/Kconfig b/cmd/Kconfig

> > index c0332235261f..7751001819d0 100644

> > --- a/cmd/Kconfig

> > +++ b/cmd/Kconfig

> > @@ -397,6 +397,7 @@ menu "Memory commands"

> >  config CMD_CRC32

> >  	bool "crc32"

> >  	select HASH

> > +	default n if ARCH_SUNXI

> 

> Is that meant to solve issues with 32 bit boards as well? Or shall we

> use "ARCH_SUNXI && ARM64" here?

> 

> In case including 32-bit is intentional, this is:

> 

> for [PATCH 01/11] - [PATCH 04/11]:

> Reviewed-by: Andre Przywara <andre.przywara@arm.com>


This is intentional. I really want to have an experience as close as
possible on the arm and arm64 boards, and having the same commands set
is one of the things that matter to achieve that :)

> If no one shoot any of those options down, you might want to squash them

> into one patch, since they all go into the same file.


That works for me :)

> Unless you need to meet some company patch count target before the end

> of the year :-D


I guess I missed that deadline already... ;)

> P.S. Shall we mark those patches somehow, so that we can easily revert

> them later? Those commands can be useful.


That might be a good idea. Do you have a suggestion?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Jagan Teki Jan. 10, 2018, 6:17 a.m. | #4
On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The sunXi arm64 build has overflown, leading to the main U-boot binary
> overwriting the environment when flashing the new image, or even worse,
> overwriting itself when we're calling saveenv.
>
> Disable this command that is not critical until we can adress the issue
> properly.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---

Applied to u-boot-sunxi/master
Jagan Teki Jan. 11, 2018, 5:54 p.m. | #5
On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The sunXi arm64 build has overflown, leading to the main U-boot binary
> overwriting the environment when flashing the new image, or even worse,
> overwriting itself when we're calling saveenv.
>
> Disable this command that is not critical until we can adress the issue
> properly.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  cmd/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index c0332235261f..7751001819d0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -397,6 +397,7 @@ menu "Memory commands"
>  config CMD_CRC32
>         bool "crc32"
>         select HASH
> +       default n if ARCH_SUNXI
>         default y
>         help
>           Compute CRC32.

this make unselect HASH so, few of the boards have build issues, we
can select sunxi directly on HASH kconfig.

[1] https://travis-ci.org/openedev/u-boot-sunxi/jobs/327576344
Tom Rini Jan. 11, 2018, 6:41 p.m. | #6
On Thu, Jan 11, 2018 at 11:24:28PM +0530, Jagan Teki wrote:
> On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard

> <maxime.ripard@free-electrons.com> wrote:

> > The sunXi arm64 build has overflown, leading to the main U-boot binary

> > overwriting the environment when flashing the new image, or even worse,

> > overwriting itself when we're calling saveenv.

> >

> > Disable this command that is not critical until we can adress the issue

> > properly.

> >

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

> > ---

> >  cmd/Kconfig | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/cmd/Kconfig b/cmd/Kconfig

> > index c0332235261f..7751001819d0 100644

> > --- a/cmd/Kconfig

> > +++ b/cmd/Kconfig

> > @@ -397,6 +397,7 @@ menu "Memory commands"

> >  config CMD_CRC32

> >         bool "crc32"

> >         select HASH

> > +       default n if ARCH_SUNXI

> >         default y

> >         help

> >           Compute CRC32.

> 

> this make unselect HASH so, few of the boards have build issues, we

> can select sunxi directly on HASH kconfig.


OK, hold on.  This series was supposed to be pulled in to v2018.01, to
fix size issues, before we could take the env series that I said can
come in for v2018.03, yes?  So we should just not do this series and
take Maxime's update to the env series which I expect to be posted soon
so I can merge it, and sunxi can move to env in FAT and not have the
problem it has on aarch64 atm.

-- 
Tom
Maxime Ripard Jan. 12, 2018, 8:56 a.m. | #7
Hi,

On Thu, Jan 11, 2018 at 11:24:28PM +0530, Jagan Teki wrote:
> On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard

> <maxime.ripard@free-electrons.com> wrote:

> > The sunXi arm64 build has overflown, leading to the main U-boot binary

> > overwriting the environment when flashing the new image, or even worse,

> > overwriting itself when we're calling saveenv.

> >

> > Disable this command that is not critical until we can adress the issue

> > properly.

> >

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

> > ---

> >  cmd/Kconfig | 1 +

> >  1 file changed, 1 insertion(+)

> >

> > diff --git a/cmd/Kconfig b/cmd/Kconfig

> > index c0332235261f..7751001819d0 100644

> > --- a/cmd/Kconfig

> > +++ b/cmd/Kconfig

> > @@ -397,6 +397,7 @@ menu "Memory commands"

> >  config CMD_CRC32

> >         bool "crc32"

> >         select HASH

> > +       default n if ARCH_SUNXI

> >         default y

> >         help

> >           Compute CRC32.

> 

> this make unselect HASH so, few of the boards have build issues, we

> can select sunxi directly on HASH kconfig.

> 

> [1] https://travis-ci.org/openedev/u-boot-sunxi/jobs/327576344


All the errors on the build 3 (which is the one that failed in your
history) are related to git issues it seems, and not build failures.

Which boards are failing?
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard Jan. 12, 2018, 9 a.m. | #8
On Thu, Jan 11, 2018 at 01:41:41PM -0500, Tom Rini wrote:
> On Thu, Jan 11, 2018 at 11:24:28PM +0530, Jagan Teki wrote:

> > On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard

> > <maxime.ripard@free-electrons.com> wrote:

> > > The sunXi arm64 build has overflown, leading to the main U-boot binary

> > > overwriting the environment when flashing the new image, or even worse,

> > > overwriting itself when we're calling saveenv.

> > >

> > > Disable this command that is not critical until we can adress the issue

> > > properly.

> > >

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

> > > ---

> > >  cmd/Kconfig | 1 +

> > >  1 file changed, 1 insertion(+)

> > >

> > > diff --git a/cmd/Kconfig b/cmd/Kconfig

> > > index c0332235261f..7751001819d0 100644

> > > --- a/cmd/Kconfig

> > > +++ b/cmd/Kconfig

> > > @@ -397,6 +397,7 @@ menu "Memory commands"

> > >  config CMD_CRC32

> > >         bool "crc32"

> > >         select HASH

> > > +       default n if ARCH_SUNXI

> > >         default y

> > >         help

> > >           Compute CRC32.

> > 

> > this make unselect HASH so, few of the boards have build issues, we

> > can select sunxi directly on HASH kconfig.

> 

> OK, hold on.  This series was supposed to be pulled in to v2018.01, to

> fix size issues, before we could take the env series that I said can

> come in for v2018.03, yes?  So we should just not do this series and

> take Maxime's update to the env series which I expect to be posted soon

> so I can merge it, and sunxi can move to env in FAT and not have the

> problem it has on aarch64 atm.


They are a bit orthogonal actually.

This serie addresses the build failures we have today in order to
still be able to build binaries that work.

The FAT transition is more of a longer-term goal in order to not have
the size restriction we have eventually (hopefully 2018.05).

So the first one mitigates the problem, while the second one allows us
to get rid of the problem entirely in the future.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Jagan Teki Jan. 12, 2018, 11:33 a.m. | #9
On Fri, Jan 12, 2018 at 2:26 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Jan 11, 2018 at 11:24:28PM +0530, Jagan Teki wrote:
>> On Thu, Dec 21, 2017 at 6:10 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The sunXi arm64 build has overflown, leading to the main U-boot binary
>> > overwriting the environment when flashing the new image, or even worse,
>> > overwriting itself when we're calling saveenv.
>> >
>> > Disable this command that is not critical until we can adress the issue
>> > properly.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  cmd/Kconfig | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/cmd/Kconfig b/cmd/Kconfig
>> > index c0332235261f..7751001819d0 100644
>> > --- a/cmd/Kconfig
>> > +++ b/cmd/Kconfig
>> > @@ -397,6 +397,7 @@ menu "Memory commands"
>> >  config CMD_CRC32
>> >         bool "crc32"
>> >         select HASH
>> > +       default n if ARCH_SUNXI
>> >         default y
>> >         help
>> >           Compute CRC32.
>>
>> this make unselect HASH so, few of the boards have build issues, we
>> can select sunxi directly on HASH kconfig.
>>
>> [1] https://travis-ci.org/openedev/u-boot-sunxi/jobs/327576344
>
> All the errors on the build 3 (which is the one that failed in your
> history) are related to git issues it seems, and not build failures.

Not exactly, boards which uses dfu from 5i/7i/8i are failed [2]

[2] https://travis-ci.org/openedev/u-boot-sunxi

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index c0332235261f..7751001819d0 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -397,6 +397,7 @@  menu "Memory commands"
 config CMD_CRC32
 	bool "crc32"
 	select HASH
+	default n if ARCH_SUNXI
 	default y
 	help
 	  Compute CRC32.