diff mbox series

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

Message ID 20230512200331.51457-11-francis.laniel@amarulasolutions.com
State RFC
Delegated to: Tom Rini
Headers show
Series Modernize U-Boot shell | expand

Commit Message

Francis Laniel May 12, 2023, 8:03 p.m. UTC 
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(+)

Comments

Heinrich Schuchardt May 13, 2023, 1:07 a.m. UTC | #1 
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.

Running scripts/checkpatch.pl indicates that we should use
IS_ENABLED(CONFIG_*) here.

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__ */
Francis Laniel May 13, 2023, 8:29 p.m. UTC | #2 
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__ */
diff mbox series

Patch

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
+	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__ */