Date:   Tue, 24 Jan 2023 18:12:15 -0800
From:   Saravana Kannan <saravanak@...gle.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Frank Rowand <frowand.list@...il.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org,
        Bjorn Andersson <andersson@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Abel Vesa <abel.vesa@...aro.org>
Subject: Re: [RESEND PATCH] of: property: do not create clocks device link for
 clock controllers

On Wed, Jan 18, 2023 at 5:35 AM Rob Herring <robh+dt@...nel.org> wrote:
>
> On Wed, Jan 18, 2023 at 3:11 AM Dmitry Baryshkov
> <dmitry.baryshkov@...aro.org> wrote:
> >
> > Do not create device link for clock controllers. 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, but with two PHYs, dispcc doesn't get probed at all, thus
> > breaking display support.
> >
> > Cc: Bjorn Andersson <andersson@...nel.org>
> > Cc: Stephen Boyd <sboyd@...nel.org>
> > Cc: Saravana Kannan <saravanak@...gle.com>
> > Cc: Abel Vesa <abel.vesa@...aro.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > ---
> >
> > This patch has been posted a year ago in January 2022 ([1]). Since that time
> > Saravana failed to produce patches to assist in debugging the issue
> > ([2]) or to fix the issue ([3]). The issue we observe has been described
> > by Abel at ([4]). As we work on adding support for Dual DSI
> > configurations, the issue becomes more and more important, since binding
> > the whole display subsystem fails.

I did send out a patch series[1] to try and fix this. Heck I even
talked about this in LPC 2022. So I don't think it's accurate to say I
didn't help debug this or fix this. There's some email thread in lore
where Abel gave more details and I figured out the issue and we didn't
need any more debugging. And then I sent out [1]. Sorry I missed you
in the cc lise for [1] -- I try to keep track of everyone to cc but
things slip through the cracks sometimes. But at the same time, it's
easy to check for emails from me before saying I didn't help or didn't
send out fixes :)

If you do try to give [1] a shot, there are a bunch of bugs that
people pointed out for which I gave fixes on top of [1] in the
replies. I was supposed to work on v2 over the holidays, but that
didn't happen because of stuff outside my control.

> That's ample time to fix this, so I intend to apply this. But I'll
> give it a few days for comments.

Rob, I'd recommend not applying this because it'll fix it for Dmitry
but break someone else's use case. That's the whole reason it takes me
a while to send out patches -- it's easy to fix it for a subset of
devices, but fixing something without breaking someone else is harder
(I still believe it's doable) and it takes a while to test them on all
the devices I want to test before sending them out.

-Saravana
[1] - https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/

Powered by blists - more mailing lists