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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240903173615b69cfd0d@mail.local>
Date: Tue, 3 Sep 2024 19:36:15 +0200
From: Alexandre Belloni <alexandre.belloni@...tlin.com>
To: claudiu beznea <claudiu.beznea@...on.dev>
Cc: geert+renesas@...der.be, mturquette@...libre.com, sboyd@...nel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	magnus.damm@...il.com, p.zabel@...gutronix.de,
	linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-rtc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3
 available on Renesas RZ/G3S SoC

On 03/09/2024 19:09:00+0300, claudiu beznea wrote:
> > 
> > Also, w/o this change the RTC remains blocked under the following scenarios
> > if the interrupts are not disabled in remove:
> 
> This intrigued me and did some further investigation. I found that
> something is wrong on the driver as described bellow.
> 
> The failures described in the previous emails were due to the fact that the
> RTC counter clock was requested by the driver as devres managed resource.
> This is the clock that feeds the RTC counting logic.
> 
> With the bellow diff applied on top of this series scenarios 1/ and 2/
> described in the previous email works just fine.
> 
> For scenario 3/ the system is not powered up (as expected) but there are no
> more failures on the next boots.
> 
> diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> index f908d2a1017d..c9adcadc58c0 100644
> --- a/drivers/rtc/rtc-renesas-rtca3.c
> +++ b/drivers/rtc/rtc-renesas-rtca3.c
> @@ -747,10 +747,14 @@ static int rtca3_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
> 
> -       clk = devm_clk_get_enabled(dev, "counter");
> +       clk = devm_clk_get(dev, "counter");
>         if (IS_ERR(clk))
>                 return PTR_ERR(clk);
> 
> +       ret = clk_prepare_enable(clk);
> +       if (ret)
> +               return ret;
> +
>         spin_lock_init(&priv->lock);
>         atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
>         init_completion(&priv->set_alarm_completion);
> @@ -783,7 +787,7 @@ static void rtca3_remove(struct platform_device *pdev)
> 
>         guard(spinlock_irqsave)(&priv->lock);
> 
> -       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);
>  }
> 
> Question is: the RTC counter clock should stay on when the driver is
> unbinded, right?

Yes, the lifecycle of the RTC and its ressources is longer than the
system's as the whole point is to keep the time and date running while
the system is off.

> 
> Thank you,
> Claudiu Beznea
> 
> > 
> > 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);
> >  }
> > 
> > Thank you,
> > Claudiu Beznea
> > 

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