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

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

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

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