linux
linux copied to clipboard
raspberrypi
rtc_pcf8563: /sys/class/rtc/rtc0/wakealarm is missing
Describe the bug
PiKVM uses the PCF8563 chip to implement a second hardware watchdog. When updating the kernel from 6.1.61 to 6.6.21 (d3d24545684c6b6637603e14443bbd67dae59413), the /sys/class/rtc/rtc0/wakealarm file disappeared at all while hwclock seems to be working fine. So the alarm is now broken.
Steps to reproduce the behaviour
Connect any PCF8563-based clock to I2C and add the overlay to /boot/config.txt: dtoverlay=i2c-rtc,pcf8563. After reboot you should see the file /sys/class/rtc/rtc0/wakealarm.
Device (s)
Raspberry Pi CM4 Lite
System
Arch Linux ARM
Feb 29 2024 12:24:53
Copyright (c) 2012 Broadcom
version f4e2138c2adc8f3a92a3a65939e458f11d7298ba (clean) (release) (start)
Linux pikvm 6.6.21-1-rpi #1 SMP Tue Mar 12 14:24:18 UTC 2024 armv7l GNU/Linux
Logs
Nothing special. Looks exactly as on old working kernel.
[root@pikvm ~]# dmesg | grep rtc
[ 9.019505] rtc-pcf8563 1-0051: registered as rtc0
[ 9.047961] rtc-pcf8563 1-0051: setting system clock to 2024-03-12T19:47:50 UTC (1710272870)
Additional context
For PiKVM, we have dyndbg enabled in the kernel, so if you can tell me what to enable for diagnostics, I can do it.
How is wakealarm working on 6.1? The i2c-rtc overlay doesn't configure an IRQ for it, and without one RTC_FEATURE_ALARM is cleared.
This is a surprise to me because it has always just worked, even on 5.x (and 4.x as I remember)
How do you think it works without an IRQ? Is the kernel polling for the alarm status on a timer? That doesn't sound like it would work well as a watchdog.
The kernel RTC admin guide says:
For example, not every RTC is hooked up to an IRQ, so they can't all issue alarms;
Probably, somehow the IRQ was still being configured. We're using it on 6.1 right now. The PCF chip controls a solid state relay that cuts the CM4 power line. I raise the wakealarm, perform a poweroff, and after a minute the alarm actually goes off. The CM4 is powered off, as intended in our scheme.
This is a listing on 6.1. Okay, to be honest I don't see any related with alarm. However, the file for wakealarm is here.
CPU0 CPU1 CPU2 CPU3
25: 0 0 0 0 GICv2 29 Level arch_timer
26: 45332995 21755804 18241068 20138026 GICv2 30 Level arch_timer
29: 24782431 0 0 0 GICv2 65 Level fe00b880.mailbox
30: 342 0 0 0 GICv2 114 Level DMA IRQ
31: 0 0 0 0 GICv2 116 Level DMA IRQ
32: 0 0 0 0 GICv2 117 Level DMA IRQ
33: 0 0 0 0 GICv2 118 Level DMA IRQ
34: 0 0 0 0 GICv2 119 Level DMA IRQ
36: 0 0 0 0 GICv2 122 Level DMA IRQ
37: 0 0 0 0 GICv2 123 Level DMA IRQ
38: 0 0 0 0 GICv2 48 Level arm-pmu
39: 0 0 0 0 GICv2 49 Level arm-pmu
40: 0 0 0 0 GICv2 50 Level arm-pmu
41: 0 0 0 0 GICv2 51 Level arm-pmu
46: 0 0 0 0 GICv2 175 Level PCIe PME
53: 14 0 0 0 BRCM STB PCIe MSI 1572864 Edge nvme0q0
54: 6321664 0 0 0 GICv2 189 Level eth0
55: 2179664 0 0 0 GICv2 190 Level eth0
56: 111919 0 0 0 BRCM STB PCIe MSI 1572865 Edge nvme0q1
57: 124274 0 0 0 BRCM STB PCIe MSI 1572866 Edge nvme0q2
58: 87315 0 0 0 BRCM STB PCIe MSI 1572867 Edge nvme0q3
59: 56532 0 0 0 BRCM STB PCIe MSI 1572868 Edge nvme0q4
60: 39 0 0 0 BRCM STB PCIe MSI 3670016 Edge xhci_hcd
61: 70 0 0 0 GICv2 66 Level VCHIQ doorbell
62: 523212 0 0 0 GICv2 158 Level mmc1, mmc0
63: 14 0 0 0 GICv2 153 Level uart-pl011
64: 0 0 0 0 GICv2 150 Level fe204000.spi
65: 4 0 0 0 GICv2 105 Level fe980000.usb, fe980000.usb
66: 0 0 0 0 GICv2 130 Level feb10000.codec
67: 22946788 0 0 0 GICv2 149 Level fe205000.i2c, fe804000.i2c
68: 32273179 0 0 0 GICv2 135 Level unicam_capture0
69: 0 0 0 0 GICv2 106 Level v3d
70: 94 0 0 0 GICv2 129 Level vc4 hvs
71: 0 0 0 0 interrupt-controller@7ef00100 4 Level vc4 hdmi hpd connected
72: 0 0 0 0 interrupt-controller@7ef00100 5 Level vc4 hdmi hpd disconnected
73: 0 0 0 0 interrupt-controller@7ef00100 1 Level vc4 hdmi cec rx
74: 0 0 0 0 interrupt-controller@7ef00100 0 Level vc4 hdmi cec tx
75: 0 0 0 0 interrupt-controller@7ef00100 10 Level vc4 hdmi hpd connected
76: 0 0 0 0 interrupt-controller@7ef00100 11 Level vc4 hdmi hpd disconnected
77: 0 0 0 0 interrupt-controller@7ef00100 7 Level vc4 hdmi cec rx
78: 0 0 0 0 interrupt-controller@7ef00100 8 Level vc4 hdmi cec tx
79: 0 0 0 0 GICv2 107 Level fe004000.txp
80: 0 0 0 0 GICv2 141 Level vc4 crtc
81: 6447477 0 0 0 GICv2 142 Level vc4 crtc, vc4 crtc
82: 41 0 0 0 GICv2 133 Level vc4 crtc
83: 0 0 0 0 GICv2 138 Level vc4 crtc
84: 18923299 0 0 0 pinctrl-bcm2835 16 Edge kvmd-fan::hall
85: 130853 0 0 0 pinctrl-bcm2835 4 Edge kvmd::atx
86: 286664 0 0 0 pinctrl-bcm2835 5 Edge kvmd::atx
IPI0: 0 0 0 0 CPU wakeup interrupts
IPI1: 0 0 0 0 Timer broadcast interrupts
IPI2: 164154 164743 164088 102267 Rescheduling interrupts
IPI3: 16661836 28139626 8133115 7773366 Function call interrupts
IPI4: 0 0 0 0 CPU stop interrupts
IPI5: 1471127 8079578 1358197 1431498 IRQ work interrupts
IPI6: 0 0 0 0 completion interrupts
[root@pikvm kvmd-webterm]# tree /sys/class/rtc/rtc0/
/sys/class/rtc/rtc0/
|-- device -> ../../../1-0051
|-- power
| |-- autosuspend_delay_ms
| |-- control
| |-- runtime_active_time
| |-- runtime_status
| `-- runtime_suspended_time
|-- subsystem -> ../../../../../../../../class/rtc
|-- date
|-- dev
|-- hctosys
|-- max_user_freq
|-- name
|-- range
|-- since_epoch
|-- time
|-- uevent
`-- wakealarm
4 directories, 15 files
[root@pikvm kvmd-webterm]# uname -a
Linux pikvm 6.1.61-1-rpi-ARCH #1 SMP Fri Nov 3 20:48:52 MSK 2023 armv7l GNU/Linux
In more detail, how we use it. We have a small demon that raises the wakealarm and then bumpes it. If the OS freezes or power off has been performed, the PC chip pulls the relay: https://github.com/pikvm/kvmd/blob/master/kvmd/apps/watchdog/init.py
So we need to get this functionality back somehow, because it's used by all customers.
The dependency on a configured IRQ was introduced with this commit: https://github.com/raspberrypi/linux/commit/60cfac17d0a1c28cd41959e95ba1e0ecc47165e7
I can't currently see why it was not present in rpi-6.1.y, since it seems to have been introduced around 6.1-rc6.
Yours is a fairly unusual use case, with the IRQ not being wired to the SPI, but it isn't invalid. Obviously it is possible for us to revert it in our downstream trees, but we try to avoid accumulating downstream patches that don't have a limited life.
I think you need to petition the kernel devs to revert it. @vwax submitted it and @alexandrebelloni approved it - perhaps they have some input.
It is not unusual but the driver lacks support for this case. Can you try this patch: https://github.com/alexandrebelloni/linux/commit/7b6c32af91d78fabc56461ffd27310cf8344f884 after adding the wakeup-source property to your node?
I understand. Actually, I thought everything was fine here - I just used the API for the chip.
@pelwell I've reverted that commit and everything is working now.
@alexandrebelloni Thank you! I'll try now.
But please guide me a bit: what exactly I need to do with wakeup-source? Just add it to options? dtoverlay=i2c-rtc,pcf8563,wakeup-source?
You'll have to patch the overlay as well so it knows that the wakeup-source property makes sense for the pcf8563 (we can add this in our tree once you've got it all working):
wakeup-source = <&ds1339>,"wakeup-source?",
<&ds3231>,"wakeup-source?",
<&mcp7940x>,"wakeup-source?",
<&mcp7941x>,"wakeup-source?",
- <&m41t62>,"wakeup-source?";
+ <&m41t62>,"wakeup-source?",
+ <&pcf8563>,"wakeup-source?";
The dependency on a configured IRQ was introduced with this commit: 60cfac1
I can't currently see why it was not present in rpi-6.1.y, since it seems to have been introduced around 6.1-rc6.
https://github.com/torvalds/linux/commit/60cfac17d0a1c28cd41959e95ba1e0ecc47165e7 says it features in tags 6.2-rc1 and later.
Okay, I've tested it and everything working fine. Patch https://github.com/alexandrebelloni/linux/commit/7b6c32af91d78fabc56461ffd27310cf8344f884 plus dtsi edit plus config.txt
Thank you!
Thank you, Alexandre. Can I assume that your patch will be submitted at some point?
@alexandrebelloni Sup? Sorry for pinging. Will your patch be submitted to the kernel upstream?
Yes but we are in the middle f the merge window and sending my pull request to Linus has the priority. This will have to wait for the next cycle.