[PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Alexandre Belloni]

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