| 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-dragging-grapple-f26e4361e009@thorsis.com>
Date: Thu, 22 Aug 2024 08:53:59 +0200
From: Alexander Dahl <ada@...rsis.com>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: Alexander Dahl <ada@...rsis.com>, 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 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?
> 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?
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
Powered by blists - more mailing lists