Re: [PATCH 1/2] Fix usage of CONFIG_PREBOOT

2022-11-21 Thread Tom Rini
On Sun, Jul 10, 2022 at 01:42:55PM +0200, Pali Rohár wrote:

> Due to usage of PREBOOT in Kconfig, macro CONFIG_PREBOOT is always defined
> when CONFIG_USE_PREBOOT is enabled. In case CONFIG_PREBOOT is not
> explicitly enabled it is set to empty C string and therefore
> '#ifdef CONFIG_PREBOOT' guard does not work. Fix this issue by introducing
> a new Kconfig symbol PREBOOT_DEFINED which cause to define new C macro
> CONFIG_PREBOOT_DEFINED only when CONFIG_PREBOOT is really defined.
> 
> Change usage of '#ifdef CONFIG_PREBOOT' by '#ifdef CONFIG_USE_PREBOOT' for
> code which checks if preboot code would be called and by
> '#ifdef CONFIG_PREBOOT_DEFINED' for defining preboot code.
> 
> Signed-off-by: Pali Rohár 

For the record, we need to long term figure out a better solution to
user configurable default environment stuff.  Doing this via Kconfig is
fragile / problematic. However, as this patch does solve a real problem,
applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/2] Fix usage of CONFIG_PREBOOT

2022-07-12 Thread Pali Rohár
On Tuesday 12 July 2022 04:58:51 Simon Glass wrote:
> Hi Pali,
> 
> On Sun, 10 Jul 2022 at 05:43, Pali Rohár  wrote:
> >
> > Due to usage of PREBOOT in Kconfig, macro CONFIG_PREBOOT is always defined
> > when CONFIG_USE_PREBOOT is enabled. In case CONFIG_PREBOOT is not
> > explicitly enabled it is set to empty C string and therefore
> > '#ifdef CONFIG_PREBOOT' guard does not work. Fix this issue by introducing
> > a new Kconfig symbol PREBOOT_DEFINED which cause to define new C macro
> > CONFIG_PREBOOT_DEFINED only when CONFIG_PREBOOT is really defined.
> >
> > Change usage of '#ifdef CONFIG_PREBOOT' by '#ifdef CONFIG_USE_PREBOOT' for
> > code which checks if preboot code would be called and by
> > '#ifdef CONFIG_PREBOOT_DEFINED' for defining preboot code.
> >
> > Signed-off-by: Pali Rohár 
> > ---
> >  board/boundary/nitrogen6x/nitrogen6x.c | 4 ++--
> >  boot/Kconfig   | 4 
> >  include/env_default.h  | 2 +-
> >  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> Can you not use:
> 
> #idef CONFIG_USE_PREBOOT
> 
> ?

Where?

> You should not be checking for the existence of a string Kconfig.

I do not see other option, because this is how kconfig is working. When
string option is not set then kconfig defines it to empty string.

> Regards,
> Simon


Re: [PATCH 1/2] Fix usage of CONFIG_PREBOOT

2022-07-12 Thread Simon Glass
Hi Pali,

On Sun, 10 Jul 2022 at 05:43, Pali Rohár  wrote:
>
> Due to usage of PREBOOT in Kconfig, macro CONFIG_PREBOOT is always defined
> when CONFIG_USE_PREBOOT is enabled. In case CONFIG_PREBOOT is not
> explicitly enabled it is set to empty C string and therefore
> '#ifdef CONFIG_PREBOOT' guard does not work. Fix this issue by introducing
> a new Kconfig symbol PREBOOT_DEFINED which cause to define new C macro
> CONFIG_PREBOOT_DEFINED only when CONFIG_PREBOOT is really defined.
>
> Change usage of '#ifdef CONFIG_PREBOOT' by '#ifdef CONFIG_USE_PREBOOT' for
> code which checks if preboot code would be called and by
> '#ifdef CONFIG_PREBOOT_DEFINED' for defining preboot code.
>
> Signed-off-by: Pali Rohár 
> ---
>  board/boundary/nitrogen6x/nitrogen6x.c | 4 ++--
>  boot/Kconfig   | 4 
>  include/env_default.h  | 2 +-
>  3 files changed, 7 insertions(+), 3 deletions(-)

Can you not use:

#idef CONFIG_USE_PREBOOT

?

You should not be checking for the existence of a string Kconfig.

Regards,
Simon