| Message ID | bce2ca405ef96b1363fd1370887409d9e8468422.1660659437.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rtc: mpfs: Remove printing of stray CR | expand |
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 >
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.
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,
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);
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(-)