On 02/09/2024 17:49:14+0300, claudiu beznea wrote: > >> + /* Disable alarm, periodic interrupts. */ > >> + rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > > > > Why do you disable alarms on driver remove? I think you need to add a > > comment if this is because it can't system up, else this is a bad > > practice. > > The RTC cannot power on the system after a power off. It can't also resume > it from a deep sleep state (when only the SoC area where the RTC resides > remains power on (there is no way to signal from RTC to the power supply > chain that an alarm happened)). It can only wake it up from s2idle mode > where all SoC components remains powered on. > > Also, w/o this change the RTC remains blocked under the following scenarios > if the interrupts are not disabled in remove: > > 1/ Configure wake alarm and unbind the RTC driver with the following commands: > # echo +10 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind > # sleep 12 > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind > > When rebinding the re-configuration of the RTC device times out: > [ 121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > -sh: echo: write error: Connection timed out > > 2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with > the following commands: > # echo s2idle > /sys/power/mem_sleep > # echo +10 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind > # echo mem > /sys/power/state > # #system is resumed by non RTC wakeup source (as the RTC alarm is not > working anymore in this case) > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind > > The system is not waked up from RTC alarm (as expected) and the rebinding > fails again: > > [ 172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > -sh: echo: write error: Connection timed out > > 3/ configure the RTC alarm, unbind and power off (with the following commands): > # echo +60 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind > # poweroff > > The system is not started after 60 seconds and at the next reboot the RTC > configuration on probe is failing the same: > > [ 0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > > In all scenarios the RTC is recovered only if removing/re-applying the > power to the SoC area where it resides. > > These tests were done with the patches in this series and then I tried it > with the following diff on top of the patches in this series. The results > were the same: > > diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c > index 822c055b6e4d..720fdac3adc6 100644 > --- a/drivers/rtc/rtc-renesas-rtca3.c > +++ b/drivers/rtc/rtc-renesas-rtca3.c > @@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct > rtca3_priv *priv) > usleep_range(sleep_us, sleep_us + 10); > > /* Disable alarm and carry interrupts. */ > - mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE; > + mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE; > ret = rtca3_alarm_irq_set_helper(priv, mask, 0); > if (ret) > return ret; > @@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev) > guard(spinlock_irqsave)(&priv->lock); > > /* Disable alarm, periodic interrupts. */ > - rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > + //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > } Thanks for the detailed explanation. Can you add a small comment, I really want t avoid people cargo-culting this behavior as this has already been the case. -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
