linux-kernel - Re: [PATCH v7 06/19] rtc: sun6i: Add support for RTCs without external LOSCs

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: <20210616134749.mzp52dvbjmiabgl3@gilmour>
Date:   Wed, 16 Jun 2021 15:47:49 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Andre Przywara <andre.przywara@....com>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Rob Herring <robh@...nel.org>, Icenowy Zheng <icenowy@...c.io>,
        Samuel Holland <samuel@...lland.org>,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...glegroups.com,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Ondrej Jirman <megous@...ous.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        linux-rtc@...r.kernel.org
Subject: Re: [PATCH v7 06/19] rtc: sun6i: Add support for RTCs without
 external LOSCs

On Wed, Jun 16, 2021 at 11:14:52AM +0100, Andre Przywara wrote:
> On Wed, 16 Jun 2021 11:14:31 +0200
> Maxime Ripard <maxime@...no.tech> wrote:
> 
> Hi,
> 
> > On Tue, Jun 15, 2021 at 12:06:23PM +0100, Andre Przywara wrote:
> > > Some newer Allwinner RTCs (for instance the one in the H616 SoC) lack
> > > a pin for an external 32768 Hz oscillator. As a consequence, this LOSC
> > > can't be selected as the RTC clock source, and we must rely on the
> > > internal RC oscillator.
> > > To allow additions of clocks to the RTC node, add a feature bit to ignore
> > > any provided clocks for now (the current code would think this is the
> > > external LOSC). Later DTs and code can then for instance add the PLL
> > > based clock input, and older kernel won't get confused.
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara@....com>  
> > 
> > Honestly, I don't really know if it's worth it at this point.
> > 
> > If we sums this up:
> > 
> >  - The RTC has 2 features that we use, mostly centered around 2
> >    registers set plus a global one
> > 
> >  - Those 2 features are programmed in a completely different way
> > 
> >  - Even the common part is different, given the discussion around the
> >    clocks that we have.
> > 
> > What is there to share in that driver aside from the probe, and maybe
> > the interrupt handling? Instead of complicating this further with more
> > special case that you were (rightfully) complaining about, shouldn't we
> > just acknowledge the fact that it's a completely separate design and
> > should be treated as such, with a completely separate driver?
> 
> If you mean to have a separate clock driver, and one RTC driver: I
> agree, and IIUC Samuel has a prototype, covering the H6 and D1 as well:
> https://github.com/smaeul/linux/commit/6f8f761db1d8dd4b6abf006fb7e2427da79321c2
> 
> The only problem I see that they are sharing MMIO registers. Maybe it
> works because the RTC part never touches anything below 0x10, and the
> clock part just needs the first four registers?
> But this means we can't easily change this for the H6, as the
> existing H6 RTC code adds 0x10 to the MMIO base, and also old DTs will
> have the RTC base address in their RTC reg property.
> 
> If we can somehow solve this (let the clock driver point to the RTC
> node to get a regmap?) I am all in, for the reasons you mentioned.

I meant having a separate RTC+clocks driver. I'm not really sure why we
need to split them.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)