[PATCH v1 0/3] fix some binding check errors for marvell

[PATCH v1 0/3] fix some binding check errors for marvell

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Thies series fixes some Errors reported by dtbs_check. First part is taken
from my old series [1] where i converted sata-platform txt binding to yaml
because it wasn't picked up.

[1] https://lore.kernel.org/linux-arm-kernel/20220311210357.222830-1-linux@fw-web.de/

Frank Wunderlich (3):
  arm64: dts: marvell: Fix anyOf conditional failed
  arm64: dts: marvell: drop additional phy-names for sata
  dt-bindings: ata: ahci-platform: add missing iommus property

 Documentation/devicetree/bindings/ata/ahci-platform.yaml   | 3 +++
 arch/arm64/boot/dts/marvell/armada-7040-db.dts             | 1 +
 arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts       | 2 ++
 arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts | 1 +
 arch/arm64/boot/dts/marvell/armada-8040-db.dts             | 5 +++--
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi         | 3 +--
 arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts    | 2 ++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi              | 2 ++
 arch/arm64/boot/dts/marvell/cn9130-crb-B.dts               | 1 +
 arch/arm64/boot/dts/marvell/cn9131-db.dtsi                 | 1 +
 arch/arm64/boot/dts/marvell/cn9132-db.dtsi                 | 1 +
 11 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.43.0

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

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

after converting the ahci-platform binding to yaml the following files
reporting "'anyOf' conditional failed" on

sata@540000: sata-port@0

armada-7040-db.dts
armada-8040-clearfog-gt-8k.dts
armada-8040-mcbin.dts
armada-8040-mcbin-singleshot.dts
cn9130-db.dts
cn9130-db-B.dts
cn9131-db.dts
cn9131-db-B.dts
cn9132-db.dts
cn9132-db-B.dts

the following files reporting 'anyOf' conditional failed on

sata@540000: sata-port@1

cn9132-db.dts
cn9132-db-B.dts
cn9130-crb-B.dts

'phys' is a required property
'target-supply' is a required property
From schema: Documentation/devicetree/bindings/ata/ahci-platform.yaml

This is caused by defining sata-ports incomplete in armada-cp11x.dtsi
and overriding only a subset of ports with the needed
phys/target-supply property.

Fix this by disabling the node-templates and enabling the needed nodes.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v7:
- rebase on mvebu/dt64
- drop fixes tag
i only try to fix binding-check errors.
as i cannot test it on hardware, please verify my changes are correct

there are still some there, but they should be fixed by someone having the hardware.

v5: add fixes-tag

the dtsi uses a macro for the node-label defined in armada-common.dtsi

CP11X_LABEL(sata0): sata@540000 {

so i hope i catched all right nodes to be enabled...
have enabled all cpX_sata0 sata-portY childs
---
 arch/arm64/boot/dts/marvell/armada-7040-db.dts             | 1 +
 arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts       | 2 ++
 arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts | 1 +
 arch/arm64/boot/dts/marvell/armada-8040-db.dts             | 3 +++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi         | 1 +
 arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts    | 2 ++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi              | 2 ++
 arch/arm64/boot/dts/marvell/cn9130-crb-B.dts               | 1 +
 arch/arm64/boot/dts/marvell/cn9131-db.dtsi                 | 1 +
 arch/arm64/boot/dts/marvell/cn9132-db.dtsi                 | 1 +
 10 files changed, 15 insertions(+)

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@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@0 {
 		phys = <&cp0_comphy2 0>;
+		status = "okay";
 	};
 
 	/* M.2-2250 B-key (J39) */
 	sata-port@1 {
 		phys = <&cp0_comphy3 1>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
index 7005a32a6e1e..225a54ab688d 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
@@ -475,6 +475,7 @@ &cp1_sata0 {
 
 	sata-port@1 {
 		phys = <&cp1_comphy0 1>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
index 2ec19d364e62..fe5d6cb9d692 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
@@ -145,9 +145,12 @@ &cp0_sata0 {
 
 	sata-port@0 {
 		phys = <&cp0_comphy1 0>;
+		status = "okay";
 	};
+
 	sata-port@1 {
 		phys = <&cp0_comphy3 1>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
index e88ff5b179c8..5043cf2eb33e 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
@@ -245,6 +245,7 @@ &cp0_sata0 {
 	/* CPM Lane 5 - U29 */
 	sata-port@1 {
 		phys = <&cp0_comphy5 1>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
index 3e5e0651ce68..9c25a88581e4 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
@@ -408,10 +408,12 @@ &cp0_sata0 {
 
 	sata-port@0 {
 		phys = <&cp0_comphy2 0>;
+		status = "okay";
 	};
 
 	sata-port@1 {
 		phys = <&cp0_comphy5 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@540000 {
 
 			sata-port@0 {
 				reg = <0>;
+				status = "disabled";
 			};
 
 			sata-port@1 {
 				reg = <1>;
+				status = "disabled";
 			};
 		};
 
diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb-B.dts b/arch/arm64/boot/dts/marvell/cn9130-crb-B.dts
index 0904cb0309ae..34194745f79e 100644
--- a/arch/arm64/boot/dts/marvell/cn9130-crb-B.dts
+++ b/arch/arm64/boot/dts/marvell/cn9130-crb-B.dts
@@ -28,6 +28,7 @@ sata-port@0 {
 		status = "okay";
 		/* Generic PHY, providing serdes lanes */
 		phys = <&cp0_comphy2 0>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/cn9131-db.dtsi b/arch/arm64/boot/dts/marvell/cn9131-db.dtsi
index ad7360c83048..626042fce7e2 100644
--- a/arch/arm64/boot/dts/marvell/cn9131-db.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9131-db.dtsi
@@ -127,6 +127,7 @@ &cp1_sata0 {
 	sata-port@1 {
 		/* Generic PHY, providing serdes lanes */
 		phys = <&cp1_comphy5 1>;
+		status = "okay";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/cn9132-db.dtsi b/arch/arm64/boot/dts/marvell/cn9132-db.dtsi
index e753cfdac697..f91fc69905b8 100644
--- a/arch/arm64/boot/dts/marvell/cn9132-db.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9132-db.dtsi
@@ -175,6 +175,7 @@ &cp2_sata0 {
 	sata-port@0 {
 		/* Generic PHY, providing serdes lanes */
 		phys = <&cp2_comphy2 0>;
+		status = "okay";
 	};
 };
 
-- 
2.43.0

[PATCH v1 2/3] arm64: dts: marvell: drop additional phy-names for sata

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Commit facbe7092f8a ("arm64: dts: marvell: Drop undocumented SATA phy names")
drops some phy-names from devicetrees but misses some. Drop them too.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/marvell/armada-8040-db.dts     | 2 --
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 2 --
 2 files changed, 4 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
index fe5d6cb9d692..9d45e881a97d 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
@@ -307,11 +307,9 @@ &cp1_sata0 {
 
 	sata-port@0 {
 		phys = <&cp1_comphy1 0>;
-		phy-names = "cp1-sata0-0-phy";
 	};
 	sata-port@1 {
 		phys = <&cp1_comphy3 1>;
-		phy-names = "cp1-sata0-1-phy";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
index 5043cf2eb33e..0d4a5fd9503f 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
@@ -345,13 +345,11 @@ &cp1_sata0 {
 	/* CPS Lane 1 - U32 */
 	sata-port@0 {
 		phys = <&cp1_comphy1 0>;
-		phy-names = "cp1-sata0-0-phy";
 	};
 
 	/* CPS Lane 3 - U31 */
 	sata-port@1 {
 		phys = <&cp1_comphy3 1>;
-		phy-names = "cp1-sata0-1-phy";
 	};
 };
 
-- 
2.43.0

[PATCH v1 3/3] dt-bindings: ata: ahci-platform: add missing iommus property

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

fix dtbs_check errors in following files:

arch/arm64/boot/dts/marvell/armada-7040-db.dtb: sata@540000:
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dtb: sata@540000:
arch/arm64/boot/dts/marvell/armada-8040-db.dtb: sata@540000:
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtb: sata@540000:
arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dtb: sata@540000:
  Unevaluated properties are not allowed ('iommus' was unexpected)

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
iommus seems to be added with
83a3545d9c37 ("arm64: dts: marvell: add SMMU support")
which seems not be documented in txt before i converted the binding.
---
 Documentation/devicetree/bindings/ata/ahci-platform.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
index ef19468e3022..cc35cdc02840 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -84,6 +84,9 @@ properties:
     minItems: 1
     maxItems: 3
 
+  iommus:
+    maxItems: 1
+
 patternProperties:
   "^sata-port@[0-9a-f]+$":
     $ref: /schemas/ata/ahci-common.yaml#/$defs/ahci-port
-- 
2.43.0

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

[Andrew Lunn]

On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> after converting the ahci-platform binding to yaml the following files
> reporting "'anyOf' conditional failed" on
> 
> sata@540000: sata-port@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@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@0 {
>  		phys = <&cp0_comphy2 0>;
> +		status = "okay";
>  	};
>  
>  	/* M.2-2250 B-key (J39) */
>  	sata-port@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@540000 {
>  
>  			sata-port@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

Re: [PATCH v1 2/3] arm64: dts: marvell: drop additional phy-names for sata

[Andrew Lunn]

On Sat, Nov 09, 2024 at 10:46:20AM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Commit facbe7092f8a ("arm64: dts: marvell: Drop undocumented SATA phy names")
> drops some phy-names from devicetrees but misses some. Drop them too.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v1 3/3] dt-bindings: ata: ahci-platform: add missing iommus property

[Andrew Lunn]

On Sat, Nov 09, 2024 at 10:46:21AM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> fix dtbs_check errors in following files:
> 
> arch/arm64/boot/dts/marvell/armada-7040-db.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-db.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dtb: sata@540000:
>   Unevaluated properties are not allowed ('iommus' was unexpected)
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Frank Wunderlich]

Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
>On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> after converting the ahci-platform binding to yaml the following files
>> reporting "'anyOf' conditional failed" on
>> 
>> sata@540000: sata-port@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@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@0 {
>>  		phys = <&cp0_comphy2 0>;
>> +		status = "okay";
>>  	};
>>  
>>  	/* M.2-2250 B-key (J39) */
>>  	sata-port@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@540000 {
>>  
>>  			sata-port@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.

As you know, phys that can supply different types (sata,usb,pcie,*gmii,...),but only one mode can be used per phy. So only one controller can be used with it,the other(s) can not. I do not know marvell,but there are similar in mediatek (xsphy) and rockchip (combphy).

From my PoV it makes sense to check only enabled nodes for required properties,but i do not know internals of dtbs_check. This patch is 2 years old and i only rebased it and run dtbs check with the others to have a clean result...i can test again without this one to check if anyOf is shown again.

regards Frank

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

[Frank Wunderlich]

> Gesendet: Sonntag, 10. November 2024 um 10:25
&gt; Von: "Frank Wunderlich" <frank-w@public-files.de>
&gt; Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
&gt; &gt;On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
&gt; &gt;&gt; From: Frank Wunderlich <frank-w@public-files.de>
&gt; &gt;&gt;
&gt; &gt;&gt; after converting the ahci-platform binding to yaml the following files
&gt; &gt;&gt; reporting "'anyOf' conditional failed" on
&gt; &gt;&gt;
&gt; &gt;&gt; sata@540000: sata-port@0
...
&gt; &gt;
&gt; &gt;I don't know the yaml too well, but it is not obvious how adding a few
&gt; &gt;status = "disabled"; status = "okay"; fixes a "'anyOf' conditional failed".
&gt; &gt;
&gt; &gt;Maybe you can expand the explanation a bit?
&gt; &gt;
&gt; &gt;	Andrew
&gt;
&gt; Hi angelo,
&gt;
&gt; I guess the dtbs_check only checks required properties from yaml if the node is enabled.
&gt;
&gt; As you know, phys that can supply different types (sata,usb,pcie,*gmii,...),but only one mode can be used per phy. So only one controller can be used with it,the other(s) can not. I do not know marvell,but there are similar in mediatek (xsphy) and rockchip (combphy).
&gt;
&gt; From my PoV it makes sense to check only enabled nodes for required properties,but i do not know internals of dtbs_check. This patch is 2 years old and i only rebased it and run dtbs check with the others to have a clean result...i can test again without this one to check if anyOf is shown again.
&gt;
&gt; regards Frank

Hi

issue is still there and patch is still needed...without it i get these messages:

$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ata/ahci-platform.yaml
  UPD     include/config/kernel.release
  DTC [C] arch/arm64/boot/dts/marvell/armada-7040-db.dtb
arch/arm64/boot/dts/marvell/armada-7040-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/armada-7040-mochabin.dtb
  DTC [C] arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dtb
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/armada-8040-db.dtb
  DTC [C] arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtb
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dtb
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dtb
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-db.dtb
arch/arm64/boot/dts/marvell/cn9130-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-db-B.dtb
arch/arm64/boot/dts/marvell/cn9130-db-B.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9131-db.dtb
arch/arm64/boot/dts/marvell/cn9131-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
/media/data_ext/git/kernel/BPI-R2-4.14/arch/arm64/boot/dts/marvell/cn9131-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9131-db-B.dtb
arch/arm64/boot/dts/marvell/cn9131-db-B.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/cn9131-db-B.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9132-db.dtb
arch/arm64/boot/dts/marvell/cn9132-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/cn9132-db.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/cn9132-db.dtb: sata@540000: sata-port@1: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9132-db-B.dtb
arch/arm64/boot/dts/marvell/cn9132-db-B.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/cn9132-db-B.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/cn9132-db-B.dtb: sata@540000: sata-port@1: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-crb-A.dtb
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-crb-B.dtb
arch/arm64/boot/dts/marvell/cn9130-crb-B.dtb: sata@540000: sata-port@1: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dtb
arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-cf-base.dtb
arch/arm64/boot/dts/marvell/cn9130-cf-base.dtb: sata@540000: sata-port@1: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9130-cf-pro.dtb
arch/arm64/boot/dts/marvell/cn9130-cf-pro.dtb: sata@540000: sata-port@1: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dtb
arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dtb: sata@540000: sata-port@0: 'anyOf' conditional failed, one must be fixed:
	'phys' is a required property
	'target-supply' is a required property
	from schema $id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
  DTC [C] arch/arm64/boot/dts/marvell/cn9132-clearfog.dtb

that imho confirms my guess that only enabled nodes are checked and without the disabled this node is always enabled and
by disabling the SoC-node and enabling at board-level let the others (here printed) disabled and so not need the required
properties.

i can try to add short description about it, something like this:

The dtbs-check only checks enabled nodes and there required nodes must be present. Nodes are enabled by default (current state for sata@540000 node), but some boards seem to use the phy somewhere else or just not want to use the sata contoller and so miss the required properties 'phys' and 'target-supply'. So disable the sata@540000 node at SoC level and enable it where it is filled with required properties.

maybe adding this phrase to commit is enough?

regards Frank</frank-w@public-files.de></andrew@lunn.ch></frank-w@public-files.de>

Re: [PATCH v1 2/3] arm64: dts: marvell: drop additional phy-names for sata

[Damien Le Moal]

On 11/9/24 18:46, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Commit facbe7092f8a ("arm64: dts: marvell: Drop undocumented SATA phy names")

Isn't a Fixes tag for the above commit desired here ?

> drops some phy-names from devicetrees but misses some. Drop them too.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  arch/arm64/boot/dts/marvell/armada-8040-db.dts     | 2 --
>  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> index fe5d6cb9d692..9d45e881a97d 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> @@ -307,11 +307,9 @@ &cp1_sata0 {
>  
>  	sata-port@0 {
>  		phys = <&cp1_comphy1 0>;
> -		phy-names = "cp1-sata0-0-phy";
>  	};
>  	sata-port@1 {
>  		phys = <&cp1_comphy3 1>;
> -		phy-names = "cp1-sata0-1-phy";
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> index 5043cf2eb33e..0d4a5fd9503f 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> @@ -345,13 +345,11 @@ &cp1_sata0 {
>  	/* CPS Lane 1 - U32 */
>  	sata-port@0 {
>  		phys = <&cp1_comphy1 0>;
> -		phy-names = "cp1-sata0-0-phy";
>  	};
>  
>  	/* CPS Lane 3 - U31 */
>  	sata-port@1 {
>  		phys = <&cp1_comphy3 1>;
> -		phy-names = "cp1-sata0-1-phy";
>  	};
>  };
>  


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1 2/3] arm64: dts: marvell: drop additional phy-names for sata

[Frank Wunderlich]

Hi

Am 11. November 2024 01:05:48 MEZ schrieb Damien Le Moal <dlemoal@kernel.org>:
>On 11/9/24 18:46, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Commit facbe7092f8a ("arm64: dts: marvell: Drop undocumented SATA phy names")
>
>Isn't a Fixes tag for the above commit desired here ?

I have not added one and deopped from first part because of this:

<https://lore.kernel.org/linux-arm-kernel/20220315024952.3we7hiwrkskb4tsy@vireshk-i7/>

>> drops some phy-names from devicetrees but misses some. Drop them too.
>> 
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>


regards Frank

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

[Rob Herring]

On Sun, Nov 10, 2024 at 3:25 AM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
> >On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >>
> >> after converting the ahci-platform binding to yaml the following files
> >> reporting "'anyOf' conditional failed" on
> >>
> >> sata@540000: sata-port@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@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@0 {
> >>              phys = <&cp0_comphy2 0>;
> >> +            status = "okay";
> >>      };
> >>
> >>      /* M.2-2250 B-key (J39) */
> >>      sata-port@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@540000 {
> >>
> >>                      sata-port@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.

Rob

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

[Andrew Lunn]

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@public-files.de> wrote:
> >
> > Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
> > >On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> > >> From: Frank Wunderlich <frank-w@public-files.de>
> > >>
> > >> after converting the ahci-platform binding to yaml the following files
> > >> reporting "'anyOf' conditional failed" on
> > >>
> > >> sata@540000: sata-port@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@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@0 {
> > >>              phys = <&cp0_comphy2 0>;
> > >> +            status = "okay";
> > >>      };
> > >>
> > >>      /* M.2-2250 B-key (J39) */
> > >>      sata-port@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@540000 {
> > >>
> > >>                      sata-port@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@0 always uses phys = <&cp0_comphy2 0> and sata-port@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?

	Andrew

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

[Frank Wunderlich]

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

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

Re: [PATCH v1 3/3] dt-bindings: ata: ahci-platform: add missing iommus property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Sat, Nov 09, 2024 at 10:46:21AM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> fix dtbs_check errors in following files:
> 
> arch/arm64/boot/dts/marvell/armada-7040-db.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-db.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtb: sata@540000:
> arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dtb: sata@540000:
>   Unevaluated properties are not allowed ('iommus' was unexpected)
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Rob Herring]

On Mon, Nov 11, 2024 at 06:15:16PM +0100, Andrew Lunn wrote:
> 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@public-files.de> wrote:
> > >
> > > Am 9. November 2024 18:29:44 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
> > > >On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> > > >> From: Frank Wunderlich <frank-w@public-files.de>
> > > >>
> > > >> after converting the ahci-platform binding to yaml the following files
> > > >> reporting "'anyOf' conditional failed" on
> > > >>
> > > >> sata@540000: sata-port@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@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@0 {
> > > >>              phys = <&cp0_comphy2 0>;
> > > >> +            status = "okay";
> > > >>      };
> > > >>
> > > >>      /* M.2-2250 B-key (J39) */
> > > >>      sata-port@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@540000 {
> > > >>
> > > >>                      sata-port@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@0 always uses phys = <&cp0_comphy2 0> and sata-port@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?

Depends if the phy connection/assignment is really fixed or all boards 
so far just happen to use the same one. If it is fixed and it's just a 
matter of only one user can be active at a time, then yes, moving to the 
SoC dtsi makes sense. The connection in the h/w is there, enabled or 
not. Also, then the board is only dealing with "status" like many of the 
blocks.

Rob

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

[Rob Herring]

On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>

Thanks for reviving these.

> after converting the ahci-platform binding to yaml the following files
> reporting "'anyOf' conditional failed" on

Here and the subject, "fixing anyOf" isn't very specific and is just an 
implementation detail of the schema. "Add missing required 'phys' 
property" would be more exact.

Rob

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

[Frank Wunderlich]

> Gesendet: Montag, 11. November 2024 um 21:36
&gt; Von: "Rob Herring" <robh@kernel.org>
&gt; An: "Frank Wunderlich" <linux@fw-web.de>
&gt; CC: "Damien Le Moal" <dlemoal@kernel.org>, "Niklas Cassel" <cassel@kernel.org>, "Krzysztof Kozlowski" <krzk+dt@kernel.org>, "Conor Dooley" <conor+dt@kernel.org>, "Andrew Lunn" <andrew@lunn.ch>, "Gregory Clement" <gregory.clement@bootlin.com>, "Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>, "Russell King" <linux@armlinux.org.uk>, "Frank Wunderlich" <frank-w@public-files.de>, "Hans de Goede" <hdegoede@redhat.com>, "Jens Axboe" <axboe@kernel.dk>, linux-ide@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
&gt; Betreff: Re: [PATCH v1 1/3] arm64: dts: marvell: Fix anyOf conditional failed
&gt;
&gt; On Sat, Nov 09, 2024 at 10:46:19AM +0100, Frank Wunderlich wrote:
&gt; &gt; From: Frank Wunderlich <frank-w@public-files.de>
&gt;
&gt; Thanks for reviving these.
&gt;
&gt; &gt; after converting the ahci-platform binding to yaml the following files
&gt; &gt; reporting "'anyOf' conditional failed" on
&gt;
&gt; Here and the subject, "fixing anyOf" isn't very specific and is just an
&gt; implementation detail of the schema. "Add missing required 'phys'
&gt; property" would be more exact.

imho it does not match what patch does...i do not add required phys...i just disable the nodes and enable them only where phys is set.

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

Re: (subset) [PATCH v1 0/3] fix some binding check errors for marvell

[Niklas Cassel]

On Sat, 09 Nov 2024 10:46:18 +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Thies series fixes some Errors reported by dtbs_check. First part is taken
> from my old series [1] where i converted sata-platform txt binding to yaml
> because it wasn't picked up.
> 
> [1] https://lore.kernel.org/linux-arm-kernel/20220311210357.222830-1-linux@fw-web.de/
> 
> [...]

Applied to libata/linux.git (for-6.13), thanks!

[3/3] dt-bindings: ata: ahci-platform: add missing iommus property
      https://git.kernel.org/libata/linux/c/e8a2389e


Assuming that patches 1/3 and 2/3 will go via the Marvell tree:
git://git.kernel.org/pub/scm/linux/kernel/git/gclement/mvebu.git


Kind regards,
Niklas

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

[Niklas Cassel]

On Mon, Nov 11, 2024 at 10:25:12AM -0600, Rob Herring wrote:
> > >
> > >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.
> 
> Rob

Hello Rob,

If we look at e.g. this binding:
Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml

We can see that it does not define iommu-map in the binding,
likewise the binding does have:
unevaluatedProperties: false


If I apply my patch that adds iommu-map for e.g. the pcie2x1l0 node:
(the patch does not add anything to the binding above):
https://lore.kernel.org/linux-rockchip/20241107123732.1160063-2-cassel@kernel.org/


If look at the pcie2x1l0 node, it is marked as status = "disabled"
in arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi

but is marked as status = "enabled"
in arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts

If I run CHECK_DTBS for this dts/dtb:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=y rockchip/rk3588-rock-5b.dtb
  DTC [C] arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
$ 

No warnings.

What am I missing?

Considering the warning in this series where the binding also
had unevaluatedProperties: false
I would have expected the same error for the pcie2x1l0 node.

(And if I look at most PCI controler bindings, they actually do define
iommu-map, so it seems a requirement for it to be defined if used.)


Kind regards,
Niklas

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

[Niklas Cassel]

On Tue, Nov 12, 2024 at 01:36:51PM +0100, Niklas Cassel wrote:
> On Mon, Nov 11, 2024 at 10:25:12AM -0600, Rob Herring wrote:
> > > >
> > > >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.
> > 
> > Rob
> 
> Hello Rob,
> 
> If we look at e.g. this binding:
> Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> 
> We can see that it does not define iommu-map in the binding,
> likewise the binding does have:
> unevaluatedProperties: false
> 
> 
> If I apply my patch that adds iommu-map for e.g. the pcie2x1l0 node:
> (the patch does not add anything to the binding above):
> https://lore.kernel.org/linux-rockchip/20241107123732.1160063-2-cassel@kernel.org/
> 
> 
> If look at the pcie2x1l0 node, it is marked as status = "disabled"
> in arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> 
> but is marked as status = "enabled"
> in arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> 
> If I run CHECK_DTBS for this dts/dtb:
> $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=y rockchip/rk3588-rock-5b.dtb
>   DTC [C] arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
> $ 
> 
> No warnings.
> 
> What am I missing?
> 
> Considering the warning in this series where the binding also
> had unevaluatedProperties: false
> I would have expected the same error for the pcie2x1l0 node.
> 
> (And if I look at most PCI controler bindings, they actually do define
> iommu-map, so it seems a requirement for it to be defined if used.)
> 

Or perhaps the question should have been, if iommu-map is an exception,
why isn't iommus also an exception?


Kind regards,
Niklas

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

[Frank Wunderlich]

Hi,

just a gentle ping to have it finally merged

regards Frank

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

[Frank Wunderlich]

is there any new state here? got no answer for my last 2 Messages

https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241109094623.37518-2-linux@fw-web.de/

sorry for the html-entities...they came from my gmx webmailer, it is reported multiple times, but i cannot do more here :(

regards Frank


> Gesendet: Montag, 2. Dezember 2024 um 12:19
> Betreff: Aw: Re: [PATCH v1 1/3] arm64: dts: marvell: Fix anyOf conditional failed
>
> Hi,
>
> just a gentle ping to have it finally merged
>
> regards Frank

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

[Niklas Cassel]

On Thu, Jan 02, 2025 at 06:44:32PM +0000, Frank Wunderlich wrote:
> is there any new state here? got no answer for my last 2 Messages
> 
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241109094623.37518-2-linux@fw-web.de/
> 
> sorry for the html-entities...they came from my gmx webmailer, it is reported multiple times, but i cannot do more here :(
> 

Since this is a marvell change, I expect it to go via marvell / mvebu tree.


Kind regards,
Niklas

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

[Rob Herring]

On Thu, Jan 2, 2025 at 12:44 PM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> is there any new state here? got no answer for my last 2 Messages
>
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241109094623.37518-2-linux@fw-web.de/
>
> sorry for the html-entities...they came from my gmx webmailer, it is reported multiple times, but i cannot do more here :(

As a maintainer, when I see any discussion or comments, I drop the
patch from my queue. That may have happened here. It is best to resend
if it's been more than 2 weeks. Though Marvell maintainer response
times are often longer than that sadly.

You haven't addressed my comment either. The subject needs work. If
you don't like my suggestion, then come up with your own.

Rob

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

[Frank Wunderlich]

Am 2. Januar 2025 20:46:05 MEZ schrieb Rob Herring <robh@kernel.org>:
>On Thu, Jan 2, 2025 at 12:44 PM Frank Wunderlich
><frank-w@public-files.de> wrote:
>>
>> is there any new state here? got no answer for my last 2 Messages
>>
>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241109094623.37518-2-linux@fw-web.de/
>>
>> sorry for the html-entities...they came from my gmx webmailer, it is reported multiple times, but i cannot do more here :(
>
>As a maintainer, when I see any discussion or comments, I drop the
>patch from my queue. That may have happened here. It is best to resend
>if it's been more than 2 weeks. Though Marvell maintainer response
>times are often longer than that sadly.
>
>You haven't addressed my comment either. The subject needs work. If
>you don't like my suggestion, then come up with your own.

I have addressed your comment:

>> Here and the subject, "fixing anyOf" isn't very specific and is just an
>> implementation detail of the schema. "Add missing required 'phys'
>> property" would be more exact.
> imho it does not match what patch does...i do not add required phys...i just disable the nodes and enable them only where phys is set.

But maybe i can name it better.

Maybe something like this:

"Fix binding error caused by incomplete nodes."

>Rob


regards Frank

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

[Gregory CLEMENT]

Hello Frank,

> is there any new state here? got no answer for my last 2 Messages

Actually I waited for a new version following the review and I didn't
saw it. Maybe I missed it.

Grégory

>
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241109094623.37518-2-linux@fw-web.de/
>
> sorry for the html-entities...they came from my gmx webmailer, it is reported multiple times, but i cannot do more here :(
>
> regards Frank
>
>
>> Gesendet: Montag, 2. Dezember 2024 um 12:19
>> Betreff: Aw: Re: [PATCH v1 1/3] arm64: dts: marvell: Fix anyOf conditional failed
>>
>> Hi,
>>
>> just a gentle ping to have it finally merged
>>
>> regards Frank

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Frank Wunderlich]

Am 6. Januar 2025 16:28:57 MEZ schrieb Gregory CLEMENT <gregory.clement@bootlin.com>:
>Hello Frank,
>
>> is there any new state here? got no answer for my last 2 Messages
>
>Actually I waited for a new version following the review and I didn't
>saw it. Maybe I missed it.

No,sent v2 last days as the suggestion from rob was imho not matching what the patch does (i do not add phys property). So i wrote back, but there it hangs.

I hope new title is better.

<https://patchwork.kernel.org/project/linux-arm-kernel/patch/20250103103724.6223-2-linux@fw-web.de/>

Anyone with marvell boards have tested it?

>Grégory


regards Frank