[PATCH 05/25] treewide: Correct use of long help

[Simon Glass]

Hi Tom,

On Thu, 5 Oct 2023 at 20:16, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Oct 05, 2023 at 07:41:49PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 5 Oct 2023 at 08:53, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Wed, Oct 04, 2023 at 07:23:47PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sun, 24 Sept 2023 at 17:27, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Sun, Sep 24, 2023 at 02:39:23PM -0600, Simon Glass wrote:
> > > > > > Some commands assume that CONFIG_SYS_LONGHELP is always defined.
> > > > > > Declaration of long help should be bracketed by an #ifdef to avoid an
> > > > > > 'unused variable' warning.
> > > > > >
> > > > > > Fix this treewide.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > [snip]
> > > > > > diff --git a/arch/arm/mach-imx/cmd_dek.c b/arch/arm/mach-imx/cmd_dek.c
> > > > > > index 6fa5b41fcd38..25ea7d3b37da 100644
> > > > > > --- a/arch/arm/mach-imx/cmd_dek.c
> > > > > > +++ b/arch/arm/mach-imx/cmd_dek.c
> > > > > > @@ -393,11 +393,12 @@ static int do_dek_blob(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > >       return blob_encap_dek(src_addr, dst_addr, len);
> > > > > >  }
> > > > > >
> > > > > > -/***************************************************/
> > > > > > +#if IS_ENABLED(CONFIG_SYS_LONGHELP)
> > > > > >  static char dek_blob_help_text[] =
> > > > > >       "src dst len            - Encapsulate and create blob of data\n"
> > > > > >       "                         $len bits long at address $src and\n"
> > > > > >       "                         store the result at address $dst.\n";
> > > > > > +#endif
> > > > > >
> > > > > >  U_BOOT_CMD(
> > > > > >       dek_blob, 4, 1, do_dek_blob,
> > > > >
> > > > > This really doesn't read nicely.  I would rather (globally and fix
> > > > > existing users) __maybe_unused this instead.  I think there's just one
> > > > > example today that isn't "foo_help_text".
> > > >
> > > > Hmm, what do you think about adding a __longhelp symbol to cause the
> > > > linker to discard it when not needed?
> > >
> > > Well, I don't think we need linker list magic when __maybe_unused will
> > > just have them be discarded normally.
> >
> > Yes, perhaps things are in a better state than they used to be, but
> > there is a linker discard for commands at present.
>
> Yes, but __maybe_unused means we don't get a warning about it, and it's
> literally discarded as part of --gc-sections as it done everywhere with
> maybe 3 exceptions?

Actually with this series we get a lot closer to that. The problem
with the status quo is that disabling CMDLINE doesn't disable most
commands, relying instead on discarding them at link time.

With this series, it looks like we can in fact do that, so I should
remove the discards as well.

The one proviso is that this does drop a lot of things we want...e.g.
BOOTSTD_DEFAULTS cannot be enabled due to its use of 'select', meaning
that common filesystems are dropped. So we'll need more effort after
this, to allow (for example) bootmeths that don't need commands to
work correctly. But I think this series at least provides a better
starting point for teasing things apart.

So OK I'll go with __maybe_unused for the ones that need it, which
isn't too many given that many of the strings are just included
directly in the macro. It is considerably easier than trying to be to
clever about things.

Regards,
Simon