2020-04-16 00:25:55

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

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 da26a584dca0..a3e85186f8e6 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -182,16 +182,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.25.1


2020-04-16 02:58:38

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On (20/04/15 20:00), Andy Shevchenko wrote:
[..]
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index da26a584dca0..a3e85186f8e6 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -182,16 +182,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);
> }

So can this be instead:

struct rtc_time tm;

rtc_time64_to_tm(time, &tm);
dev_info(.... "%ptR", &tm);

?

If it can, then I'd probably say something like "can we then just use
rtc_time64_to_tm()"?

-ss

2020-04-16 03:06:08

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On (20/04/16 11:53), Sergey Senozhatsky wrote:
> > + 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);
> > }
>
> So can this be instead:
>
> struct rtc_time tm;
>
> rtc_time64_to_tm(time, &tm);
> dev_info(.... "%ptR", &tm);
^^^^
%ptRdt ?

P.S. I wonder what's the longest English word which can be constructed
from a valid vsprintf() specifiers sequence (consecutive specifiers).

-ss

2020-04-21 13:11:14

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On Thu, Apr 16, 2020 at 11:53:58AM +0900, Sergey Senozhatsky wrote:
> On (20/04/15 20:00), Andy Shevchenko wrote:
> [..]
> > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> > index da26a584dca0..a3e85186f8e6 100644
> > --- a/drivers/firmware/raspberrypi.c
> > +++ b/drivers/firmware/raspberrypi.c
> > @@ -182,16 +182,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);
> > }
>
> So can this be instead:
>
> struct rtc_time tm;
>
> rtc_time64_to_tm(time, &tm);
> dev_info(.... "%ptR", &tm);
>
> ?
>
> If it can, then I'd probably say something like "can we then just use
> rtc_time64_to_tm()"?

Same comment as per previous patch (TL;DR: no, it can't).

--
With Best Regards,
Andy Shevchenko


2020-06-16 15:55:32

by Nicolas Saenz Julienne

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

Hi,

On Wed, 2020-04-15 at 20:00 +0300, Andy Shevchenko wrote:
> 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]>
> ---

as originally reported by Stefan Wahren, this patch is likely to be the cause
for a regression on RPi3b+ 32bit mode (multi_v7_defconfig,
5.8.0-rc1-00019-ga5dc8300df75):

[ 3.759892] raspberrypi-firmware soc:firmware: Attached to firmware from 18446744073709048425-02-19T15:52:16

Whereas the same doesn't seem to happen in 64bit mode:

[ 1.584126] raspberrypi-firmware soc:firmware: Attached to firmware from 2020-02-12T12:39:27

Regards,
Nicolas



Attachments:
signature.asc (499.00 B)
This is a digitally signed message part

2020-06-16 16:16:01

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On Tue, Jun 16, 2020 at 05:53:23PM +0200, Nicolas Saenz Julienne wrote:
> On Wed, 2020-04-15 at 20:00 +0300, Andy Shevchenko wrote:
> > 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]>
> > ---
>
> as originally reported by Stefan Wahren, this patch is likely to be the cause
> for a regression on RPi3b+ 32bit mode (multi_v7_defconfig,
> 5.8.0-rc1-00019-ga5dc8300df75):
>
> [ 3.759892] raspberrypi-firmware soc:firmware: Attached to firmware from 18446744073709048425-02-19T15:52:16
>
> Whereas the same doesn't seem to happen in 64bit mode:
>
> [ 1.584126] raspberrypi-firmware soc:firmware: Attached to firmware from 2020-02-12T12:39:27

Had you chance to run test_printf on that machine and see if it reports any failure?
If no, can you provide a raw date which makes this happen?

Ah, I think I have an idea what is going on... stay tuned!

Does below fix it?

--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -181,6 +181,7 @@ EXPORT_SYMBOL_GPL(rpi_firmware_property);
static void
rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
{
+ time64_t date_and_time;
u32 packet;
int ret = rpi_firmware_property(fw,
RPI_FIRMWARE_GET_FIRMWARE_REVISION,
@@ -189,7 +190,8 @@ rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
if (ret)
return;

- dev_info(fw->cl.dev, "Attached to firmware from %ptT\n", &packet);
+ date_and_time = packet;
+ dev_info(fw->cl.dev, "Attached to firmware from %ptT\n", &date_and_time);
}

--
With Best Regards,
Andy Shevchenko


2020-06-16 16:24:23

by Nicolas Saenz Julienne

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On Tue, 2020-06-16 at 19:13 +0300, Andy Shevchenko wrote:
> On Tue, Jun 16, 2020 at 05:53:23PM +0200, Nicolas Saenz Julienne wrote:
> > On Wed, 2020-04-15 at 20:00 +0300, Andy Shevchenko wrote:
> > > 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]>
> > > ---
> >
> > as originally reported by Stefan Wahren, this patch is likely to be the
> > cause
> > for a regression on RPi3b+ 32bit mode (multi_v7_defconfig,
> > 5.8.0-rc1-00019-ga5dc8300df75):
> >
> > [ 3.759892] raspberrypi-firmware soc:firmware: Attached to firmware from
> > 18446744073709048425-02-19T15:52:16
> >
> > Whereas the same doesn't seem to happen in 64bit mode:
> >
> > [ 1.584126] raspberrypi-firmware soc:firmware: Attached to firmware from
> > 2020-02-12T12:39:27
>
> Had you chance to run test_printf on that machine and see if it reports any
> failure?
> If no, can you provide a raw date which makes this happen?
>
> Ah, I think I have an idea what is going on... stay tuned!
>
> Does below fix it?

Yes :)

> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -181,6 +181,7 @@ EXPORT_SYMBOL_GPL(rpi_firmware_property);
> static void
> rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
> {
> + time64_t date_and_time;
> u32 packet;
> int ret = rpi_firmware_property(fw,
> RPI_FIRMWARE_GET_FIRMWARE_REVISION,
> @@ -189,7 +190,8 @@ rpi_firmware_print_firmware_revision(struct rpi_firmware
> *fw)
> if (ret)
> return;
>
> - dev_info(fw->cl.dev, "Attached to firmware from %ptT\n", &packet);
> + date_and_time = packet;
> + dev_info(fw->cl.dev, "Attached to firmware from %ptT\n",
> &date_and_time);
> }
>


Attachments:
signature.asc (499.00 B)
This is a digitally signed message part

2020-06-16 16:35:30

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] ARM: bcm2835: Switch to use %ptT

On Tue, Jun 16, 2020 at 06:22:17PM +0200, Nicolas Saenz Julienne wrote:
> On Tue, 2020-06-16 at 19:13 +0300, Andy Shevchenko wrote:
> > On Tue, Jun 16, 2020 at 05:53:23PM +0200, Nicolas Saenz Julienne wrote:
> > > On Wed, 2020-04-15 at 20:00 +0300, Andy Shevchenko wrote:
> > > > 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]>
> > > > ---
> > >
> > > as originally reported by Stefan Wahren, this patch is likely to be the
> > > cause
> > > for a regression on RPi3b+ 32bit mode (multi_v7_defconfig,
> > > 5.8.0-rc1-00019-ga5dc8300df75):
> > >
> > > [ 3.759892] raspberrypi-firmware soc:firmware: Attached to firmware from
> > > 18446744073709048425-02-19T15:52:16
> > >
> > > Whereas the same doesn't seem to happen in 64bit mode:
> > >
> > > [ 1.584126] raspberrypi-firmware soc:firmware: Attached to firmware from
> > > 2020-02-12T12:39:27
> >
> > Had you chance to run test_printf on that machine and see if it reports any
> > failure?
> > If no, can you provide a raw date which makes this happen?
> >
> > Ah, I think I have an idea what is going on... stay tuned!
> >
> > Does below fix it?
>
> Yes :)

Patch has been just sent. I hope it can go via printk tree, because MAINTAINERS keeps silent for this file.

--
With Best Regards,
Andy Shevchenko