* [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-01 15:24 [PATCH v4 0/5] Add sata nodes to rk356x Frank Wunderlich
@ 2022-03-01 15:24 ` Frank Wunderlich
2022-03-02 9:46 ` Krzysztof Kozlowski
2022-03-02 18:14 ` Rob Herring
2022-03-01 15:24 ` [PATCH v4 2/5] arm64: dts: broadcom: Fix sata nodename Frank Wunderlich
` (3 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-01 15:24 UTC (permalink / raw)
To: devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
From: Frank Wunderlich <frank-w@public-files.de>
Create a yaml file for dtbs_check from the old txt binding.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v4:
- fix min vs. max
- fix indention of examples
- move up sata-common.yaml
- reorder compatible
- add descriptions/maxitems
- fix compatible-structure
- fix typo in example achi vs. ahci
- add clock-names and reg-names
- fix ns2 errors in separate patch
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
---
.../devicetree/bindings/ata/ahci-platform.txt | 79 ---------
.../bindings/ata/ahci-platform.yaml | 162 ++++++++++++++++++
2 files changed, 162 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 = <®_sata0>;
- };
-
- sata1: sata-port@1 {
- reg = <1>;
- phys = <&sata_phy 1>;
- target-supply = <®_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..cf67ddfc6afb
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -0,0 +1,162 @@
+# 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 <hdegoede@redhat.com>
+ - Jens Axboe <axboe@kernel.dk>
+
+allOf:
+- $ref: "sata-common.yaml#"
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - brcm,iproc-ahci
+ - marvell,armada-8k-ahci
+ - marvell,berlin2q-ahci
+ - const: generic-ahci
+ - enum:
+ - brcm,iproc-ahci
+ - cavium,octeon-7130-ahci
+ - hisilicon,hisi-ahci
+ - ibm,476gtr-ahci
+ - marvell,armada-3700-ahci
+ - marvell,armada-380-ahci
+ - snps,dwc-ahci
+ - snps,spear-ahci
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ description:
+ Clock IDs array as required by the controller.
+ minItems: 1
+ maxItems: 3
+
+ interrupts:
+ description:
+ specifies the interrupt number for the controller.
+ maxItems: 1
+
+ ahci-supply:
+ description:
+ regulator for AHCI controller
+
+ clock-names:
+ description:
+ Names of clocks corresponding to IDs in the clock property.
+ minItems: 1
+ maxItems: 3
+
+ dma-coherent:
+ true
+
+ phy-supply:
+ description:
+ regulator for PHY power
+
+ phys:
+ description:
+ List of all PHYs on this controller
+ maxItems: 1
+
+ phy-names:
+ description:
+ Name specifier for the PHYs
+ maxItems: 1
+
+ ports-implemented:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ 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.
+ maxItems: 1
+
+ reg-names:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ target-supply:
+ description:
+ regulator for SATA target power
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+patternProperties:
+ "^sata-port@[0-9a-f]+$":
+ 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 ]
+
+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-ahci", "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 = <®_sata0>;
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ target-supply = <®_sata1>;
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-01 15:24 ` [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml Frank Wunderlich
@ 2022-03-02 9:46 ` Krzysztof Kozlowski
2022-03-02 10:31 ` Aw: " Frank Wunderlich
2022-03-02 18:14 ` Rob Herring
1 sibling, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 9:46 UTC (permalink / raw)
To: Frank Wunderlich, devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring, Hans de Goede,
Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis,
Michael Riesch, linux-arm-kernel, linux-rockchip, Ray Jui,
Scott Branden, bcm-kernel-feedback-list
On 01/03/2022 16:24, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Create a yaml file for dtbs_check from the old txt binding.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v4:
> - fix min vs. max
> - fix indention of examples
> - move up sata-common.yaml
> - reorder compatible
> - add descriptions/maxitems
> - fix compatible-structure
> - fix typo in example achi vs. ahci
> - add clock-names and reg-names
> - fix ns2 errors in separate patch
> 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
> ---
> .../devicetree/bindings/ata/ahci-platform.txt | 79 ---------
> .../bindings/ata/ahci-platform.yaml | 162 ++++++++++++++++++
> 2 files changed, 162 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 = <®_sata0>;
> - };
> -
> - sata1: sata-port@1 {
> - reg = <1>;
> - phys = <&sata_phy 1>;
> - target-supply = <®_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..cf67ddfc6afb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> @@ -0,0 +1,162 @@
> +# 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 <hdegoede@redhat.com>
> + - Jens Axboe <axboe@kernel.dk>
> +
> +allOf:
> +- $ref: "sata-common.yaml#"
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - brcm,iproc-ahci
> + - marvell,armada-8k-ahci
> + - marvell,berlin2q-ahci
> + - const: generic-ahci
> + - enum:
> + - brcm,iproc-ahci
This one is already earlier in generic-ahci, so you can skip him here.
> + - cavium,octeon-7130-ahci
> + - hisilicon,hisi-ahci
> + - ibm,476gtr-ahci
> + - marvell,armada-3700-ahci
> + - marvell,armada-380-ahci
> + - snps,dwc-ahci
> + - snps,spear-ahci
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description:
> + Clock IDs array as required by the controller.
> + minItems: 1
> + maxItems: 3
> +
> + interrupts:
> + description:
> + specifies the interrupt number for the controller.
Skip description, it's obvious.
> + maxItems: 1
> +
> + ahci-supply:
> + description:
> + regulator for AHCI controller
> +
> + clock-names:
> + description:
> + Names of clocks corresponding to IDs in the clock property.
> + minItems: 1
> + maxItems: 3
Put the clock-names next to clocks.
> +
> + dma-coherent:
> + true
New line not needed. "dma-coherent: true"
> +
> + phy-supply:
> + description:
> + regulator for PHY power
> +
> + phys:
> + description:
> + List of all PHYs on this controller
> + maxItems: 1
> +
> + phy-names:
> + description:
> + Name specifier for the PHYs
> + maxItems: 1
> +
> + ports-implemented:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + 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.
> + maxItems: 1
maxItems are incorrect here, this is not an array.
> +
> + reg-names:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + target-supply:
> + description:
> + regulator for SATA target power
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^sata-port@[0-9a-f]+$":
> + type: object
> + description:
> + Subnode with configuration of the Ports.
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + phys:
> + minItems: 1
maxItems: 1
> +
> + target-supply:
> + description:
> + regulator for SATA target power
> +
> + required:
> + - reg
> +
> + anyOf:
> + - required: [ phys ]
> + - required: [ target-supply ]
> +
> +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-ahci", "generic-ahci";
> + reg = <0xe90000 0x1000>;
You still have wrong address.
> + 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 = <®_sata0>;
> + };
> +
> + sata1: sata-port@1 {
> + reg = <1>;
> + phys = <&sata_phy 1>;
> + target-supply = <®_sata1>;
> + };
> + };
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread* Aw: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-02 9:46 ` Krzysztof Kozlowski
@ 2022-03-02 10:31 ` Frank Wunderlich
2022-03-02 10:59 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-02 10:31 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Frank Wunderlich, devicetree, Damien Le Moal, Rob Herring,
Hans de Goede, Jens Axboe, linux-ide, linux-kernel,
Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel,
linux-rockchip, Ray Jui, Scott Branden, bcm-kernel-feedback-list
Hi
> Gesendet: Mittwoch, 02. März 2022 um 10:46 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com>
> On 01/03/2022 16:24, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - brcm,iproc-ahci
> > + - marvell,armada-8k-ahci
> > + - marvell,berlin2q-ahci
> > + - const: generic-ahci
> > + - enum:
> > + - brcm,iproc-ahci
>
> This one is already earlier in generic-ahci, so you can skip him here.
OK, i drop it
> > + - cavium,octeon-7130-ahci
> > + - hisilicon,hisi-ahci
> > + - ibm,476gtr-ahci
> > + - marvell,armada-3700-ahci
> > + - marvell,armada-380-ahci
> > + - snps,dwc-ahci
> > + - snps,spear-ahci
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + description:
> > + Clock IDs array as required by the controller.
> > + minItems: 1
> > + maxItems: 3
> > +
> > + interrupts:
> > + description:
> > + specifies the interrupt number for the controller.
>
> Skip description, it's obvious.
OK
> > + maxItems: 1
> > +
> > + ahci-supply:
> > + description:
> > + regulator for AHCI controller
> > +
> > + clock-names:
> > + description:
> > + Names of clocks corresponding to IDs in the clock property.
> > + minItems: 1
> > + maxItems: 3
>
> Put the clock-names next to clocks.
had moved them down as they are optional, but ok, i move up
> > +
> > + dma-coherent:
> > + true
>
> New line not needed. "dma-coherent: true"
OK
> > + ports-implemented:
> > + $ref: '/schemas/types.yaml#/definitions/uint32'
> > + 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.
> > + maxItems: 1
>
> maxItems are incorrect here, this is not an array.
right, i'll change it
> > +patternProperties:
> > + "^sata-port@[0-9a-f]+$":
> > + type: object
> > + description:
> > + Subnode with configuration of the Ports.
> > +
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + phys:
> > + minItems: 1
>
> maxItems: 1
oh, missed that
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/berlin2q.h>
> > + sata@f7e90000 {
> > + compatible = "marvell,berlin2q-ahci", "generic-ahci";
> > + reg = <0xe90000 0x1000>;
>
> You still have wrong address.
only took the example from the txt, but i guess reg should be <0xf7e90000 0x1000>; ?
will also fix all subjects to
"dt-bindings: ata: ahci-platform: ..."
Thanks
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: Aw: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-02 10:31 ` Aw: " Frank Wunderlich
@ 2022-03-02 10:59 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 10:59 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, devicetree, Damien Le Moal, Rob Herring,
Hans de Goede, Jens Axboe, linux-ide, linux-kernel,
Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel,
linux-rockchip, Ray Jui, Scott Branden, bcm-kernel-feedback-list
On 02/03/2022 11:31, Frank Wunderlich wrote:
>
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/clock/berlin2q.h>
>>> + sata@f7e90000 {
>>> + compatible = "marvell,berlin2q-ahci", "generic-ahci";
>>> + reg = <0xe90000 0x1000>;
>>
>> You still have wrong address.
>
> only took the example from the txt, but i guess reg should be <0xf7e90000 0x1000>; ?
>
Yes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-01 15:24 ` [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml Frank Wunderlich
2022-03-02 9:46 ` Krzysztof Kozlowski
@ 2022-03-02 18:14 ` Rob Herring
2022-03-03 7:04 ` Aw: " Frank Wunderlich
1 sibling, 1 reply; 17+ messages in thread
From: Rob Herring @ 2022-03-02 18:14 UTC (permalink / raw)
To: Frank Wunderlich
Cc: devicetree, Frank Wunderlich, Damien Le Moal,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
On Tue, Mar 01, 2022 at 04:24:17PM +0100, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Create a yaml file for dtbs_check from the old txt binding.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v4:
> - fix min vs. max
> - fix indention of examples
> - move up sata-common.yaml
> - reorder compatible
> - add descriptions/maxitems
> - fix compatible-structure
> - fix typo in example achi vs. ahci
> - add clock-names and reg-names
> - fix ns2 errors in separate patch
> 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
> ---
> .../devicetree/bindings/ata/ahci-platform.txt | 79 ---------
> .../bindings/ata/ahci-platform.yaml | 162 ++++++++++++++++++
> 2 files changed, 162 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 = <®_sata0>;
> - };
> -
> - sata1: sata-port@1 {
> - reg = <1>;
> - phys = <&sata_phy 1>;
> - target-supply = <®_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..cf67ddfc6afb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> @@ -0,0 +1,162 @@
> +# 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
blank line.
> +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.
You need a '|' after 'description' if you want to maintain the
paragraphs.
> +
> +maintainers:
> + - Hans de Goede <hdegoede@redhat.com>
> + - Jens Axboe <axboe@kernel.dk>
> +
> +allOf:
> +- $ref: "sata-common.yaml#"
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - brcm,iproc-ahci
> + - marvell,armada-8k-ahci
> + - marvell,berlin2q-ahci
> + - const: generic-ahci
> + - enum:
> + - brcm,iproc-ahci
> + - cavium,octeon-7130-ahci
> + - hisilicon,hisi-ahci
> + - ibm,476gtr-ahci
> + - marvell,armada-3700-ahci
> + - marvell,armada-380-ahci
> + - snps,dwc-ahci
> + - snps,spear-ahci
Install yamllint and run 'make dt_binding_check'. It's going to
complain about the indentation.
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description:
> + Clock IDs array as required by the controller.
> + minItems: 1
> + maxItems: 3
> +
> + interrupts:
> + description:
> + specifies the interrupt number for the controller.
> + maxItems: 1
> +
> + ahci-supply:
> + description:
> + regulator for AHCI controller
> +
> + clock-names:
Group with 'clocks'
> + description:
> + Names of clocks corresponding to IDs in the clock property.
> + minItems: 1
> + maxItems: 3
> +
> + dma-coherent:
> + true
> +
> + phy-supply:
> + description:
> + regulator for PHY power
> +
> + phys:
> + description:
> + List of all PHYs on this controller
> + maxItems: 1
> +
> + phy-names:
> + description:
> + Name specifier for the PHYs
> + maxItems: 1
> +
> + ports-implemented:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + 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.
> + maxItems: 1
A uint32 is only ever 1 item. Drop.
IIRC, isn't the max here 0xff? Add constraints.
> +
> + reg-names:
> + maxItems: 1
Group with 'reg'.
> +
> + resets:
> + maxItems: 1
> +
> + target-supply:
> + description:
> + regulator for SATA target power
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^sata-port@[0-9a-f]+$":
> + type: object
additionalProperties: false
> + 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 ]
> +
> +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-ahci", "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 = <®_sata0>;
> + };
> +
> + sata1: sata-port@1 {
> + reg = <1>;
> + phys = <&sata_phy 1>;
> + target-supply = <®_sata1>;
> + };
> + };
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread* Aw: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-02 18:14 ` Rob Herring
@ 2022-03-03 7:04 ` Frank Wunderlich
2022-03-04 23:37 ` Rob Herring
0 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-03 7:04 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Wunderlich, devicetree, Damien Le Moal,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
Hi Rob,
thanks for review,
have prepared the changes based on yours and krzysztof comments
https://github.com/frank-w/BPI-R2-4.14/commits/5.17-next-20220225
(just ignore the top 2 commits) i thought i had a size-cells-error, but did not get them again after reverting this part, seems they are fixed by inclusion of the sata-common binding
> Gesendet: Mittwoch, 02. März 2022 um 19:14 Uhr
> Von: "Rob Herring" <robh@kernel.org>
> An: "Frank Wunderlich" <linux@fw-web.de>
> On Tue, Mar 01, 2022 at 04:24:17PM +0100, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> > new file mode 100644
> > index 000000000000..cf67ddfc6afb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> > @@ -0,0 +1,162 @@
> > +# 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
>
> blank line.
done
> > +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.
>
> You need a '|' after 'description' if you want to maintain the
> paragraphs.
ok added | to all multiline descriptions
> > +
> > +maintainers:
> > + - Hans de Goede <hdegoede@redhat.com>
> > + - Jens Axboe <axboe@kernel.dk>
> > +
> > +allOf:
> > +- $ref: "sata-common.yaml#"
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - brcm,iproc-ahci
> > + - marvell,armada-8k-ahci
> > + - marvell,berlin2q-ahci
> > + - const: generic-ahci
> > + - enum:
> > + - brcm,iproc-ahci
> > + - cavium,octeon-7130-ahci
> > + - hisilicon,hisi-ahci
> > + - ibm,476gtr-ahci
> > + - marvell,armada-3700-ahci
> > + - marvell,armada-380-ahci
> > + - snps,dwc-ahci
> > + - snps,spear-ahci
>
> Install yamllint and run 'make dt_binding_check'. It's going to
> complain about the indentation.
you're right, i had no yamllint installed, so i have not seen these indention errors
> > + ahci-supply:
> > + description:
> > + regulator for AHCI controller
> > +
> > + clock-names:
>
> Group with 'clocks'
ok, already done in my tree because of krzysztofs comment
> > + ports-implemented:
> > + $ref: '/schemas/types.yaml#/definitions/uint32'
> > + 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.
> > + maxItems: 1
>
> A uint32 is only ever 1 item. Drop.
>
> IIRC, isn't the max here 0xff? Add constraints.
i've found it only set to 0x1 so i have currently set the maximum to 0x1, is this ok?
If some higher value is needed binding needs to be touched...
> > +
> > + reg-names:
> > + maxItems: 1
>
> Group with 'reg'.
ok
> > +patternProperties:
> > + "^sata-port@[0-9a-f]+$":
> > + type: object
>
> additionalProperties: false
ok added to my tree
and needed to add phy-names because some marvell boards using this
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dt.yaml: sata@540000: sata-port@1: 'phy-names' does not match any of the regexes: 'pinctrl-[0-9]+'
now i have only the marvell-errors about incomplete sata-port subnode (without phy/target-supply) like i mention in the patch...how to proceed with this?
regards Frank
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-03 7:04 ` Aw: " Frank Wunderlich
@ 2022-03-04 23:37 ` Rob Herring
2022-03-05 10:07 ` Aw: " Frank Wunderlich
0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2022-03-04 23:37 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, devicetree, Damien Le Moal,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, open list:ARM/Rockchip SoC...,
Ray Jui, Scott Branden,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE
On Thu, Mar 3, 2022 at 1:04 AM Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Hi Rob,
>
> thanks for review,
>
> have prepared the changes based on yours and krzysztof comments
>
> https://github.com/frank-w/BPI-R2-4.14/commits/5.17-next-20220225
>
> (just ignore the top 2 commits) i thought i had a size-cells-error, but did not get them again after reverting this part, seems they are fixed by inclusion of the sata-common binding
>
> > Gesendet: Mittwoch, 02. März 2022 um 19:14 Uhr
> > Von: "Rob Herring" <robh@kernel.org>
> > An: "Frank Wunderlich" <linux@fw-web.de>
>
> > On Tue, Mar 01, 2022 at 04:24:17PM +0100, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
>
> > > diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> > > new file mode 100644
> > > index 000000000000..cf67ddfc6afb
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
> > > @@ -0,0 +1,162 @@
> > > +# 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
> >
> > blank line.
>
> done
>
> > > +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.
> >
> > You need a '|' after 'description' if you want to maintain the
> > paragraphs.
>
> ok added | to all multiline descriptions
>
> > > +
> > > +maintainers:
> > > + - Hans de Goede <hdegoede@redhat.com>
> > > + - Jens Axboe <axboe@kernel.dk>
> > > +
> > > +allOf:
> > > +- $ref: "sata-common.yaml#"
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - brcm,iproc-ahci
> > > + - marvell,armada-8k-ahci
> > > + - marvell,berlin2q-ahci
> > > + - const: generic-ahci
> > > + - enum:
> > > + - brcm,iproc-ahci
> > > + - cavium,octeon-7130-ahci
> > > + - hisilicon,hisi-ahci
> > > + - ibm,476gtr-ahci
> > > + - marvell,armada-3700-ahci
> > > + - marvell,armada-380-ahci
> > > + - snps,dwc-ahci
> > > + - snps,spear-ahci
> >
> > Install yamllint and run 'make dt_binding_check'. It's going to
> > complain about the indentation.
>
> you're right, i had no yamllint installed, so i have not seen these indention errors
>
> > > + ahci-supply:
> > > + description:
> > > + regulator for AHCI controller
> > > +
> > > + clock-names:
> >
> > Group with 'clocks'
>
> ok, already done in my tree because of krzysztofs comment
>
> > > + ports-implemented:
> > > + $ref: '/schemas/types.yaml#/definitions/uint32'
> > > + 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.
> > > + maxItems: 1
> >
> > A uint32 is only ever 1 item. Drop.
> >
> > IIRC, isn't the max here 0xff? Add constraints.
>
> i've found it only set to 0x1 so i have currently set the maximum to 0x1, is this ok?
> If some higher value is needed binding needs to be touched...
There's a spec for it, so no need to look at what's used. Calxeda AHCI
had 5 ports IIRC.
> > > + reg-names:
> > > + maxItems: 1
> >
> > Group with 'reg'.
>
> ok
>
> > > +patternProperties:
> > > + "^sata-port@[0-9a-f]+$":
> > > + type: object
> >
> > additionalProperties: false
>
> ok added to my tree
>
> and needed to add phy-names because some marvell boards using this
>
> arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dt.yaml: sata@540000: sata-port@1: 'phy-names' does not match any of the regexes: 'pinctrl-[0-9]+'
>
> now i have only the marvell-errors about incomplete sata-port subnode (without phy/target-supply) like i mention in the patch...how to proceed with this?
So the child nodes are incomplete? They should be disabled then
(status = "disabled") and that turns off required properties checks.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* Aw: Re: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-04 23:37 ` Rob Herring
@ 2022-03-05 10:07 ` Frank Wunderlich
2022-03-05 10:19 ` Frank Wunderlich
0 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-05 10:07 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Wunderlich, devicetree, Damien Le Moal,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, open list:ARM/Rockchip SoC...,
Ray Jui, Scott Branden,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE
Hi
> Gesendet: Samstag, 05. März 2022 um 00:37 Uhr
> Von: "Rob Herring" <robh@kernel.org>
> > > > + ports-implemented:
> > > > + $ref: '/schemas/types.yaml#/definitions/uint32'
> > > > + 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.
> > > > + maxItems: 1
> > >
> > > A uint32 is only ever 1 item. Drop.
> > >
> > > IIRC, isn't the max here 0xff? Add constraints.
> >
> > i've found it only set to 0x1 so i have currently set the maximum to 0x1, is this ok?
> > If some higher value is needed binding needs to be touched...
>
> There's a spec for it, so no need to look at what's used. Calxeda AHCI
> had 5 ports IIRC.
as far as i understand code in libahci.c line 535+ [1]
i guess i need to set lower 5 bits to 1 for 5 ports, right? resulting in max value 0x1f
> > now i have only the marvell-errors about incomplete sata-port subnode (without phy/target-supply) like i mention in the patch...how to proceed with this?
>
> So the child nodes are incomplete? They should be disabled then
> (status = "disabled") and that turns off required properties checks.
thanks, have disable the nodes and reenable them where phys/target-supply was added [2]...
now the dtbs_check is clean
[1] https://elixir.bootlin.com/linux/v5.17-rc6/source/drivers/ata/libahci.c#L535
[2] https://github.com/frank-w/BPI-R2-4.14/commits/5.17-next-20220225
regards Frank
^ permalink raw reply [flat|nested] 17+ messages in thread
* Aw: Re: Re: [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml
2022-03-05 10:07 ` Aw: " Frank Wunderlich
@ 2022-03-05 10:19 ` Frank Wunderlich
0 siblings, 0 replies; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-05 10:19 UTC (permalink / raw)
To: Frank Wunderlich, Rob Herring
Cc: Rob Herring, Frank Wunderlich, devicetree, Damien Le Moal,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, open list:ARM/Rockchip SoC...,
Ray Jui, Scott Branden,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE
Take a look again of the mentioned calxeda...
it looks like it only uses the compatible "calxeda,hb-ahci" handled by
drivers/ata/sata_highbank.c and seems not using the ahci-platform.c
obj-$(CONFIG_SATA_HIGHBANK) += sata_highbank.o libahci.o
so imho the maximum 0x1 still should be right
regards Frank
> > Gesendet: Samstag, 05. März 2022 um 00:37 Uhr
> > Von: "Rob Herring" <robh@kernel.org>
>
> > There's a spec for it, so no need to look at what's used. Calxeda AHCI
> > had 5 ports IIRC.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 2/5] arm64: dts: broadcom: Fix sata nodename
2022-03-01 15:24 [PATCH v4 0/5] Add sata nodes to rk356x Frank Wunderlich
2022-03-01 15:24 ` [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml Frank Wunderlich
@ 2022-03-01 15:24 ` Frank Wunderlich
2022-03-01 21:29 ` Florian Fainelli
2022-03-01 15:24 ` [PATCH v4 3/5] dt-bindings: Add power-domains property to ahci-platform Frank Wunderlich
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-01 15:24 UTC (permalink / raw)
To: devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
From: Frank Wunderlich <frank-w@public-files.de>
After converting ahci-platform txt binding to yaml nodename is reported
as not matching the standard:
arch/arm64/boot/dts/broadcom/northstar2/ns2-svk.dt.yaml:
ahci@663f2000: $nodename:0: 'ahci@663f2000' does not match '^sata(@.*)?$'
Fix it to match binding.
Fixes: ac9aae00f0fc ("arm64: dts: Add SATA3 AHCI and SATA3 PHY DT nodes for NS2")
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v4:
- new Patch
---
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
index 2cfeaf3b0a87..8c218689fef7 100644
--- a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
@@ -687,7 +687,7 @@ sata_phy1: sata-phy@1 {
};
};
- sata: ahci@663f2000 {
+ sata: sata@663f2000 {
compatible = "brcm,iproc-ahci", "generic-ahci";
reg = <0x663f2000 0x1000>;
dma-coherent;
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/5] arm64: dts: broadcom: Fix sata nodename
2022-03-01 15:24 ` [PATCH v4 2/5] arm64: dts: broadcom: Fix sata nodename Frank Wunderlich
@ 2022-03-01 21:29 ` Florian Fainelli
0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2022-03-01 21:29 UTC (permalink / raw)
To: bcm-kernel-feedback-list, Frank Wunderlich, devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden
On Tue, 1 Mar 2022 16:24:18 +0100, Frank Wunderlich <linux@fw-web.de> wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> After converting ahci-platform txt binding to yaml nodename is reported
> as not matching the standard:
>
> arch/arm64/boot/dts/broadcom/northstar2/ns2-svk.dt.yaml:
> ahci@663f2000: $nodename:0: 'ahci@663f2000' does not match '^sata(@.*)?$'
>
> Fix it to match binding.
>
> Fixes: ac9aae00f0fc ("arm64: dts: Add SATA3 AHCI and SATA3 PHY DT nodes for NS2")
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, thanks!
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 3/5] dt-bindings: Add power-domains property to ahci-platform
2022-03-01 15:24 [PATCH v4 0/5] Add sata nodes to rk356x Frank Wunderlich
2022-03-01 15:24 ` [PATCH v4 1/5] dt-bindings: Convert ahci-platform DT bindings to yaml Frank Wunderlich
2022-03-01 15:24 ` [PATCH v4 2/5] arm64: dts: broadcom: Fix sata nodename Frank Wunderlich
@ 2022-03-01 15:24 ` Frank Wunderlich
2022-03-02 9:46 ` Krzysztof Kozlowski
2022-03-01 15:24 ` [PATCH v4 4/5] dt-bindings: Add rk3568-dwc3-ahci compatible Frank Wunderlich
2022-03-01 15:24 ` [PATCH v4 5/5] arm64: dts: rockchip: Add sata nodes to rk356x Frank Wunderlich
4 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-01 15:24 UTC (permalink / raw)
To: devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
From: Frank Wunderlich <frank-w@public-files.de>
Some SoC using power-domains property so add it here
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
changes in v4: none
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 cf67ddfc6afb..ea1af283d125 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -89,6 +89,9 @@ properties:
some embedded SoCs.
maxItems: 1
+ power-domains:
+ maxItems: 1
+
reg-names:
maxItems: 1
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 3/5] dt-bindings: Add power-domains property to ahci-platform
2022-03-01 15:24 ` [PATCH v4 3/5] dt-bindings: Add power-domains property to ahci-platform Frank Wunderlich
@ 2022-03-02 9:46 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 9:46 UTC (permalink / raw)
To: Frank Wunderlich, devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring, Hans de Goede,
Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis,
Michael Riesch, linux-arm-kernel, linux-rockchip, Ray Jui,
Scott Branden, bcm-kernel-feedback-list
On 01/03/2022 16:24, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Some SoC using power-domains property so add it here
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> changes in v4: none
> changes in v3:
> - new patch
> ---
> Documentation/devicetree/bindings/ata/ahci-platform.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 4/5] dt-bindings: Add rk3568-dwc3-ahci compatible
2022-03-01 15:24 [PATCH v4 0/5] Add sata nodes to rk356x Frank Wunderlich
` (2 preceding siblings ...)
2022-03-01 15:24 ` [PATCH v4 3/5] dt-bindings: Add power-domains property to ahci-platform Frank Wunderlich
@ 2022-03-01 15:24 ` Frank Wunderlich
2022-03-02 9:47 ` Krzysztof Kozlowski
2022-03-01 15:24 ` [PATCH v4 5/5] arm64: dts: rockchip: Add sata nodes to rk356x Frank Wunderlich
4 siblings, 1 reply; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-01 15:24 UTC (permalink / raw)
To: devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
From: Frank Wunderlich <frank-w@public-files.de>
Add SoC specific compatible for rk3568 ahci controller
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v4: new Patch
Documentation/devicetree/bindings/ata/ahci-platform.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
index ea1af283d125..7111cf448f45 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml
@@ -29,6 +29,10 @@ properties:
- marvell,armada-8k-ahci
- marvell,berlin2q-ahci
- const: generic-ahci
+ - items:
+ - enum:
+ - rockchip,rk3568-dwc-ahci
+ - const: snps,dwc-ahci
- enum:
- brcm,iproc-ahci
- cavium,octeon-7130-ahci
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 4/5] dt-bindings: Add rk3568-dwc3-ahci compatible
2022-03-01 15:24 ` [PATCH v4 4/5] dt-bindings: Add rk3568-dwc3-ahci compatible Frank Wunderlich
@ 2022-03-02 9:47 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 9:47 UTC (permalink / raw)
To: Frank Wunderlich, devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring, Hans de Goede,
Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis,
Michael Riesch, linux-arm-kernel, linux-rockchip, Ray Jui,
Scott Branden, bcm-kernel-feedback-list
On 01/03/2022 16:24, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Add SoC specific compatible for rk3568 ahci controller
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v4: new Patch
> Documentation/devicetree/bindings/ata/ahci-platform.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
Here and in previous bindings patches - you have wrong subject. I
already said it with v3. I gave you exact subject prefix to use.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 5/5] arm64: dts: rockchip: Add sata nodes to rk356x
2022-03-01 15:24 [PATCH v4 0/5] Add sata nodes to rk356x Frank Wunderlich
` (3 preceding siblings ...)
2022-03-01 15:24 ` [PATCH v4 4/5] dt-bindings: Add rk3568-dwc3-ahci compatible Frank Wunderlich
@ 2022-03-01 15:24 ` Frank Wunderlich
4 siblings, 0 replies; 17+ messages in thread
From: Frank Wunderlich @ 2022-03-01 15:24 UTC (permalink / raw)
To: devicetree
Cc: Frank Wunderlich, Damien Le Moal, Rob Herring,
Krzysztof Kozlowski, Hans de Goede, Jens Axboe, linux-ide,
linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch,
linux-arm-kernel, linux-rockchip, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
From: Frank Wunderlich <frank-w@public-files.de>
RK356x supports up to 3 sata controllers which were compatible with the
existing snps,dwc-ahci binding.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
changes in v4:
- drop newline in dts
- re-add clock-names
- add soc specific compatible
changes in v3:
- fix combphy error by moving sata0 to rk3568.dtsi
- remove clock-names and interrupt-names
changes in v2:
- added sata0 + 1, but have only tested sata2
---
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 14 ++++++++++++
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 28 ++++++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 5b0f528d6818..3e07d9f6a2d1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -8,6 +8,20 @@
/ {
compatible = "rockchip,rk3568";
+ sata0: sata@fc000000 {
+ compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfc000000 0 0x1000>;
+ clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>,
+ <&cru CLK_SATA0_RXOOB>;
+ clock-names = "sata", "pmalive", "rxoob";
+ interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&combphy0 PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ ports-implemented = <0x1>;
+ power-domains = <&power RK3568_PD_PIPE>;
+ status = "disabled";
+ };
+
pipe_phy_grf0: syscon@fdc70000 {
compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
reg = <0x0 0xfdc70000 0x0 0x1000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 7cdef800cb3c..264dd030e703 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -230,6 +230,34 @@ scmi_shmem: sram@0 {
};
};
+ sata1: sata@fc400000 {
+ compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfc400000 0 0x1000>;
+ clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>,
+ <&cru CLK_SATA1_RXOOB>;
+ clock-names = "sata", "pmalive", "rxoob";
+ interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&combphy1 PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ ports-implemented = <0x1>;
+ power-domains = <&power RK3568_PD_PIPE>;
+ status = "disabled";
+ };
+
+ sata2: sata@fc800000 {
+ compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfc800000 0 0x1000>;
+ clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>,
+ <&cru CLK_SATA2_RXOOB>;
+ clock-names = "sata", "pmalive", "rxoob";
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&combphy2 PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ ports-implemented = <0x1>;
+ power-domains = <&power RK3568_PD_PIPE>;
+ status = "disabled";
+ };
+
gic: interrupt-controller@fd400000 {
compatible = "arm,gic-v3";
reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread