Received-SPF: pass (google.com: domain of linux-kernel+bounces-112170-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240224052436.3552333-1-saravanak@google.com> <ZfvN5jDrftG-YRG4@titan>
In-Reply-To: <ZfvN5jDrftG-YRG4@titan>
From: Saravana Kannan <saravanak@google.com>
Date: Fri, 22 Mar 2024 18:53:57 -0700
Message-ID: <CAGETcx8+vw0Vr0NWzjOAvxAZ07M4U7BWPAgO9avCngW0-9e_kA@mail.gmail.com>
Subject: Re: [PATCH] of: property: fw_devlink: Fix stupid bug in
 remote-endpoint parsing
To: John Watts <contact@jookia.org>
Cc: Rob Herring <robh+dt@kernel.org>, Frank Rowand <frowand.list@gmail.com>, 
	=?UTF-8?Q?Herv=C3=A9_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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 20, 2024 at 11:05=E2=80=AFPM John Watts <contact@jookia.org> wr=
ote:
>
> On Fri, Feb 23, 2024 at 09:24:35PM -0800, Saravana Kannan wrote:
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minu=
te
> > incorrect edit of "index !=3D0" into "!index". This patch fixes it to b=
e
> > "index > 0" to match the comment right next to it.
> >
> > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a r=
emote-endpoint property")
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > Using Link: instead of Closes: because Luca reported two separate issue=
s.
> >
> > Sorry about introducing a stupid bug in an -rcX Rob.
> >
> > -Saravana
>
> Hi there,
>
> Could this be reverted? It breaks my audio-graph-card2 setup:
>
> [   17.116290] platform 2034000.i2s: deferred probe pending: platform: wa=
it for supplier /sound/multi
> [   17.125370] platform test_codec: deferred probe pending: platform: wai=
t for supplier /sound/multi
> [   17.134257] platform sound: deferred probe pending: asoc-audio-graph-c=
ard2: parse error

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?

But the better fix would be to use the new "post-init-providers"
property. See below.

>
> / {
>         ...
>
>
>         test_codec {
>                 compatible =3D "test-codec";
>                 prefix =3D "Test codec";
>                 #sound-dai-cells =3D <0>;

post-init-provider =3D <&multi>;

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

>                 port {
>                         test_ep: endpoint {
>                                 remote-endpoint =3D <&card_ep_2>;
>                         };
>                 };
>         };
>
>         sound {
>                 compatible =3D "audio-graph-card2";
>                 label =3D "CS5368";
>                 links =3D <&i2s2_port>;
>                 multi {
>                         #address-cells =3D <1>;
>                         #size-cells =3D <0>;
>                         convert-channels =3D <16>;
>                         port@0 {
>                                 reg =3D <0>;
>                                 card_ep_0: endpoint {
>                                         remote-endpoint =3D <&i2s2_ep>;
>                                 };
>                         };
>                         //port@1 {
>                         //      reg =3D <1>;
>                         //      card_ep_1: endpoint {
>                         //              remote-endpoint =3D <&cs5368_ep>;
>                         //      };
>                         //};
>                         port@1 {
>                                 reg =3D <1>;
>                                 card_ep_2: endpoint {
>                                         remote-endpoint =3D <&test_ep>;
>                                 };
>                         };
>                 };
>         };
> };
>
>
> &i2s2 {
>         pinctrl-0 =3D <&i2s2_pins>, <&i2s2_din_pins>;
>         pinctrl-names =3D "default";
>         status =3D "okay";
>         i2s2_port: port {
>                 i2s2_ep: endpoint {
>                         format =3D "dsp_a";
>                         bitclock-master;
>                         frame-master;
>                         remote-endpoint =3D <&card_ep_0>;
>                 };
>         };
> };
>
> John.