[PATCH v4 1/4] cmd: bind: Add unbind command with driver filter
Simon Glass
sjg at chromium.org
Wed Aug 2 23:31:28 CEST 2023
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
> {
> 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
> 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.
> } else if (!by_node && bind) {
> int index = (argc > 2) ? dectoul(argv[2], NULL) : 0;
>
> @@ -251,7 +251,7 @@ U_BOOT_CMD(
> U_BOOT_CMD(
> unbind, 4, 0, do_bind_unbind,
> "Unbind a device from a driver",
> - "<node path>\n"
> + "<node path> [<driver>]\n"
> "unbind <class> <index>\n"
> "unbind <class> <index> <driver>\n"
> );
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 6e26b64fb81..52fceb69341 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -877,15 +877,17 @@ int device_get_child_by_of_offset(const struct udevice *parent, int node,
> }
>
> static struct udevice *_device_find_global_by_ofnode(struct udevice *parent,
> - ofnode ofnode)
> + ofnode ofnode,
> + const char *drv)
> {
> struct udevice *dev, *found;
>
> - if (ofnode_equal(dev_ofnode(parent), ofnode))
> + if (ofnode_equal(dev_ofnode(parent), ofnode) &&
> + (!drv || (drv && !strcmp(parent->driver->name, drv))))
> return parent;
>
> device_foreach_child(dev, parent) {
> - found = _device_find_global_by_ofnode(dev, ofnode);
> + found = _device_find_global_by_ofnode(dev, ofnode, drv);
> if (found)
> return found;
> }
> @@ -895,7 +897,15 @@ static struct udevice *_device_find_global_by_ofnode(struct udevice *parent,
>
> int device_find_global_by_ofnode(ofnode ofnode, struct udevice **devp)
> {
> - *devp = _device_find_global_by_ofnode(gd->dm_root, ofnode);
> + *devp = _device_find_global_by_ofnode(gd->dm_root, ofnode, NULL);
> +
> + return *devp ? 0 : -ENOENT;
> +}
> +
> +int device_find_global_by_ofnode_driver(ofnode ofnode, const char *drv,
> + struct udevice **devp)
> +{
> + *devp = _device_find_global_by_ofnode(gd->dm_root, ofnode, drv);
>
> return *devp ? 0 : -ENOENT;
> }
> @@ -904,7 +914,7 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp)
> {
> struct udevice *dev;
>
> - dev = _device_find_global_by_ofnode(gd->dm_root, ofnode);
> + dev = _device_find_global_by_ofnode(gd->dm_root, ofnode, NULL);
> return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
> }
>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index b86bf90609b..5f05ae0924f 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -748,6 +748,23 @@ int device_get_child_by_of_offset(const struct udevice *parent, int of_offset,
>
> int device_find_global_by_ofnode(ofnode node, struct udevice **devp);
>
> +/**
> + * device_find_global_by_ofnode_driver() - Get a device based on ofnode and driver
> + *
> + * Locates a device by its device tree ofnode and driver currently bound to
> + * it, searching globally throughout the all driver model devices.
> + *
> + * The device is NOT probed
> + *
> + * @node: Device tree ofnode to find
> + * @drv: Driver name bound to device
> + * @devp: Returns pointer to device if found, otherwise this is set to NULL
> + * Return: 0 if OK, -ve on error
> + */
> +
> +int device_find_global_by_ofnode_driver(ofnode node, const char *drv,
> + struct udevice **devp);
> +
> /**
> * device_get_global_by_ofnode() - Get a device based on ofnode
> *
> --
> 2.40.1
>
I suppose the test is in another patch, but it is often better to put
it in the same patch.
Regards,
Simon
More information about the U-Boot
mailing list