[PATCH v1] rtc: mpfs: fix counter upload completion condition

[PATCH v1] rtc: mpfs: fix counter upload completion condition

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The condition that needs to be checked for upload completion is the
UPLOAD bit in the completion register going low. The original iterations
of this driver used a do-while and this was converted to a
read_poll_timeout() during upstreaming without the condition being
inverted as it should have been.

I suspect that this went unnoticed until now because a) the first read
was done when the bit was still set, immediately completing the
read_poll_timeout() and b) because the RTC doesn't hold time when power
is removed from the SoC reducing its utility (I for one keep it
disabled). If my first suspicion was true when the driver was
upstreamed, it's not true any longer though, hence the detection of the
problem.

Fixes: 0b31d703598dc ("rtc: Add driver for Microchip PolarFire SoC")
CC: stable@vger.kernel.org
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
CC: Valentina.FernandezAlanis@microchip.com
CC: Conor Dooley <conor.dooley@microchip.com>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
CC: linux-riscv@lists.infradead.org
CC: linux-rtc@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/rtc/rtc-mpfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
index 6aa3eae575d2a..ece6de4a6adbd 100644
--- a/drivers/rtc/rtc-mpfs.c
+++ b/drivers/rtc/rtc-mpfs.c
@@ -112,7 +112,7 @@ static int mpfs_rtc_settime(struct device *dev, struct rtc_time *tm)
 	ctrl |= CONTROL_UPLOAD_BIT;
 	writel(ctrl, rtcdev->base + CONTROL_REG);
 
-	ret = read_poll_timeout(readl, prog, prog & CONTROL_UPLOAD_BIT, 0, UPLOAD_TIMEOUT_US,
+	ret = read_poll_timeout(readl, prog, !(prog & CONTROL_UPLOAD_BIT), 0, UPLOAD_TIMEOUT_US,
 				false, rtcdev->base + CONTROL_REG);
 	if (ret) {
 		dev_err(dev, "timed out uploading time to rtc");
-- 
2.53.0

Re: [PATCH v1] rtc: mpfs: fix counter upload completion condition

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]

Hey Alexandrew,

On Wed, May 13, 2026 at 06:55:55PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The condition that needs to be checked for upload completion is the
> UPLOAD bit in the completion register going low. The original iterations
> of this driver used a do-while and this was converted to a
> read_poll_timeout() during upstreaming without the condition being
> inverted as it should have been.
> 
> I suspect that this went unnoticed until now because a) the first read
> was done when the bit was still set, immediately completing the
> read_poll_timeout() and b) because the RTC doesn't hold time when power
> is removed from the SoC reducing its utility (I for one keep it
> disabled). If my first suspicion was true when the driver was
> upstreamed, it's not true any longer though, hence the detection of the
> problem.
> 
> Fixes: 0b31d703598dc ("rtc: Add driver for Microchip PolarFire SoC")
> CC: stable@vger.kernel.org
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Any chance this could be applied as 7.1 fixes material?

Apologies if I missed an application mail somewhere,
Conor.

> ---
> CC: Valentina.FernandezAlanis@microchip.com
> CC: Conor Dooley <conor.dooley@microchip.com>
> CC: Daire McNamara <daire.mcnamara@microchip.com>
> CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
> CC: linux-riscv@lists.infradead.org
> CC: linux-rtc@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/rtc/rtc-mpfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
> index 6aa3eae575d2a..ece6de4a6adbd 100644
> --- a/drivers/rtc/rtc-mpfs.c
> +++ b/drivers/rtc/rtc-mpfs.c
> @@ -112,7 +112,7 @@ static int mpfs_rtc_settime(struct device *dev, struct rtc_time *tm)
>  	ctrl |= CONTROL_UPLOAD_BIT;
>  	writel(ctrl, rtcdev->base + CONTROL_REG);
>  
> -	ret = read_poll_timeout(readl, prog, prog & CONTROL_UPLOAD_BIT, 0, UPLOAD_TIMEOUT_US,
> +	ret = read_poll_timeout(readl, prog, !(prog & CONTROL_UPLOAD_BIT), 0, UPLOAD_TIMEOUT_US,
>  				false, rtcdev->base + CONTROL_REG);
>  	if (ret) {
>  		dev_err(dev, "timed out uploading time to rtc");
> -- 
> 2.53.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1] rtc: mpfs: fix counter upload completion condition

[Valentina.FernandezAlanis]

On 13/05/2026 18:55, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The condition that needs to be checked for upload completion is the
> UPLOAD bit in the completion register going low. The original iterations
> of this driver used a do-while and this was converted to a
> read_poll_timeout() during upstreaming without the condition being
> inverted as it should have been.
>
> I suspect that this went unnoticed until now because a) the first read
> was done when the bit was still set, immediately completing the
> read_poll_timeout() and b) because the RTC doesn't hold time when power
> is removed from the SoC reducing its utility (I for one keep it
> disabled). If my first suspicion was true when the driver was
> upstreamed, it's not true any longer though, hence the detection of the
> problem.
>
> Fixes: 0b31d703598dc ("rtc: Add driver for Microchip PolarFire SoC")
> CC: stable@vger.kernel.org
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Tested-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
> ---
> CC: Valentina.FernandezAlanis@microchip.com
> CC: Conor Dooley <conor.dooley@microchip.com>
> CC: Daire McNamara <daire.mcnamara@microchip.com>
> CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
> CC: linux-riscv@lists.infradead.org
> CC: linux-rtc@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>   drivers/rtc/rtc-mpfs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
> index 6aa3eae575d2a..ece6de4a6adbd 100644
> --- a/drivers/rtc/rtc-mpfs.c
> +++ b/drivers/rtc/rtc-mpfs.c
> @@ -112,7 +112,7 @@ static int mpfs_rtc_settime(struct device *dev, struct rtc_time *tm)
>   	ctrl |= CONTROL_UPLOAD_BIT;
>   	writel(ctrl, rtcdev->base + CONTROL_REG);
>   
> -	ret = read_poll_timeout(readl, prog, prog & CONTROL_UPLOAD_BIT, 0, UPLOAD_TIMEOUT_US,
> +	ret = read_poll_timeout(readl, prog, !(prog & CONTROL_UPLOAD_BIT), 0, UPLOAD_TIMEOUT_US,
>   				false, rtcdev->base + CONTROL_REG);
>   	if (ret) {
>   		dev_err(dev, "timed out uploading time to rtc");

Re: [PATCH v1] rtc: mpfs: fix counter upload completion condition

[Alexandre Belloni]

On Wed, 13 May 2026 18:55:55 +0100, Conor Dooley wrote:
> The condition that needs to be checked for upload completion is the
> UPLOAD bit in the completion register going low. The original iterations
> of this driver used a do-while and this was converted to a
> read_poll_timeout() during upstreaming without the condition being
> inverted as it should have been.
> 
> I suspect that this went unnoticed until now because a) the first read
> was done when the bit was still set, immediately completing the
> read_poll_timeout() and b) because the RTC doesn't hold time when power
> is removed from the SoC reducing its utility (I for one keep it
> disabled). If my first suspicion was true when the driver was
> upstreamed, it's not true any longer though, hence the detection of the
> problem.
> 
> [...]

Applied, thanks!

[1/1] rtc: mpfs: fix counter upload completion condition
      https://git.kernel.org/abelloni/c/9792ff8afa90

Best regards,

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

Re: [PATCH v1] rtc: mpfs: fix counter upload completion condition

[Geert Uytterhoeven]

Hi Conor,

On Wed, 13 May 2026 at 20:04, Conor Dooley <conor@kernel.org> wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The condition that needs to be checked for upload completion is the
> UPLOAD bit in the completion register going low. The original iterations
> of this driver used a do-while and this was converted to a
> read_poll_timeout() during upstreaming without the condition being
> inverted as it should have been.
>
> I suspect that this went unnoticed until now because a) the first read
> was done when the bit was still set, immediately completing the
> read_poll_timeout() and b) because the RTC doesn't hold time when power
> is removed from the SoC reducing its utility (I for one keep it
> disabled). If my first suspicion was true when the driver was
> upstreamed, it's not true any longer though, hence the detection of the
> problem.
>
> Fixes: 0b31d703598dc ("rtc: Add driver for Microchip PolarFire SoC")
> CC: stable@vger.kernel.org
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Thanks, this landed as commit 9792ff8afa9017fe ("rtc: mpfs: fix counter
upload completion condition") in v7.2-rc1, and finally the endless
stream of:

    mpfs_rtc 20124000.rtc: timed out uploading time to rtc

is gone!

And no, it didn't go unnoticed, at least not for me, but you couldn't
reproduce it reliably before:
https://lore.kernel.org/bce2ca405ef96b1363fd1370887409d9e8468422.1660659437.git.geert+renesas@glider.be/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1] rtc: mpfs: fix counter upload completion condition

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

On Tue, Jun 30, 2026 at 05:18:13PM +0200, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Wed, 13 May 2026 at 20:04, Conor Dooley <conor@kernel.org> wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> >
> > The condition that needs to be checked for upload completion is the
> > UPLOAD bit in the completion register going low. The original iterations
> > of this driver used a do-while and this was converted to a
> > read_poll_timeout() during upstreaming without the condition being
> > inverted as it should have been.
> >
> > I suspect that this went unnoticed until now because a) the first read
> > was done when the bit was still set, immediately completing the
> > read_poll_timeout() and b) because the RTC doesn't hold time when power
> > is removed from the SoC reducing its utility (I for one keep it
> > disabled). If my first suspicion was true when the driver was
> > upstreamed, it's not true any longer though, hence the detection of the
> > problem.
> >
> > Fixes: 0b31d703598dc ("rtc: Add driver for Microchip PolarFire SoC")
> > CC: stable@vger.kernel.org
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks, this landed as commit 9792ff8afa9017fe ("rtc: mpfs: fix counter
> upload completion condition") in v7.2-rc1, and finally the endless
> stream of:
> 
>     mpfs_rtc 20124000.rtc: timed out uploading time to rtc
> 
> is gone!
> 
> And no, it didn't go unnoticed, at least not for me, but you couldn't
> reproduce it reliably before:
> https://lore.kernel.org/bce2ca405ef96b1363fd1370887409d9e8468422.1660659437.git.geert+renesas@glider.be/

Oh wow, old mystery solved and not "unnoticed" at all!
To be honest, there's a good chance it'd have been investigated more
thoroughly sooner if I had the driver enabled, but I don't given it
doesn't hold time powered off. I'm glad it's eventually been solved
though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]