From: <Conor.Dooley@microchip.com> To: <geert+renesas@glider.be>, <Daire.McNamara@microchip.com>, <a.zummo@towertech.it>, <alexandre.belloni@bootlin.com> Cc: <linux-riscv@lists.infradead.org>, <linux-rtc@vger.kernel.org> Subject: Re: [PATCH] rtc: mpfs: Remove printing of stray CR Date: Tue, 16 Aug 2022 17:27:51 +0000 [thread overview] Message-ID: <4d414583-1011-4f01-870e-5d3ad812109a@microchip.com> (raw) In-Reply-To: <bce2ca405ef96b1363fd1370887409d9e8468422.1660659437.git.geert+renesas@glider.be> 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 >
WARNING: multiple messages have this Message-ID (diff)
From: <Conor.Dooley@microchip.com> To: <geert+renesas@glider.be>, <Daire.McNamara@microchip.com>, <a.zummo@towertech.it>, <alexandre.belloni@bootlin.com> Cc: <linux-riscv@lists.infradead.org>, <linux-rtc@vger.kernel.org> Subject: Re: [PATCH] rtc: mpfs: Remove printing of stray CR Date: Tue, 16 Aug 2022 17:27:51 +0000 [thread overview] Message-ID: <4d414583-1011-4f01-870e-5d3ad812109a@microchip.com> (raw) In-Reply-To: <bce2ca405ef96b1363fd1370887409d9e8468422.1660659437.git.geert+renesas@glider.be> 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
next prev parent reply other threads:[~2022-08-16 17:28 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-08-16 14:18 [PATCH] rtc: mpfs: Remove printing of stray CR Geert Uytterhoeven 2022-08-16 14:18 ` Geert Uytterhoeven 2022-08-16 17:27 ` Conor.Dooley [this message] 2022-08-16 17:27 ` Conor.Dooley 2022-08-20 17:05 ` Conor.Dooley 2022-08-20 17:05 ` Conor.Dooley 2022-08-23 20:18 ` Alexandre Belloni 2022-08-23 20:18 ` Alexandre Belloni
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4d414583-1011-4f01-870e-5d3ad812109a@microchip.com \ --to=conor.dooley@microchip.com \ --cc=Daire.McNamara@microchip.com \ --cc=a.zummo@towertech.it \ --cc=alexandre.belloni@bootlin.com \ --cc=geert+renesas@glider.be \ --cc=linux-riscv@lists.infradead.org \ --cc=linux-rtc@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.