From: Saravana Kannan <saravanak@google.com>
To: Rob Herring <robh@kernel.org>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
linux-clk <linux-clk@vger.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
Collabora Kernel ML <kernel@collabora.com>
Subject: Re: [RFC] clk: add boot clock support
Date: Thu, 25 Mar 2021 18:55:52 -0700 [thread overview]
Message-ID: <CAGETcx9JmtbwAq_fpU5KfUzjcTw-uHPqKo3gAGjQwht=wxY8yg@mail.gmail.com> (raw)
In-Reply-To: <20210326012720.GA2113788@robh.at.kernel.org>
On Thu, Mar 25, 2021 at 6:27 PM Rob Herring <robh@kernel.org> wrote:
>
> +Saravana
>
> On Thu, Mar 18, 2021 at 10:03:18PM +0100, Sebastian Reichel wrote:
> > On Congatec's QMX6 system on module one of the i.MX6 fixed clocks
> > is provided by an I2C RTC. Specifying this properly results in a
> > circular dependency, since the I2C RTC (and thus its clock) cannot
> > be initialized without the i.MX6 clock controller being initialized.
> >
> > With current code the following path is executed when i.MX6 clock
> > controller is probed (and ckil clock is specified to be the I2C RTC
> > via DT):
> >
> > 1. imx6q_obtain_fixed_clk_hw(ccm_node, "ckil", 0);
> > 2. of_clk_get_by_name(ccm_node, "ckil");
> > 3. __of_clk_get(ccm_node, 0, ccm_node->full_name, "ckil");
> > 4. of_clk_get_hw(ccm_node, 0, "ckil")
> > 5. spec = of_parse_clkspec(ccm_node, 0, "ckil"); // get phandle
> > 6. of_clk_get_hw_from_clkspec(&spec); // returns -EPROBE_DEFER
> > 7. error is propagated back, i.MX6q clock controller is probe deferred
> > 8. I2C controller is never initialized without clock controller
> > I2C RTC is never initialized without I2C controller
> > CKIL clock is never initialized without I2C RTC
> > clock controller is never initialized without CKIL
> >
> > To fix the circular dependency this registers a dummy clock when
> > the RTC clock is tried to be acquired. The dummy clock will later
> > be unregistered when the proper clock is registered for the RTC
> > DT node. IIUIC clk_core_reparent_orphans() will take care of
> > fixing up the clock tree.
> >
> > NOTE: For now the patch is compile tested only. If this approach
> > is the correct one I will do some testing and properly submit this.
> > You can find all the details about the hardware in the following
> > patchset:
> >
> > https://lore.kernel.org/linux-devicetree/20210222171247.97609-1-sebastian.reichel@collabora.com/
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> > .../bindings/clock/clock-bindings.txt | 7 +
> > drivers/clk/clk.c | 146 ++++++++++++++++++
> > 2 files changed, 153 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> > index f2ea53832ac6..66d67ff4aa0f 100644
> > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> > @@ -32,6 +32,13 @@ clock-output-names: Recommended to be a list of strings of clock output signal
> > Clock consumer nodes must never directly reference
> > the provider's clock-output-names property.
> >
> > +boot-clock-frequencies: This property is used to specify that a clock is enabled
> > + by default with the provided frequency at boot time. This
> > + is required to break circular clock dependencies. For clock
> > + providers with #clock-cells = 0 this is a single u32
> > + with the frequency in Hz. Otherwise it's a list of
> > + clock cell specifier + frequency in Hz.
>
> Seems alright to me. I hadn't thought about the aspect of needing to
> know the frequency. Other cases probably don't as you only need the
> clocks once both components have registered.
>
> Note this could be lost being threaded in the other series.
I read this thread and tried to understand it. But my head isn't right
today (lack of sleep) so I couldn't wrap my head around it. I'll look
at it again after the weekend. In the meantime, Sebastian can you
please point me to the DT file and the specific device nodes (names or
line number) where this cycle is present?
Keeping a clock on until all its consumers probe is part of my TODO
list (next item after fw_devlink=on lands). I already have it working
in AOSP, but need to clean it up for upstream. fw_devlink can also
break *some* cycles (not all). So I'm wondering if the kernel will
solve this automatically soon(ish). If it can solve it automatically,
I'd rather not add new DT bindings because it'll make it more work for
fw_devlink.
Thanks,
Saravana
next prev parent reply other threads:[~2021-03-26 1:57 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-22 17:12 [PATCHv1 0/6] Support for GE B1x5v2 Sebastian Reichel
2021-02-22 17:12 ` [PATCHv1 1/6] rtc: m41t80: add support for protected clock Sebastian Reichel
2021-02-22 21:20 ` Alexandre Belloni
2021-02-22 21:26 ` Alexandre Belloni
2021-02-23 1:26 ` Sebastian Reichel
2021-03-06 19:56 ` Rob Herring
2021-03-08 14:03 ` Sebastian Reichel
2021-03-16 21:51 ` Rob Herring
2021-03-18 21:03 ` [RFC] clk: add boot clock support Sebastian Reichel
2021-03-26 1:27 ` Rob Herring
2021-03-26 1:55 ` Saravana Kannan [this message]
2021-03-26 9:52 ` Sebastian Reichel
2021-03-29 20:03 ` Saravana Kannan
2021-03-29 21:53 ` Sebastian Reichel
2021-03-30 0:36 ` Saravana Kannan
2021-03-30 9:09 ` Sebastian Reichel
2021-03-30 17:05 ` Saravana Kannan
2021-04-05 22:43 ` Sebastian Reichel
2021-04-05 23:51 ` Saravana Kannan
2021-02-22 17:12 ` [PATCHv1 2/6] drm/imx: Add 8 pixel alignment fix Sebastian Reichel
2021-02-22 17:12 ` [PATCHv1 3/6] dt-bindings: vendor-prefixes: add congatec Sebastian Reichel
2021-03-06 19:57 ` Rob Herring
2021-02-22 17:12 ` [PATCHv1 4/6] dt-bindings: arm: fsl: add GE B1x5pv2 boards Sebastian Reichel
2021-03-06 19:58 ` Rob Herring
2021-02-22 17:12 ` [PATCHv1 5/6] dt-bindings: mtd: jedec,spi-nor: add sst25vf032b Sebastian Reichel
2021-02-23 0:15 ` Rob Herring
2021-02-23 1:33 ` Sebastian Reichel
2021-02-22 17:12 ` [PATCHv1 6/6] ARM: dts: imx6: Add GE B1x5v2 Sebastian Reichel
Reply instructions:
You may reply publicly 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='CAGETcx9JmtbwAq_fpU5KfUzjcTw-uHPqKo3gAGjQwht=wxY8yg@mail.gmail.com' \
--to=saravanak@google.com \
--cc=devicetree@vger.kernel.org \
--cc=kernel@collabora.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=sebastian.reichel@collabora.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).