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

From: "Ondřej Jirman" <megous@megous.com>
To: "Jernej Škrabec" <jernej.skrabec@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	devicetree@vger.kernel.org,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com,
	Rob Herring <robh+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org
Subject: Re: [linux-sunxi] [PATCH v2 2/3] rtc: sun6i: Add support for H6 RTC
Date: Sat, 24 Aug 2019 23:27:46 +0200	[thread overview]
Message-ID: <20190824212746.a5pyilkrrvysjjbd@core.my.home> (raw)
In-Reply-To: <1690798.2HKiRSsjat@jernej-laptop>

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.
> 
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel