There are users which print time and date represented by content of
time64_t type in human readable format.
Instead of open coding that each time introduce %ptT[dt][r] specifier.
Few test cases for %ptT specifier has been added as well.
Cc: Hans Verkuil <[email protected]>
Cc: Mathias Nyman <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Jonathan Hunter <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
Documentation/core-api/printk-formats.rst | 22 +++++++------
lib/test_printf.c | 11 +++++--
lib/vsprintf.c | 39 ++++++++++++++++++++++-
3 files changed, 59 insertions(+), 13 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index a7fae4538946..e33c130db76a 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -412,21 +412,23 @@ Examples::
Passed by reference.
-Time and date (struct rtc_time)
--------------------------------
+Time and date
+-------------
::
- %ptR YYYY-mm-ddTHH:MM:SS
- %ptRd YYYY-mm-dd
- %ptRt HH:MM:SS
- %ptR[dt][r]
+ %pt[RT] YYYY-mm-ddTHH:MM:SS
+ %pt[RT]d YYYY-mm-dd
+ %pt[RT]t HH:MM:SS
+ %pt[RT][dt][r]
-For printing date and time as represented by struct rtc_time structure in
-human readable format.
+For printing date and time as represented by
+ R struct rtc_time structure
+ T time64_t type
+in human readable format.
-By default year will be incremented by 1900 and month by 1. Use %ptRr (raw)
-to suppress this behaviour.
+By default year will be incremented by 1900 and month by 1.
+Use %pt[RT]r (raw) to suppress this behaviour.
Passed by reference.
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 659b6cc0d483..a0295fbd874f 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -450,7 +450,7 @@ struct_va_format(void)
}
static void __init
-struct_rtc_time(void)
+time_and_date(void)
{
/* 1543210543 */
const struct rtc_time tm = {
@@ -461,6 +461,8 @@ struct_rtc_time(void)
.tm_mon = 10,
.tm_year = 118,
};
+ /* 2019-01-04T15:32:23 */
+ time64_t t = 1546615943;
test_hashed("%pt", &tm);
@@ -470,6 +472,11 @@ struct_rtc_time(void)
test("05:35:43|0118-10-26", "%ptRtr|%ptRdr", &tm, &tm);
test("05:35:43|2018-11-26", "%ptRttr|%ptRdtr", &tm, &tm);
test("05:35:43 tr|2018-11-26 tr", "%ptRt tr|%ptRd tr", &tm, &tm);
+
+ test("2019-01-04T15:32:23", "%ptT", &t);
+ 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);
}
static void __init
@@ -583,7 +590,7 @@ test_pointer(void)
uuid();
dentry();
struct_va_format();
- struct_rtc_time();
+ time_and_date();
struct_clk();
bitmap();
netdev_features();
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3add92329bae..0457af8b10a9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -31,6 +31,7 @@
#include <linux/dcache.h>
#include <linux/cred.h>
#include <linux/rtc.h>
+#include <linux/time.h>
#include <linux/uuid.h>
#include <linux/of.h>
#include <net/addrconf.h>
@@ -1633,6 +1634,39 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm, const char *fmt)
return buf;
}
+static noinline_for_stack
+void time64_to_rtc_time(time64_t time, struct rtc_time *rtc_time)
+{
+#ifdef CONFIG_RTC_LIB
+ rtc_time64_to_tm(time, rtc_time);
+#else
+ struct tm tm;
+
+ time64_to_tm(time, 0, &tm);
+
+ rtc_time->tm_sec = tm.tm_sec;
+ rtc_time->tm_min = tm.tm_min;
+ rtc_time->tm_hour = tm.tm_hour;
+ rtc_time->tm_mday = tm.tm_mday;
+ rtc_time->tm_mon = tm.tm_mon;
+ rtc_time->tm_year = tm.tm_year;
+ rtc_time->tm_wday = tm.tm_wday;
+ rtc_time->tm_yday = tm.tm_yday;
+
+ rtc_time->tm_isdst = 0;
+#endif
+}
+
+static noinline_for_stack
+char *time64_str(char *buf, char *end, const time64_t *t, const char *fmt)
+{
+ struct rtc_time tm;
+
+ time64_to_rtc_time(*t, &tm);
+
+ return rtc_str(buf, end, &tm, fmt);
+}
+
static noinline_for_stack
char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
const char *fmt)
@@ -1640,6 +1674,8 @@ char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
switch (fmt[1]) {
case 'R':
return rtc_str(buf, end, (const struct rtc_time *)ptr, fmt);
+ case 'T':
+ return time64_str(buf, end, (const time64_t *)ptr, fmt);
default:
return ptr_to_id(buf, end, ptr, spec);
}
@@ -1924,8 +1960,9 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
* - 'd[234]' For a dentry name (optionally 2-4 last components)
* - 'D[234]' Same as 'd' but for a struct file
* - 'g' For block_device name (gendisk + partition number)
- * - 't[R][dt][r]' For time and date as represented:
+ * - 't[RT][dt][r]' For time and date as represented by:
* R struct rtc_time
+ * T time64_t
* - 'C' For a clock, it prints the name (Common Clock Framework) or address
* (legacy clock framework) of the clock
* - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
--
2.19.2
Use %ptT instead of open coded variant to print content of
time64_t type in human readable format.
Cc: Hans Verkuil <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/media/usb/pulse8-cec/pulse8-cec.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c
index b085b14f3f87..05f997e9ce28 100644
--- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
+++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
@@ -328,7 +328,6 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
u8 *data = pulse8->data + 1;
u8 cmd[2];
int err;
- struct tm tm;
time64_t date;
pulse8->vers = 0;
@@ -349,10 +348,7 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
if (err)
return err;
date = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | data[3];
- time64_to_tm(date, 0, &tm);
- dev_info(pulse8->dev, "Firmware build date %04ld.%02d.%02d %02d:%02d:%02d\n",
- tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
- tm.tm_hour, tm.tm_min, tm.tm_sec);
+ dev_info(pulse8->dev, "Firmware build date %ptT\n", &date);
dev_dbg(pulse8->dev, "Persistent config:\n");
cmd[0] = MSGCODE_GET_AUTO_ENABLED;
--
2.19.2
Use %ptT instead of open coded variant to print content of
time64_t type in human readable format.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/firmware/raspberrypi.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index a13558154ac3..ac5aeab1eb04 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -181,16 +181,10 @@ rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
RPI_FIRMWARE_GET_FIRMWARE_REVISION,
&packet, sizeof(packet));
- if (ret == 0) {
- struct tm tm;
-
- time64_to_tm(packet, 0, &tm);
+ if (ret)
+ return;
- dev_info(fw->cl.dev,
- "Attached to firmware from %04ld-%02d-%02d %02d:%02d\n",
- tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
- tm.tm_hour, tm.tm_min);
- }
+ dev_info(fw->cl.dev, "Attached to firmware from %ptT\n", &packet);
}
static void
--
2.19.2
Use %ptT instead of open coded variant to print content of
time64_t type in human readable format.
Cc: Mathias Nyman <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Jonathan Hunter <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
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 938ff06c0349..ed3eea3876e2 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -820,7 +820,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
const struct firmware *fw;
unsigned long timeout;
time64_t timestamp;
- struct tm time;
u64 address;
u32 value;
int err;
@@ -925,11 +924,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: %ptT UTC\n", ×tamp);
return 0;
}
--
2.19.2
On Thu, Jan 10, 2019 at 10:58:58PM +0100, Alexandre Belloni wrote:
> On 08/01/2019 16:25:28+0100, Petr Mladek wrote:
> > On Fri 2019-01-04 21:30:06, Andy Shevchenko wrote:
> > > There are users which print time and date represented by content of
> > > time64_t type in human readable format.
> > >
> > > Instead of open coding that each time introduce %ptT[dt][r] specifier.
> > >
> > > Few test cases for %ptT specifier has been added as well.
> > > +void time64_to_rtc_time(time64_t time, struct rtc_time *rtc_time)
> > > +{
> > > +#ifdef CONFIG_RTC_LIB
> > > + rtc_time64_to_tm(time, rtc_time);
> > I wonder if the conversion between struct tm and rtc_time
> > might be useful in general.
> >
> > It might make sense to de-duplicate time64_to_tm() and
> > rtc_time64_to_tm() implementations.
> Looking at 57f1f0874f42, this seemed to be the plan at the time
> time_to_tm was introduced but this was never done. Seeing that tm and
> rtc_time are quite similar, we could probably always use time64_to_tm as
> it is more accurate than rtc_time64_to_tm as the latter assumes a
> specific year range.
So, do I understand correctly that dropping #ifdef along with
rtc_time64_to_tm() call is sufficient for now?
> Maybe be rtc_str should take a struct tm instead of an rtc_time so
> time64_to_rtc_time always uses time64_to_tm.
Because this one, while sounding plausible, maybe too invasive on current
state of affairs.
> > > +#else
> > > + struct tm tm;
> > > +
> > > + time64_to_tm(time, 0, &tm);
> > > +
> > > + rtc_time->tm_sec = tm.tm_sec;
> > > + rtc_time->tm_min = tm.tm_min;
> > > + rtc_time->tm_hour = tm.tm_hour;
> > > + rtc_time->tm_mday = tm.tm_mday;
> > > + rtc_time->tm_mon = tm.tm_mon;
> > > + rtc_time->tm_year = tm.tm_year;
> > > + rtc_time->tm_wday = tm.tm_wday;
> > > + rtc_time->tm_yday = tm.tm_yday;
> > > +
> > > + rtc_time->tm_isdst = 0;
> > > +#endif
> > > +}
> > > +
> > > +static noinline_for_stack
> > > +char *time64_str(char *buf, char *end, const time64_t *t, const char *fmt)
> > > +{
> > > + struct rtc_time tm;
> > > +
> > > + time64_to_rtc_time(*t, &tm);
> > > +
> > > + return rtc_str(buf, end, &tm, fmt);
> > > +}
--
With Best Regards,
Andy Shevchenko
On 26/07/2019 16:20:37+0300, Andy Shevchenko wrote:
> On Thu, Jan 10, 2019 at 10:58:58PM +0100, Alexandre Belloni wrote:
> > On 08/01/2019 16:25:28+0100, Petr Mladek wrote:
> > > On Fri 2019-01-04 21:30:06, Andy Shevchenko wrote:
> > > > There are users which print time and date represented by content of
> > > > time64_t type in human readable format.
> > > >
> > > > Instead of open coding that each time introduce %ptT[dt][r] specifier.
> > > >
> > > > Few test cases for %ptT specifier has been added as well.
>
> > > > +void time64_to_rtc_time(time64_t time, struct rtc_time *rtc_time)
> > > > +{
> > > > +#ifdef CONFIG_RTC_LIB
> > > > + rtc_time64_to_tm(time, rtc_time);
>
> > > I wonder if the conversion between struct tm and rtc_time
> > > might be useful in general.
> > >
> > > It might make sense to de-duplicate time64_to_tm() and
> > > rtc_time64_to_tm() implementations.
>
> > Looking at 57f1f0874f42, this seemed to be the plan at the time
> > time_to_tm was introduced but this was never done. Seeing that tm and
> > rtc_time are quite similar, we could probably always use time64_to_tm as
> > it is more accurate than rtc_time64_to_tm as the latter assumes a
> > specific year range.
>
> So, do I understand correctly that dropping #ifdef along with
> rtc_time64_to_tm() call is sufficient for now?
>
I'd be fine with that.
> > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > time64_to_rtc_time always uses time64_to_tm.
>
> Because this one, while sounding plausible, maybe too invasive on current
> state of affairs.
>
Well, if the kernel struct tm had an int tm_year instead of long
tm_year, then you could simply cast a struct rtc_time to a struct tm.
I'm not sure was was the rationale to have a long, especially since
userspace has an int.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> On 26/07/2019 16:20:37+0300, Andy Shevchenko wrote:
> > On Thu, Jan 10, 2019 at 10:58:58PM +0100, Alexandre Belloni wrote:
> > > On 08/01/2019 16:25:28+0100, Petr Mladek wrote:
> > > > On Fri 2019-01-04 21:30:06, Andy Shevchenko wrote:
> > > > > There are users which print time and date represented by content of
> > > > > time64_t type in human readable format.
> > > > >
> > > > > Instead of open coding that each time introduce %ptT[dt][r] specifier.
> > > > >
> > > > > Few test cases for %ptT specifier has been added as well.
> >
> > > > > +void time64_to_rtc_time(time64_t time, struct rtc_time *rtc_time)
> > > > > +{
> > > > > +#ifdef CONFIG_RTC_LIB
> > > > > + rtc_time64_to_tm(time, rtc_time);
> >
> > > > I wonder if the conversion between struct tm and rtc_time
> > > > might be useful in general.
> > > >
> > > > It might make sense to de-duplicate time64_to_tm() and
> > > > rtc_time64_to_tm() implementations.
> >
> > > Looking at 57f1f0874f42, this seemed to be the plan at the time
> > > time_to_tm was introduced but this was never done. Seeing that tm and
> > > rtc_time are quite similar, we could probably always use time64_to_tm as
> > > it is more accurate than rtc_time64_to_tm as the latter assumes a
> > > specific year range.
> >
> > So, do I understand correctly that dropping #ifdef along with
> > rtc_time64_to_tm() call is sufficient for now?
> >
>
> I'd be fine with that.
Good, thanks! I'll send v2 soon.
> > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > time64_to_rtc_time always uses time64_to_tm.
> >
> > Because this one, while sounding plausible, maybe too invasive on current
> > state of affairs.
>
> Well, if the kernel struct tm had an int tm_year instead of long
> tm_year, then you could simply cast a struct rtc_time to a struct tm.
I don't think so. It will be error prone from endianess prospective on
64-bit platforms.
> I'm not sure was was the rationale to have a long, especially since
> userspace has an int.
Yeah, this is strange, I guess we simple may long -> int in kernel's struct tm.
--
With Best Regards,
Andy Shevchenko
On 01/10/2019 14:36:55+0300, Andy Shevchenko wrote:
> On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> > > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > > time64_to_rtc_time always uses time64_to_tm.
> > >
> > > Because this one, while sounding plausible, maybe too invasive on current
> > > state of affairs.
> >
> > Well, if the kernel struct tm had an int tm_year instead of long
> > tm_year, then you could simply cast a struct rtc_time to a struct tm.
>
> I don't think so. It will be error prone from endianess prospective on
> 64-bit platforms.
>
I don't get why, as long as the first members of both structs are the
same, this should work.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Fri, Jan 04, 2019 at 09:30:09PM +0200, Andy Shevchenko wrote:
> Use %ptT instead of open coded variant to print content of
> time64_t type in human readable format.
Any comments on this?
>
> Cc: Mathias Nyman <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Jonathan Hunter <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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 938ff06c0349..ed3eea3876e2 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -820,7 +820,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> const struct firmware *fw;
> unsigned long timeout;
> time64_t timestamp;
> - struct tm time;
> u64 address;
> u32 value;
> int err;
> @@ -925,11 +924,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: %ptT UTC\n", ×tamp);
>
> return 0;
> }
> --
> 2.19.2
>
--
With Best Regards,
Andy Shevchenko
On Fri, Jan 04, 2019 at 09:30:08PM +0200, Andy Shevchenko wrote:
> Use %ptT instead of open coded variant to print content of
> time64_t type in human readable format.
Hans, any objections on this?
> Cc: Hans Verkuil <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/media/usb/pulse8-cec/pulse8-cec.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c
> index b085b14f3f87..05f997e9ce28 100644
> --- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
> +++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
> @@ -328,7 +328,6 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
> u8 *data = pulse8->data + 1;
> u8 cmd[2];
> int err;
> - struct tm tm;
> time64_t date;
>
> pulse8->vers = 0;
> @@ -349,10 +348,7 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
> if (err)
> return err;
> date = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | data[3];
> - time64_to_tm(date, 0, &tm);
> - dev_info(pulse8->dev, "Firmware build date %04ld.%02d.%02d %02d:%02d:%02d\n",
> - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> - tm.tm_hour, tm.tm_min, tm.tm_sec);
> + dev_info(pulse8->dev, "Firmware build date %ptT\n", &date);
>
> dev_dbg(pulse8->dev, "Persistent config:\n");
> cmd[0] = MSGCODE_GET_AUTO_ENABLED;
> --
> 2.19.2
>
--
With Best Regards,
Andy Shevchenko
On Tue, Oct 01, 2019 at 01:48:16PM +0200, Alexandre Belloni wrote:
> On 01/10/2019 14:36:55+0300, Andy Shevchenko wrote:
> > On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> > > > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > > > time64_to_rtc_time always uses time64_to_tm.
> > > >
> > > > Because this one, while sounding plausible, maybe too invasive on current
> > > > state of affairs.
> > >
> > > Well, if the kernel struct tm had an int tm_year instead of long
> > > tm_year, then you could simply cast a struct rtc_time to a struct tm.
> >
> > I don't think so. It will be error prone from endianess prospective on
> > 64-bit platforms.
> >
>
> I don't get why, as long as the first members of both structs are the
> same, this should work.
On BE 64-bit we will always get tm_year == 0, won't we?
--
With Best Regards,
Andy Shevchenko
On 01/10/2019 15:11:54+0300, Andy Shevchenko wrote:
> On Tue, Oct 01, 2019 at 01:48:16PM +0200, Alexandre Belloni wrote:
> > On 01/10/2019 14:36:55+0300, Andy Shevchenko wrote:
> > > On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> > > > > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > > > > time64_to_rtc_time always uses time64_to_tm.
> > > > >
> > > > > Because this one, while sounding plausible, maybe too invasive on current
> > > > > state of affairs.
> > > >
> > > > Well, if the kernel struct tm had an int tm_year instead of long
> > > > tm_year, then you could simply cast a struct rtc_time to a struct tm.
> > >
> > > I don't think so. It will be error prone from endianess prospective on
> > > 64-bit platforms.
> > >
> >
> > I don't get why, as long as the first members of both structs are the
> > same, this should work.
>
> On BE 64-bit we will always get tm_year == 0, won't we?
>
Not if you have int tm_year in struct tm. I guess we can change the
kernel struct tm because it is not part of the ABI.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 10/1/19 1:57 PM, Andy Shevchenko wrote:
> On Fri, Jan 04, 2019 at 09:30:08PM +0200, Andy Shevchenko wrote:
>> Use %ptT instead of open coded variant to print content of
>> time64_t type in human readable format.
>
> Hans, any objections on this?
>
>> Cc: Hans Verkuil <[email protected]>
>> Signed-off-by: Andy Shevchenko <[email protected]>
Acked-by: Hans Verkuil <[email protected]>
Thanks!
Hans
>> ---
>> drivers/media/usb/pulse8-cec/pulse8-cec.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c
>> index b085b14f3f87..05f997e9ce28 100644
>> --- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
>> +++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
>> @@ -328,7 +328,6 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
>> u8 *data = pulse8->data + 1;
>> u8 cmd[2];
>> int err;
>> - struct tm tm;
>> time64_t date;
>>
>> pulse8->vers = 0;
>> @@ -349,10 +348,7 @@ static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
>> if (err)
>> return err;
>> date = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | data[3];
>> - time64_to_tm(date, 0, &tm);
>> - dev_info(pulse8->dev, "Firmware build date %04ld.%02d.%02d %02d:%02d:%02d\n",
>> - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
>> - tm.tm_hour, tm.tm_min, tm.tm_sec);
>> + dev_info(pulse8->dev, "Firmware build date %ptT\n", &date);
>>
>> dev_dbg(pulse8->dev, "Persistent config:\n");
>> cmd[0] = MSGCODE_GET_AUTO_ENABLED;
>> --
>> 2.19.2
>>
>
On 1.10.2019 14.56, Andy Shevchenko wrote:
> On Fri, Jan 04, 2019 at 09:30:09PM +0200, Andy Shevchenko wrote:
>> Use %ptT instead of open coded variant to print content of
>> time64_t type in human readable format.
>
> Any comments on this?
Untested, but looks ok to me.
xhci-tegra.c is written by Thierry Reding, so its more up to him
>
>>
>> Cc: Mathias Nyman <[email protected]>
>> Cc: Thierry Reding <[email protected]>
>> Cc: Jonathan Hunter <[email protected]>
>> Signed-off-by: Andy Shevchenko <[email protected]>
>> ---
>> 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 938ff06c0349..ed3eea3876e2 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -820,7 +820,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
>> const struct firmware *fw;
>> unsigned long timeout;
>> time64_t timestamp;
>> - struct tm time;
>> u64 address;
>> u32 value;
>> int err;
>> @@ -925,11 +924,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: %ptT UTC\n", ×tamp);
>>
>> return 0;
>> }
>> --
>> 2.19.2
>>
>
On Tue, Oct 01, 2019 at 03:20:46PM +0300, Mathias Nyman wrote:
> On 1.10.2019 14.56, Andy Shevchenko wrote:
> > On Fri, Jan 04, 2019 at 09:30:09PM +0200, Andy Shevchenko wrote:
> > > Use %ptT instead of open coded variant to print content of
> > > time64_t type in human readable format.
> >
> > Any comments on this?
>
> Untested, but looks ok to me.
Thanks!
> xhci-tegra.c is written by Thierry Reding, so its more up to him
Thierry, do you have any objections here?
>
> >
> > >
> > > Cc: Mathias Nyman <[email protected]>
> > > Cc: Thierry Reding <[email protected]>
> > > Cc: Jonathan Hunter <[email protected]>
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > ---
> > > 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 938ff06c0349..ed3eea3876e2 100644
> > > --- a/drivers/usb/host/xhci-tegra.c
> > > +++ b/drivers/usb/host/xhci-tegra.c
> > > @@ -820,7 +820,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> > > const struct firmware *fw;
> > > unsigned long timeout;
> > > time64_t timestamp;
> > > - struct tm time;
> > > u64 address;
> > > u32 value;
> > > int err;
> > > @@ -925,11 +924,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: %ptT UTC\n", ×tamp);
> > > return 0;
> > > }
> > > --
> > > 2.19.2
> > >
> >
>
--
With Best Regards,
Andy Shevchenko
On Fri, Jan 04, 2019 at 09:30:09PM +0200, Andy Shevchenko wrote:
> Use %ptT instead of open coded variant to print content of
> time64_t type in human readable format.
>
> Cc: Mathias Nyman <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Jonathan Hunter <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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 938ff06c0349..ed3eea3876e2 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -820,7 +820,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> const struct firmware *fw;
> unsigned long timeout;
> time64_t timestamp;
> - struct tm time;
> u64 address;
> u32 value;
> int err;
> @@ -925,11 +924,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: %ptT UTC\n", ×tamp);
If I understand correctly, this will now print:
Firmware timestamp: YYYY-mm-ddTHH:MM:SS UTC
whereas it earlier printed:
Firmware timestamp: YYYY-mm-dd HH:MM:SS UTC
So the 'T' character is different now. Could we make this something
along the lines of:
dev_info(dev, "Firmware timestamp: %ptTd %ptTt UTC\n", ×tamp,
×tamp);
To keep the output identical? It's possible that there are some scripts
that parse the log to find out which firmware was loaded.
Thierry
On Tue, Oct 01, 2019 at 02:13:21PM +0200, Alexandre Belloni wrote:
> On 01/10/2019 15:11:54+0300, Andy Shevchenko wrote:
> > On Tue, Oct 01, 2019 at 01:48:16PM +0200, Alexandre Belloni wrote:
> > > On 01/10/2019 14:36:55+0300, Andy Shevchenko wrote:
> > > > On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> > > > > > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > > > > > time64_to_rtc_time always uses time64_to_tm.
> > > > > >
> > > > > > Because this one, while sounding plausible, maybe too invasive on current
> > > > > > state of affairs.
> > > > >
> > > > > Well, if the kernel struct tm had an int tm_year instead of long
> > > > > tm_year, then you could simply cast a struct rtc_time to a struct tm.
> > > >
> > > > I don't think so. It will be error prone from endianess prospective on
> > > > 64-bit platforms.
> > > >
> > >
> > > I don't get why, as long as the first members of both structs are the
> > > same, this should work.
> >
> > On BE 64-bit we will always get tm_year == 0, won't we?
> >
>
> Not if you have int tm_year in struct tm. I guess we can change the
> kernel struct tm because it is not part of the ABI.
We can, but:
- it will require to change all `printf("%ld", tm_year)` cases at the same
time in entire kernel (and also some functions might start producing
warnings when some variable will be cut to int)
- it is out of scope of this series
So, I will leave it untouched for now.
--
With Best Regards,
Andy Shevchenko
On 01/10/2019 16:33:35+0300, Andy Shevchenko wrote:
> On Tue, Oct 01, 2019 at 02:13:21PM +0200, Alexandre Belloni wrote:
> > On 01/10/2019 15:11:54+0300, Andy Shevchenko wrote:
> > > On Tue, Oct 01, 2019 at 01:48:16PM +0200, Alexandre Belloni wrote:
> > > > On 01/10/2019 14:36:55+0300, Andy Shevchenko wrote:
> > > > > On Mon, Sep 30, 2019 at 10:08:09PM +0200, Alexandre Belloni wrote:
> > > > > > > > Maybe be rtc_str should take a struct tm instead of an rtc_time so
> > > > > > > > time64_to_rtc_time always uses time64_to_tm.
> > > > > > >
> > > > > > > Because this one, while sounding plausible, maybe too invasive on current
> > > > > > > state of affairs.
> > > > > >
> > > > > > Well, if the kernel struct tm had an int tm_year instead of long
> > > > > > tm_year, then you could simply cast a struct rtc_time to a struct tm.
> > > > >
> > > > > I don't think so. It will be error prone from endianess prospective on
> > > > > 64-bit platforms.
> > > > >
> > > >
> > > > I don't get why, as long as the first members of both structs are the
> > > > same, this should work.
> > >
> > > On BE 64-bit we will always get tm_year == 0, won't we?
> > >
> >
> > Not if you have int tm_year in struct tm. I guess we can change the
> > kernel struct tm because it is not part of the ABI.
>
> We can, but:
> - it will require to change all `printf("%ld", tm_year)` cases at the same
> time in entire kernel (and also some functions might start producing
> warnings when some variable will be cut to int)
> - it is out of scope of this series
>
> So, I will leave it untouched for now.
>
That's fine, I was not asking to do it as a prerequisite ;)
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, Oct 1, 2019 at 4:33 PM Thierry Reding <[email protected]> wrote:
> On Fri, Jan 04, 2019 at 09:30:09PM +0200, Andy Shevchenko wrote:
> > Use %ptT instead of open coded variant to print content of
> > time64_t type in human readable format.
> > - 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: %ptT UTC\n", ×tamp);
>
> If I understand correctly, this will now print:
>
> Firmware timestamp: YYYY-mm-ddTHH:MM:SS UTC
>
> whereas it earlier printed:
>
> Firmware timestamp: YYYY-mm-dd HH:MM:SS UTC
>
> So the 'T' character is different now.
> Could we make this something
> along the lines of:
>
> dev_info(dev, "Firmware timestamp: %ptTd %ptTt UTC\n", ×tamp,
> ×tamp);
>
> To keep the output identical?
Yes, we can...
> It's possible that there are some scripts
> that parse the log to find out which firmware was loaded.
...but if you have scripts parsing kernel log, something is odd.
As far as I understand kernel log isn't ABI, no-one should rely on its output.
--
With Best Regards,
Andy Shevchenko