[U-Boot] [PATCH v2 0/3] dm: display devices in a tree-like format for easy debug
When I was reviewing I2C DM series, I found it difficult to track what is going on. "When is this node is bound/probe?" etc.
I think this tool is helpful to know which device is bound/probe.
Devices are listed like "tree" command does.
Changes in v2: - Fix the tree format
Masahiro Yamada (3): lib: string: move strlcpy() to a common place dm: add a command to list devices in a tree-like format sandbox: enable showdev command
common/Makefile | 1 + common/cmd_showdev.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/gadget/ether.c | 24 ---------------- include/configs/sandbox.h | 1 + include/linux/string.h | 3 ++ lib/string.c | 25 +++++++++++++++++ 6 files changed, 100 insertions(+), 24 deletions(-) create mode 100644 common/cmd_showdev.c
Move strlcpy() definition from drivers/usb/gadget/ether.c to lib/string.c because it is a very useful function. Let's add the prototype to include/linux/string.h too.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com ---
Changes in v2: None
drivers/usb/gadget/ether.c | 24 ------------------------ include/linux/string.h | 3 +++ lib/string.c | 25 +++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index d0dd29f..ba442d5 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -852,30 +852,6 @@ DEFINE_CACHE_ALIGN_BUFFER(u8, control_req, USB_BUFSIZ); DEFINE_CACHE_ALIGN_BUFFER(u8, status_req, STATUS_BYTECOUNT); #endif
- -/** - * strlcpy - Copy a %NUL terminated string into a sized buffer - * @dest: Where to copy the string to - * @src: Where to copy the string from - * @size: size of destination buffer - * - * Compatible with *BSD: the result is always a valid - * NUL-terminated string that fits in the buffer (unless, - * of course, the buffer size is zero). It does not pad - * out the result like strncpy() does. - */ -size_t strlcpy(char *dest, const char *src, size_t size) -{ - size_t ret = strlen(src); - - if (size) { - size_t len = (ret >= size) ? size - 1 : ret; - memcpy(dest, src, len); - dest[len] = '\0'; - } - return ret; -} - /*============================================================================*/
/* diff --git a/include/linux/string.h b/include/linux/string.h index 96348d6..c7047ba 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -30,6 +30,9 @@ extern char * strcpy(char *,const char *); #ifndef __HAVE_ARCH_STRNCPY extern char * strncpy(char *,const char *, __kernel_size_t); #endif +#ifndef __HAVE_ARCH_STRLCPY +size_t strlcpy(char *, const char *, size_t); +#endif #ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); #endif diff --git a/lib/string.c b/lib/string.c index 29c2ca7..87c9a40 100644 --- a/lib/string.c +++ b/lib/string.c @@ -102,6 +102,31 @@ char * strncpy(char * dest,const char *src,size_t count) } #endif
+#ifndef __HAVE_ARCH_STRLCPY +/** + * strlcpy - Copy a C-string into a sized buffer + * @dest: Where to copy the string to + * @src: Where to copy the string from + * @size: size of destination buffer + * + * Compatible with *BSD: the result is always a valid + * NUL-terminated string that fits in the buffer (unless, + * of course, the buffer size is zero). It does not pad + * out the result like strncpy() does. + */ +size_t strlcpy(char *dest, const char *src, size_t size) +{ + size_t ret = strlen(src); + + if (size) { + size_t len = (ret >= size) ? size - 1 : ret; + memcpy(dest, src, len); + dest[len] = '\0'; + } + return ret; +} +#endif + #ifndef __HAVE_ARCH_STRCAT /** * strcat - Append one %NUL-terminated string to another
On 20 November 2014 12:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Move strlcpy() definition from drivers/usb/gadget/ether.c to lib/string.c because it is a very useful function. Let's add the prototype to include/linux/string.h too.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
Changes in v2: None
Acked-by: Simon Glass sjg@chromium.org
On 20 November 2014 at 07:27, Simon Glass sjg@chromium.org wrote:
On 20 November 2014 12:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Move strlcpy() definition from drivers/usb/gadget/ether.c to lib/string.c because it is a very useful function. Let's add the prototype to include/linux/string.h too.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
Changes in v2: None
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!
We are adding more and more drivers to driver model these days. Compared with the ad-hoc driver system, it seems pretty difficult to understand how drivers are working on driver model. For ex. - Which devices have been bound? - Which devices have been probed? - Which is the parent of this device? etc.
I hope this tool will help us test/review driver model patches.
Just hit "showdev" on the command line and it will list all the bound devices in a tree-like format with Class and Probed flag.
=> showdev Class Probed Name ---------------------------------------- root [ + ] root_driver demo [ ] |-- demo_shape_drv demo [ ] |-- demo_simple_drv demo [ ] |-- demo_shape_drv demo [ ] |-- demo_simple_drv demo [ ] |-- demo_shape_drv test [ ] |-- test_drv test [ ] |-- test_drv test [ ] |-- test_drv gpio [ ] |-- gpio_sandbox serial [ ] |-- serial_sandbox serial [ + ] |-- serial demo [ ] |-- triangle demo [ ] |-- square demo [ ] |-- hexagon gpio [ ] |-- gpios i2c [ + ] |-- i2c@0 i2c_eeprom [ + ] | |-- eeprom@2c i2c_emul [ + ] | | -- emul i2c_generic [ + ] | -- generic_59 spi [ ] |-- spi@0 spi_emul [ ] | -- flash@0 cros_ec [ + ] -- cros-ec@0
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com ---
Changes in v2: - Fix the tree format
common/Makefile | 1 + common/cmd_showdev.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 common/cmd_showdev.c
diff --git a/common/Makefile b/common/Makefile index 6cc4de8..c26c764 100644 --- a/common/Makefile +++ b/common/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o obj-$(CONFIG_CMD_DATE) += cmd_date.o obj-$(CONFIG_CMD_DEMO) += cmd_demo.o +obj-$(CONFIG_CMD_SHOWDEV) += cmd_showdev.o obj-$(CONFIG_CMD_SOUND) += cmd_sound.o ifdef CONFIG_4xx obj-$(CONFIG_CMD_SETGETDCR) += cmd_dcr.o diff --git a/common/cmd_showdev.c b/common/cmd_showdev.c new file mode 100644 index 0000000..f73bfba --- /dev/null +++ b/common/cmd_showdev.c @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2014 Panasonic Corporation + * Author: Masahiro Yamada yamada.m@jp.panasonic.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <linux/string.h> +#include <linux/list.h> +#include <dm/device.h> +#include <dm/uclass.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define INDENT1 " " +#define INDENT2 "| " +#define INDENT3 "\-- " +#define INDENT4 "|-- " + +static void show_devices(struct udevice *dev, int depth, int last_flag) +{ + int i, is_last; + struct udevice *child; + char class_name[12]; + + /* print the first 11 characters to not break the tree-format. */ + strlcpy(class_name, dev->uclass->uc_drv->name, sizeof(class_name)); + printf(" %-11s ", class_name); + + printf("[ %c ] ", dev->flags & DM_FLAG_ACTIVATED ? '+' : ' '); + + for (i = depth; i >= 0; i--) { + is_last = (last_flag >> i) & 1; + if (i) { + if (is_last) + printf(INDENT1); + else + printf(INDENT2); + } else { + if (is_last) + printf(INDENT3); + else + printf(INDENT4); + } + } + + printf("%s\n", dev->name); + + list_for_each_entry(child, &dev->child_head, sibling_node) { + is_last = list_is_last(&child->sibling_node, &dev->child_head); + show_devices(child, depth + 1, (last_flag << 1) | is_last); + } +} + +static int do_showdev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + printf(" Class Probed Name\n"); + printf("----------------------------------------\n"); + + show_devices(gd->dm_root, -1, 0); + + return 0; +} + +U_BOOT_CMD( + showdev, 1, 1, do_showdev, + "show devices in a tree-like format.", + "" +);
Hi Masahiro,
On 20 November 2014 12:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
We are adding more and more drivers to driver model these days. Compared with the ad-hoc driver system, it seems pretty difficult to understand how drivers are working on driver model. For ex.
- Which devices have been bound?
- Which devices have been probed?
- Which is the parent of this device?
etc.
I hope this tool will help us test/review driver model patches.
Just hit "showdev" on the command line and it will list all the bound devices in a tree-like format with Class and Probed flag.
This looks very similar to the 'dm tree' command. Can we unify these? Perhaps move the commands into common/cmd_dm.c? Then we can use the CONFIG_CMD_DM option instead of a new one.
Also I think 'dm xxx' is better than things 'showdev'. The 'dm' prefix can be used for all DM commands.
=> showdev Class Probed Name
root [ + ] root_driver demo [ ] |-- demo_shape_drv demo [ ] |-- demo_simple_drv demo [ ] |-- demo_shape_drv demo [ ] |-- demo_simple_drv demo [ ] |-- demo_shape_drv test [ ] |-- test_drv test [ ] |-- test_drv test [ ] |-- test_drv gpio [ ] |-- gpio_sandbox serial [ ] |-- serial_sandbox serial [ + ] |-- serial demo [ ] |-- triangle demo [ ] |-- square demo [ ] |-- hexagon gpio [ ] |-- gpios i2c [ + ] |-- i2c@0 i2c_eeprom [ + ] | |-- eeprom@2c i2c_emul [ + ] | | -- emul i2c_generic [ + ] | -- generic_59 spi [ ] |-- spi@0 spi_emul [ ] | -- flash@0 cros_ec [ + ] -- cros-ec@0
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
Changes in v2:
- Fix the tree format
common/Makefile | 1 + common/cmd_showdev.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 common/cmd_showdev.c
diff --git a/common/Makefile b/common/Makefile index 6cc4de8..c26c764 100644 --- a/common/Makefile +++ b/common/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o obj-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o obj-$(CONFIG_CMD_DATE) += cmd_date.o obj-$(CONFIG_CMD_DEMO) += cmd_demo.o +obj-$(CONFIG_CMD_SHOWDEV) += cmd_showdev.o obj-$(CONFIG_CMD_SOUND) += cmd_sound.o ifdef CONFIG_4xx obj-$(CONFIG_CMD_SETGETDCR) += cmd_dcr.o diff --git a/common/cmd_showdev.c b/common/cmd_showdev.c new file mode 100644 index 0000000..f73bfba --- /dev/null +++ b/common/cmd_showdev.c @@ -0,0 +1,70 @@ +/*
- Copyright (C) 2014 Panasonic Corporation
- Author: Masahiro Yamada yamada.m@jp.panasonic.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <linux/string.h> +#include <linux/list.h> +#include <dm/device.h> +#include <dm/uclass.h>
+DECLARE_GLOBAL_DATA_PTR;
+#define INDENT1 " " +#define INDENT2 "| " +#define INDENT3 "\-- " +#define INDENT4 "|-- "
Is there really any value in these?
+static void show_devices(struct udevice *dev, int depth, int last_flag) +{
int i, is_last;struct udevice *child;char class_name[12];/* print the first 11 characters to not break the tree-format. */strlcpy(class_name, dev->uclass->uc_drv->name, sizeof(class_name));printf(" %-11s ", class_name);printf("[ %c ] ", dev->flags & DM_FLAG_ACTIVATED ? '+' : ' ');
This line could be combined with the above.
for (i = depth; i >= 0; i--) {is_last = (last_flag >> i) & 1;if (i) {if (is_last)printf(INDENT1);elseprintf(INDENT2);} else {if (is_last)printf(INDENT3);elseprintf(INDENT4);}}printf("%s\n", dev->name);list_for_each_entry(child, &dev->child_head, sibling_node) {is_last = list_is_last(&child->sibling_node, &dev->child_head);show_devices(child, depth + 1, (last_flag << 1) | is_last);}+}
+static int do_showdev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
printf(" Class Probed Name\n");
Remove the first space?
printf("----------------------------------------\n");show_devices(gd->dm_root, -1, 0);return 0;+}
+U_BOOT_CMD(
showdev, 1, 1, do_showdev,"show devices in a tree-like format.",""+);
1.9.1
Regards, Simon
Hi Simon,
On Thu, 20 Nov 2014 15:00:41 +0000 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 20 November 2014 12:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
We are adding more and more drivers to driver model these days. Compared with the ad-hoc driver system, it seems pretty difficult to understand how drivers are working on driver model. For ex.
- Which devices have been bound?
- Which devices have been probed?
- Which is the parent of this device?
etc.
I hope this tool will help us test/review driver model patches.
Just hit "showdev" on the command line and it will list all the bound devices in a tree-like format with Class and Probed flag.
This looks very similar to the 'dm tree' command. Can we unify these? Perhaps move the commands into common/cmd_dm.c? Then we can use the CONFIG_CMD_DM option instead of a new one.
Also I think 'dm xxx' is better than things 'showdev'. The 'dm' prefix can be used for all DM commands.
Sorry, I did not know you had already implemented a similar one. There is no point to have two commands to do equivalent stuff.
The difference is some appearances and that pointer addresses are missing from mine.
I think my 4 columns indentation is more readable, but it might be my personal preference.
So, if you prefer the current one, feel free to reject mine, of course. Or shall I change "dm tree" to something like mine?
What shall I do?
+#include <common.h> +#include <linux/string.h> +#include <linux/list.h> +#include <dm/device.h> +#include <dm/uclass.h>
+DECLARE_GLOBAL_DATA_PTR;
+#define INDENT1 " " +#define INDENT2 "| " +#define INDENT3 "\-- " +#define INDENT4 "|-- "
Is there really any value in these?
Not really. I just thought that it might be helpful if we want to change the indentation width.
+static void show_devices(struct udevice *dev, int depth, int last_flag) +{
int i, is_last;struct udevice *child;char class_name[12];/* print the first 11 characters to not break the tree-format. */strlcpy(class_name, dev->uclass->uc_drv->name, sizeof(class_name));printf(" %-11s ", class_name);printf("[ %c ] ", dev->flags & DM_FLAG_ACTIVATED ? '+' : ' ');This line could be combined with the above.
Yes, indeed.
for (i = depth; i >= 0; i--) {is_last = (last_flag >> i) & 1;if (i) {if (is_last)printf(INDENT1);elseprintf(INDENT2);} else {if (is_last)printf(INDENT3);elseprintf(INDENT4);}}printf("%s\n", dev->name);list_for_each_entry(child, &dev->child_head, sibling_node) {is_last = list_is_last(&child->sibling_node, &dev->child_head);show_devices(child, depth + 1, (last_flag << 1) | is_last);}+}
+static int do_showdev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
printf(" Class Probed Name\n");Remove the first space?
I inserted the first space for readability, but it might not be necessary.
BTW, I read your code but I could not understand why you used list_for_each_entry_safe() in display_succ(), whilst list_for_each_entry() in do_dm_dump_uclass().
Is there a special reason for inconsintency?
Best Regards Masahiro Yamada
Hi Masahiro,
On 21 November 2014 08:04, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Thu, 20 Nov 2014 15:00:41 +0000 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 20 November 2014 12:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
We are adding more and more drivers to driver model these days. Compared with the ad-hoc driver system, it seems pretty difficult to understand how drivers are working on driver model. For ex.
- Which devices have been bound?
- Which devices have been probed?
- Which is the parent of this device?
etc.
I hope this tool will help us test/review driver model patches.
Just hit "showdev" on the command line and it will list all the bound devices in a tree-like format with Class and Probed flag.
This looks very similar to the 'dm tree' command. Can we unify these? Perhaps move the commands into common/cmd_dm.c? Then we can use the CONFIG_CMD_DM option instead of a new one.
Also I think 'dm xxx' is better than things 'showdev'. The 'dm' prefix can be used for all DM commands.
Sorry, I did not know you had already implemented a similar one. There is no point to have two commands to do equivalent stuff.
The difference is some appearances and that pointer addresses are missing from mine.
I think my 4 columns indentation is more readable, but it might be my personal preference.
So, if you prefer the current one, feel free to reject mine, of course. Or shall I change "dm tree" to something like mine?
What shall I do?
Yours is prettier - I've been meaning to fix the formatting one day. So let's go with yours if you you can address the comments and overwrite or adjust 'dm tree'.
+#include <common.h> +#include <linux/string.h> +#include <linux/list.h> +#include <dm/device.h> +#include <dm/uclass.h>
+DECLARE_GLOBAL_DATA_PTR;
+#define INDENT1 " " +#define INDENT2 "| " +#define INDENT3 "\-- " +#define INDENT4 "|-- "
Is there really any value in these?
Not really. I just thought that it might be helpful if we want to change the indentation width.
+static void show_devices(struct udevice *dev, int depth, int last_flag) +{
int i, is_last;struct udevice *child;char class_name[12];/* print the first 11 characters to not break the tree-format. */strlcpy(class_name, dev->uclass->uc_drv->name, sizeof(class_name));printf(" %-11s ", class_name);printf("[ %c ] ", dev->flags & DM_FLAG_ACTIVATED ? '+' : ' ');This line could be combined with the above.
Yes, indeed.
for (i = depth; i >= 0; i--) {is_last = (last_flag >> i) & 1;if (i) {if (is_last)printf(INDENT1);elseprintf(INDENT2);} else {if (is_last)printf(INDENT3);elseprintf(INDENT4);}}printf("%s\n", dev->name);list_for_each_entry(child, &dev->child_head, sibling_node) {is_last = list_is_last(&child->sibling_node, &dev->child_head);show_devices(child, depth + 1, (last_flag << 1) | is_last);}+}
+static int do_showdev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
printf(" Class Probed Name\n");Remove the first space?
I inserted the first space for readability, but it might not be necessary.
BTW, I read your code but I could not understand why you used list_for_each_entry_safe() in display_succ(), whilst list_for_each_entry() in do_dm_dump_uclass().
Is there a special reason for inconsintency?
I don't think we need the safe version. Previously the code may have probed devices as it worked, which could cause problems. But that was a bug.
Regards, Simon
Of course, this command can be used on any boards besides sandbox.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com ---
Changes in v2: None
include/configs/sandbox.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index ee4b244..334142b 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -26,6 +26,7 @@ #define CONFIG_DM #define CONFIG_CMD_DEMO #define CONFIG_CMD_DM +#define CONFIG_CMD_SHOWDEV #define CONFIG_DM_DEMO #define CONFIG_DM_DEMO_SIMPLE #define CONFIG_DM_DEMO_SHAPE
participants (2)
-
Masahiro Yamada -
Simon Glass