Re: [RFC PATCH v8 10/23] global_data.h: add GD_FLG_HUSH_OLD_PARSER flag

[Francis Laniel <francis.laniel@amarulasolutions.com>]

Le samedi 13 mai 2023, 02:07:57 WEST Heinrich Schuchardt a écrit :
> On 5/12/23 22:03, Francis Laniel wrote:
> > This flag is used to indicate we are using the hush parser.
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > ---
> > 
> >   common/cli.c                      | 2 ++
> >   include/asm-generic/global_data.h | 4 ++++
> >   2 files changed, 6 insertions(+)
> > 
> > diff --git a/common/cli.c b/common/cli.c
> > index 3916a7b10a..e5fe1060d0 100644
> > --- a/common/cli.c
> > +++ b/common/cli.c
> > @@ -268,6 +268,8 @@ void cli_loop(void)
> > 
> >   void cli_init(void)
> >   {
> >   #ifdef CONFIG_HUSH_PARSER
> 
> GD_FLG_HUSH_OLD_PARSER should depend on CONFIG_HUSH_OLD_PARSER.

I am not sure how to handle this as it would mean having something like this:
#ifdef CONFIG_HUSH_OLD_PARSER
	/**
	 * @GD_FLG_HUSH_OLD_PARSER: Use hush old parser.
	 */
	GD_FLG_HUSH_OLD_PARSER = 0x400000,
#endif
#ifdef CONFIG_HUSH_2021_PARSER
	/**
	 * @GD_FLG_HUSH_2021_PARSER: Use hush 2021 parser.
	 */
	GD_FLG_HUSH_2021_PARSER = 0x800000,
#endif
So, this would mean some flag would exist depending on CONFIG_*, which would 
then make the testing quite hard.
Indeed, for now, none of gd_flags is defined depending on some CONFIG_*.
Nonetheless, maybe using gd_flags here is not the correct solution?
Do you have in mind any idea which would be a better fit?
 
> Running scripts/checkpatch.pl indicates that we should use
> IS_ENABLED(CONFIG_*) here.

This code was not added in this contribution.
I may clean it but the current contribution is quite big, so I would like to 
avoid making it bigger by polishing other code if possible.
I will nonetheless checkpatch.pl every commits here for the next version!

> Best regards
> 
> Heinrich
> 
> > +	if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER))
> > +		gd->flags |= GD_FLG_HUSH_OLD_PARSER;
> > 
> >   	u_boot_hush_start();
> >   
> >   #endif
> > 
> > diff --git a/include/asm-generic/global_data.h
> > b/include/asm-generic/global_data.h index a1e1b9d640..120f1189ee 100644
> > --- a/include/asm-generic/global_data.h
> > +++ b/include/asm-generic/global_data.h
> > @@ -654,6 +654,10 @@ enum gd_flags {
> > 
> >   	 * @GD_FLG_OF_TAG_MIGRATE: Device tree has old u-boot,dm- tags
> >   	 */
> >   	
> >   	GD_FLG_OF_TAG_MIGRATE = 0x200000,
> > 
> > +	/**
> > +	 * @GD_FLG_HUSH_OLD_PARSER: Use hush old parser.
> > +	 */
> > +	GD_FLG_HUSH_OLD_PARSER = 0x400000,
> > 
> >   };
> >   
> >   #endif /* __ASSEMBLY__ */