Aw: Re: [PATCH v1 1/3] arm64: dts: marvell: Fix anyOf conditional failed

[Frank Wunderlich]

> Gesendet: Montag, 11. November 2024 um 18:15
> Von: "Andrew Lunn" <andrew at lunn.ch>
> An: "Rob Herring" <robh at kernel.org>
> CC: frank-w at public-files.de, "Frank Wunderlich" <linux at fw-web.de>, "Damien Le Moal" <dlemoal at kernel.org>, "Niklas Cassel" <cassel at kernel.org>, "Krzysztof Kozlowski" <krzk+dt at kernel.org>, "Conor Dooley" <conor+dt at kernel.org>, "Gregory Clement" <gregory.clement at bootlin.com>, "Sebastian Hesselbarth" <sebastian.hesselbarth at gmail.com>, "Russell King" <linux at armlinux.org.uk>, "Hans de Goede" <hdegoede at redhat.com>, "Jens Axboe" <axboe at kernel.dk>, linux-ide at vger.kernel.org, devicetree at vger.kernel.org, linux-kernel at vger.kernel.org, linux-arm-kernel at lists.infradead.org
> Betreff: Re: [PATCH v1 1/3] arm64: dts: marvell: Fix anyOf conditional failed
>
> On Mon, Nov 11, 2024 at 10:25:12AM -0600, Rob Herring wrote:
> > On Sun, Nov 10, 2024 at 3:25 AM Frank Wunderlich
> > <frank-w at public-files.de> wrote:
> > >
> > > Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew at lunn.ch>:
> > > >On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> > > >> From: Frank Wunderlich <frank-w at public-files.de>
> > > >>
> > > >> after converting the ahci-platform binding to yaml the following files
> > > >> reporting "'anyOf' conditional failed" on
> > > >>
> > > >> sata at 540000: sata-port at 0
> > > >> diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > >> index 1e0ab35cc686..2b5e45d2c5a6 100644
> > > >> --- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > >> +++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> > > >> @@ -214,6 +214,7 @@ &cp0_sata0 {
> > > >>
> > > >>      sata-port at 1 {
> > > >>              phys = <&cp0_comphy3 1>;
> > > >> +            status = "okay";
> > > >>      };
> > > >>  };
> > > >
> > > >>
> > > >> diff --git a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> > > >> index 7af949092b91..6bdc4f1e6939 100644
> > > >> --- a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> > > >> +++ b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> > > >> @@ -433,11 +433,13 @@ &cp0_sata0 {
> > > >>      /* 7 + 12 SATA connector (J24) */
> > > >>      sata-port at 0 {
> > > >>              phys = <&cp0_comphy2 0>;
> > > >> +            status = "okay";
> > > >>      };
> > > >>
> > > >>      /* M.2-2250 B-key (J39) */
> > > >>      sata-port at 1 {
> > > >>              phys = <&cp0_comphy3 1>;
> > > >> +            status = "okay";
> > > >>      };
> > > >>  };
> > > >> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > > >> index 7e595ac80043..161beec0b6b0 100644
> > > >> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > > >> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > > >> @@ -347,10 +347,12 @@ CP11X_LABEL(sata0): sata at 540000 {
> > > >>
> > > >>                      sata-port at 0 {
> > > >>                              reg = <0>;
> > > >> +                            status = "disabled";
> > > >>                      };
> > > >
> > > >I don't know the yaml too well, but it is not obvious how adding a few
> > > >status = "disabled"; status = "okay"; fixes a "'anyOf' conditional failed".
> > > >
> > > >Maybe you can expand the explanation a bit?
> > > >
> > > >       Andrew
> > >
> > > Hi angelo,
> > >
> > > I guess the dtbs_check only checks required properties from yaml if the node is enabled.
> > 
> > Yes, that is exactly how it works.
> 
> So from this, can i imply that phys is a required property?
> 
> Looking at the above patch, it appears that for armada-*.dts,
> sata-port at 0 always uses phys = <&cp0_comphy2 0> and sata-port at 1 uses
> phys = <&cp0_comphy3 1>. Is this an actual SoC property? Could it be
> moved up into the .dtsi file? Or is it really a board property?

as i said the phy may operate in different modes (not know marvell here, but on other vendors phys are defined at board level), maybe the boards where the phy is missing the phy is used in another mode. Without knowing the SoC and boards disable it at SoC-level and enable only the nodes containing a phys property is all i can do here to fix the issue. Imho it is always a good idea to enable only the conrollers a board will use.

> 	Andrew
> </frank-w at public-files.de></andrew at lunn.ch></frank-w at public-files.de></axboe at kernel.dk></hdegoede at redhat.com></linux at armlinux.org.uk></sebastian.hesselbarth at gmail.com></gregory.clement at bootlin.com></conor+dt at kernel.org></krzk+dt at kernel.org></cassel at kernel.org></dlemoal at kernel.org></linux at fw-web.de></robh at kernel.org></andrew at lunn.ch>