2022-02-28 00:06:23

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 0/3] Add sata nodes to rk356x

From: Frank Wunderlich <[email protected]>

This Series converts the binding for ahci-platform to yaml and adds
sata nodes to rockchip rk356x device trees.

v3:
- add conversion to sata-series
- fix some errors in dt_binding_check and dtbs_check
- move to unevaluated properties = false
- add power-domain to yaml
- move sata0 to rk3568.dtsi
- drop clock-names and interrupt-names

Frank Wunderlich (3):
dt-bindings: Convert ahci-platform DT bindings to yaml
dt-bindings: Add power-domains property to ahci-platform
arm64: dts: rockchip: Add sata nodes to rk356x

.../devicetree/bindings/ata/ahci-platform.txt | 79 ----------
.../bindings/ata/ahci-platform.yaml | 143 ++++++++++++++++++
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 14 ++
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 26 ++++
4 files changed, 183 insertions(+), 79 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.txt
create mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.yaml

--
2.25.1


2022-02-28 00:53:24

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform

From: Frank Wunderlich <[email protected]>

Some SoC using power-domains property so add it here

Signed-off-by: Frank Wunderlich <[email protected]>
---
changes in v3:
- new patch
---
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 cc246b312c59..cc3710fe4fd4 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -67,6 +67,9 @@ properties:
some embedded SoCs.
minItems: 1

+ power-domains:
+ maxItems: 1
+
resets:
minItems: 1

--
2.25.1

2022-02-28 02:14:52

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml

From: Frank Wunderlich <[email protected]>

Create a yaml file for dtbs_check from the old txt binding.

Signed-off-by: Frank Wunderlich <[email protected]>
---
v3:
- add conversion to sata-series
- fix some errors in dt_binding_check and dtbs_check
- move to unevaluated properties = false

---

imho all errors should be fixed in the dts not in the yaml...

errors about the subitem requirement that was defined in txt but not fixed some marvell dts

some dts for Marvell SoC bring error
'phys' is a required property
'target-supply' is a required property

problem is in arch/arm64/boot/dts/marvell/armada-cp11x.dtsi:331
here the sata-port@0 is defined, but not overridden with phy/target-supply in any following dts

====================================================================

arch/arm64/boot/dts/broadcom/northstar2
ns2-svk.dt.yaml:
ns2-xmc.dt.yaml:
ahci@663f2000:
$nodename:0: 'ahci@663f2000' does not match '^sata(@.*)?$'

Unevaluated properties are not allowed
('reg-names', '#address-cells', '#size-cells' were unexpected)
---
.../devicetree/bindings/ata/ahci-platform.txt | 79 ----------
.../bindings/ata/ahci-platform.yaml | 140 ++++++++++++++++++
2 files changed, 140 insertions(+), 79 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.txt
create mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.yaml

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
deleted file mode 100644
index 77091a277642..000000000000
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ /dev/null
@@ -1,79 +0,0 @@
-* AHCI SATA Controller
-
-SATA nodes are defined to describe on-chip Serial ATA controllers.
-Each SATA controller should have its own node.
-
-It is possible, but not required, to represent each port as a sub-node.
-It allows to enable each port independently when dealing with multiple
-PHYs.
-
-Required properties:
-- compatible : compatible string, one of:
- - "brcm,iproc-ahci"
- - "hisilicon,hisi-ahci"
- - "cavium,octeon-7130-ahci"
- - "ibm,476gtr-ahci"
- - "marvell,armada-380-ahci"
- - "marvell,armada-3700-ahci"
- - "snps,dwc-ahci"
- - "snps,spear-ahci"
- - "generic-ahci"
-- interrupts : <interrupt mapping for SATA IRQ>
-- reg : <registers mapping>
-
-Please note that when using "generic-ahci" you must also specify a SoC specific
-compatible:
- compatible = "manufacturer,soc-model-ahci", "generic-ahci";
-
-Optional properties:
-- dma-coherent : Present if dma operations are coherent
-- clocks : a list of phandle + clock specifier pairs
-- resets : a list of phandle + reset specifier pairs
-- target-supply : regulator for SATA target power
-- phy-supply : regulator for PHY power
-- phys : reference to the SATA PHY node
-- phy-names : must be "sata-phy"
-- ahci-supply : regulator for AHCI controller
-- ports-implemented : Mask that indicates which ports that the HBA supports
- are available for software to use. Useful if PORTS_IMPL
- is not programmed by the BIOS, which is true with
- some embedded SOC's.
-
-Required properties when using sub-nodes:
-- #address-cells : number of cells to encode an address
-- #size-cells : number of cells representing the size of an address
-
-Sub-nodes required properties:
-- reg : the port number
-And at least one of the following properties:
-- phys : reference to the SATA PHY node
-- target-supply : regulator for SATA target power
-
-Examples:
- sata@ffe08000 {
- compatible = "snps,spear-ahci";
- reg = <0xffe08000 0x1000>;
- interrupts = <115>;
- };
-
-With sub-nodes:
- sata@f7e90000 {
- compatible = "marvell,berlin2q-achi", "generic-ahci";
- reg = <0xe90000 0x1000>;
- interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&chip CLKID_SATA>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- sata0: sata-port@0 {
- reg = <0>;
- phys = <&sata_phy 0>;
- target-supply = <&reg_sata0>;
- };
-
- sata1: sata-port@1 {
- reg = <1>;
- phys = <&sata_phy 1>;
- target-supply = <&reg_sata1>;;
- };
- };
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
new file mode 100644
index 000000000000..cc246b312c59
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AHCI SATA Controller
+description:
+ SATA nodes are defined to describe on-chip Serial ATA controllers.
+ Each SATA controller should have its own node.
+
+ It is possible, but not required, to represent each port as a sub-node.
+ It allows to enable each port independently when dealing with multiple
+ PHYs.
+
+maintainers:
+ - Hans de Goede <[email protected]>
+ - Jens Axboe <[email protected]>
+
+properties:
+ compatible:
+ contains:
+ enum:
+ - brcm,iproc-ahci
+ - cavium,octeon-7130-ahci
+ - generic-ahci
+ - hisilicon,hisi-ahci
+ - ibm,476gtr-ahci
+ - marvell,armada-380-ahci
+ - marvell,armada-3700-ahci
+ - snps,dwc-ahci
+ - snps,spear-ahci
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 3
+
+ interrupts:
+ minItems: 1
+
+ ahci-supply:
+ description:
+ regulator for AHCI controller
+
+ dma-coherent:
+ description:
+ Present if dma operations are coherent
+
+ phy-supply:
+ description:
+ regulator for PHY power
+
+ phys:
+ minItems: 1
+
+ phy-names:
+ minItems: 1
+
+ ports-implemented:
+ description:
+ Mask that indicates which ports that the HBA supports
+ are available for software to use. Useful if PORTS_IMPL
+ is not programmed by the BIOS, which is true with
+ some embedded SoCs.
+ minItems: 1
+
+ resets:
+ minItems: 1
+
+ target-supply:
+ description:
+ regulator for SATA target power
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+patternProperties:
+ "^sata-port@[0-9]+$":
+ type: object
+ description:
+ Subnode with configuration of the Ports.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ phys:
+ minItems: 1
+
+ target-supply:
+ description:
+ regulator for SATA target power
+
+ required:
+ - reg
+
+ anyOf:
+ - required: [ phys ]
+ - required: [ target-supply ]
+
+allOf:
+- $ref: "sata-common.yaml#"
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ sata@ffe08000 {
+ compatible = "snps,spear-ahci";
+ reg = <0xffe08000 0x1000>;
+ interrupts = <115>;
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/berlin2q.h>
+ sata@f7e90000 {
+ compatible = "marvell,berlin2q-achi", "generic-ahci";
+ reg = <0xe90000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ target-supply = <&reg_sata0>;
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ target-supply = <&reg_sata1>;
+ };
+ };
--
2.25.1

2022-02-28 10:50:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml

On 27/02/2022 19:27, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> Create a yaml file for dtbs_check from the old txt binding.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> v3:
> - add conversion to sata-series
> - fix some errors in dt_binding_check and dtbs_check
> - move to unevaluated properties = false

You missed devicetree mailing list (corrupted address).

>
> ---
>
> imho all errors should be fixed in the dts not in the yaml...
>
> errors about the subitem requirement that was defined in txt but not fixed some marvell dts
>
> some dts for Marvell SoC bring error
> 'phys' is a required property
> 'target-supply' is a required property
>
> problem is in arch/arm64/boot/dts/marvell/armada-cp11x.dtsi:331
> here the sata-port@0 is defined, but not overridden with phy/target-supply in any following dts
>
> ====================================================================
>
> arch/arm64/boot/dts/broadcom/northstar2
> ns2-svk.dt.yaml:
> ns2-xmc.dt.yaml:
> ahci@663f2000:
> $nodename:0: 'ahci@663f2000' does not match '^sata(@.*)?$'
>
> Unevaluated properties are not allowed
> ('reg-names', '#address-cells', '#size-cells' were unexpected)
> ---
> .../devicetree/bindings/ata/ahci-platform.txt | 79 ----------
> .../bindings/ata/ahci-platform.yaml | 140 ++++++++++++++++++
> 2 files changed, 140 insertions(+), 79 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.txt
> create mode 100644 Documentation/devicetree/bindings/ata/ahci-platform.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> deleted file mode 100644
> index 77091a277642..000000000000
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ /dev/null
> @@ -1,79 +0,0 @@
> -* AHCI SATA Controller
> -
> -SATA nodes are defined to describe on-chip Serial ATA controllers.
> -Each SATA controller should have its own node.
> -
> -It is possible, but not required, to represent each port as a sub-node.
> -It allows to enable each port independently when dealing with multiple
> -PHYs.
> -
> -Required properties:
> -- compatible : compatible string, one of:
> - - "brcm,iproc-ahci"
> - - "hisilicon,hisi-ahci"
> - - "cavium,octeon-7130-ahci"
> - - "ibm,476gtr-ahci"
> - - "marvell,armada-380-ahci"
> - - "marvell,armada-3700-ahci"
> - - "snps,dwc-ahci"
> - - "snps,spear-ahci"
> - - "generic-ahci"
> -- interrupts : <interrupt mapping for SATA IRQ>
> -- reg : <registers mapping>
> -
> -Please note that when using "generic-ahci" you must also specify a SoC specific
> -compatible:
> - compatible = "manufacturer,soc-model-ahci", "generic-ahci";
> -
> -Optional properties:
> -- dma-coherent : Present if dma operations are coherent
> -- clocks : a list of phandle + clock specifier pairs
> -- resets : a list of phandle + reset specifier pairs
> -- target-supply : regulator for SATA target power
> -- phy-supply : regulator for PHY power
> -- phys : reference to the SATA PHY node
> -- phy-names : must be "sata-phy"
> -- ahci-supply : regulator for AHCI controller
> -- ports-implemented : Mask that indicates which ports that the HBA supports
> - are available for software to use. Useful if PORTS_IMPL
> - is not programmed by the BIOS, which is true with
> - some embedded SOC's.
> -
> -Required properties when using sub-nodes:
> -- #address-cells : number of cells to encode an address
> -- #size-cells : number of cells representing the size of an address
> -
> -Sub-nodes required properties:
> -- reg : the port number
> -And at least one of the following properties:
> -- phys : reference to the SATA PHY node
> -- target-supply : regulator for SATA target power
> -
> -Examples:
> - sata@ffe08000 {
> - compatible = "snps,spear-ahci";
> - reg = <0xffe08000 0x1000>;
> - interrupts = <115>;
> - };
> -
> -With sub-nodes:
> - sata@f7e90000 {
> - compatible = "marvell,berlin2q-achi", "generic-ahci";
> - reg = <0xe90000 0x1000>;
> - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&chip CLKID_SATA>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - sata0: sata-port@0 {
> - reg = <0>;
> - phys = <&sata_phy 0>;
> - target-supply = <&reg_sata0>;
> - };
> -
> - sata1: sata-port@1 {
> - reg = <1>;
> - phys = <&sata_phy 1>;
> - target-supply = <&reg_sata1>;;
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> new file mode 100644
> index 000000000000..cc246b312c59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/ahci-platform.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AHCI SATA Controller
> +description:
> + SATA nodes are defined to describe on-chip Serial ATA controllers.
> + Each SATA controller should have its own node.
> +
> + It is possible, but not required, to represent each port as a sub-node.
> + It allows to enable each port independently when dealing with multiple
> + PHYs.
> +
> +maintainers:
> + - Hans de Goede <[email protected]>
> + - Jens Axboe <[email protected]>
> +
> +properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,iproc-ahci
> + - cavium,octeon-7130-ahci
> + - generic-ahci
> + - hisilicon,hisi-ahci
> + - ibm,476gtr-ahci
> + - marvell,armada-380-ahci
> + - marvell,armada-3700-ahci

Order entries alphabetically.

> + - snps,dwc-ahci
> + - snps,spear-ahci

You converted the TXT bindings explicitly, but you missed the comment
just below the 'reg' about generic-ahci. The generic-ahci never comes alone.

The snps,dwc-ahci could come, although history shows that Synapsys
blocks are commonly re-used and they should have specific compatible.
Current users still have single snps,dwc-ahci, so it could be fixed a
bit later.

On the other hand, I expect to fix all the DTS in the same series where
the bindings are corrected.

> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 3

Items should be described. Next or this patch could add also clock-names.

> +
> + interrupts:
> + minItems: 1

You mean maxItems?

> +
> + ahci-supply:
> + description:
> + regulator for AHCI controller
> +
> + dma-coherent:
> + description:
> + Present if dma operations are coherent

Skip description, it's obvious. Just 'true'.

> +
> + phy-supply:
> + description:
> + regulator for PHY power
> +
> + phys:
> + minItems: 1

maxItems?
> +
> + phy-names:
> + minItems: 1

Describe the items.

> +
> + ports-implemented:
> + description:
> + Mask that indicates which ports that the HBA supports
> + are available for software to use. Useful if PORTS_IMPL
> + is not programmed by the BIOS, which is true with
> + some embedded SoCs.
> + minItems: 1

You need a type and maxItems.

> +
> + resets:
> + minItems: 1

maxItems?

> +
> + target-supply:
> + description:
> + regulator for SATA target power
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^sata-port@[0-9]+$":

You limit number of ports to 10. On purpose? What about 0xa? 0xb?

> + type: object
> + description:
> + Subnode with configuration of the Ports.
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + phys:
> + minItems: 1

maxItems? Why do you put everywhere minItems? Are several phys really
expected?

> +
> + target-supply:
> + description:
> + regulator for SATA target power
> +
> + required:
> + - reg
> +
> + anyOf:
> + - required: [ phys ]
> + - required: [ target-supply ]
> +
> +allOf:
> +- $ref: "sata-common.yaml#"

This goes before properties.

> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + sata@ffe08000 {

Wrong indentation. It starts just below |

> + compatible = "snps,spear-ahci";
> + reg = <0xffe08000 0x1000>;
> + interrupts = <115>;
> + };
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/berlin2q.h>
> + sata@f7e90000 {
> + compatible = "marvell,berlin2q-achi", "generic-ahci";

This clearly won't pass your checks. I don't think you run
dt_binding_check. You must test your bindings first.

> + reg = <0xe90000 0x1000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&chip CLKID_SATA>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata0: sata-port@0 {
> + reg = <0>;
> + phys = <&sata_phy 0>;
> + target-supply = <&reg_sata0>;
> + };
> +
> + sata1: sata-port@1 {
> + reg = <1>;
> + phys = <&sata_phy 1>;
> + target-supply = <&reg_sata1>;
> + };
> + };


Best regards,
Krzysztof

2022-02-28 10:55:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform

On 27/02/2022 19:27, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> Some SoC using power-domains property so add it here

Full stop.

Subject: "dt-bindings: ata: ahci-platform: Add power-domains property"

>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> changes in v3:
> - new patch
> ---
> 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 cc246b312c59..cc3710fe4fd4 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> @@ -67,6 +67,9 @@ properties:
> some embedded SoCs.
> minItems: 1
>
> + power-domains:
> + maxItems: 1
> +
> resets:
> minItems: 1
>


Best regards,
Krzysztof

2022-02-28 17:26:46

by Frank Wunderlich

[permalink] [raw]
Subject: Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml

Hi Krzysztof,

thanks for first review.

> Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr
> Von: "Krzysztof Kozlowski" <[email protected]>

> On 27/02/2022 19:27, Frank Wunderlich wrote:
> > From: Frank Wunderlich <[email protected]>

> You missed devicetree mailing list (corrupted address).

sorry, devicetree ML was To, but forget the Cc-Header (prepared addresses in coverletter)

> > imho all errors should be fixed in the dts not in the yaml...

> > -- reg : <registers mapping>
> > -
> > -Please note that when using "generic-ahci" you must also specify a SoC specific
> > -compatible:
> > - compatible = "manufacturer,soc-model-ahci", "generic-ahci";
...
> > +properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - brcm,iproc-ahci
> > + - cavium,octeon-7130-ahci
> > + - generic-ahci
> > + - hisilicon,hisi-ahci
> > + - ibm,476gtr-ahci
> > + - marvell,armada-380-ahci
> > + - marvell,armada-3700-ahci
>
> Order entries alphabetically.

ok

> > + - snps,dwc-ahci
> > + - snps,spear-ahci
>
> You converted the TXT bindings explicitly, but you missed the comment
> just below the 'reg' about generic-ahci. The generic-ahci never comes alone.

How should this comment be added? description above/below the compatible-property?
Sorry for dumb questions...this is my first yaml ;)

e.g.

properties:
compatible:
description:
Please note that when using "generic-ahci" you must also specify a SoC specific
compatible:
compatible = "manufacturer,soc-model-ahci", "generic-ahci";
contains:
enum:

> The snps,dwc-ahci could come, although history shows that Synapsys
> blocks are commonly re-used and they should have specific compatible.
> Current users still have single snps,dwc-ahci, so it could be fixed a
> bit later.
>
> On the other hand, I expect to fix all the DTS in the same series where
> the bindings are corrected.

i don't know the marvell/broadcom-hw so i cannot fix them.
Just converted the txt to check my rockchip sata nodes and to be more
future-proof (no more exceptions like the marvell/broadcom)

> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 3
>
> Items should be described. Next or this patch could add also clock-names.

i was told to drop clock-names (same for interrupt-names and reset-names) from dts
and in txt it was not there so have not added it

https://patchwork.kernel.org/comment/24755956/

> > +
> > + interrupts:
> > + minItems: 1
>
> You mean maxItems?

no, minItems, as interrupts suggests 1+ (same for phys)

> > +
> > + ahci-supply:
> > + description:
> > + regulator for AHCI controller
> > +
> > + dma-coherent:
> > + description:
> > + Present if dma operations are coherent
>
> Skip description, it's obvious. Just 'true'.

ok, took this from the txt

> > +
> > + phy-supply:
> > + description:
> > + regulator for PHY power
> > +
> > + phys:
> > + minItems: 1
>
> maxItems?
> > +
> > + phy-names:
> > + minItems: 1
>
> Describe the items.
>
> > +
> > + ports-implemented:
> > + description:
> > + Mask that indicates which ports that the HBA supports
> > + are available for software to use. Useful if PORTS_IMPL
> > + is not programmed by the BIOS, which is true with
> > + some embedded SoCs.
> > + minItems: 1
>
> You need a type and maxItems.

what will be the type of a mask?

> > +
> > + resets:
> > + minItems: 1
>
> maxItems?

if there is a known maximum....

> > +
> > + target-supply:
> > + description:
> > + regulator for SATA target power
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +patternProperties:
> > + "^sata-port@[0-9]+$":
>
> You limit number of ports to 10. On purpose? What about 0xa? 0xb?

oh, right, there can be hexadecimal...
thought this is only true for the main-node (address) and have only seen @0, @1 and @2

> > + type: object
> > + description:
> > + Subnode with configuration of the Ports.
> > +
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + phys:
> > + minItems: 1
>
> maxItems? Why do you put everywhere minItems? Are several phys really
> expected?

name suggests that it can be more than 1. i know from usb subsystem (dwc3 usb3) that a device can have more than one phy, and because in the txt there are no ranges i set everywhere MinItems to 1 with open end as i do not know all possibilities. Anything else will be trial and error...for all properties

> > +
> > + target-supply:
> > + description:
> > + regulator for SATA target power
> > +
> > + required:
> > + - reg
> > +
> > + anyOf:
> > + - required: [ phys ]
> > + - required: [ target-supply ]
> > +
> > +allOf:
> > +- $ref: "sata-common.yaml#"
>
> This goes before properties.
>
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + sata@ffe08000 {
>
> Wrong indentation. It starts just below |

will fix it

> > + compatible = "snps,spear-ahci";
> > + reg = <0xffe08000 0x1000>;
> > + interrupts = <115>;
> > + };
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/berlin2q.h>
> > + sata@f7e90000 {
> > + compatible = "marvell,berlin2q-achi", "generic-ahci";
>
> This clearly won't pass your checks. I don't think you run
> dt_binding_check. You must test your bindings first.

i had them tested ...needed to add the includes...after that the dt_bindings_check was without errors/warnings

these are the commands i used:

ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ata/ahci-platform.yaml
ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ata/ahci-platform.yaml

> Best regards,
> Krzysztof

regards Frank