On Fri, Jan 27, 2023 at 9:34 PM Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote: > > 28 января 2023 г. 07:54:14 GMT+03:00, Saravana Kannan <saravanak@xxxxxxxxxx> пишет: > >On Fri, Jan 27, 2023 at 8:41 PM Dmitry Baryshkov > ><dmitry.baryshkov@xxxxxxxxxx> wrote: > >> > >> On Fri, 27 Jan 2023 at 01:12, Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > >> > > >> > On Thu, Jan 26, 2023 at 2:51 PM Dmitry Baryshkov > >> > <dmitry.baryshkov@xxxxxxxxxx> wrote: > >> > > > >> > > 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. > >> > > >> > A clock controller can depend on other clock controllers for non clock > >> > tree reasons. For example, it might need a clock ON to access its > >> > registers. So, while the CCF can handle orphans properly, that's not > >> > the only dependency. Also, fw_devlink is not just about probing > >> > either. It also has to do with proper sync_state() callbacks. > >> > >> Just a question, please excuse if I'm misunderstanding it. Does > >> fw_devlink created this way also impose any runtime PM dependencies? > > > >If you set fw_devlink=rpm in the command line. The default is just "on". > > So you plan to switch to rpm at some point? Ideally, but it's a loooong way off. I need to fix all the issues people are pointing out right now before I try to go for that being the default. -Saravana
