[U-Boot] [PATCH v2 3/5] cmd: Add bind/unbind commands to bind a device to a driver from the command line

Michal Simek monstr at monstr.eu
Thu Jun 21 08:00:05 UTC 2018


On 18.6.2018 15:56, Jean-Jacques Hiblot wrote:
> In some cases it can be useful to be able to bind a device to a driver from
> the command line.
> The obvious example is for versatile devices such as USB gadget.
> Another use case is when the devices are not yet ready at startup and
> require some setup before the drivers are bound (ex: FPGA which bitsream is
> fetched from a mass storage or ethernet)
> 
> usage example:
> 
> bind usb_dev_generic 0 usb_ether
> unbind usb_dev_generic 0 usb_ether
> or
> unbind eth 1
> 
> bind /ocp/omap_dwc3 at 48380000/usb at 48390000 usb_ether
> unbind /ocp/omap_dwc3 at 48380000/usb at 48390000
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> 
> ---
> 
> Changes in v2:
> - Make the bind/unbind command generic, not specific to usb device.
> - Update the API to be able to bind/unbind based on DTS node path
> - Add a Kconfig option to select the bind/unbind commands
> 
>  cmd/Kconfig  |   9 +++
>  cmd/Makefile |   1 +
>  cmd/bind.c   | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 266 insertions(+)
>  create mode 100644 cmd/bind.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1eb55e5..d6bbfba 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -607,6 +607,15 @@ config CMD_ADC
>  	  Shows ADC device info and permit printing one-shot analog converted
>  	  data from a named Analog to Digital Converter.
>  
> +config CMD_BIND
> +	bool "bind/unbind - Bind or unbind a device to/from a driver"
> +	depends on DM
> +	help
> +	  Bind or unbind a device to/from a driver from the command line.
> +	  This is useful in situations where a device may be handled by several
> +	  drivers. For example, this can be used to bind a UDC to the usb ether
> +	  gadget driver from the command line.
> +
>  config CMD_CLK
>  	bool "clk - Show clock frequencies"
>  	help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index e0088df..b12aca3 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_SOURCE) += source.o
>  obj-$(CONFIG_CMD_SOURCE) += source.o
>  obj-$(CONFIG_CMD_BDI) += bdinfo.o
>  obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
> +obj-$(CONFIG_CMD_BIND) += bind.o
>  obj-$(CONFIG_CMD_BINOP) += binop.o
>  obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
>  obj-$(CONFIG_CMD_BMP) += bmp.o
> diff --git a/cmd/bind.c b/cmd/bind.c
> new file mode 100644
> index 0000000..60d84d6
> --- /dev/null
> +++ b/cmd/bind.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 JJ Hiblot <jjhiblot at ti.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/uclass-internal.h>
> +
> +static int bind_by_class_index(const char *uclass, int index,
> +			       const char *drv_name)
> +{
> +	static enum uclass_id uclass_id;
> +	struct udevice *dev;
> +	struct udevice *parent;
> +	int ret;
> +	struct driver *drv;
> +
> +	drv = lists_driver_lookup_name(drv_name);
> +	if (!drv) {
> +		printf("Cannot find driver '%s'\n", drv_name);
> +		return -ENOENT;
> +	}
> +
> +	uclass_id = uclass_get_by_name(uclass);
> +	if (uclass_id == UCLASS_INVALID) {
> +		printf("%s is not a valid uclass\n", uclass);
> +		return -EINVAL;
> +	}
> +
> +	ret = uclass_find_device(uclass_id, index, &parent);
> +	if (!parent || ret) {
> +		printf("Cannot find device %d of class %s\n", index, uclass);
> +		return ret;
> +	}
> +
> +	ret = device_bind_with_driver_data(parent, drv, drv->name, 0,
> +					   ofnode_null(), &dev);
> +	if (!dev || ret) {
> +		printf("Unable to bind. err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int unbind_by_class_index(const char *uclass, int index)
> +{
> +	static enum uclass_id uclass_id;
> +	struct udevice *dev;
> +	int ret;
> +
> +	uclass_id = uclass_get_by_name(uclass);
> +	if (uclass_id == UCLASS_INVALID) {
> +		printf("%s is not a valid uclass\n", uclass);
> +		return -EINVAL;
> +	}
> +
> +	ret = uclass_find_device(uclass_id, index, &dev);
> +	if (!dev || ret) {
> +		printf("Cannot find device %d of class %s\n", index, uclass);
> +		return ret;
> +	}
> +
> +	ret = device_unbind(dev);
> +	if (ret) {
> +		printf("Unable to unbind. err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int unbind_child_by_class_index(const char *uclass, int index,
> +				       const char *drv_name)
> +{
> +	static enum uclass_id uclass_id;
> +	struct udevice *parent;
> +	struct udevice *dev, *n;
> +	int ret;
> +	struct driver *drv;
> +
> +	drv = lists_driver_lookup_name(drv_name);
> +	if (!drv) {
> +		printf("Cannot find driver '%s'\n", drv_name);
> +		return -ENOENT;
> +	}
> +
> +	uclass_id = uclass_get_by_name(uclass);
> +	if (uclass_id == UCLASS_INVALID) {
> +		printf("%s is not a valid uclass\n", uclass);
> +		return -EINVAL;
> +	}
> +
> +	ret = uclass_find_device(uclass_id, index, &parent);
> +	if (!parent || ret) {
> +		printf("Cannot find device %d of class %s\n", index, uclass);
> +		return ret;
> +	}
> +
> +	list_for_each_entry_safe(dev, n, &parent->child_head, sibling_node) {
> +		int rc;
> +
> +		if (dev->driver != drv)
> +			continue;
> +
> +		rc = device_unbind(dev);
> +		if (rc && !ret) {
> +			printf("failed to unbind %s/%s\n", dev->name,
> +			       drv->name);
> +			ret = rc;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int bind_by_node_path(const char *path, const char *drv_name)
> +{
> +	struct udevice *dev;
> +	struct udevice *parent = NULL;
> +	int ret;
> +	int id;
> +	ofnode ofnode;
> +	struct driver *drv;
> +
> +	drv = lists_driver_lookup_name(drv_name);
> +	if (!drv) {
> +		printf("%s is not a valid driver name\n", drv_name);
> +		return -ENOENT;
> +	}
> +
> +	ofnode = ofnode_path(path);
> +	if (!ofnode_valid(ofnode)) {
> +		printf("%s is not a valid node path\n", path);
> +		return -EINVAL;
> +	}
> +
> +	while (ofnode_valid(ofnode)) {
> +		for (id = 0; id < UCLASS_COUNT; id++) {
> +			if (!uclass_find_device_by_ofnode(id, ofnode, &parent))
> +				break;
> +		}
> +		if (parent)
> +			break;
> +		ofnode = ofnode_get_parent(ofnode);
> +	}
> +
> +	if (!parent) {
> +		printf("Cannot find a parent device for node path %s\n", path);
> +		return -ENODEV;
> +	}
> +
> +	ret = device_bind_with_driver_data(parent, drv, drv->name, 0,
> +					   ofnode_path(path), &dev);
> +	if (!dev || ret) {
> +		printf("Unable to bind. err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int unbind_by_node_path(const char *path)
> +{
> +	struct udevice *dev;
> +	int ret;
> +	int id;
> +	ofnode ofnode;
> +
> +	ofnode = ofnode_path(path);
> +	if (!ofnode_valid(ofnode)) {
> +		printf("%s is not a valid node path\n", path);
> +		return -EINVAL;
> +	}
> +
> +	for (id = 0; id < UCLASS_COUNT; id++) {
> +		if (uclass_find_device_by_ofnode(id, ofnode, &dev) == 0)
> +			break;
> +	}
> +
> +	if (!dev) {
> +		printf("Cannot find a device with path %s\n", path);
> +		return -ENODEV;
> +	}
> +
> +	ret = device_unbind(dev);
> +	if (ret) {
> +		printf("Unable to unbind. err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int do_bind_unbind(cmd_tbl_t *cmdtp, int flag, int argc,
> +			  char * const argv[])
> +{
> +	int ret;
> +	bool bind;
> +	bool by_node;
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	bind = (argv[0][0] == 'b');
> +	by_node = (argv[1][0] == '/');
> +
> +	if (by_node && bind) {
> +		if (argc != 3)
> +			return CMD_RET_USAGE;
> +		ret = bind_by_node_path(argv[1], argv[2]);
> +	} else if (by_node && !bind) {
> +		if (argc != 2)
> +			return CMD_RET_USAGE;
> +		ret = unbind_by_node_path(argv[1]);
> +	} else if (!by_node && bind) {
> +		int index = (argc > 2) ? simple_strtoul(argv[2], NULL, 10) : 0;
> +
> +		if (argc != 4)
> +			return CMD_RET_USAGE;
> +		ret = bind_by_class_index(argv[1], index, argv[3]);
> +	} else if (!by_node && !bind) {
> +		int index = (argc > 2) ? simple_strtoul(argv[2], NULL, 10) : 0;
> +
> +		if (argc == 3)
> +			ret = unbind_by_class_index(argv[1], index);
> +		else if (argc == 4)
> +			ret = unbind_child_by_class_index(argv[1], index,
> +							  argv[3]);
> +		else
> +			return CMD_RET_USAGE;
> +	}
> +
> +	if (ret)
> +		return CMD_RET_FAILURE;
> +	else
> +		return CMD_RET_SUCCESS;

I am getting compilation warnings bout this ret handling.

cmd/bind.c: In function ‘do_bind_unbind’:
cmd/bind.c:237:5: warning: ‘ret’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  if (ret)
     ^



> +}
> +
> +U_BOOT_CMD(
> +	bind,	4,	0,	do_bind_unbind,
> +	"Bind a device to a driver",
> +	"<node path> <driver>\n"
> +	"bind <class> <index> <driver>\n"
> +);
> +
> +U_BOOT_CMD(
> +	unbind,	4,	0,	do_bind_unbind,
> +	"Unbind a device from a driver",
> +	"<node path>\n"
> +	"unbind <class> <index>\n"
> +	"unbind <class> <index> <driver>\n"
> +);
> 

I have played with this series with zynq_gpio driver on zcu100.
It can be only one instance of this driver but I can call bind multiple
times and instance is listed with new index. Log below.

ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> bind /amba/gpio at ff0a0000 gpio_zynq
EFI: Initializing UCLASS_EFI
Calling zynq_gpio_bind
ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 gpio        1  [   ]   gpio_zynq   |   |   `-- gpio_zynq
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP>

I can read dev->seq in probe and fail if dev->seq is not 0 but not sure
if this should be checked in probe.

The second thing is that when I call unbind for probed device that it is
failing because it hasn't been removed.

ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> gpio status 0
Calling zynq_gpio_ofdata_to_platdata
Calling zynq_gpio_probe 0
ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [ + ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> unbind /amba/gpio at ff0a0000
EFI: Initializing UCLASS_EFI
activated
Unable to unbind. err:-22

That's why I am curious if this command enable to unbind device then
there should be also an option to remove that device too.

More generic note. I have been taking information about structure from
dm tree command and I would consider to move test/dm/cmd_dm.c to cmd
folder and extend this file by these new functionality.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180621/e152deb0/attachment.sig>


More information about the U-Boot mailing list