Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Marek,

marex@denx.de wrote on Thu, 5 Oct 2023 15:01:51 +0200:

> On 10/2/23 15:46, Miquel Raynal wrote:
> > While it may sound totally obvious for the regular U-Boot developer to
> > get the parameters of the bind/unbind commands from the output of 'dm
> > tree', it did not felt straightforward to me until I was explicitly
> > told to look there. And even when I knew the command, I did not make a
> > direct link between the arguments of this command and the columns
> > returned by 'dm tree'.
> > 
> > Several of us lost a lot of time because of that, I would like to kindly
> > help other users by slightly improving this textual line. Unfortunately,
> > because of how this string is used (like within the 'help' command) I
> > cannot detail much more, but at least the pointer is there.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > Resend: no change.
> > 
> > Changes in v2:
> > * Moved the details in the long text section as suggested by Heinrich.
> > * Rephrased the help text as well, not fully following Heinrich
> >    suggestion, but taking inspiration from it.
> > ---
> >   cmd/bind.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/cmd/bind.c b/cmd/bind.c
> > index 4d1b7885e60..be0d4d2a711 100644
> > --- a/cmd/bind.c
> > +++ b/cmd/bind.c
> > @@ -246,6 +246,8 @@ U_BOOT_CMD(
> >   	"Bind a device to a driver",
> >   	"<node path> <driver>\n"
> >   	"bind <class> <index> <driver>\n"
> > +	"Use 'dm tree' to list all devices registered in the driver model,\n"
> > +	"their path, class, index and current driver.\n"
> >   );  
> >   >   U_BOOT_CMD(  
> > @@ -254,4 +256,6 @@ U_BOOT_CMD(
> >   	"<node path>\n"
> >   	"unbind <class> <index>\n"
> >   	"unbind <class> <index> <driver>\n"
> > +	"Use 'dm tree' to list all devices registered in the driver model,\n"
> > +	"their path, class, index and current driver.\n"  
> 
> 'dm tree' depends on CMD_DM , you might need some if (IS_ENABLED()) here.

Well, no, and that's my point actually. People need to know that this
command exist and should be used for this purpose. If you only tell
them "use 'dm tree'" conditionally, you loose part of the audience:
people not aware of this command or its usefulness.

Thanks,
Miquèl