[PATCH] rtc: mpfs: Remove printing of stray CR

[PATCH] rtc: mpfs: Remove printing of stray CR

[Geert Uytterhoeven]

During boot, the driver prints out a stray carriage return character.
Remove it, together with the preceding space character.

While at it, change prescaler to "unsigned long", as returned by
clk_get_rate(), to avoid truncating very large clock rates, and update
the format specifiers.

Fixes: 0b31d703598dc199 ("rtc: Add driver for Microchip PolarFire SoC")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Apparently updating the RTC when Debian userspace starts fails, causing
an infinite stream of:

    mpfs_rtc 20124000.rtc: timed out uploading time to rtc

Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.
---
 drivers/rtc/rtc-mpfs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
index f14d1925e0c94dfb..944ad10365164c4d 100644
--- a/drivers/rtc/rtc-mpfs.c
+++ b/drivers/rtc/rtc-mpfs.c
@@ -233,7 +233,7 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
 {
 	struct mpfs_rtc_dev *rtcdev;
 	struct clk *clk;
-	u32 prescaler;
+	unsigned long prescaler;
 	int wakeup_irq, ret;
 
 	rtcdev = devm_kzalloc(&pdev->dev, sizeof(struct mpfs_rtc_dev), GFP_KERNEL);
@@ -275,14 +275,13 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
 
 	/* prescaler hardware adds 1 to reg value */
 	prescaler = clk_get_rate(devm_clk_get(&pdev->dev, "rtcref")) - 1;
-
 	if (prescaler > MAX_PRESCALER_COUNT) {
-		dev_dbg(&pdev->dev, "invalid prescaler %d\n", prescaler);
+		dev_dbg(&pdev->dev, "invalid prescaler %lu\n", prescaler);
 		return -EINVAL;
 	}
 
 	writel(prescaler, rtcdev->base + PRESCALER_REG);
-	dev_info(&pdev->dev, "prescaler set to: 0x%X \r\n", prescaler);
+	dev_info(&pdev->dev, "prescaler set to: %lu\n", prescaler);
 
 	device_init_wakeup(&pdev->dev, true);
 	ret = dev_pm_set_wake_irq(&pdev->dev, wakeup_irq);
-- 
2.25.1

[PATCH] rtc: mpfs: Remove printing of stray CR

[Geert Uytterhoeven]

During boot, the driver prints out a stray carriage return character.
Remove it, together with the preceding space character.

While at it, change prescaler to "unsigned long", as returned by
clk_get_rate(), to avoid truncating very large clock rates, and update
the format specifiers.

Fixes: 0b31d703598dc199 ("rtc: Add driver for Microchip PolarFire SoC")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Apparently updating the RTC when Debian userspace starts fails, causing
an infinite stream of:

    mpfs_rtc 20124000.rtc: timed out uploading time to rtc

Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.
---
 drivers/rtc/rtc-mpfs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
index f14d1925e0c94dfb..944ad10365164c4d 100644
--- a/drivers/rtc/rtc-mpfs.c
+++ b/drivers/rtc/rtc-mpfs.c
@@ -233,7 +233,7 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
 {
 	struct mpfs_rtc_dev *rtcdev;
 	struct clk *clk;
-	u32 prescaler;
+	unsigned long prescaler;
 	int wakeup_irq, ret;
 
 	rtcdev = devm_kzalloc(&pdev->dev, sizeof(struct mpfs_rtc_dev), GFP_KERNEL);
@@ -275,14 +275,13 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
 
 	/* prescaler hardware adds 1 to reg value */
 	prescaler = clk_get_rate(devm_clk_get(&pdev->dev, "rtcref")) - 1;
-
 	if (prescaler > MAX_PRESCALER_COUNT) {
-		dev_dbg(&pdev->dev, "invalid prescaler %d\n", prescaler);
+		dev_dbg(&pdev->dev, "invalid prescaler %lu\n", prescaler);
 		return -EINVAL;
 	}
 
 	writel(prescaler, rtcdev->base + PRESCALER_REG);
-	dev_info(&pdev->dev, "prescaler set to: 0x%X \r\n", prescaler);
+	dev_info(&pdev->dev, "prescaler set to: %lu\n", prescaler);
 
 	device_init_wakeup(&pdev->dev, true);
 	ret = dev_pm_set_wake_irq(&pdev->dev, wakeup_irq);
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Conor.Dooley]

Hey Geert, thanks for the patch.

On 16/08/2022 15:18, Geert Uytterhoeven wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> During boot, the driver prints out a stray carriage return character.
> Remove it, together with the preceding space character.
> 
> While at it, change prescaler to "unsigned long", as returned by
> clk_get_rate(), to avoid truncating very large clock rates, and update
> the format specifiers.

If you manage to into Linux with a reference clock that high
let me know ASAP ;)

> 
> Fixes: 0b31d703598dc199 ("rtc: Add driver for Microchip PolarFire SoC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Apparently updating the RTC when Debian userspace starts fails, causing
> an infinite stream of:
> 
>     mpfs_rtc 20124000.rtc: timed out uploading time to rtc
> 
> Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.

I didn't see this once during development, nor when I tested before
I left work today. Tested when I got home, happened once the first
time I tried it & never saw it again after that...

I'll take a look this week and see if I can figure out a cause.

As I mentioned on IRC, I wondered if there was an interaction between
the HSS you're running & the reset controller series that you applied.

I looked back at the the HSS, and there was a point where it did not
take the RTC out of reset - but that predates the version you have
(0.99.16) by over 6 months.

I would still be quite interested in seeing if it repro's without
the reset series.


> ---
>  drivers/rtc/rtc-mpfs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
> index f14d1925e0c94dfb..944ad10365164c4d 100644
> --- a/drivers/rtc/rtc-mpfs.c
> +++ b/drivers/rtc/rtc-mpfs.c
> @@ -233,7 +233,7 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
>  {
>         struct mpfs_rtc_dev *rtcdev;
>         struct clk *clk;
> -       u32 prescaler;
> +       unsigned long prescaler;
>         int wakeup_irq, ret;
> 
>         rtcdev = devm_kzalloc(&pdev->dev, sizeof(struct mpfs_rtc_dev), GFP_KERNEL);
> @@ -275,14 +275,13 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
> 
>         /* prescaler hardware adds 1 to reg value */
>         prescaler = clk_get_rate(devm_clk_get(&pdev->dev, "rtcref")) - 1;
> -
>         if (prescaler > MAX_PRESCALER_COUNT) {
> -               dev_dbg(&pdev->dev, "invalid prescaler %d\n", prescaler);
> +               dev_dbg(&pdev->dev, "invalid prescaler %lu\n", prescaler);
>                 return -EINVAL;
>         }
> 
>         writel(prescaler, rtcdev->base + PRESCALER_REG);
> -       dev_info(&pdev->dev, "prescaler set to: 0x%X \r\n", prescaler);
> +       dev_info(&pdev->dev, "prescaler set to: %lu\n", prescaler);

TBQH, this does not need to be a dev_info() print. I don't think it
provides any value to a regular user.

Either way:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks again,
Conor.

> 
>         device_init_wakeup(&pdev->dev, true);
>         ret = dev_pm_set_wake_irq(&pdev->dev, wakeup_irq);
> --
> 2.25.1
> 

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Conor.Dooley]

Hey Geert, thanks for the patch.

On 16/08/2022 15:18, Geert Uytterhoeven wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> During boot, the driver prints out a stray carriage return character.
> Remove it, together with the preceding space character.
> 
> While at it, change prescaler to "unsigned long", as returned by
> clk_get_rate(), to avoid truncating very large clock rates, and update
> the format specifiers.

If you manage to into Linux with a reference clock that high
let me know ASAP ;)

> 
> Fixes: 0b31d703598dc199 ("rtc: Add driver for Microchip PolarFire SoC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Apparently updating the RTC when Debian userspace starts fails, causing
> an infinite stream of:
> 
>     mpfs_rtc 20124000.rtc: timed out uploading time to rtc
> 
> Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.

I didn't see this once during development, nor when I tested before
I left work today. Tested when I got home, happened once the first
time I tried it & never saw it again after that...

I'll take a look this week and see if I can figure out a cause.

As I mentioned on IRC, I wondered if there was an interaction between
the HSS you're running & the reset controller series that you applied.

I looked back at the the HSS, and there was a point where it did not
take the RTC out of reset - but that predates the version you have
(0.99.16) by over 6 months.

I would still be quite interested in seeing if it repro's without
the reset series.


> ---
>  drivers/rtc/rtc-mpfs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
> index f14d1925e0c94dfb..944ad10365164c4d 100644
> --- a/drivers/rtc/rtc-mpfs.c
> +++ b/drivers/rtc/rtc-mpfs.c
> @@ -233,7 +233,7 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
>  {
>         struct mpfs_rtc_dev *rtcdev;
>         struct clk *clk;
> -       u32 prescaler;
> +       unsigned long prescaler;
>         int wakeup_irq, ret;
> 
>         rtcdev = devm_kzalloc(&pdev->dev, sizeof(struct mpfs_rtc_dev), GFP_KERNEL);
> @@ -275,14 +275,13 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
> 
>         /* prescaler hardware adds 1 to reg value */
>         prescaler = clk_get_rate(devm_clk_get(&pdev->dev, "rtcref")) - 1;
> -
>         if (prescaler > MAX_PRESCALER_COUNT) {
> -               dev_dbg(&pdev->dev, "invalid prescaler %d\n", prescaler);
> +               dev_dbg(&pdev->dev, "invalid prescaler %lu\n", prescaler);
>                 return -EINVAL;
>         }
> 
>         writel(prescaler, rtcdev->base + PRESCALER_REG);
> -       dev_info(&pdev->dev, "prescaler set to: 0x%X \r\n", prescaler);
> +       dev_info(&pdev->dev, "prescaler set to: %lu\n", prescaler);

TBQH, this does not need to be a dev_info() print. I don't think it
provides any value to a regular user.

Either way:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks again,
Conor.

> 
>         device_init_wakeup(&pdev->dev, true);
>         ret = dev_pm_set_wake_irq(&pdev->dev, wakeup_irq);
> --
> 2.25.1
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Conor.Dooley]

On 16/08/2022 18:27, Conor Dooley wrote:
> On 16/08/2022 15:18, Geert Uytterhoeven wrote:
>> Apparently updating the RTC when Debian userspace starts fails, causing
>> an infinite stream of:
>>
>>     mpfs_rtc 20124000.rtc: timed out uploading time to rtc
>>
>> Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.
> 
> I didn't see this once during development, nor when I tested before
> I left work today. Tested when I got home, happened once the first
> time I tried it & never saw it again after that...
> 
> I'll take a look this week and see if I can figure out a cause.
> 
> As I mentioned on IRC, I wondered if there was an interaction between
> the HSS you're running & the reset controller series that you applied.
> 
> I looked back at the the HSS, and there was a point where it did not
> take the RTC out of reset - but that predates the version you have
> (0.99.16) by over 6 months.
> 
> I would still be quite interested in seeing if it repro's without
> the reset series.

I have managed to repro this intermittently on v6.0-rc1, although
quite infrequently. I don't have the reset series applied nor am
I on an old HSS. I don't see the stream that you do w/ my Ubuntu
nfs, just a single timeout, when it occurs. Haven't really had a
chance to debug yet, but I'll LYK.

Thanks,
Conor.

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Conor.Dooley]

On 16/08/2022 18:27, Conor Dooley wrote:
> On 16/08/2022 15:18, Geert Uytterhoeven wrote:
>> Apparently updating the RTC when Debian userspace starts fails, causing
>> an infinite stream of:
>>
>>     mpfs_rtc 20124000.rtc: timed out uploading time to rtc
>>
>> Increasing UPLOAD_TIMEOUT_US from 50 to 50000 doesn't help.
> 
> I didn't see this once during development, nor when I tested before
> I left work today. Tested when I got home, happened once the first
> time I tried it & never saw it again after that...
> 
> I'll take a look this week and see if I can figure out a cause.
> 
> As I mentioned on IRC, I wondered if there was an interaction between
> the HSS you're running & the reset controller series that you applied.
> 
> I looked back at the the HSS, and there was a point where it did not
> take the RTC out of reset - but that predates the version you have
> (0.99.16) by over 6 months.
> 
> I would still be quite interested in seeing if it repro's without
> the reset series.

I have managed to repro this intermittently on v6.0-rc1, although
quite infrequently. I don't have the reset series applied nor am
I on an old HSS. I don't see the stream that you do w/ my Ubuntu
nfs, just a single timeout, when it occurs. Haven't really had a
chance to debug yet, but I'll LYK.

Thanks,
Conor.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Alexandre Belloni]

On Tue, 16 Aug 2022 16:18:25 +0200, Geert Uytterhoeven wrote:
> During boot, the driver prints out a stray carriage return character.
> Remove it, together with the preceding space character.
> 
> While at it, change prescaler to "unsigned long", as returned by
> clk_get_rate(), to avoid truncating very large clock rates, and update
> the format specifiers.
> 
> [...]

Applied, thanks!

[1/1] rtc: mpfs: Remove printing of stray CR
      commit: 07ae9278b423500f93e10869b1a50276d82050ec

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] rtc: mpfs: Remove printing of stray CR

[Alexandre Belloni]

On Tue, 16 Aug 2022 16:18:25 +0200, Geert Uytterhoeven wrote:
> During boot, the driver prints out a stray carriage return character.
> Remove it, together with the preceding space character.
> 
> While at it, change prescaler to "unsigned long", as returned by
> clk_get_rate(), to avoid truncating very large clock rates, and update
> the format specifiers.
> 
> [...]

Applied, thanks!

[1/1] rtc: mpfs: Remove printing of stray CR
      commit: 07ae9278b423500f93e10869b1a50276d82050ec

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com