[PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

[PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

[Andy Shevchenko]

ISO 8601 defines 'T' as a separator between date and time. Though,
some ABIs use time and date with ' ' separator instead.

Add a flavour to the %pt specifier to override default separator.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/core-api/printk-formats.rst |  6 +++++-
 lib/test_printf.c                         |  5 +++++
 lib/vsprintf.c                            | 19 ++++++++++++++++---
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index f063a384c7c8..bc85fd4685e7 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -514,9 +514,10 @@ Time and date
 ::
 
 	%pt[RT]			YYYY-mm-ddTHH:MM:SS
+	%pt[RT]s		YYYY-mm-dd HH:MM:SS
 	%pt[RT]d		YYYY-mm-dd
 	%pt[RT]t		HH:MM:SS
-	%pt[RT][dt][r]
+	%pt[RT][dt][rs]
 
 For printing date and time as represented by::
 
@@ -528,6 +529,9 @@ in human readable format.
 By default year will be incremented by 1900 and month by 1.
 Use %pt[RT]r (raw) to suppress this behaviour.
 
+The %pt[RT]s (space) will override ISO 8601 by using ' ' instead of 'T'
+between date and time. It won't have any effect when date or time is omitted.
+
 Passed by reference.
 
 struct clk
diff --git a/lib/test_printf.c b/lib/test_printf.c
index ec0d5976bb69..8ac71aee46af 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -528,6 +528,11 @@ time_and_date(void)
 	test("0119-00-04T15:32:23", "%ptTr", &t);
 	test("15:32:23|2019-01-04", "%ptTt|%ptTd", &t, &t);
 	test("15:32:23|0119-00-04", "%ptTtr|%ptTdr", &t, &t);
+
+	test("2019-01-04 15:32:23", "%ptTs", &t);
+	test("0119-00-04 15:32:23", "%ptTsr", &t);
+	test("15:32:23|2019-01-04", "%ptTts|%ptTds", &t, &t);
+	test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t);
 }
 
 static void __init
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f0c35d9b65bf..5f36c7a43cdc 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1834,7 +1834,8 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
 	      struct printf_spec spec, const char *fmt)
 {
 	bool have_t = true, have_d = true;
-	bool raw = false;
+	bool raw = false, space = false;
+	bool found = true;
 	int count = 2;
 
 	if (check_pointer(&buf, end, tm, spec))
@@ -1851,14 +1852,26 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
 		break;
 	}
 
-	raw = fmt[count] == 'r';
+	do {
+		switch (fmt[count++]) {
+		case 'r':
+			raw = true;
+			break;
+		case 's':
+			space = true;
+			break;
+		default:
+			found = false;
+			break;
+		}
+	} while (found);
 
 	if (have_d)
 		buf = date_str(buf, end, tm, raw);
 	if (have_d && have_t) {
 		/* Respect ISO 8601 */
 		if (buf < end)
-			*buf = 'T';
+			*buf = space ? ' ' : 'T';
 		buf++;
 	}
 	if (have_t)
-- 
2.30.2

[PATCH v1 2/4] kdb: Switch to use %ptTs

[Andy Shevchenko]

Use %ptTs instead of open-coded variant to print contents
of time64_t type in human readable form.

Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: kgdb-bugreport@lists.sourceforge.net
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/debug/kdb/kdb_main.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 1baa96a2ecb8..622410c45da1 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2488,7 +2488,6 @@ static void kdb_sysinfo(struct sysinfo *val)
 static int kdb_summary(int argc, const char **argv)
 {
 	time64_t now;
-	struct tm tm;
 	struct sysinfo val;
 
 	if (argc)
@@ -2502,13 +2501,7 @@ static int kdb_summary(int argc, const char **argv)
 	kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
 
 	now = __ktime_get_real_seconds();
-	time64_to_tm(now, 0, &tm);
-	kdb_printf("date       %04ld-%02d-%02d %02d:%02d:%02d "
-		   "tz_minuteswest %d\n",
-		1900+tm.tm_year, tm.tm_mon+1, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec,
-		sys_tz.tz_minuteswest);
-
+	kdb_printf("date       %ptTs tz_minuteswest %d\n", &now, sys_tz.tz_minuteswest);
 	kdb_sysinfo(&val);
 	kdb_printf("uptime     ");
 	if (val.uptime > (24*60*60)) {
-- 
2.30.2

[PATCH v1 3/4] nilfs2: Switch to use %ptTs

[Andy Shevchenko]

Use %ptTs instead of open coded variant to print contents
of time64_t type in human readable form.

Use sysfs_emit() at the same time in the changed functions.

Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Cc: linux-nilfs@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 fs/nilfs2/sysfs.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
index 303d71430bdd..4e10423f0448 100644
--- a/fs/nilfs2/sysfs.c
+++ b/fs/nilfs2/sysfs.c
@@ -19,19 +19,6 @@
 /* /sys/fs/<nilfs>/ */
 static struct kset *nilfs_kset;
 
-#define NILFS_SHOW_TIME(time_t_val, buf) ({ \
-		struct tm res; \
-		int count = 0; \
-		time64_to_tm(time_t_val, 0, &res); \
-		res.tm_year += 1900; \
-		res.tm_mon += 1; \
-		count = scnprintf(buf, PAGE_SIZE, \
-				    "%ld-%.2d-%.2d %.2d:%.2d:%.2d\n", \
-				    res.tm_year, res.tm_mon, res.tm_mday, \
-				    res.tm_hour, res.tm_min, res.tm_sec);\
-		count; \
-})
-
 #define NILFS_DEV_INT_GROUP_OPS(name, parent_name) \
 static ssize_t nilfs_##name##_attr_show(struct kobject *kobj, \
 					struct attribute *attr, char *buf) \
@@ -576,7 +563,7 @@ nilfs_segctor_last_seg_write_time_show(struct nilfs_segctor_attr *attr,
 	ctime = nilfs->ns_ctime;
 	up_read(&nilfs->ns_segctor_sem);
 
-	return NILFS_SHOW_TIME(ctime, buf);
+	return sysfs_emit(buf, "%ptTs\n", &ctime);
 }
 
 static ssize_t
@@ -604,7 +591,7 @@ nilfs_segctor_last_nongc_write_time_show(struct nilfs_segctor_attr *attr,
 	nongc_ctime = nilfs->ns_nongc_ctime;
 	up_read(&nilfs->ns_segctor_sem);
 
-	return NILFS_SHOW_TIME(nongc_ctime, buf);
+	return sysfs_emit(buf, "%ptTs\n", &nongc_ctime);
 }
 
 static ssize_t
@@ -724,7 +711,7 @@ nilfs_superblock_sb_write_time_show(struct nilfs_superblock_attr *attr,
 	sbwtime = nilfs->ns_sbwtime;
 	up_read(&nilfs->ns_sem);
 
-	return NILFS_SHOW_TIME(sbwtime, buf);
+	return sysfs_emit(buf, "%ptTs\n", &sbwtime);
 }
 
 static ssize_t
-- 
2.30.2

[PATCH v1 4/4] usb: host: xhci-tegra: Switch to use %ptTs

[Andy Shevchenko]

Use %ptTs instead of open coded variant to print contents
of time64_t type in human readable form.

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/host/xhci-tegra.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index ce97ff054c68..937b78cba89b 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -890,7 +890,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
 	struct xhci_op_regs __iomem *op;
 	unsigned long timeout;
 	time64_t timestamp;
-	struct tm time;
 	u64 address;
 	u32 value;
 	int err;
@@ -987,11 +986,8 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
 	}
 
 	timestamp = le32_to_cpu(header->fwimg_created_time);
-	time64_to_tm(timestamp, 0, &time);
 
-	dev_info(dev, "Firmware timestamp: %ld-%02d-%02d %02d:%02d:%02d UTC\n",
-		 time.tm_year + 1900, time.tm_mon + 1, time.tm_mday,
-		 time.tm_hour, time.tm_min, time.tm_sec);
+	dev_info(dev, "Firmware timestamp: %ptTs UTC\n", &timestamp);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH v1 2/4] kdb: Switch to use %ptTs

[Doug Anderson]

Hi,

On Mon, May 10, 2021 at 8:04 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use %ptTs instead of open-coded variant to print contents
> of time64_t type in human readable form.
>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: kgdb-bugreport@lists.sourceforge.net
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  kernel/debug/kdb/kdb_main.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)

I kinda doubt anyone would really care if we just switched kdb to just
the old "%ptT". Probably no machines are parsing this string.

...but in any case, now that the nifty new format is there we might as
well use it. Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v1 2/4] kdb: Switch to use %ptTs

[Andy Shevchenko]

On Mon, May 10, 2021 at 05:05:46PM -0700, Doug Anderson wrote:
> On Mon, May 10, 2021 at 8:04 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Use %ptTs instead of open-coded variant to print contents
> > of time64_t type in human readable form.
> >
> > Cc: Jason Wessel <jason.wessel@windriver.com>
> > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > Cc: kgdb-bugreport@lists.sourceforge.net
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  kernel/debug/kdb/kdb_main.c | 9 +--------
> >  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> I kinda doubt anyone would really care if we just switched kdb to just
> the old "%ptT". Probably no machines are parsing this string.

Formally it's an ABI.

> ...but in any case, now that the nifty new format is there we might as
> well use it. Thus:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

[Petr Mladek]

On Mon 2021-05-10 18:04:10, Andy Shevchenko wrote:
> ISO 8601 defines 'T' as a separator between date and time. Though,
> some ABIs use time and date with ' ' separator instead.
> 
> Add a flavour to the %pt specifier to override default separator.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  Documentation/core-api/printk-formats.rst |  6 +++++-
>  lib/test_printf.c                         |  5 +++++
>  lib/vsprintf.c                            | 19 ++++++++++++++++---
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index f063a384c7c8..bc85fd4685e7 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -514,9 +514,10 @@ Time and date
>  ::
>  
>  	%pt[RT]			YYYY-mm-ddTHH:MM:SS
> +	%pt[RT]s		YYYY-mm-dd HH:MM:SS
>  	%pt[RT]d		YYYY-mm-dd
>  	%pt[RT]t		HH:MM:SS
> -	%pt[RT][dt][r]
> +	%pt[RT][dt][rs]

Sigh, we do not have clear rules what [xy] means. It might be:

      + always use one of them
      + optionally use one of them
      + always use any of them
      + optionally use any of them

%pt[RT][dt][rs] is a great mix:

      + R or T is required, the rest is optional
      + 'd' or 't' can be used but both together are not supported 
      + any variant of 'r' and 's' is supported including various ordering

Honestly, I do not know about any magic solution that might make it
easier to understand these monster modifiers.

Well, what about using the following at least in this case:

	%pt[RT][dt][r][s]

It might help to understand that both 'r' and 's' can be used at the
same time.

An attempt to distinguishing all the possibilities might be:

	%pt{RT}[{dt}][r][s]

where [] means that it is optional and {} means one of them must be
chosen. But I am not sure if it really makes the life easier. Anyway,
this would be for another patch that updates the entire printk-formats.rst.

>  
>  For printing date and time as represented by::
>  
> @@ -528,6 +529,9 @@ in human readable format.
>  By default year will be incremented by 1900 and month by 1.
>  Use %pt[RT]r (raw) to suppress this behaviour.
>  
> +The %pt[RT]s (space) will override ISO 8601 by using ' ' instead of 'T'
> +between date and time. It won't have any effect when date or time is omitted.
> +
>  Passed by reference.
>  
>  struct clk
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index f0c35d9b65bf..5f36c7a43cdc 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1834,7 +1834,8 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
>  	      struct printf_spec spec, const char *fmt)
>  {
>  	bool have_t = true, have_d = true;
> -	bool raw = false;
> +	bool raw = false, space = false;
> +	bool found = true;
>  	int count = 2;
>  
>  	if (check_pointer(&buf, end, tm, spec))
> @@ -1851,14 +1852,26 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
>  		break;
>  	}
>  
> -	raw = fmt[count] == 'r';
> +	do {
> +		switch (fmt[count++]) {
> +		case 'r':
> +			raw = true;
> +			break;
> +		case 's':
> +			space = true;
> +			break;
> +		default:
> +			found = false;
> +			break;
> +		}
> +	} while (found);
>  
>  	if (have_d)
>  		buf = date_str(buf, end, tm, raw);
>  	if (have_d && have_t) {
>  		/* Respect ISO 8601 */

The comment is slightly misleding now. What about something like?

		/* 'T' by ISO 8601. */

Or maybe call the variable: iso_8601, remove the comment, and
invert the logic:


		bool iso_8601 = true;

		case 's':
			iso_8601 = false;
			break;

		*buf = iso_8601 ? 'T' : ' ';

>  		if (buf < end)
> -			*buf = 'T';
> +			*buf = space ? ' ' : 'T';
>  		buf++;
>  	}
>  	if (have_t)

Best Regards,
Petr

Re: [PATCH v1 2/4] kdb: Switch to use %ptTs

[Petr Mladek]

On Mon 2021-05-10 18:04:11, Andy Shevchenko wrote:
> Use %ptTs instead of open-coded variant to print contents
> of time64_t type in human readable form.
> 
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: kgdb-bugreport@lists.sourceforge.net
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v1 3/4] nilfs2: Switch to use %ptTs

[Petr Mladek]

On Mon 2021-05-10 18:04:12, Andy Shevchenko wrote:
> Use %ptTs instead of open coded variant to print contents
> of time64_t type in human readable form.
> 
> Use sysfs_emit() at the same time in the changed functions.
> 
> Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> Cc: linux-nilfs@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v1 4/4] usb: host: xhci-tegra: Switch to use %ptTs

[Petr Mladek]

On Mon 2021-05-10 18:04:13, Andy Shevchenko wrote:
> Use %ptTs instead of open coded variant to print contents
> of time64_t type in human readable form.
> 
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

[Andy Shevchenko]

On Tue, May 11, 2021 at 04:14:56PM +0200, Petr Mladek wrote:
> On Mon 2021-05-10 18:04:10, Andy Shevchenko wrote:
> > ISO 8601 defines 'T' as a separator between date and time. Though,
> > some ABIs use time and date with ' ' separator instead.
> > 
> > Add a flavour to the %pt specifier to override default separator.

...

> >  	%pt[RT]			YYYY-mm-ddTHH:MM:SS
> > +	%pt[RT]s		YYYY-mm-dd HH:MM:SS
> >  	%pt[RT]d		YYYY-mm-dd
> >  	%pt[RT]t		HH:MM:SS
> > -	%pt[RT][dt][r]
> > +	%pt[RT][dt][rs]
> 
> Sigh, we do not have clear rules what [xy] means. It might be:
> 
>       + always use one of them
>       + optionally use one of them
>       + always use any of them
>       + optionally use any of them
> 
> %pt[RT][dt][rs] is a great mix:
> 
>       + R or T is required, the rest is optional
>       + 'd' or 't' can be used but both together are not supported 
>       + any variant of 'r' and 's' is supported including various ordering
> 
> Honestly, I do not know about any magic solution that might make it
> easier to understand these monster modifiers.
> 
> Well, what about using the following at least in this case:
> 
> 	%pt[RT][dt][r][s]
> 
> It might help to understand that both 'r' and 's' can be used at the
> same time.

This is the case, yes, thanks for catching it.

> An attempt to distinguishing all the possibilities might be:
> 
> 	%pt{RT}[{dt}][r][s]
> 
> where [] means that it is optional and {} means one of them must be
> chosen. But I am not sure if it really makes the life easier. Anyway,
> this would be for another patch that updates the entire printk-formats.rst.

No, this is not the case, the d and t can go in any combinations: none, d, t,
dt, or td.

> 
> >  For printing date and time as represented by::
> >  
> > @@ -528,6 +529,9 @@ in human readable format.
> >  By default year will be incremented by 1900 and month by 1.
> >  Use %pt[RT]r (raw) to suppress this behaviour.
> >  
> > +The %pt[RT]s (space) will override ISO 8601 by using ' ' instead of 'T'
> > +between date and time. It won't have any effect when date or time is omitted.

...

> > +	do {
> > +		switch (fmt[count++]) {
> > +		case 'r':
> > +			raw = true;
> > +			break;
> > +		case 's':
> > +			space = true;
> > +			break;
> > +		default:
> > +			found = false;
> > +			break;
> > +		}
> > +	} while (found);
> >  
> >  	if (have_d)
> >  		buf = date_str(buf, end, tm, raw);
> >  	if (have_d && have_t) {
> >  		/* Respect ISO 8601 */
> 
> The comment is slightly misleding now. What about something like?
> 
> 		/* 'T' by ISO 8601. */
> 
> Or maybe call the variable: iso_8601, remove the comment, and
> invert the logic:

Okay, I will think how to improve, thanks!

> 		bool iso_8601 = true;
> 
> 		case 's':
> 			iso_8601 = false;
> 			break;
> 
> 		*buf = iso_8601 ? 'T' : ' ';
> 
> >  		if (buf < end)
> > -			*buf = 'T';
> > +			*buf = space ? ' ' : 'T';
> >  		buf++;
> >  	}

-- 
With Best Regards,
Andy Shevchenko