linux-kernel - Re: [linux-sunxi] [PATCH v2 2/3] rtc: sun6i: Add support for H6 RTC

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: <20190824212746.a5pyilkrrvysjjbd@core.my.home>
Date:   Sat, 24 Aug 2019 23:27:46 +0200
From:   Ondřej Jirman <megous@...ous.com>
To:     Jernej Škrabec <jernej.skrabec@...il.com>
Cc:     linux-sunxi@...glegroups.com, Mark Rutland <mark.rutland@....com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        devicetree@...r.kernel.org,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        linux-kernel@...r.kernel.org, Chen-Yu Tsai <wens@...e.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, linux-rtc@...r.kernel.org
Subject: Re: [linux-sunxi] [PATCH v2 2/3] rtc: sun6i: Add support for H6 RTC

Hello Jernej,

On Sat, Aug 24, 2019 at 11:09:49PM +0200, Jernej Škrabec wrote:
> > Visually?
> > 
> > That would explain why it doesn't work for you. The mainline RTC driver
> > disables auto-switch feature, and if your board doesn't have a crystal for
> > LOSC, RTC will not generate a clock for the RTC.
> > 
> > H6's dtsi describes by default a situatiuon with external 32k crystal
> > oscillator. See ext_osc32k node. That's incorrect for your board if it
> > doesn't have the crystal. You need to fix this in the DTS for your board
> > instead of patching the driver.
> 
> I see that reparenting is supported, but I'm not sure how to fix that in DT. 
> Any suggestion?

You may try removing the clocks property from rtc node.

> > 
> > The driver has parent clock selection logic in case the LOSC crystal is not
> > used.
> > 
> > Your patch enables automatic detection of LOSC failure and RTC changes clock
> > to LOSC automatically, despite what's described in the DTS. That may fix
> > the issue, but is not the correct solution.
> > 
> > Registers on my board look like this (external 32k osc is used) for
> > reference:
> > 
> > LOSC_CTRL_REG[7000000]: 8011
> > 	KEY_FIELD                      ???                  (0)
> > 	LOSC_AUTO_SWT_BYPASS           EN                   (1)
> > 	LOSC_AUTO_SWT_EN               DIS                  (0)
> > 	EXT_LOSC_EN                    EN                   (1)
> > 	EXT_LOSC_GSM                   LOW                  (0)
> > 	BATTERY_DIR                    DISCHARGE            (0)
> > 	LOSC_SRC_SEL                   EXT32k               (1)
> > 
> > LOSC_AUTO_SWT_STA_REG[7000004]: 1
> > 	EXT_LOSC_STA                   OK                   (0)
> > 	LOSC_AUTO_SWT_PEND             NOEFF                (0)
> > 	LOSC_SRC_SEL_STA               EXT32K               (1)
> > 
> 
> In my case LOSC_CTRL_REG has value 0x4010 and LOSC_AUTO_SWT_STA_REG
> has value 0x4, so there is issue with external crystal (it's missing) and RTC 
> switched to internal one.
> 
> BTW, what's wrong with automatic switching? Why is it disabled?

It always was disabled on mainline (bit 14 was set to 0 even before my patch).
H6 just probably has another extra undocummented bit, that's needed to disables
it properly.

You probably don't want a glitch to switch your RTC from high-precision
clock to a low precision one possibly without any indication in the userspace
or a kernel log.

Regardless of all this, DTS needs to have a correct description of the HW,
which means if RTC module is not connected to the 32.757kHz crystal/clock,
clocks property should be empty.

regards,
	o.

> Best regards,
> Jernej
> 
> > regards,
> > 	o.
> > 
> > > > The real issue probably is that the mainline driver is missing this:
> > > > 
> > > > https://megous.com/git/linux/tree/drivers/rtc/rtc-sunxi.c?h=h6-4.9-bsp#n
> > > > 650
> > > 
> > > Not sure what you mean by that. ext vs. int source selection?
> > > 
> > > 
> > > 
> > > Best regards,
> > > Jernej
> > > 
> > > > regards,
> > > > 
> > > > 	o.
> 
> 
> 
>