[PATCH v4 1/4] cmd: bind: Add unbind command with driver filter

[Marek Vasut]

On 8/2/23 23:31, Simon Glass wrote:
> Hi Marek,
> 
> On Wed, 2 Aug 2023 at 06:47, Marek Vasut <marex at denx.de> wrote:
>>
>> Extend the driver core to perform lookup by both OF node and driver
>> bound to the node. Use this to look up specific device instances to
>> unbind from nodes in the unbind command. One example where this is
>> needed is USB peripheral controller, which may have multiple gadget
>> drivers bound to it. The unbind command has to select that specific
>> gadget driver instance to unbind from the controller, not unbind the
>> controller driver itself from the controller.
>>
>> USB ethernet gadget usage looks as follows with this change. Notice
>> the extra 'usb_ether' addition in the 'unbind' command at the end.
>> "
>> bind /soc/usb-otg at 49000000 usb_ether
>> setenv ethact usb_ether
>> setenv loadaddr 0xc2000000
>> setenv ipaddr 10.0.0.2
>> setenv serverip 10.0.0.1
>> setenv netmask 255.255.255.0
>> tftpboot 0xc2000000 10.0.0.1:test.file
>> unbind /soc/usb-otg at 49000000 usb_ether
>> "
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> ---
>> Cc: Kevin Hilman <khilman at baylibre.com>
>> Cc: Lukasz Majewski <lukma at denx.de>
>> Cc: Miquel Raynal <miquel.raynal at bootlin.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> ---
>> V2: No change
>> V3: No change
>> V4: No change
>> ---
>>   cmd/bind.c            | 10 +++++-----
>>   drivers/core/device.c | 20 +++++++++++++++-----
>>   include/dm/device.h   | 17 +++++++++++++++++
>>   3 files changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/cmd/bind.c b/cmd/bind.c
>> index 4d1b7885e60..3137cdf6cb5 100644
>> --- a/cmd/bind.c
>> +++ b/cmd/bind.c
>> @@ -162,7 +162,7 @@ static int bind_by_node_path(const char *path, const char *drv_name)
>>          return 0;
>>   }
>>
>> -static int unbind_by_node_path(const char *path)
>> +static int unbind_by_node_path(const char *path, const char *drv_name)
> 
> Can you add a function comment? I am wondering what drm_name is for

drv_name means driver name.

>>   {
>>          struct udevice *dev;
>>          int ret;
>> @@ -174,7 +174,7 @@ static int unbind_by_node_path(const char *path)
>>                  return -EINVAL;
>>          }
>>
>> -       ret = device_find_global_by_ofnode(ofnode, &dev);
>> +       ret = device_find_global_by_ofnode_driver(ofnode, drv_name, &dev);
>>
>>          if (!dev || ret) {
>>                  printf("Cannot find a device with path %s\n", path);
>> @@ -214,9 +214,9 @@ static int do_bind_unbind(struct cmd_tbl *cmdtp, int flag, int argc,
>>                          return CMD_RET_USAGE;
>>                  ret = bind_by_node_path(argv[1], argv[2]);
>>          } else if (by_node && !bind) {
>> -               if (argc != 2)
>> +               if (argc != 2 && argc != 3)
> 
> how about: argv < 2

No.

>>                          return CMD_RET_USAGE;
>> -               ret = unbind_by_node_path(argv[1]);
>> +               ret = unbind_by_node_path(argv[1], argv[2]);
> 
> but if argc is 2, how can we access argv[2]? Is it because we accept
> NULL? In that case, please add a comment.

We accept NULL.

[...]