Re: [RESEND PATCH] of: property: do not create clocks device link for clock controllers

From: Dmitry Baryshkov
Date: Thu Jan 26 2023 - 17:51:10 EST

On 25/01/2023 21:09, Rob Herring wrote:
On Tue, Jan 24, 2023 at 06:12:15PM -0800, Saravana Kannan wrote:
On Wed, Jan 18, 2023 at 5:35 AM Rob Herring <robh+dt@xxxxxxxxxx> wrote:

On Wed, Jan 18, 2023 at 3:11 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> 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@xxxxxxxxxx>
Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
Cc: Saravana Kannan <saravanak@xxxxxxxxxx>
Cc: Abel Vesa <abel.vesa@xxxxxxxxxx>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---

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.

This case is really simple, I think. Clock controllers (and clock-core-framework) are prepared to handle clock orphans properly. Moreover they have been supposed to work in such way for quite a while. In other words, I don't think we should save them from this -EPROBE_DEFERRED.

Thus I think it is better to let them continue doing their job of handling probe deferrals on their own, at least for the time being. And then, when your patches are finished, we can think about reenabling current behaviour. As a reminder, currently, all Qualcomm platforms trying to use double DSI configuration are broken and have to use fw_devlink= kernel params.

Stephen, do you have any comments regarding this fw_devlink usage vs CCF?


Okay, will give it a bit longer.

Rob

--
With best wishes
Dmitry