From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Saravana Kannan <saravanak@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Stephen Boyd <swboyd@chromium.org>,
Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH] of: property: do not create clocks device link for clock controllers
Date: Tue, 30 Nov 2021 03:24:29 +0300 [thread overview]
Message-ID: <CAA8EJpr4+B1Q3voOr__3M37jxxBRi0g3avzE1Y8CDLkZB+rSGg@mail.gmail.com> (raw)
In-Reply-To: <CAGETcx86iFm04PxnmrRQhooxtd4_kv87rAu=T0RauqFXmRLKNA@mail.gmail.com>
On Tue, 30 Nov 2021 at 02:53, Saravana Kannan <saravanak@google.com> wrote:
>
> On Mon, Nov 29, 2021 at 3:48 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Thu, Nov 25, 2021 at 10:36 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > Do not create device link for clock controllers.
> >
> > Nak.
> >
> > > Some of the clocks
> > > provided to the device via OF can be the clocks that are just parents to
> > > the clocks provided by this clock controller. Clock subsystem already
> > > has support for handling missing clock parents correctly (clock
> > > orphans). Later when the parent clock is registered, clocks get
> > > populated properly.
> > >
> > > An example of the system where this matters is the SDM8450 MTP board
> > > (see arch/arm64/boot/dts/qcom/sdm845-mtp.dts). Here the dispcc uses
> > > clocks provided by dsi0_phy and dsi1_phy device tree nodes. However the
> > > dispcc itself provides clocks to both PHYs, to the PHY parent device,
> > > etc. With just dsi0_phy in place devlink is able to break the
> > > dependency,
> >
> > Right, because I wrote code to make sure we handle these clock
> > controller cases properly. If that logic isn't smart enough, let's fix
> > that.
As I said, devlink was delaying dispcc probing ,waiting for the second
DSI PHY clock provider.
Thus came my proposal to let clock orphans handle the case (which it
does perfectly).
> >
> > > but with two PHYs, dispcc doesn't get probed at all, thus
> > > breaking display support.
> >
> > Then let's find out why and fix this instead of hiding some
> > dependencies from fw_devlink. You could be breaking other cases/boards
> > with this change you are making.
>
> Btw, forgot to mention. I'll look into this one and try to find the
> reason why it wasn't handled automatically. And then come up with a
> fix.
>
> If you want to find out why fw_devlink didn't notice the cycle
> correctly for the case of 2 PHYs vs 1 PHY, I'd appreciate that too.
>
> Btw, same comment for remote-endpoint. I'll look into what's going on
> in that case. Btw, I'm assuming all the code and DT you are testing
> this on is already upstream. Can you please confirm that?
All the code and basic DT is upstreamed. The DT part I
referenced/posted was written for the custom extender for the
qrb5165-rb5 board that I use here to test MSM DRM driver, but the
result DT should be more or less the same as smd845-mtp.
>
> -Saravana
>
> >
> > -Saravana
> >
> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Cc: Stephen Boyd <swboyd@chromium.org>
> > > Cc: Saravana Kannan <saravanak@google.com>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > > drivers/of/property.c | 16 +++++++++++++++-
> > > 1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > > index a3483484a5a2..f7229e4030e3 100644
> > > --- a/drivers/of/property.c
> > > +++ b/drivers/of/property.c
> > > @@ -1264,7 +1264,6 @@ struct supplier_bindings {
> > > bool node_not_dev;
> > > };
> > >
> > > -DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")
> > > DEFINE_SIMPLE_PROP(interconnects, "interconnects", "#interconnect-cells")
> > > DEFINE_SIMPLE_PROP(iommus, "iommus", "#iommu-cells")
> > > DEFINE_SIMPLE_PROP(mboxes, "mboxes", "#mbox-cells")
> > > @@ -1294,6 +1293,21 @@ DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
> > > DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
> > > DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
> > >
> > > +static struct device_node *parse_clocks(struct device_node *np,
> > > + const char *prop_name, int index)
> > > +{
> > > + /*
> > > + * Do not create clock-related device links for clocks controllers,
> > > + * clock orphans will handle missing clock parents automatically.
> > > + */
> > > + if (!strcmp(prop_name, "clocks") &&
> > > + of_find_property(np, "#clock-cells", NULL))
> > > + return NULL;
> > > +
> > > + return parse_prop_cells(np, prop_name, index, "clocks",
> > > + "#clock-cells");
> > > +}
> > > +
> > > static struct device_node *parse_gpios(struct device_node *np,
> > > const char *prop_name, int index)
> > > {
> > > --
> > > 2.33.0
> > >
--
With best wishes
Dmitry
next prev parent reply other threads:[~2021-11-30 0:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-25 18:36 [PATCH] of: property: do not create clocks device link for clock controllers Dmitry Baryshkov
2021-11-29 23:48 ` Saravana Kannan
2021-11-29 23:52 ` Saravana Kannan
2021-11-30 0:24 ` Dmitry Baryshkov [this message]
2021-12-07 2:00 ` Dmitry Baryshkov
2021-12-07 2:24 ` Saravana Kannan
2022-01-10 20:55 ` Dmitry Baryshkov
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=CAA8EJpr4+B1Q3voOr__3M37jxxBRi0g3avzE1Y8CDLkZB+rSGg@mail.gmail.com \
--to=dmitry.baryshkov@linaro.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=saravanak@google.com \
--cc=swboyd@chromium.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
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 NNTP newsgroup(s).