Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help
On 10/5/23 17:54, Miquel Raynal wrote: Hi Marek, ma...@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 --- 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", " \n" "bind \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( "\n" "unbind \n" "unbind \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. If you tell them 'Use "dm tree"' and it prints "no command found" , then it's a bug. You likely want to add 'imply CMD_DM' into CMD_BIND Kconfig to make sure CMD_DM is available by default, but also that users can turn it off if they really want to.
Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help
Hi Marek, ma...@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 > > --- > > 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", > > " \n" > > "bind \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( > > "\n" > > "unbind \n" > > "unbind \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
Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help
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 --- 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", " \n" "bind \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( "\n" "unbind \n" "unbind \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.
Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help
On lun., oct. 02, 2023 at 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 Reviewed-by: Mattijs Korpershoek > --- > 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", > " \n" > "bind \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( > "\n" > "unbind \n" > "unbind \n" > + "Use 'dm tree' to list all devices registered in the driver model,\n" > + "their path, class, index and current driver.\n" > ); > -- > 2.34.1
Re: [PATCH RESEND v2 1/2] cmd: bind: Try to improve the (un)bind help
On Mon, 2 Oct 2023 at 07: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 > --- > 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(+) > Reviewed-by: Simon Glass
