Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Stephen Boyd <sboyd@kernel.org>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-clk@vger.kernel.org, linux-rtc@vger.kernel.org,
	Alessandro Zummo <a.zummo@towertech.it>,
	Russell King <linux@armlinux.org.uk>,
	Michael Turquette <mturquette@baylibre.com>,
	linux-kernel@vger.kernel.org, kernel@collabora.com
Subject: Re: [RFCv1] rtc: m41t80: disable clock provider support
Date: Wed, 13 Nov 2019 23:27:26 +0100
Message-ID: <20191113222726.ifjp2bhwmplm5r7z@earth.universe> (raw)
In-Reply-To: <20191112222012.157CC20674@mail.kernel.org>

[-- Attachment #1: Type: text/plain, Size: 3179 bytes --]

Hi Stephen,

On Tue, Nov 12, 2019 at 02:20:11PM -0800, Stephen Boyd wrote:
> Quoting Sebastian Reichel (2019-11-12 07:15:26)
> > On Fri, Nov 08, 2019 at 10:53:33PM -0800, Stephen Boyd wrote:
> > > Quoting Sebastian Reichel (2019-11-08 17:41:51)
> > > > On Fri, Nov 08, 2019 at 04:24:48PM -0800, Stephen Boyd wrote:
> > > > > 
> > > > > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 17:01 Sebastian Reichel
2019-11-08 17:53 ` Alexandre Belloni
2019-11-08 22:34   ` Sebastian Reichel
2019-11-09  0:24     ` Stephen Boyd
2019-11-09  1:41       ` Sebastian Reichel
2019-11-09  6:53         ` Stephen Boyd
2019-11-12 15:15           ` Sebastian Reichel
2019-11-12 22:20             ` Stephen Boyd
2019-11-13 22:27               ` Sebastian Reichel [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191113222726.ifjp2bhwmplm5r7z@earth.universe \
    --to=sebastian.reichel@collabora.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=kernel@collabora.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git