[PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Josua Mayer]

HummingBoard has two RTCs, first integrated within SoC that can be used to
wake up from sleep - and a second on the carrier board including back-up
battery which is intended for keeping time during power-off.

Add aliases for both, ensuring that the battery-backed clock is primary
rtc and used by default during boot for restoring system time.

Fixes keeping time across power-cycle observed on Debian,
which sets RTC_HCTOSYS_DEVICE="rtc0".

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi  | 7 ++++++-
 arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard2.dtsi | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi
index bfade7149080..a955c77cd499 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi
@@ -41,6 +41,11 @@
 #include <dt-bindings/sound/fsl-imx-audmux.h>
 
 / {
+	aliases {
+		rtc0 = &carrier_rtc;
+		rtc1 = &snvs_rtc;
+	};
+
 	/* Will be filled by the bootloader */
 	memory@10000000 {
 		device_type = "memory";
@@ -187,7 +192,7 @@ &i2c1 {
 	status = "okay";
 
 	/* Pro baseboard model */
-	rtc@68 {
+	carrier_rtc: rtc@68 {
 		compatible = "nxp,pcf8523";
 		reg = <0x68>;
 	};
diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard2.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard2.dtsi
index 0883ef99cded..e6017f9bf640 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard2.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard2.dtsi
@@ -41,6 +41,11 @@
 #include <dt-bindings/sound/fsl-imx-audmux.h>
 
 / {
+	aliases {
+		rtc0 = &pcf8523;
+		rtc1 = &snvs_rtc;
+	};
+
 	/* Will be filled by the bootloader */
 	memory@10000000 {
 		device_type = "memory";

---
base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
change-id: 20240118-imx6-hb-primary-rtc-047d47fd6eaa

Sincerely,
-- 
Josua Mayer <josua@solid-run.com>

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Fabio Estevam]

On Thu, Jan 18, 2024 at 12:01 PM Josua Mayer <josua@solid-run.com> wrote:
>
> HummingBoard has two RTCs, first integrated within SoC that can be used to
> wake up from sleep - and a second on the carrier board including back-up
> battery which is intended for keeping time during power-off.
>
> Add aliases for both, ensuring that the battery-backed clock is primary
> rtc and used by default during boot for restoring system time.
>
> Fixes keeping time across power-cycle observed on Debian,
> which sets RTC_HCTOSYS_DEVICE="rtc0".
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Russell King (Oracle)]

On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
> HummingBoard has two RTCs, first integrated within SoC that can be used to
> wake up from sleep - and a second on the carrier board including back-up
> battery which is intended for keeping time during power-off.
> 
> Add aliases for both, ensuring that the battery-backed clock is primary
> rtc and used by default during boot for restoring system time.

Given that the snvs RTC isn't battery backed, should we even be enabling
that in DT?

Also, have you seen any issues such as:

[    0.933249] rtc-pcf8523 0-0068: failed to set xtal load capacitance: -11
[    0.933505] rtc-pcf8523: probe of 0-0068 failed with error -11

which seems to be exhibiting itself on my SolidSense board?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Josua Mayer]

Am 18.01.24 um 17:07 schrieb Russell King (Oracle):
> On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
>> HummingBoard has two RTCs, first integrated within SoC that can be used to
>> wake up from sleep - and a second on the carrier board including back-up
>> battery which is intended for keeping time during power-off.
>>
>> Add aliases for both, ensuring that the battery-backed clock is primary
>> rtc and used by default during boot for restoring system time.
> Given that the snvs RTC isn't battery backed, should we even be enabling
> that in DT?
In imx6qdl.dtsi it is not disabled.
According to Jon it is useful because it can wake up the soc from sleep,
whereas the external rtc can't.
>
> Also, have you seen any issues such as:
>
> [    0.933249] rtc-pcf8523 0-0068: failed to set xtal load capacitance: -11
> [    0.933505] rtc-pcf8523: probe of 0-0068 failed with error -11
>
> which seems to be exhibiting itself on my SolidSense board?
Not on my HummingBoard Gate Rev. 1.4., but indeed on my solidsense
unit too, which is probably same age as yours.
Only tested imx6dl-hummingboard2-emmc-som-v15.dtb,
but solidsense one should make no difference.

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Josua Mayer]

Am 19.01.24 um 13:07 schrieb Josua Mayer:
> Am 18.01.24 um 17:07 schrieb Russell King (Oracle):
>> On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
>>> HummingBoard has two RTCs, first integrated within SoC that can be used to
>>> wake up from sleep - and a second on the carrier board including back-up
>>> battery which is intended for keeping time during power-off.
>>>
>>> Add aliases for both, ensuring that the battery-backed clock is primary
>>> rtc and used by default during boot for restoring system time.
>> Given that the snvs RTC isn't battery backed, should we even be enabling
>> that in DT?
> In imx6qdl.dtsi it is not disabled.
> According to Jon it is useful because it can wake up the soc from sleep,
> whereas the external rtc can't.
>> Also, have you seen any issues such as:
>>
>> [    0.933249] rtc-pcf8523 0-0068: failed to set xtal load capacitance: -11
>> [    0.933505] rtc-pcf8523: probe of 0-0068 failed with error -11
>>
>> which seems to be exhibiting itself on my SolidSense board?
> Not on my HummingBoard Gate Rev. 1.4., but indeed on my solidsense
> unit too, which is probably same age as yours.
> Only tested imx6dl-hummingboard2-emmc-som-v15.dtb,
> but solidsense one should make no difference.

I was reading control registers 1-3:
debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x00
0x00
debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x01
0x00
debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x02
0x04

^^ This means low voltage on back up battery

After a few power-cycles that error went away.
Why pcf8523_load_capacitance would ever return EAGAIN I don't see.

In any case now that probe succeeded, I read these values:
0x80
0x00
0x04

And consequently when trying to read or write time I receive
[   88.844038] rtc-pcf8523 0-0068: low voltage detected, time is unreliable

Apparently all of my HummingBoard-2 / SolidSense have completely
drained their backup batteries to 0.1V over the past 4 years
while not in use.
FYI replacement part is MS621T.

Long story short I don't think the EAGAIN during probe is related
to adding aliases.
HOWEVER imo pcf8523_probe should return an error when
pcf8523_load_capacitance fails.

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Russell King (Oracle)]

On Fri, Jan 19, 2024 at 01:46:26PM +0000, Josua Mayer wrote:
> Am 19.01.24 um 13:07 schrieb Josua Mayer:
> > Am 18.01.24 um 17:07 schrieb Russell King (Oracle):
> >> On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
> >>> HummingBoard has two RTCs, first integrated within SoC that can be used to
> >>> wake up from sleep - and a second on the carrier board including back-up
> >>> battery which is intended for keeping time during power-off.
> >>>
> >>> Add aliases for both, ensuring that the battery-backed clock is primary
> >>> rtc and used by default during boot for restoring system time.
> >> Given that the snvs RTC isn't battery backed, should we even be enabling
> >> that in DT?
> > In imx6qdl.dtsi it is not disabled.
> > According to Jon it is useful because it can wake up the soc from sleep,
> > whereas the external rtc can't.
> >> Also, have you seen any issues such as:
> >>
> >> [    0.933249] rtc-pcf8523 0-0068: failed to set xtal load capacitance: -11
> >> [    0.933505] rtc-pcf8523: probe of 0-0068 failed with error -11
> >>
> >> which seems to be exhibiting itself on my SolidSense board?
> > Not on my HummingBoard Gate Rev. 1.4., but indeed on my solidsense
> > unit too, which is probably same age as yours.
> > Only tested imx6dl-hummingboard2-emmc-som-v15.dtb,
> > but solidsense one should make no difference.
> 
> I was reading control registers 1-3:
> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x00
> 0x00
> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x01
> 0x00
> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x02
> 0x04
> 
> ^^ This means low voltage on back up battery

Interesting - in my case, the solidsense has been powered on for months
(it's my internet router on the boat).

I've rebooted it again today, and this time it seems to have been
successful, and is using the time from it.

> After a few power-cycles that error went away.
> Why pcf8523_load_capacitance would ever return EAGAIN I don't see.
> 
> In any case now that probe succeeded, I read these values:
> 0x80
> 0x00
> 0x04

For me, after the last reboot, they contain:
0x80
0x00
0x08

> Long story short I don't think the EAGAIN during probe is related
> to adding aliases.
> HOWEVER imo pcf8523_probe should return an error when
> pcf8523_load_capacitance fails.

I think the real question is where is the EAGAIN coming from and
why.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Josua Mayer]

Am 19.01.24 um 15:33 schrieb Russell King (Oracle):
> On Fri, Jan 19, 2024 at 01:46:26PM +0000, Josua Mayer wrote:
>> Am 19.01.24 um 13:07 schrieb Josua Mayer:
>>> Am 18.01.24 um 17:07 schrieb Russell King (Oracle):
>>>> On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
>>>>> HummingBoard has two RTCs, first integrated within SoC that can be used to
>>>>> wake up from sleep - and a second on the carrier board including back-up
>>>>> battery which is intended for keeping time during power-off.
>>>>>
>>>>> Add aliases for both, ensuring that the battery-backed clock is primary
>>>>> rtc and used by default during boot for restoring system time.
>>>> Given that the snvs RTC isn't battery backed, should we even be enabling
>>>> that in DT?
>>> In imx6qdl.dtsi it is not disabled.
>>> According to Jon it is useful because it can wake up the soc from sleep,
>>> whereas the external rtc can't.
>>>> Also, have you seen any issues such as:
>>>>
>>>> [    0.933249] rtc-pcf8523 0-0068: failed to set xtal load capacitance: -11
>>>> [    0.933505] rtc-pcf8523: probe of 0-0068 failed with error -11
>>>>
>>>> which seems to be exhibiting itself on my SolidSense board?
>>> Not on my HummingBoard Gate Rev. 1.4., but indeed on my solidsense
>>> unit too, which is probably same age as yours.
>>> Only tested imx6dl-hummingboard2-emmc-som-v15.dtb,
>>> but solidsense one should make no difference.
>> I was reading control registers 1-3:
>> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x00
>> 0x00
>> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x01
>> 0x00
>> debian@sr-imx6:~$ sudo i2cget -y -a -f 0 0x68 0x02
>> 0x04
>>
>> ^^ This means low voltage on back up battery
> Interesting - in my case, the solidsense has been powered on for months
> (it's my internet router on the boat).
>
> I've rebooted it again today, and this time it seems to have been
> successful, and is using the time from it.
>
>> After a few power-cycles that error went away.
>> Why pcf8523_load_capacitance would ever return EAGAIN I don't see.
>>
>> In any case now that probe succeeded, I read these values:
>> 0x80
>> 0x00
>> 0x04
> For me, after the last reboot, they contain:
> 0x80
> 0x00
> 0x08
>
>> Long story short I don't think the EAGAIN during probe is related
>> to adding aliases.
>> HOWEVER imo pcf8523_probe should return an error when
>> pcf8523_load_capacitance fails.
> I think the real question is where is the EAGAIN coming from and
> why.
I have tried reproducing this on 6.7.0 with imx_v6_v7_defconfig:
I use expect [1] to capture kernel rtc messages during boot,
explicitly overwrite load capacitance bit to ensure
regmap_update_bits in pcf8523_load_capacitance has work
to do during next probe, and finally trigger software reboot.

On HummingBoard-2 I  have not seen the issue during 80
reboot cycles, and on solidsense not seen during 25 cycles.

Maybe it needs an almost-all-modconfig kernel?
Or maybe it only happens when the rtc has lost time?
Perhaps I can try disassembling the backup battery ... .

[1]
#!/usr/bin/expect -f

set tty [lindex $argv 0]
set logfile [lindex $argv 1]
set count 0

# log full output to file, but only status messages to stdout
if {$logfile != ""} {
    log_file -a $logfile
    log_user 0
}

#exec stty -F $tty 115200 raw -clocal -echo -istrip -hup
#spawn -open [open $tty w+]
spawn tio $tty

while {true} {
    expect {
        -re "Debian GNU/Linux 11 sr-imx6 ttymxc0" {
            set count [expr $count + 1]
            set time [clock seconds]
            set timestr [clock format $time -format "%D %T"]
            send_user "\[$timestr\] Successful boots to Linux: $count\n"
        }
        -re "sr-imx6 login:" {
            send "debian\n"
            expect -re "Password:"
            send "debian\n"
            expect -re {debian@sr-imx6:~$}
            send "sudo i2cset -f -y 0 0x68 0x00 0x00; sudo reboot\n"
        }
        -re "password for debian:" {
            send "debian\n"
        }
        -re {\] (rtc-pcf8523.*)\n} {
            set matched $expect_out(1,string)
            set time [clock seconds]
            set timestr [clock format $time -format "%D %T"]
            send_user "\[$timestr\] $matched\n"
        }
    }
}

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Russell King (Oracle)]

On Mon, Jan 29, 2024 at 07:26:07PM +0000, Josua Mayer wrote:
> I have tried reproducing this on 6.7.0 with imx_v6_v7_defconfig:
> I use expect [1] to capture kernel rtc messages during boot,
> explicitly overwrite load capacitance bit to ensure
> regmap_update_bits in pcf8523_load_capacitance has work
> to do during next probe, and finally trigger software reboot.
> 
> On HummingBoard-2 I  have not seen the issue during 80
> reboot cycles, and on solidsense not seen during 25 cycles.

I think at this point we should put it down to something spurious,
or maybe it was specific to having run an older kernel, or something.

Unless it can be reproduced, I don't see much point in spending more
time on it - I haven't noticed it happening again recently.

That said, I tend not to reboot this platform very often!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] ARM: dts: imx6qdl-hummingboard: Add rtc0 and rtc1 aliases to fix hctosys

[Shawn Guo]

On Thu, Jan 18, 2024 at 04:01:10PM +0100, Josua Mayer wrote:
> HummingBoard has two RTCs, first integrated within SoC that can be used to
> wake up from sleep - and a second on the carrier board including back-up
> battery which is intended for keeping time during power-off.
> 
> Add aliases for both, ensuring that the battery-backed clock is primary
> rtc and used by default during boot for restoring system time.
> 
> Fixes keeping time across power-cycle observed on Debian,
> which sets RTC_HCTOSYS_DEVICE="rtc0".
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Applied, thanks!