| 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
| ||
|
Message-ID: <20240822-mute-ruby-fc8c86240f3c@thorsis.com>
Date: Thu, 22 Aug 2024 12:25:26 +0200
From: Alexander Dahl <ada@...rsis.com>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: linux-clk@...r.kernel.org, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
Codrin Ciubotariu <codrin.ciubotariu@...rochip.com>,
Sandeep Sheriker Mallikarjun <sandeepsheriker.mallikarjun@...rochip.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Tudor Ambarus <tudor.ambarus@...aro.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] ARM: dts: microchip: sam9x60: Fix rtc/rtt clocks
Hello Alexandre,
Am Thu, Aug 22, 2024 at 10:17:46AM +0200 schrieb Alexandre Belloni:
> On 22/08/2024 08:53:59+0200, Alexander Dahl wrote:
> > Hello Alexandre,
> >
> > Am Thu, Aug 22, 2024 at 01:52:05AM +0200 schrieb Alexandre Belloni:
> > > On 21/08/2024 07:51:36+0200, Alexander Dahl wrote:
> > > > The RTC and RTT peripherals use the "timing domain slow clock (TD_SLCK),
> > > > sourced from the 32.768 kHz crystal oscillator.
> > > >
> > > > (The previously used Monitoring domain slow clock (MD_SLCK) is sourced
> > > > from an internal RC oscillator which is most probably not precise enough
> > > > for real time clock purposes.)
> > > >
> > > > Fixes: 1e5f532c2737 ("ARM: dts: at91: sam9x60: add device tree for soc and board")
> > > > Fixes: 5f6b33f46346 ("ARM: dts: sam9x60: add rtt")
> > > > Signed-off-by: Alexander Dahl <ada@...rsis.com>
> > > > ---
> > > >
> > > > Notes:
> > > > Picked the wrong patch in the first try. This v2 one has a slightly
> > > > adapted commit message and more context below.
> > > >
> > > > This obviously requires a 32.768 kHz crystal oscillator to be present,
> > > > but the sam9x60.dtsi does contain that, and the clock-controllers
> > > > reference that, so I assume it's always present.
> > >
> > > The crystal is optional so this is going to break the boards that don't
> > > have one. I don't really mind but this should probably be part of the
> > > commit message.
> >
> > Okay right, according to the datasheet (Figure 27.1 SCKC Block
> > Diagram) you don't need that crystal, you can clear TD_OSCSEL and
> > td_slck runs from the internal rc then. However, td_slck is always
> > present, it either sources from the internal slow rc oscillator or the
> > crystal oscillator. And the datasheet says in section 29.1 (PMC):
> >
> > "The Slow Clock Controller (SCKC) selects the source of TD_SLCK
> > (drives the real-time part (RTT/RTC)). The source of MD_SLCK
> > (drives the rest of the system controller: wake-up logic,
> > watchdog, PMC, etc.) is always the slow RC oscillator."
> >
> > md_slck and td_slck are both registered by the at91 sckc driver, and
> > the td_slck gets two parents in of_sam9x60_sckc_setup() when
> > registered by at91_clk_register_sam9x5_slow(). The parent can be
> > switched by clk_sam9x5_slow_set_parent() from sam9x5_slow_ops then,
> > correctly setting the OSCSEL bit.
> >
> > The whole idea of the patch is giving the rtc/rtt td_slck as a parent
> > as documented in the datasheet. I don't see how this should be
> > affected by the parents of td_slck? Am I missing something?
> >
>
> You are right, I got confused because you were referring t the 32768 Hz
> crystal in your commit message and though you aimed at selected the
> parent of td_slck (and also, I didn't really work on the sam9x60).
Thanks for confirming.
Should I reword the commit message then to make it easier to
understand?
> > > This makes me realise that we always assumed the RC oscillator was
> > > running at 32768 while the sam9x60 datasheet refers to it has a 32kHz
> > > oscillator. However the RTC only has a 32768 divider...
> >
> > When sourced from the internal rc oscillator, this would mean the
> > output would be incorrect, right? How could one prove this?
>
>
> I guess you could have a look at how the RTC is drifting when selecting
> the RC osc as the parent but it will anyway be way less precise than the
> crystal so i'm not sure how you could get a conclusive result.
I would have to look deeper into rtc for that.
Maybe in a calm minute. ;-)
Greets
Alex
>
> >
> > Greets
> > Alex
> >
> > >
> > > >
> > > > /sys/kernel/debug/clk/clk_summary content excerpt before:
> > > >
> > > > slow_rc_osc 1 1 0 32768 93750000 0 50000 Y deviceless no_connection_id
> > > > md_slck 4 4 0 32768 0 0 50000 Y fffffea8.rtc no_connection_id
> > > > fffffe20.rtc no_connection_id
> > > > fffffe10.poweroff no_connection_id
> > > > fffffe00.reset-controller no_connection_id
> > > > timer@...08000 slow_clk
> > > > deviceless no_connection_id
> > > > …
> > > > slow_xtal 0 0 0 32768 0 0 50000 Y deviceless no_connection_id
> > > > slow_osc 0 0 0 32768 0 0 50000 Y deviceless no_connection_id
> > > > td_slck 0 0 0 32768 0 0 50000 Y deviceless no_connection_id
> > > >
> > > > And after:
> > > >
> > > > slow_rc_osc 1 1 0 32768 93750000 0 50000 Y deviceless no_connection_id
> > > > md_slck 2 2 0 32768 0 0 50000 Y fffffe10.poweroff no_connection_id
> > > > fffffe00.reset-controller no_connection_id
> > > > timer@...08000 slow_clk
> > > > deviceless no_connection_id
> > > > …
> > > > slow_xtal 1 1 0 32768 0 0 50000 Y deviceless no_connection_id
> > > > slow_osc 1 1 0 32768 0 0 50000 Y deviceless no_connection_id
> > > > td_slck 2 2 0 32768 0 0 50000 Y fffffea8.rtc no_connection_id
> > > > fffffe20.rtc no_connection_id
> > > > deviceless no_connection_id
> > > >
> > > > arch/arm/boot/dts/microchip/sam9x60.dtsi | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > index 291540e5d81e..d077afd5024d 100644
> > > > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > @@ -1312,7 +1312,7 @@ rtt: rtc@...ffe20 {
> > > > compatible = "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt";
> > > > reg = <0xfffffe20 0x20>;
> > > > interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
> > > > - clocks = <&clk32k 0>;
> > > > + clocks = <&clk32k 1>;
> > > > };
> > > >
> > > > pit: timer@...ffe40 {
> > > > @@ -1338,7 +1338,7 @@ rtc: rtc@...ffea8 {
> > > > compatible = "microchip,sam9x60-rtc", "atmel,at91sam9x5-rtc";
> > > > reg = <0xfffffea8 0x100>;
> > > > interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
> > > > - clocks = <&clk32k 0>;
> > > > + clocks = <&clk32k 1>;
> > > > };
> > > >
> > > > watchdog: watchdog@...fff80 {
> > > >
> > > > base-commit: 47ac09b91befbb6a235ab620c32af719f8208399
> > > > --
> > > > 2.39.2
> > > >
> > >
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Powered by blists - more mailing lists