Re: [RFCv1] rtc: m41t80: disable clock provider support

Hi Stephen,

On Tue, Nov 12, 2019 at 02:20:11PM -0800, Stephen Boyd wrote:

Quoting Sebastian Reichel (2019-11-12 07:15:26)

quoted

On Fri, Nov 08, 2019 at 10:53:33PM -0800, Stephen Boyd wrote:

quoted

Quoting Sebastian Reichel (2019-11-08 17:41:51)

quoted

On Fri, Nov 08, 2019 at 04:24:48PM -0800, Stephen Boyd wrote:

quoted

Is this the chicken-egg scenario? I read this thread but I can't follow
along with what the problem is. Sorry.

Yes. The board has an I2C based RTC (m41t62), which provides a programmable 1
Hz to 32 kHz square wave (SQW) output defaulting to 32 kHz. The board designers
connected the RTC's SQW output to the i.MX6 CKIL clock input instead of adding
another oscillator. The i.MX6 CCM acquires that clock in imx6q_clocks_init()
(and assumes it is a fixed clock):

hws[IMX6QDL_CLK_CKIL] = imx6q_obtain_fixed_clk_hw(ccm_node, "ckil", 0);

Who uses the IMX6QDL_CLK_CKIL though? Grep on kernel sources shows me
nothing.

The manual specifies, that CKIL is synchronized with the main system
clock. The resulting clock is used by all kind of IP cores inside
the i.MX6, for example the SNVS RTC and watchdog. I couldn't find
any registers to configure the CKIL pipeline and CKIL input is
usually a fixed clock, so current implementation might be "broken"
without anyone noticing. Checking a running i.MX6 system, that
actually seems to be the case :(

$ cat /sys/kernel/debug/clk/ckil/clk_rate         
32768
$ cat /sys/kernel/debug/clk/ckil/clk_enable_count 
0
$ cat /sys/kernel/debug/clk/ckil/clk_prepare_count 
0
$ cat /sys/kernel/debug/clk/ckil/clk_flags         
CLK_IS_BASIC

I suppose an easy fix would be to mark that clock as critical and
that would also keep the parent clocks enabled?

Yes. It sounds like some sort of low frequency timer clk. It probably
should always be left enabled with CLK_IS_CRITICAL then.

Right, system expects that clock to be always available including
low power states. This is supposed not to be turned off at all.

I gave it a try today (I defined ckil clock in DT as fixed
rate clock with divider and multiplier set to 1 and used
the RTC as parent clock) and it happened exactly what I
expected: I received -EPROBE_DEFER. This results in
the problem, that I pointed out.

Actually imx6 clock manager driver registers a fixed clock, when
the DT part fails (incl. a -EPROBE_DEFER error), so it still boots.
But then the reference to the parent clock is obviously missing,
so RTC clock is not enabled and CKIL is effectivly missing.

If the error code is handled properly the boot does not finish,
since the i2c bus driver probe defers without the clock manager's
clocks being available. Without the i2c bus driver, the RTC driver
is not probed, so the clock never appears.

The simplest fix would be to export of_clk_detect_critical()
and call it in the RTC driver. Reading the comment above the
function I suppose this is not an acceptable solution?

-- Sebastian