From: Saravana Kannan <saravanak@google.com>
To: John Watts <contact@jookia.org>
Cc: "Rob Herring" <robh+dt@kernel.org>,
"Frank Rowand" <frowand.list@gmail.com>,
"Hervé Codina" <herve.codina@bootlin.com>,
"Luca Ceresoli" <luca.ceresoli@bootlin.com>,
kernel-team@android.com, "Rob Herring" <robh@kernel.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing
Date: Mon, 25 Mar 2024 15:49:44 -0700 [thread overview]
Message-ID: <CAGETcx_=MmfgDajM16iJ4Of9Yr2Sy6ZpU=MyhYgnmOJFUTD_oA@mail.gmail.com> (raw)
In-Reply-To: <Zf7I65PiOR2wX1Uo@titan>
On Sat, Mar 23, 2024 at 5:20 AM John Watts <contact@jookia.org> wrote:
>
> Hello again,
>
> On Fri, Mar 22, 2024 at 06:53:57PM -0700, Saravana Kannan wrote:
> > Hmmm.... cycle detection should work here and not enforce probe
> > ordering. I'd appreciate help with debugging that. Let me look at it
> > on Monday. Can you enabled all the debug logs in drivers/base/core.c
> > and tell me what cycle detection is telling about these nodes?
>
> Hmm. It's not saying anything more than what I've already sent.
Sorry, I was asking for the logs. But now I'm looking at this again, I
think I understand what's going on.
> I think this is because /sound/multi isn't a device, it's just a
> subnode used in audio-graph-card2.
Ok, I think I understand now what's going on. fw_devlink does not know
that "sound" device will not populate "multi" as a child device.
Typically in such situations, "sound" would probe as a device and add
its child DT nodes devices. At that point, the cycle is only between
"multi" and "test_codec" and fw_devlink will detect that and not
enforce any ordering. However, in this case, "sound" doesn't have any
child devices and just depends on the remote endpoints directly.
We already have "ports", "in-ports" and "out-ports". Is there a reason
none of them will work for your use case and it has to be "multi"?
When you use one of those 3 recognized node names, things are handled
correctly.
Btw, between "test_codec" and "sound", which one is supposed to probe
first? I'm guessing "test_codec" needs to probe first for "sound" to
probe?
> Removing the multi { } section and using direct graph connections
> 'fixes' this.
I think the right fix is the use of post-init-providers. Because even
if you do the above, all it does is let fw_devlink see that there's a
cyclic dependency in DT. And it'll stop enforcing the probe and
suspend/resume ordering. Ideally we want to enforce a specific order
here. test_codec first and then sound.
> I think this might be because usually in a graph each node containing
> ports is a device, such as a display panel, a bridge, an LCD
> controller. These kind of form a dependency chain.
>
> In this case all the ports in multi act as a way to glue multiple
> ports together for the audio-graph-card2.
>
> Does that help?
Maybe. But the logs would be more helpful.
>
> > But the better fix would be to use the new "post-init-providers"
> > property. See below.
> >
> > >
> > > / {
> > > ...
> > >
> > >
> > > test_codec {
> > > compatible = "test-codec";
> > > prefix = "Test codec";
> > > #sound-dai-cells = <0>;
> >
> > post-init-provider = <&multi>;
Did you try this? Did it help?
-Saravana
> >
> > Right now there's a cyclic dependency between test_codec and multi and
> > this tells the kernel that test codec needs to probe first.
> >
> > Similar additions to the other nodes blocked on multi.
> >
> > Thanks,
> > Saravana
>
> John.
next prev parent reply other threads:[~2024-03-25 22:50 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-24 5:24 [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing Saravana Kannan
2024-02-24 5:28 ` Saravana Kannan
2024-02-26 12:04 ` Luca Ceresoli
2024-02-29 0:20 ` Rob Herring
2024-02-26 8:19 ` Luca Ceresoli
2024-02-26 8:56 ` Herve Codina
2024-02-29 10:11 ` Geert Uytterhoeven
2024-02-29 22:45 ` Saravana Kannan
2024-02-29 22:47 ` Saravana Kannan
2024-03-14 1:48 ` Saravana Kannan
2024-03-14 8:46 ` Geert Uytterhoeven
2024-03-15 5:50 ` Saravana Kannan
2024-03-01 21:28 ` Rob Herring
2024-03-21 6:04 ` John Watts
2024-03-23 1:53 ` Saravana Kannan
2024-03-23 12:19 ` John Watts
2024-03-25 22:49 ` Saravana Kannan [this message]
2024-03-26 0:42 ` John Watts
2024-03-26 1:35 ` Saravana Kannan
2024-03-26 1:42 ` John Watts
2024-03-26 2:31 ` John Watts
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='CAGETcx_=MmfgDajM16iJ4Of9Yr2Sy6ZpU=MyhYgnmOJFUTD_oA@mail.gmail.com' \
--to=saravanak@google.com \
--cc=contact@jookia.org \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=herve.codina@bootlin.com \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.