[PATCH 1/6] commands: add new uptime command

[PATCH 1/6] commands: add new uptime command

[Ahmad Fatoum]

We have a time command to record the delta of get_time_ns() to time
command execution, but have no command to just print get_time_ns().

Such a command can be useful to debug clocksources or to verify that a
system is still running and hasn't been reset by a yet unhandled
watchdog in-between.

Make development a bit easier by providing a new uptime command.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig  | 13 +++++++++
 commands/Makefile |  1 +
 commands/uptime.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 commands/uptime.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 2ce990b5616a..a59616ad1474 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2289,6 +2289,19 @@ config CMD_TIME
 	  Note: This command depends on COMMAND being interruptible,
 	  otherwise the timer may overrun resulting in incorrect results
 
+config CMD_UPTIME
+	bool "uptime"
+	help
+	  uptime - Tell how long barebox has been running
+
+	  Usage: uptime [-n]
+
+	  This command formats the number of elapsed nanoseconds
+	  as measured with the current clocksource.
+
+	  Options:
+	    -n		output elapsed time in nanoseconds
+
 config CMD_STATE
 	tristate
 	depends on STATE
diff --git a/commands/Makefile b/commands/Makefile
index 68d0d05365a5..cac1d4f2535b 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_CMD_WD)		+= wd.o
 obj-$(CONFIG_CMD_LED_TRIGGER)	+= trigger.o
 obj-$(CONFIG_CMD_USB)		+= usb.o
 obj-$(CONFIG_CMD_TIME)		+= time.o
+obj-$(CONFIG_CMD_UPTIME)	+= uptime.o
 obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
 obj-$(CONFIG_CMD_OF_DIFF)	+= of_diff.o
 obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
diff --git a/commands/uptime.c b/commands/uptime.c
new file mode 100644
index 000000000000..e220fec72139
--- /dev/null
+++ b/commands/uptime.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <common.h>
+#include <command.h>
+#include <clock.h>
+#include <linux/math64.h>
+
+#define NSEC_PER_MINUTE	(NSEC_PER_SEC * 60LL)
+#define NSEC_PER_HOUR	(NSEC_PER_MINUTE * 60LL)
+#define NSEC_PER_DAY	(NSEC_PER_HOUR * 24LL)
+#define NSEC_PER_WEEK	(NSEC_PER_DAY * 7LL)
+
+static bool print_with_unit(u64 val, const char *unit, bool comma)
+{
+	if (!val)
+		return comma;
+
+	printf("%s%llu %s%s", comma ? ", " : "", val, unit, val > 1 ? "s" : "");
+	return true;
+}
+
+static int do_uptime(int argc, char *argv[])
+{
+	u64 timestamp, weeks, days, hours, minutes;
+	bool comma = false;
+
+	timestamp = get_time_ns();
+
+	if (argc == 2 && !strcmp(argv[1], "-n")) {
+		printf("up %lluns\n", timestamp);
+		return 0;
+	}
+
+	if (argc != 1)
+		return COMMAND_ERROR_USAGE;
+
+	printf("up ");
+
+	weeks = div64_u64_rem(timestamp, NSEC_PER_WEEK, &timestamp);
+	days = div64_u64_rem(timestamp, NSEC_PER_DAY, &timestamp);
+	hours = div64_u64_rem(timestamp, NSEC_PER_HOUR, &timestamp);
+	minutes = div64_u64_rem(timestamp, NSEC_PER_MINUTE, &timestamp);
+
+	comma = print_with_unit(weeks, "week", false);
+	comma = print_with_unit(days, "day", comma);
+	comma = print_with_unit(hours, "hour", comma);
+	comma = print_with_unit(minutes, "minute", comma);
+
+	if (!comma) {
+		u64 seconds = div64_u64_rem(timestamp, NSEC_PER_SEC, &timestamp);
+		print_with_unit(seconds, "second", false);
+	}
+
+	printf("\n");
+
+	return 0;
+}
+
+BAREBOX_CMD_HELP_START(uptime)
+BAREBOX_CMD_HELP_TEXT("This command formats the number of elapsed nanoseconds")
+BAREBOX_CMD_HELP_TEXT("as measured with the current clocksource")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-n",     "output elapsed time in nanoseconds")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(uptime)
+	.cmd		= do_uptime,
+	BAREBOX_CMD_DESC("Tell how long barebox has been running")
+	BAREBOX_CMD_OPTS("[-n]")
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_HELP(cmd_uptime_help)
+BAREBOX_CMD_END
-- 
2.30.2

[PATCH 2/6] commands: time: refactor into new strjoin

[Ahmad Fatoum]

time concatenates all its remaining arguments with a space in-between
and then passes that to the command executor. This can be useful
elsewhere as well, so factor it out into a new strjoin function.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/time.c  | 11 +----------
 include/string.h |  2 ++
 lib/string.c     | 22 ++++++++++++++++++++++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/commands/time.c b/commands/time.c
index 5b8933ea6553..336128f6a9be 100644
--- a/commands/time.c
+++ b/commands/time.c
@@ -12,26 +12,17 @@ static int do_time(int argc, char *argv[])
 	unsigned char *buf;
 	u64 start, end, diff64;
 	bool nanoseconds = false;
-	int len = 1; /* '\0' */
 
 	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
 
-	for (i = 1; i < argc; i++)
-		len += strlen(argv[i]) + 1;
-
-	buf = xzalloc(len);
-
 	i = 1;
 	if (!strcmp(argv[i], "-n")) {
 		nanoseconds = true;
 		i++;
 	}
 
-	for (; i < argc; i++) {
-		strcat(buf, argv[i]);
-		strcat(buf, " ");
-	}
+	buf = strjoin(" ", &argv[i], argc - i);
 
 	start = get_time_ns();
 
diff --git a/include/string.h b/include/string.h
index d423bee6fba5..2cc727fd1d7a 100644
--- a/include/string.h
+++ b/include/string.h
@@ -17,4 +17,6 @@ void *__nokasan_default_memcpy(void * dest,const void *src,size_t count);
 
 char *parse_assignment(char *str);
 
+char *strjoin(const char *separator, char **array, size_t len);
+
 #endif /* __STRING_H */
diff --git a/lib/string.c b/lib/string.c
index 6389217d5b41..a500e8a3d1ba 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -938,3 +938,25 @@ char *parse_assignment(char *str)
 
 	return value;
 }
+
+char *strjoin(const char *separator, char **arr, size_t arrlen)
+{
+	size_t separatorlen;
+	int len = 1; /* '\0' */
+	char *buf;
+	int i;
+
+	separatorlen = strlen(separator);
+
+	for (i = 0; i < arrlen; i++)
+		len += strlen(arr[i]) + separatorlen;
+
+	buf = xzalloc(len);
+
+	for (i = 0; i < arrlen; i++) {
+		strcat(buf, arr[i]);
+		strcat(buf, separator);
+	}
+
+	return buf;
+}
-- 
2.30.2

[PATCH 3/6] string: reduce strjoin runtime, drop trailing separator

[Ahmad Fatoum]

The implementation of strjoin is a bit suboptimal. The destination
string is traversed from the beginning due to strcat and we have a
left-over separator at the end, while it should only be in-between.

Fix this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/string.h |  1 +
 lib/string.c     | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/string.h b/include/string.h
index 2cc727fd1d7a..596440ca8164 100644
--- a/include/string.h
+++ b/include/string.h
@@ -4,6 +4,7 @@
 
 #include <linux/string.h>
 
+void *mempcpy(void *dest, const void *src, size_t count);
 int strtobool(const char *str, int *val);
 char *strsep_unescaped(char **, const char *);
 char *stpcpy(char *dest, const char *src);
diff --git a/lib/string.c b/lib/string.c
index a500e8a3d1ba..edd36da4d4f2 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -603,6 +603,11 @@ void *__memcpy(void * dest, const void *src, size_t count)
 	__alias(__default_memcpy);
 #endif
 
+void *mempcpy(void *dest, const void *src, size_t count)
+{
+	return memcpy(dest, src, count) + count;
+}
+EXPORT_SYMBOL(mempcpy);
 
 #ifndef __HAVE_ARCH_MEMMOVE
 /**
@@ -943,7 +948,7 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
 {
 	size_t separatorlen;
 	int len = 1; /* '\0' */
-	char *buf;
+	char *buf, *p;
 	int i;
 
 	separatorlen = strlen(separator);
@@ -951,12 +956,14 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
 	for (i = 0; i < arrlen; i++)
 		len += strlen(arr[i]) + separatorlen;
 
-	buf = xzalloc(len);
+	p = buf = xmalloc(len);
 
 	for (i = 0; i < arrlen; i++) {
-		strcat(buf, arr[i]);
-		strcat(buf, separator);
+		p = stpcpy(p, arr[i]);
+		p = mempcpy(p, separator, separatorlen);
 	}
 
+	p[-separatorlen] = '\0';
+
 	return buf;
 }
-- 
2.30.2

[PATCH 4/6] test: self: add strjoin tests

[Ahmad Fatoum]

Just to make sure strjoin works as intended, add some simple unit tests.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/Kconfig  |  5 +++++
 test/self/Makefile |  1 +
 test/self/string.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100644 test/self/string.c

diff --git a/test/self/Kconfig b/test/self/Kconfig
index f3cb6601e3b8..3a5e7795fea8 100644
--- a/test/self/Kconfig
+++ b/test/self/Kconfig
@@ -38,6 +38,11 @@ config SELFTEST_ENABLE_ALL
 	help
 	  Selects all self-tests compatible with current configuration
 
+config SELFTEST_STRING
+	bool "string selftest"
+	help
+	  Tests some of the string library functions
+
 config SELFTEST_MALLOC
 	bool "malloc() selftest"
 	help
diff --git a/test/self/Makefile b/test/self/Makefile
index 6f2c0d394034..5d9d772d13b0 100644
--- a/test/self/Makefile
+++ b/test/self/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_SELFTEST) += core.o
+obj-$(CONFIG_SELFTEST_STRING) += string.o
 obj-$(CONFIG_SELFTEST_MALLOC) += malloc.o
 obj-$(CONFIG_SELFTEST_PRINTF) += printf.o
 obj-$(CONFIG_SELFTEST_PROGRESS_NOTIFIER) += progress-notifier.o
diff --git a/test/self/string.c b/test/self/string.c
new file mode 100644
index 000000000000..b5785da20d8d
--- /dev/null
+++ b/test/self/string.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <common.h>
+#include <bselftest.h>
+#include <malloc.h>
+#include <string.h>
+
+BSELFTEST_GLOBALS();
+
+static void __expect_streq(const char *is, const char *expect,
+			   const char *func, int line)
+{
+	total_tests++;
+	if (strcmp(is, expect)) {
+		failed_tests++;
+		printf("%s:%d: got %s, but %s expected\n", func, line, is, expect);
+	}
+}
+
+#define expect_streq(is, expect) \
+	__expect_streq(is, expect, __func__, __LINE__)
+
+static void test_string(void)
+{
+	char *strs[] = { "ayy", "bee", "cee" };
+	char *buf;
+
+	buf = strjoin("", strs, ARRAY_SIZE(strs));
+	expect_streq(buf, "ayybeecee");
+	free(buf);
+
+	buf = strjoin(" ", strs, ARRAY_SIZE(strs));
+	expect_streq(buf, "ayy bee cee");
+	free(buf);
+
+	buf = strjoin(", ", strs, ARRAY_SIZE(strs));
+	expect_streq(buf, "ayy, bee, cee");
+	free(buf);
+
+	buf = strjoin(" ", strs, 1);
+	expect_streq(buf, "ayy");
+	free(buf);
+}
+bselftest(core, test_string);
-- 
2.30.2

[PATCH 5/6] commands: drvinfo: support filtering by driver

[Ahmad Fatoum]

drvinfo can be very long especially for the in-tree defconfigs,
add optional filtering support:

  barebox@board:/ drvinfo imx7d
  Driver  Device(s)
  --------------------
  imx7d-src
          30390000.reset-controller@30390000.of
  imx7d_adc
          30610000.adc@30610000.of
          30620000.adc@30620000.of

  Use 'devinfo DEVICE' for more information

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/drvinfo.c | 13 +++++++++++++
 common/complete.c  | 21 +++++++++++++++++++++
 include/complete.h |  1 +
 3 files changed, 35 insertions(+)

diff --git a/commands/drvinfo.c b/commands/drvinfo.c
index 9f8f971ee9dd..27ff55f01d90 100644
--- a/commands/drvinfo.c
+++ b/commands/drvinfo.c
@@ -5,15 +5,23 @@
 #include <common.h>
 #include <command.h>
 #include <driver.h>
+#include <complete.h>
 
 static int do_drvinfo(int argc, char *argv[])
 {
+	char *filter = NULL;
 	struct driver_d *drv;
 	struct device_d *dev;
 
+	if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && argc > 1)
+		filter = strjoin(" ", &argv[1], argc - 1);
+
 	printf("Driver\tDevice(s)\n");
 	printf("--------------------\n");
 	for_each_driver(drv) {
+		if (filter && !str_has_prefix(drv->name, filter))
+			continue;
+
 		printf("%s\n",drv->name);
 		for_each_device(dev) {
 			if (dev->driver == drv)
@@ -24,6 +32,7 @@ static int do_drvinfo(int argc, char *argv[])
 	if (IS_ENABLED(CONFIG_CMD_DEVINFO))
 		printf("\nUse 'devinfo DEVICE' for more information\n");
 
+	free(filter);
 	return 0;
 }
 
@@ -31,5 +40,9 @@ static int do_drvinfo(int argc, char *argv[])
 BAREBOX_CMD_START(drvinfo)
 	.cmd		= do_drvinfo,
 	BAREBOX_CMD_DESC("list compiled-in device drivers")
+#ifdef CONFIG_AUTO_COMPLETE
+	BAREBOX_CMD_OPTS("[DRIVER]")
+#endif
 	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
+	BAREBOX_CMD_COMPLETE(driver_complete)
 BAREBOX_CMD_END
diff --git a/common/complete.c b/common/complete.c
index ab3c98549314..916d13d776ce 100644
--- a/common/complete.c
+++ b/common/complete.c
@@ -174,6 +174,27 @@ static int device_param_complete(struct device_d *dev, struct string_list *sl,
 	return 0;
 }
 
+int driver_complete(struct string_list *sl, char *instr)
+{
+	struct driver_d *drv;
+	int len;
+
+	if (!instr)
+		instr = "";
+
+	len = strlen(instr);
+
+	for_each_driver(drv) {
+		if (strncmp(instr, drv->name, len))
+			continue;
+
+		string_list_add_asprintf(sl, "%s ", drv->name);
+	}
+
+	return COMPLETE_CONTINUE;
+}
+EXPORT_SYMBOL(driver_complete);
+
 int empty_complete(struct string_list *sl, char *instr)
 {
 	return COMPLETE_END;
diff --git a/include/complete.h b/include/complete.h
index b0e675b5599f..2068760ac235 100644
--- a/include/complete.h
+++ b/include/complete.h
@@ -14,6 +14,7 @@ void complete_reset(void);
 
 int command_complete(struct string_list *sl, char *instr);
 int device_complete(struct string_list *sl, char *instr);
+int driver_complete(struct string_list *sl, char *instr);
 int empty_complete(struct string_list *sl, char *instr);
 int eth_complete(struct string_list *sl, char *instr);
 int command_var_complete(struct string_list *sl, char *instr);
-- 
2.30.2

[PATCH 6/6] test: self: only include ramfs selftest when CONFIG_SELFTEST_FS_RAMFS=y

[Ahmad Fatoum]

So far, we ignored the symbol and built the test always when
CONFIG_SELFTEST=y and CONFIG_FS_RAMFS=y.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/Kconfig  | 2 +-
 test/self/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/self/Kconfig b/test/self/Kconfig
index 3a5e7795fea8..9c0043e29e6a 100644
--- a/test/self/Kconfig
+++ b/test/self/Kconfig
@@ -33,7 +33,7 @@ config SELFTEST_ENABLE_ALL
 	select SELFTEST_PROGRESS_NOTIFIER
 	select SELFTEST_OF_MANIPULATION
 	select SELFTEST_ENVIRONMENT_VARIABLES if ENVIRONMENT_VARIABLES
-	select SELFTEST_FS_RAMFS
+	imply SELFTEST_FS_RAMFS
 	imply SELFTEST_TFTP
 	help
 	  Selects all self-tests compatible with current configuration
diff --git a/test/self/Makefile b/test/self/Makefile
index 5d9d772d13b0..5ec36e0660ef 100644
--- a/test/self/Makefile
+++ b/test/self/Makefile
@@ -7,4 +7,4 @@ obj-$(CONFIG_SELFTEST_PRINTF) += printf.o
 obj-$(CONFIG_SELFTEST_PROGRESS_NOTIFIER) += progress-notifier.o
 obj-$(CONFIG_SELFTEST_OF_MANIPULATION) += of_manipulation.o of_manipulation.dtb.o
 obj-$(CONFIG_SELFTEST_ENVIRONMENT_VARIABLES) += envvar.o
-obj-$(CONFIG_FS_RAMFS) += ramfs.o
+obj-$(CONFIG_SELFTEST_FS_RAMFS) += ramfs.o
-- 
2.30.2

Re: [PATCH 3/6] string: reduce strjoin runtime, drop trailing separator

[Sascha Hauer]

On Wed, Oct 26, 2022 at 08:42:02AM +0200, Ahmad Fatoum wrote:
> The implementation of strjoin is a bit suboptimal. The destination
> string is traversed from the beginning due to strcat and we have a
> left-over separator at the end, while it should only be in-between.
> 
> Fix this.

Rather than fixing a just introduced function I would expect a patch
introducing strjoin() and then another one converting the time command
over to use it.

> +void *mempcpy(void *dest, const void *src, size_t count)
> +{
> +	return memcpy(dest, src, count) + count;
> +}
> +EXPORT_SYMBOL(mempcpy);
>  
>  #ifndef __HAVE_ARCH_MEMMOVE
>  /**
> @@ -943,7 +948,7 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>  {
>  	size_t separatorlen;
>  	int len = 1; /* '\0' */
> -	char *buf;
> +	char *buf, *p;
>  	int i;
>  
>  	separatorlen = strlen(separator);
> @@ -951,12 +956,14 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>  	for (i = 0; i < arrlen; i++)
>  		len += strlen(arr[i]) + separatorlen;
>  
> -	buf = xzalloc(len);
> +	p = buf = xmalloc(len);
>  
>  	for (i = 0; i < arrlen; i++) {
> -		strcat(buf, arr[i]);
> -		strcat(buf, separator);
> +		p = stpcpy(p, arr[i]);
> +		p = mempcpy(p, separator, separatorlen);
>  	}
>  
> +	p[-separatorlen] = '\0';

That's a bit hard to read. How about:

	for (i = 0; i < arrlen; i++) {
		p = stpcpy(p, arr[i]);
		if (i < arrlen - 1)
			p = stpcpy(p, separator);
	}

That would also allow you to optimize the allocated buffer size above.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/6] commands: add new uptime command

[Sascha Hauer]

On Wed, Oct 26, 2022 at 08:42:00AM +0200, Ahmad Fatoum wrote:
> We have a time command to record the delta of get_time_ns() to time
> command execution, but have no command to just print get_time_ns().
> 
> Such a command can be useful to debug clocksources or to verify that a
> system is still running and hasn't been reset by a yet unhandled
> watchdog in-between.
> 
> Make development a bit easier by providing a new uptime command.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/Kconfig  | 13 +++++++++
>  commands/Makefile |  1 +
>  commands/uptime.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
>  create mode 100644 commands/uptime.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 2ce990b5616a..a59616ad1474 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -2289,6 +2289,19 @@ config CMD_TIME
>  	  Note: This command depends on COMMAND being interruptible,
>  	  otherwise the timer may overrun resulting in incorrect results
>  
> +config CMD_UPTIME
> +	bool "uptime"
> +	help
> +	  uptime - Tell how long barebox has been running
> +
> +	  Usage: uptime [-n]
> +
> +	  This command formats the number of elapsed nanoseconds
> +	  as measured with the current clocksource.
> +
> +	  Options:
> +	    -n		output elapsed time in nanoseconds
> +
>  config CMD_STATE
>  	tristate
>  	depends on STATE
> diff --git a/commands/Makefile b/commands/Makefile
> index 68d0d05365a5..cac1d4f2535b 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_CMD_WD)		+= wd.o
>  obj-$(CONFIG_CMD_LED_TRIGGER)	+= trigger.o
>  obj-$(CONFIG_CMD_USB)		+= usb.o
>  obj-$(CONFIG_CMD_TIME)		+= time.o
> +obj-$(CONFIG_CMD_UPTIME)	+= uptime.o
>  obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
>  obj-$(CONFIG_CMD_OF_DIFF)	+= of_diff.o
>  obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
> diff --git a/commands/uptime.c b/commands/uptime.c
> new file mode 100644
> index 000000000000..e220fec72139
> --- /dev/null
> +++ b/commands/uptime.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <common.h>
> +#include <command.h>
> +#include <clock.h>
> +#include <linux/math64.h>
> +
> +#define NSEC_PER_MINUTE	(NSEC_PER_SEC * 60LL)
> +#define NSEC_PER_HOUR	(NSEC_PER_MINUTE * 60LL)
> +#define NSEC_PER_DAY	(NSEC_PER_HOUR * 24LL)
> +#define NSEC_PER_WEEK	(NSEC_PER_DAY * 7LL)
> +
> +static bool print_with_unit(u64 val, const char *unit, bool comma)
> +{
> +	if (!val)
> +		return comma;
> +
> +	printf("%s%llu %s%s", comma ? ", " : "", val, unit, val > 1 ? "s" : "");
> +	return true;
> +}
> +
> +static int do_uptime(int argc, char *argv[])
> +{
> +	u64 timestamp, weeks, days, hours, minutes;
> +	bool comma = false;
> +
> +	timestamp = get_time_ns();
> +
> +	if (argc == 2 && !strcmp(argv[1], "-n")) {
> +		printf("up %lluns\n", timestamp);
> +		return 0;
> +	}

Use getopt(), that's what we have it for.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/6] string: reduce strjoin runtime, drop trailing separator

[Ahmad Fatoum]

Hi,

On 27.10.22 08:56, Sascha Hauer wrote:
> On Wed, Oct 26, 2022 at 08:42:02AM +0200, Ahmad Fatoum wrote:
>> The implementation of strjoin is a bit suboptimal. The destination
>> string is traversed from the beginning due to strcat and we have a
>> left-over separator at the end, while it should only be in-between.
>>
>> Fix this.
> 
> Rather than fixing a just introduced function I would expect a patch
> introducing strjoin() and then another one converting the time command
> over to use it.

What difference does it make?

>> +void *mempcpy(void *dest, const void *src, size_t count)
>> +{
>> +	return memcpy(dest, src, count) + count;
>> +}
>> +EXPORT_SYMBOL(mempcpy);
>>  
>>  #ifndef __HAVE_ARCH_MEMMOVE
>>  /**
>> @@ -943,7 +948,7 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>>  {
>>  	size_t separatorlen;
>>  	int len = 1; /* '\0' */
>> -	char *buf;
>> +	char *buf, *p;
>>  	int i;
>>  
>>  	separatorlen = strlen(separator);
>> @@ -951,12 +956,14 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>>  	for (i = 0; i < arrlen; i++)
>>  		len += strlen(arr[i]) + separatorlen;
>>  
>> -	buf = xzalloc(len);
>> +	p = buf = xmalloc(len);
>>  
>>  	for (i = 0; i < arrlen; i++) {
>> -		strcat(buf, arr[i]);
>> -		strcat(buf, separator);
>> +		p = stpcpy(p, arr[i]);
>> +		p = mempcpy(p, separator, separatorlen);
>>  	}
>>  
>> +	p[-separatorlen] = '\0';
> 
> That's a bit hard to read. How about:
> 
> 	for (i = 0; i < arrlen; i++) {
> 		p = stpcpy(p, arr[i]);
> 		if (i < arrlen - 1)
> 			p = stpcpy(p, separator);
> 	}
> 
> That would also allow you to optimize the allocated buffer size above.

I like my version more. I dislike these once-only checks inside loops.

Cheers,
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/6] commands: drvinfo: support filtering by driver

[Sascha Hauer]

On Wed, Oct 26, 2022 at 08:42:04AM +0200, Ahmad Fatoum wrote:
> drvinfo can be very long especially for the in-tree defconfigs,
> add optional filtering support:
> 
>   barebox@board:/ drvinfo imx7d
>   Driver  Device(s)
>   --------------------
>   imx7d-src
>           30390000.reset-controller@30390000.of
>   imx7d_adc
>           30610000.adc@30610000.of
>           30620000.adc@30620000.of
> 
>   Use 'devinfo DEVICE' for more information
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/drvinfo.c | 13 +++++++++++++
>  common/complete.c  | 21 +++++++++++++++++++++
>  include/complete.h |  1 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/commands/drvinfo.c b/commands/drvinfo.c
> index 9f8f971ee9dd..27ff55f01d90 100644
> --- a/commands/drvinfo.c
> +++ b/commands/drvinfo.c
> @@ -5,15 +5,23 @@
>  #include <common.h>
>  #include <command.h>
>  #include <driver.h>
> +#include <complete.h>
>  
>  static int do_drvinfo(int argc, char *argv[])
>  {
> +	char *filter = NULL;
>  	struct driver_d *drv;
>  	struct device_d *dev;
>  
> +	if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && argc > 1)
> +		filter = strjoin(" ", &argv[1], argc - 1);

Why does this depend on CONFIG_AUTO_COMPLETE?

> +
>  	printf("Driver\tDevice(s)\n");
>  	printf("--------------------\n");
>  	for_each_driver(drv) {
> +		if (filter && !str_has_prefix(drv->name, filter))
> +			continue;

I don't see how this is expected to work. When you pass multiple drivers
as argument 'filter' will be a concatenation of multiple driver names
which then matches nothing.

How about using fnmatch? We could then pass things like "*usb*"

Sascha


> +
>  		printf("%s\n",drv->name);
>  		for_each_device(dev) {
>  			if (dev->driver == drv)
> @@ -24,6 +32,7 @@ static int do_drvinfo(int argc, char *argv[])
>  	if (IS_ENABLED(CONFIG_CMD_DEVINFO))
>  		printf("\nUse 'devinfo DEVICE' for more information\n");
>  
> +	free(filter);
>  	return 0;
>  }
>  
> @@ -31,5 +40,9 @@ static int do_drvinfo(int argc, char *argv[])
>  BAREBOX_CMD_START(drvinfo)
>  	.cmd		= do_drvinfo,
>  	BAREBOX_CMD_DESC("list compiled-in device drivers")
> +#ifdef CONFIG_AUTO_COMPLETE
> +	BAREBOX_CMD_OPTS("[DRIVER]")
> +#endif
>  	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
> +	BAREBOX_CMD_COMPLETE(driver_complete)
>  BAREBOX_CMD_END
> diff --git a/common/complete.c b/common/complete.c
> index ab3c98549314..916d13d776ce 100644
> --- a/common/complete.c
> +++ b/common/complete.c
> @@ -174,6 +174,27 @@ static int device_param_complete(struct device_d *dev, struct string_list *sl,
>  	return 0;
>  }
>  
> +int driver_complete(struct string_list *sl, char *instr)
> +{
> +	struct driver_d *drv;
> +	int len;
> +
> +	if (!instr)
> +		instr = "";
> +
> +	len = strlen(instr);
> +
> +	for_each_driver(drv) {
> +		if (strncmp(instr, drv->name, len))
> +			continue;
> +
> +		string_list_add_asprintf(sl, "%s ", drv->name);
> +	}
> +
> +	return COMPLETE_CONTINUE;
> +}
> +EXPORT_SYMBOL(driver_complete);
> +
>  int empty_complete(struct string_list *sl, char *instr)
>  {
>  	return COMPLETE_END;
> diff --git a/include/complete.h b/include/complete.h
> index b0e675b5599f..2068760ac235 100644
> --- a/include/complete.h
> +++ b/include/complete.h
> @@ -14,6 +14,7 @@ void complete_reset(void);
>  
>  int command_complete(struct string_list *sl, char *instr);
>  int device_complete(struct string_list *sl, char *instr);
> +int driver_complete(struct string_list *sl, char *instr);
>  int empty_complete(struct string_list *sl, char *instr);
>  int eth_complete(struct string_list *sl, char *instr);
>  int command_var_complete(struct string_list *sl, char *instr);
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/6] string: reduce strjoin runtime, drop trailing separator

[Sascha Hauer]

On Thu, Oct 27, 2022 at 09:24:20AM +0200, Ahmad Fatoum wrote:
> Hi,
> 
> On 27.10.22 08:56, Sascha Hauer wrote:
> > On Wed, Oct 26, 2022 at 08:42:02AM +0200, Ahmad Fatoum wrote:
> >> The implementation of strjoin is a bit suboptimal. The destination
> >> string is traversed from the beginning due to strcat and we have a
> >> left-over separator at the end, while it should only be in-between.
> >>
> >> Fix this.
> > 
> > Rather than fixing a just introduced function I would expect a patch
> > introducing strjoin() and then another one converting the time command
> > over to use it.
> 
> What difference does it make?

You wouldn't have to fixup code you just introduced.

> > 
> > 	for (i = 0; i < arrlen; i++) {
> > 		p = stpcpy(p, arr[i]);
> > 		if (i < arrlen - 1)
> > 			p = stpcpy(p, separator);
> > 	}
> > 
> > That would also allow you to optimize the allocated buffer size above.
> 
> I like my version more. I dislike these once-only checks inside loops.

No problem ;)

	for (i = 0; i < arrlen - 1; i++) {
		p = stpcpy(p, arr[i]);
		p = stpcpy(p, separator);
	}

	stpcpy(p, arr[i]);

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/6] commands: drvinfo: support filtering by driver

[Ahmad Fatoum]

Hi!

On 27.10.22 09:29, Sascha Hauer wrote:
> On Wed, Oct 26, 2022 at 08:42:04AM +0200, Ahmad Fatoum wrote:
>> drvinfo can be very long especially for the in-tree defconfigs,
>> add optional filtering support:
>> +	if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && argc > 1)
>> +		filter = strjoin(" ", &argv[1], argc - 1);
> 
> Why does this depend on CONFIG_AUTO_COMPLETE?

drvinfo is something I think should always be enabled and I didn't want
to bloat it unconditionally.

> 
>> +
>>  	printf("Driver\tDevice(s)\n");
>>  	printf("--------------------\n");
>>  	for_each_driver(drv) {
>> +		if (filter && !str_has_prefix(drv->name, filter))
>> +			continue;
> 
> I don't see how this is expected to work. When you pass multiple drivers
> as argument 'filter' will be a concatenation of multiple driver names
> which then matches nothing.

Autocomplete doesn't work for elements with spaces, so when you write

  drvinfo TI<Tab>

You get

    drvinfo TI DP83

with no further ability to complete. The prefix matching can work with that
and will produce:

Driver  Device(s)
--------------------
TI DP83867
TI DP83TD510E

Use 'devinfo DEVICE' for more information

> How about using fnmatch? We could then pass things like "*usb*"

I am content with the tab completion.

Cheers,
Ahmad

> 
> Sascha
> 
> 
>> +
>>  		printf("%s\n",drv->name);
>>  		for_each_device(dev) {
>>  			if (dev->driver == drv)
>> @@ -24,6 +32,7 @@ static int do_drvinfo(int argc, char *argv[])
>>  	if (IS_ENABLED(CONFIG_CMD_DEVINFO))
>>  		printf("\nUse 'devinfo DEVICE' for more information\n");
>>  
>> +	free(filter);
>>  	return 0;
>>  }
>>  
>> @@ -31,5 +40,9 @@ static int do_drvinfo(int argc, char *argv[])
>>  BAREBOX_CMD_START(drvinfo)
>>  	.cmd		= do_drvinfo,
>>  	BAREBOX_CMD_DESC("list compiled-in device drivers")
>> +#ifdef CONFIG_AUTO_COMPLETE
>> +	BAREBOX_CMD_OPTS("[DRIVER]")
>> +#endif
>>  	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
>> +	BAREBOX_CMD_COMPLETE(driver_complete)
>>  BAREBOX_CMD_END
>> diff --git a/common/complete.c b/common/complete.c
>> index ab3c98549314..916d13d776ce 100644
>> --- a/common/complete.c
>> +++ b/common/complete.c
>> @@ -174,6 +174,27 @@ static int device_param_complete(struct device_d *dev, struct string_list *sl,
>>  	return 0;
>>  }
>>  
>> +int driver_complete(struct string_list *sl, char *instr)
>> +{
>> +	struct driver_d *drv;
>> +	int len;
>> +
>> +	if (!instr)
>> +		instr = "";
>> +
>> +	len = strlen(instr);
>> +
>> +	for_each_driver(drv) {
>> +		if (strncmp(instr, drv->name, len))
>> +			continue;
>> +
>> +		string_list_add_asprintf(sl, "%s ", drv->name);
>> +	}
>> +
>> +	return COMPLETE_CONTINUE;
>> +}
>> +EXPORT_SYMBOL(driver_complete);
>> +
>>  int empty_complete(struct string_list *sl, char *instr)
>>  {
>>  	return COMPLETE_END;
>> diff --git a/include/complete.h b/include/complete.h
>> index b0e675b5599f..2068760ac235 100644
>> --- a/include/complete.h
>> +++ b/include/complete.h
>> @@ -14,6 +14,7 @@ void complete_reset(void);
>>  
>>  int command_complete(struct string_list *sl, char *instr);
>>  int device_complete(struct string_list *sl, char *instr);
>> +int driver_complete(struct string_list *sl, char *instr);
>>  int empty_complete(struct string_list *sl, char *instr);
>>  int eth_complete(struct string_list *sl, char *instr);
>>  int command_var_complete(struct string_list *sl, char *instr);
>> -- 
>> 2.30.2
>>
>>
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/6] string: reduce strjoin runtime, drop trailing separator

[Ahmad Fatoum]

On 27.10.22 09:33, Sascha Hauer wrote:
> On Thu, Oct 27, 2022 at 09:24:20AM +0200, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 27.10.22 08:56, Sascha Hauer wrote:
>>> On Wed, Oct 26, 2022 at 08:42:02AM +0200, Ahmad Fatoum wrote:
>>>> The implementation of strjoin is a bit suboptimal. The destination
>>>> string is traversed from the beginning due to strcat and we have a
>>>> left-over separator at the end, while it should only be in-between.
>>>>
>>>> Fix this.
>>>
>>> Rather than fixing a just introduced function I would expect a patch
>>> introducing strjoin() and then another one converting the time command
>>> over to use it.
>>
>> What difference does it make?
> 
> You wouldn't have to fixup code you just introduced.

The alternative of changing stuff just to shift it around directly after
doesn't sound that much more appealing, but I can do that anyway.

> 
>>>
>>> 	for (i = 0; i < arrlen; i++) {
>>> 		p = stpcpy(p, arr[i]);
>>> 		if (i < arrlen - 1)
>>> 			p = stpcpy(p, separator);
>>> 	}
>>>
>>> That would also allow you to optimize the allocated buffer size above.
>>
>> I like my version more. I dislike these once-only checks inside loops.
> 
> No problem ;)
> 
> 	for (i = 0; i < arrlen - 1; i++) {
> 		p = stpcpy(p, arr[i]);
> 		p = stpcpy(p, separator);
> 	}
> 
> 	stpcpy(p, arr[i]);

I can do that, albeit with mempcpy. I already got separatorlen.

Cheers,
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/6] commands: drvinfo: support filtering by driver

[Sascha Hauer]

On Thu, Oct 27, 2022 at 09:51:38AM +0200, Ahmad Fatoum wrote:
> Hi!
> 
> On 27.10.22 09:29, Sascha Hauer wrote:
> > On Wed, Oct 26, 2022 at 08:42:04AM +0200, Ahmad Fatoum wrote:
> >> drvinfo can be very long especially for the in-tree defconfigs,
> >> add optional filtering support:
> >> +	if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && argc > 1)
> >> +		filter = strjoin(" ", &argv[1], argc - 1);
> > 
> > Why does this depend on CONFIG_AUTO_COMPLETE?
> 
> drvinfo is something I think should always be enabled and I didn't want
> to bloat it unconditionally.
> 
> > 
> >> +
> >>  	printf("Driver\tDevice(s)\n");
> >>  	printf("--------------------\n");
> >>  	for_each_driver(drv) {
> >> +		if (filter && !str_has_prefix(drv->name, filter))
> >> +			continue;
> > 
> > I don't see how this is expected to work. When you pass multiple drivers
> > as argument 'filter' will be a concatenation of multiple driver names
> > which then matches nothing.
> 
> Autocomplete doesn't work for elements with spaces, so when you write
> 
>   drvinfo TI<Tab>
> 
> You get
> 
>     drvinfo TI DP83
> 
> with no further ability to complete. The prefix matching can work with that
> and will produce:

Then please fix the tab completion accordingly rather than further work
around this. See the series I just sent.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |