diff mbox series

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

Message ID 20241109094623.37518-2-linux@fw-web.de
State New
Headers show
Series fix some binding check errors for marvell | expand

Commit Message

Frank Wunderlich (linux) Nov. 9, 2024, 9:46 a.m. UTC
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(+)

Comments

Andrew Lunn Nov. 9, 2024, 5:29 p.m. UTC | #1
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
Frank Wunderlich Nov. 10, 2024, 9:25 a.m. UTC | #2
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
Frank Wunderlich Nov. 10, 2024, 10:20 a.m. UTC | #3
&gt; 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>
Rob Herring (Arm) Nov. 11, 2024, 4:25 p.m. UTC | #4
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
Andrew Lunn Nov. 11, 2024, 5:15 p.m. UTC | #5
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
Frank Wunderlich Nov. 11, 2024, 6:50 p.m. UTC | #6
&gt; 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>
Rob Herring (Arm) Nov. 11, 2024, 8:31 p.m. UTC | #7
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
Rob Herring (Arm) Nov. 11, 2024, 8:36 p.m. UTC | #8
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
Frank Wunderlich Nov. 11, 2024, 9:38 p.m. UTC | #9
&gt; 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>
Niklas Cassel Nov. 12, 2024, 12:36 p.m. UTC | #10
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
Niklas Cassel Nov. 12, 2024, 12:46 p.m. UTC | #11
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
Frank Wunderlich Dec. 2, 2024, 11:19 a.m. UTC | #12
Hi,

just a gentle ping to have it finally merged

regards Frank
diff mbox series

Patch

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";
 	};
 };