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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx>
Thanks again,
Conor.
>
> device_init_wakeup(&pdev->dev, true);
> ret = dev_pm_set_wake_irq(&pdev->dev, wakeup_irq);
> --
> 2.25.1
>
