* [PATCH v3 0/3] Add sata nodes to rk356x @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich From: Frank Wunderlich <frank-w@public-files.de> 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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 0/3] Add sata nodes to rk356x @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich From: Frank Wunderlich <frank-w@public-files.de> 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 _______________________________________________ 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] 42+ messages in thread
* [PATCH v3 0/3] Add sata nodes to rk356x @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich From: Frank Wunderlich <frank-w@public-files.de> 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 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-27 18:27 ` Frank Wunderlich (?) @ 2022-02-27 18:27 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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> --- 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 = <®_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..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 <hdegoede@redhat.com> + - Jens Axboe <axboe@kernel.dk> + +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 = <®_sata0>; + }; + + sata1: sata-port@1 { + reg = <1>; + phys = <&sata_phy 1>; + target-supply = <®_sata1>; + }; + }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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> --- 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 = <®_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..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 <hdegoede@redhat.com> + - Jens Axboe <axboe@kernel.dk> + +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 = <®_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 related [flat|nested] 42+ messages in thread
* [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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> --- 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 = <®_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..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 <hdegoede@redhat.com> + - Jens Axboe <axboe@kernel.dk> + +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 = <®_sata0>; + }; + + sata1: sata-port@1 { + reg = <1>; + phys = <&sata_phy 1>; + target-supply = <®_sata1>; + }; + }; -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-27 18:27 ` Frank Wunderlich (?) @ 2022-02-28 10:08 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:08 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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, devicetree@vger.kernel.org Cc: Frank Wunderlich On 27/02/2022 19:27, 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> > --- > 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 = <®_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..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 <hdegoede@redhat.com> > + - Jens Axboe <axboe@kernel.dk> > + > +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 = <®_sata0>; > + }; > + > + sata1: sata-port@1 { > + reg = <1>; > + phys = <&sata_phy 1>; > + target-supply = <®_sata1>; > + }; > + }; Best regards, Krzysztof ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 10:08 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:08 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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, devicetree@vger.kernel.org Cc: Frank Wunderlich On 27/02/2022 19:27, 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> > --- > 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 = <®_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..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 <hdegoede@redhat.com> > + - Jens Axboe <axboe@kernel.dk> > + > +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 = <®_sata0>; > + }; > + > + sata1: sata-port@1 { > + reg = <1>; > + phys = <&sata_phy 1>; > + target-supply = <®_sata1>; > + }; > + }; Best regards, Krzysztof _______________________________________________ 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] 42+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 10:08 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:08 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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, devicetree@vger.kernel.org Cc: Frank Wunderlich On 27/02/2022 19:27, 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> > --- > 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 = <®_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..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 <hdegoede@redhat.com> > + - Jens Axboe <axboe@kernel.dk> > + > +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 = <®_sata0>; > + }; > + > + sata1: sata-port@1 { > + reg = <1>; > + phys = <&sata_phy 1>; > + target-supply = <®_sata1>; > + }; > + }; Best regards, Krzysztof _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 10:08 ` Krzysztof Kozlowski (?) @ 2022-02-28 12:19 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 12:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi Krzysztof, thanks for first review. > Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 27/02/2022 19:27, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > 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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 12:19 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 12:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi Krzysztof, thanks for first review. > Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 27/02/2022 19:27, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > 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 _______________________________________________ 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] 42+ messages in thread
* Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 12:19 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 12:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi Krzysztof, thanks for first review. > Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 27/02/2022 19:27, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > 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 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 12:19 ` Frank Wunderlich (?) @ 2022-02-28 12:38 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 12:38 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 13:19, Frank Wunderlich wrote: > Hi Krzysztof, > > thanks for first review. > >> Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 27/02/2022 19:27, Frank Wunderlich wrote: >>> From: Frank Wunderlich <frank-w@public-files.de> > >> 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 ;) No, this has to be oneOf. See for example Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > 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/ OK, then let's skip them now. The clock items should be described if it is possible. > >>> + >>> + interrupts: >>> + minItems: 1 >> >> You mean maxItems? > > no, minItems, as interrupts suggests 1+ (same for phys) You cannot have infinite number of interrupts... What suggests "1+"? What does it mean "as interrupts suggests"? Do these hardware blocks really have many interrupt lines? The 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? `git grep ports-implemented` gives pretty straightfoward answer. All DTS have u32 and driver also uses u32. > >>> + >>> + resets: >>> + minItems: 1 >> >> maxItems? > > if there is a known maximum.... Must be. You cannot have infinite number of reset lines... Please check all DTS and drivers. If there is public documentation, it also might be useful. > >>> + >>> + 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. What do you mean "name suggests"? Name of property? No, it does not suggest that. Name is standard. Please check example schema and other existing schema bindings to see how it is done. For example earlier gpio-vf610.yaml is not bad. > 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 The bindings need to be specific, so only properties which really, really can have many unknown elements we could keep here some high maxItems limit. In 99% of cases maxItems are clearly defined. > >>> + >>> + 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 Install dependencies (libyaml-dev) and you will see first error: Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: [warning] wrong indentation: expected 2 but found 0 (indentation) But the one I am thinking is indeed not visible by default. You would need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. Then you see: Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: /example-1/sata@f7e90000: failed to match any schema with compatible: ['marvell,berlin2q-achi', 'generic-ahci'] Best regards, Krzysztof ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 12:38 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 12:38 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 13:19, Frank Wunderlich wrote: > Hi Krzysztof, > > thanks for first review. > >> Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 27/02/2022 19:27, Frank Wunderlich wrote: >>> From: Frank Wunderlich <frank-w@public-files.de> > >> 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 ;) No, this has to be oneOf. See for example Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > 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/ OK, then let's skip them now. The clock items should be described if it is possible. > >>> + >>> + interrupts: >>> + minItems: 1 >> >> You mean maxItems? > > no, minItems, as interrupts suggests 1+ (same for phys) You cannot have infinite number of interrupts... What suggests "1+"? What does it mean "as interrupts suggests"? Do these hardware blocks really have many interrupt lines? The 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? `git grep ports-implemented` gives pretty straightfoward answer. All DTS have u32 and driver also uses u32. > >>> + >>> + resets: >>> + minItems: 1 >> >> maxItems? > > if there is a known maximum.... Must be. You cannot have infinite number of reset lines... Please check all DTS and drivers. If there is public documentation, it also might be useful. > >>> + >>> + 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. What do you mean "name suggests"? Name of property? No, it does not suggest that. Name is standard. Please check example schema and other existing schema bindings to see how it is done. For example earlier gpio-vf610.yaml is not bad. > 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 The bindings need to be specific, so only properties which really, really can have many unknown elements we could keep here some high maxItems limit. In 99% of cases maxItems are clearly defined. > >>> + >>> + 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 Install dependencies (libyaml-dev) and you will see first error: Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: [warning] wrong indentation: expected 2 but found 0 (indentation) But the one I am thinking is indeed not visible by default. You would need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. Then you see: Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: /example-1/sata@f7e90000: failed to match any schema with compatible: ['marvell,berlin2q-achi', 'generic-ahci'] Best regards, Krzysztof _______________________________________________ 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] 42+ messages in thread
* Re: Aw: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 12:38 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 12:38 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 13:19, Frank Wunderlich wrote: > Hi Krzysztof, > > thanks for first review. > >> Gesendet: Montag, 28. Februar 2022 um 11:08 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 27/02/2022 19:27, Frank Wunderlich wrote: >>> From: Frank Wunderlich <frank-w@public-files.de> > >> 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 ;) No, this has to be oneOf. See for example Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > 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/ OK, then let's skip them now. The clock items should be described if it is possible. > >>> + >>> + interrupts: >>> + minItems: 1 >> >> You mean maxItems? > > no, minItems, as interrupts suggests 1+ (same for phys) You cannot have infinite number of interrupts... What suggests "1+"? What does it mean "as interrupts suggests"? Do these hardware blocks really have many interrupt lines? The 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? `git grep ports-implemented` gives pretty straightfoward answer. All DTS have u32 and driver also uses u32. > >>> + >>> + resets: >>> + minItems: 1 >> >> maxItems? > > if there is a known maximum.... Must be. You cannot have infinite number of reset lines... Please check all DTS and drivers. If there is public documentation, it also might be useful. > >>> + >>> + 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. What do you mean "name suggests"? Name of property? No, it does not suggest that. Name is standard. Please check example schema and other existing schema bindings to see how it is done. For example earlier gpio-vf610.yaml is not bad. > 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 The bindings need to be specific, so only properties which really, really can have many unknown elements we could keep here some high maxItems limit. In 99% of cases maxItems are clearly defined. > >>> + >>> + 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 Install dependencies (libyaml-dev) and you will see first error: Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: [warning] wrong indentation: expected 2 but found 0 (indentation) But the one I am thinking is indeed not visible by default. You would need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. Then you see: Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: /example-1/sata@f7e90000: failed to match any schema with compatible: ['marvell,berlin2q-achi', 'generic-ahci'] Best regards, Krzysztof _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 12:38 ` Krzysztof Kozlowski (?) @ 2022-02-28 14:09 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 14:09 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi, looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 28/02/2022 13:19, Frank Wunderlich wrote: > >> 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 ;) > > No, this has to be oneOf. See for example > Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. so i end up in a struct like this compatible: oneOf: - 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 - items: - 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 > >>> + interrupts: > >>> + minItems: 1 > >> > >> You mean maxItems? > > > > no, minItems, as interrupts suggests 1+ (same for phys) > > You cannot have infinite number of interrupts... What suggests "1+"? > What does it mean "as interrupts suggests"? Do these hardware blocks > really have many interrupt lines? > > The same for phys. interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. > >>> + 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? > > `git grep ports-implemented` gives pretty straightfoward answer. All DTS > have u32 and driver also uses u32. so type should be $ref: '/schemas/types.yaml#/definitions/uint32' ? it's the only one i've found with u32 looking like a type found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml > > > >>> + > >>> + resets: > >>> + minItems: 1 > >> > >> maxItems? > > > > if there is a known maximum.... > > Must be. You cannot have infinite number of reset lines... Please check > all DTS and drivers. If there is public documentation, it also might be > useful. "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( > > > >>> + > >>> + 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. > > What do you mean "name suggests"? Name of property? No, it does not > suggest that. Name is standard. Please check example schema and other > existing schema bindings to see how it is done. For example earlier > gpio-vf610.yaml is not bad. > > 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 > > The bindings need to be specific, so only properties which really, > really can have many unknown elements we could keep here some high > maxItems limit. In 99% of cases maxItems are clearly defined. so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... but this is maybe not the right way to do as dts can contain errors which should not modify the binding. > > 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 > > Install dependencies (libyaml-dev) and you will see first error: > Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: > [warning] wrong indentation: expected 2 but found 0 (indentation) > > But the one I am thinking is indeed not visible by default. You would > need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. > Then you see: > > Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: > /example-1/sata@f7e90000: failed to match any schema with compatible: > ['marvell,berlin2q-achi', 'generic-ahci'] i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. I thought i can help getting this (simple looking) txt converted to yaml. Seems the binding needs to be done by someone who knows the drivers more than me. regards Frank ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 14:09 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 14:09 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi, looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 28/02/2022 13:19, Frank Wunderlich wrote: > >> 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 ;) > > No, this has to be oneOf. See for example > Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. so i end up in a struct like this compatible: oneOf: - 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 - items: - 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 > >>> + interrupts: > >>> + minItems: 1 > >> > >> You mean maxItems? > > > > no, minItems, as interrupts suggests 1+ (same for phys) > > You cannot have infinite number of interrupts... What suggests "1+"? > What does it mean "as interrupts suggests"? Do these hardware blocks > really have many interrupt lines? > > The same for phys. interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. > >>> + 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? > > `git grep ports-implemented` gives pretty straightfoward answer. All DTS > have u32 and driver also uses u32. so type should be $ref: '/schemas/types.yaml#/definitions/uint32' ? it's the only one i've found with u32 looking like a type found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml > > > >>> + > >>> + resets: > >>> + minItems: 1 > >> > >> maxItems? > > > > if there is a known maximum.... > > Must be. You cannot have infinite number of reset lines... Please check > all DTS and drivers. If there is public documentation, it also might be > useful. "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( > > > >>> + > >>> + 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. > > What do you mean "name suggests"? Name of property? No, it does not > suggest that. Name is standard. Please check example schema and other > existing schema bindings to see how it is done. For example earlier > gpio-vf610.yaml is not bad. > > 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 > > The bindings need to be specific, so only properties which really, > really can have many unknown elements we could keep here some high > maxItems limit. In 99% of cases maxItems are clearly defined. so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... but this is maybe not the right way to do as dts can contain errors which should not modify the binding. > > 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 > > Install dependencies (libyaml-dev) and you will see first error: > Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: > [warning] wrong indentation: expected 2 but found 0 (indentation) > > But the one I am thinking is indeed not visible by default. You would > need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. > Then you see: > > Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: > /example-1/sata@f7e90000: failed to match any schema with compatible: > ['marvell,berlin2q-achi', 'generic-ahci'] i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. I thought i can help getting this (simple looking) txt converted to yaml. Seems the binding needs to be done by someone who knows the drivers more than me. regards Frank _______________________________________________ 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] 42+ messages in thread
* Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 14:09 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 14:09 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi, looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > On 28/02/2022 13:19, Frank Wunderlich wrote: > >> 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 ;) > > No, this has to be oneOf. See for example > Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. so i end up in a struct like this compatible: oneOf: - 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 - items: - 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 > >>> + interrupts: > >>> + minItems: 1 > >> > >> You mean maxItems? > > > > no, minItems, as interrupts suggests 1+ (same for phys) > > You cannot have infinite number of interrupts... What suggests "1+"? > What does it mean "as interrupts suggests"? Do these hardware blocks > really have many interrupt lines? > > The same for phys. interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. > >>> + 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? > > `git grep ports-implemented` gives pretty straightfoward answer. All DTS > have u32 and driver also uses u32. so type should be $ref: '/schemas/types.yaml#/definitions/uint32' ? it's the only one i've found with u32 looking like a type found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml > > > >>> + > >>> + resets: > >>> + minItems: 1 > >> > >> maxItems? > > > > if there is a known maximum.... > > Must be. You cannot have infinite number of reset lines... Please check > all DTS and drivers. If there is public documentation, it also might be > useful. "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( > > > >>> + > >>> + 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. > > What do you mean "name suggests"? Name of property? No, it does not > suggest that. Name is standard. Please check example schema and other > existing schema bindings to see how it is done. For example earlier > gpio-vf610.yaml is not bad. > > 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 > > The bindings need to be specific, so only properties which really, > really can have many unknown elements we could keep here some high > maxItems limit. In 99% of cases maxItems are clearly defined. so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... but this is maybe not the right way to do as dts can contain errors which should not modify the binding. > > 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 > > Install dependencies (libyaml-dev) and you will see first error: > Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: > [warning] wrong indentation: expected 2 but found 0 (indentation) > > But the one I am thinking is indeed not visible by default. You would > need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. > Then you see: > > Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: > /example-1/sata@f7e90000: failed to match any schema with compatible: > ['marvell,berlin2q-achi', 'generic-ahci'] i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. I thought i can help getting this (simple looking) txt converted to yaml. Seems the binding needs to be done by someone who knows the drivers more than me. regards Frank _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 14:09 ` Frank Wunderlich (?) @ 2022-02-28 14:35 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 14:35 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 15:09, Frank Wunderlich wrote: > Hi, > > looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... > > i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > > > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 28/02/2022 13:19, Frank Wunderlich wrote: > >>>> 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 ;) >> >> No, this has to be oneOf. See for example >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. > > so i end up in a struct like this > > compatible: > oneOf: > - 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 > - items: > - 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 That could be one way, but instead I propose to have only second part (so enum + generic-ahci) for all compatibles mentioned in ahci_platform.c, which do not customize the driver behavior for these compatibles.. > > >>>>> + interrupts: >>>>> + minItems: 1 >>>> >>>> You mean maxItems? >>> >>> no, minItems, as interrupts suggests 1+ (same for phys) >> >> You cannot have infinite number of interrupts... What suggests "1+"? >> What does it mean "as interrupts suggests"? Do these hardware blocks >> really have many interrupt lines? >> >> The same for phys. > > interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. There is no property named "interrupt" or "phy". There is only "interrupts", "phys", "gpios", "clocks" etc. regardless whether there is one or multiple items. This is not specific to bindings, but all DTS have this. > > as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. But the sources of the driver and DTS are available, why not using them? > >>>>> + 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? >> >> `git grep ports-implemented` gives pretty straightfoward answer. All DTS >> have u32 and driver also uses u32. > > so type should be > > $ref: '/schemas/types.yaml#/definitions/uint32' Yes. See example schema. > > ? > > it's the only one i've found with u32 looking like a type > > found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml This part I do not understand. > >>> >>>>> + >>>>> + resets: >>>>> + minItems: 1 >>>> >>>> maxItems? >>> >>> if there is a known maximum.... >> >> Must be. You cannot have infinite number of reset lines... Please check >> all DTS and drivers. If there is public documentation, it also might be >> useful. > > "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( I believe that information - how many resets - is in general findable via driver sources. In all bindings conversions, if we do not have such information, we need to try to look it up. >>> >>>>> + >>>>> + 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. >> >> What do you mean "name suggests"? Name of property? No, it does not >> suggest that. Name is standard. Please check example schema and other >> existing schema bindings to see how it is done. For example earlier >> gpio-vf610.yaml is not bad. > >>> 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 >> >> The bindings need to be specific, so only properties which really, >> really can have many unknown elements we could keep here some high >> maxItems limit. In 99% of cases maxItems are clearly defined. > > so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... Why try-and-error? "git grep" works here... > > but this is maybe not the right way to do as dts can contain errors which should not modify the binding. True, but having stricter limit, even if not fully correct, is better than having too loose limit. Stricter can be always loosened. Loose requirement cannot be made stricter. > >>> 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 >> >> Install dependencies (libyaml-dev) and you will see first error: >> Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: >> [warning] wrong indentation: expected 2 but found 0 (indentation) >> >> But the one I am thinking is indeed not visible by default. You would >> need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. >> Then you see: >> >> Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: >> /example-1/sata@f7e90000: failed to match any schema with compatible: >> ['marvell,berlin2q-achi', 'generic-ahci'] > > i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. > I thought i can help getting this (simple looking) txt converted to yaml. > Seems the binding needs to be done by someone who knows the drivers more than me. > > regards Frank Best regards, Krzysztof ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 14:35 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 14:35 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 15:09, Frank Wunderlich wrote: > Hi, > > looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... > > i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > > > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 28/02/2022 13:19, Frank Wunderlich wrote: > >>>> 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 ;) >> >> No, this has to be oneOf. See for example >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. > > so i end up in a struct like this > > compatible: > oneOf: > - 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 > - items: > - 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 That could be one way, but instead I propose to have only second part (so enum + generic-ahci) for all compatibles mentioned in ahci_platform.c, which do not customize the driver behavior for these compatibles.. > > >>>>> + interrupts: >>>>> + minItems: 1 >>>> >>>> You mean maxItems? >>> >>> no, minItems, as interrupts suggests 1+ (same for phys) >> >> You cannot have infinite number of interrupts... What suggests "1+"? >> What does it mean "as interrupts suggests"? Do these hardware blocks >> really have many interrupt lines? >> >> The same for phys. > > interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. There is no property named "interrupt" or "phy". There is only "interrupts", "phys", "gpios", "clocks" etc. regardless whether there is one or multiple items. This is not specific to bindings, but all DTS have this. > > as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. But the sources of the driver and DTS are available, why not using them? > >>>>> + 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? >> >> `git grep ports-implemented` gives pretty straightfoward answer. All DTS >> have u32 and driver also uses u32. > > so type should be > > $ref: '/schemas/types.yaml#/definitions/uint32' Yes. See example schema. > > ? > > it's the only one i've found with u32 looking like a type > > found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml This part I do not understand. > >>> >>>>> + >>>>> + resets: >>>>> + minItems: 1 >>>> >>>> maxItems? >>> >>> if there is a known maximum.... >> >> Must be. You cannot have infinite number of reset lines... Please check >> all DTS and drivers. If there is public documentation, it also might be >> useful. > > "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( I believe that information - how many resets - is in general findable via driver sources. In all bindings conversions, if we do not have such information, we need to try to look it up. >>> >>>>> + >>>>> + 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. >> >> What do you mean "name suggests"? Name of property? No, it does not >> suggest that. Name is standard. Please check example schema and other >> existing schema bindings to see how it is done. For example earlier >> gpio-vf610.yaml is not bad. > >>> 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 >> >> The bindings need to be specific, so only properties which really, >> really can have many unknown elements we could keep here some high >> maxItems limit. In 99% of cases maxItems are clearly defined. > > so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... Why try-and-error? "git grep" works here... > > but this is maybe not the right way to do as dts can contain errors which should not modify the binding. True, but having stricter limit, even if not fully correct, is better than having too loose limit. Stricter can be always loosened. Loose requirement cannot be made stricter. > >>> 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 >> >> Install dependencies (libyaml-dev) and you will see first error: >> Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: >> [warning] wrong indentation: expected 2 but found 0 (indentation) >> >> But the one I am thinking is indeed not visible by default. You would >> need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. >> Then you see: >> >> Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: >> /example-1/sata@f7e90000: failed to match any schema with compatible: >> ['marvell,berlin2q-achi', 'generic-ahci'] > > i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. > I thought i can help getting this (simple looking) txt converted to yaml. > Seems the binding needs to be done by someone who knows the drivers more than me. > > regards Frank Best regards, Krzysztof _______________________________________________ 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] 42+ messages in thread
* Re: Aw: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 14:35 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 14:35 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 15:09, Frank Wunderlich wrote: > Hi, > > looks like i'm not the right person to convert the binding. I have not enough knowledge about yaml, bindings, drivers, ... > > i will try you suggestions for me as a lerning-by-doing approach, but will drop it from the series, same for part2 as it depends on first. > > > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> On 28/02/2022 13:19, Frank Wunderlich wrote: > >>>> 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 ;) >> >> No, this has to be oneOf. See for example >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > Afaik the generic-ahci should be defined optional with one of the others needed, but afaik this will duplicate the list i had. > > so i end up in a struct like this > > compatible: > oneOf: > - 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 > - items: > - 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 That could be one way, but instead I propose to have only second part (so enum + generic-ahci) for all compatibles mentioned in ahci_platform.c, which do not customize the driver behavior for these compatibles.. > > >>>>> + interrupts: >>>>> + minItems: 1 >>>> >>>> You mean maxItems? >>> >>> no, minItems, as interrupts suggests 1+ (same for phys) >> >> You cannot have infinite number of interrupts... What suggests "1+"? >> What does it mean "as interrupts suggests"? Do these hardware blocks >> really have many interrupt lines? >> >> The same for phys. > > interrupts/phys is plural of interrupt/phy, so it suggests it can be more than 1. There is no property named "interrupt" or "phy". There is only "interrupts", "phys", "gpios", "clocks" etc. regardless whether there is one or multiple items. This is not specific to bindings, but all DTS have this. > > as i said i do not know every driver with all possibilities, so i started with the min-items as we need at least 1 interrupt/phy, but yes if there are any limits then they should be added, but this needs extensive knowledge about the drivers/hardware, i don't have. But the sources of the driver and DTS are available, why not using them? > >>>>> + 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? >> >> `git grep ports-implemented` gives pretty straightfoward answer. All DTS >> have u32 and driver also uses u32. > > so type should be > > $ref: '/schemas/types.yaml#/definitions/uint32' Yes. See example schema. > > ? > > it's the only one i've found with u32 looking like a type > > found in Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml This part I do not understand. > >>> >>>>> + >>>>> + resets: >>>>> + minItems: 1 >>>> >>>> maxItems? >>> >>> if there is a known maximum.... >> >> Must be. You cannot have infinite number of reset lines... Please check >> all DTS and drivers. If there is public documentation, it also might be >> useful. > > "Please check all DTS and drivers." this is impossible for me as doing this as hobby with still limited time ;( I believe that information - how many resets - is in general findable via driver sources. In all bindings conversions, if we do not have such information, we need to try to look it up. >>> >>>>> + >>>>> + 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. >> >> What do you mean "name suggests"? Name of property? No, it does not >> suggest that. Name is standard. Please check example schema and other >> existing schema bindings to see how it is done. For example earlier >> gpio-vf610.yaml is not bad. > >>> 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 >> >> The bindings need to be specific, so only properties which really, >> really can have many unknown elements we could keep here some high >> maxItems limit. In 99% of cases maxItems are clearly defined. > > so i need to do try-and-error, setting maxItems to 1, make checks, if failing look in driver, increase, ... Why try-and-error? "git grep" works here... > > but this is maybe not the right way to do as dts can contain errors which should not modify the binding. True, but having stricter limit, even if not fully correct, is better than having too loose limit. Stricter can be always loosened. Loose requirement cannot be made stricter. > >>> 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 >> >> Install dependencies (libyaml-dev) and you will see first error: >> Documentation/devicetree/bindings/ata/ahci-platform.yaml:110:1: >> [warning] wrong indentation: expected 2 but found 0 (indentation) >> >> But the one I am thinking is indeed not visible by default. You would >> need to run it like Rob's boot is running, so add DT_CHECKER_FLAGS=-m. >> Then you see: >> >> Documentation/devicetree/bindings/ata/ahci-platform.example.dt.yaml:0:0: >> /example-1/sata@f7e90000: failed to match any schema with compatible: >> ['marvell,berlin2q-achi', 'generic-ahci'] > > i try this, but imho it's better to drop the Patch as i'm no expert in this, don't know the HW/drivers enough and this will delay the dts patch too much. > I thought i can help getting this (simple looking) txt converted to yaml. > Seems the binding needs to be done by someone who knows the drivers more than me. > > regards Frank Best regards, Krzysztof _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 14:35 ` Krzysztof Kozlowski (?) @ 2022-02-28 17:01 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 17:01 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi > Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> No, this has to be oneOf. See for example > >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > compatible: > > oneOf: > > - 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 > > - items: > > - 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 > > That could be one way, but instead I propose to have only second part > (so enum + generic-ahci) for all compatibles mentioned in > ahci_platform.c, which do not customize the driver behavior for these > compatibles.. tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) as far as i understand the logic it should be similar to this: properties: compatible: oneOf: - items: - enum: - marvell,berlin2q-achi - const: generic-ahci - items: - 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 this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. dtbs_check found some more like 'brcm,iproc-ahci' 'marvell,armada-8k-ahci' and many more it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible regards Frank ^ permalink raw reply [flat|nested] 42+ messages in thread
* Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 17:01 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 17:01 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi > Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> No, this has to be oneOf. See for example > >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > compatible: > > oneOf: > > - 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 > > - items: > > - 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 > > That could be one way, but instead I propose to have only second part > (so enum + generic-ahci) for all compatibles mentioned in > ahci_platform.c, which do not customize the driver behavior for these > compatibles.. tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) as far as i understand the logic it should be similar to this: properties: compatible: oneOf: - items: - enum: - marvell,berlin2q-achi - const: generic-ahci - items: - 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 this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. dtbs_check found some more like 'brcm,iproc-ahci' 'marvell,armada-8k-ahci' and many more it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible regards Frank _______________________________________________ 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] 42+ messages in thread
* Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 17:01 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-28 17:01 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org Hi > Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr > >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >> No, this has to be oneOf. See for example > >> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > > compatible: > > oneOf: > > - 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 > > - items: > > - 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 > > That could be one way, but instead I propose to have only second part > (so enum + generic-ahci) for all compatibles mentioned in > ahci_platform.c, which do not customize the driver behavior for these > compatibles.. tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) as far as i understand the logic it should be similar to this: properties: compatible: oneOf: - items: - enum: - marvell,berlin2q-achi - const: generic-ahci - items: - 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 this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. dtbs_check found some more like 'brcm,iproc-ahci' 'marvell,armada-8k-ahci' and many more it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible regards Frank _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml 2022-02-28 17:01 ` Frank Wunderlich (?) @ 2022-02-28 17:06 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 17:06 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 18:01, Frank Wunderlich wrote: > Hi > >> Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> >>>> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >>>> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >>>> No, this has to be oneOf. See for example >>>> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > >>> compatible: >>> oneOf: >>> - 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 >>> - items: >>> - 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 >> >> That could be one way, but instead I propose to have only second part >> (so enum + generic-ahci) for all compatibles mentioned in >> ahci_platform.c, which do not customize the driver behavior for these >> compatibles.. > > tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) > > as far as i understand the logic it should be similar to this: > > properties: > compatible: > oneOf: > - items: > - enum: > - marvell,berlin2q-achi You need to extend this enum with all the entries I mentioned before. > - const: generic-ahci > - items: No items here, directly enum. > - 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 > > this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. > > dtbs_check found some more like > > 'brcm,iproc-ahci' > 'marvell,armada-8k-ahci' > and many more > > it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible > > regards Frank Best regards, Krzysztof ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 17:06 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 17:06 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 18:01, Frank Wunderlich wrote: > Hi > >> Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> >>>> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >>>> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >>>> No, this has to be oneOf. See for example >>>> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > >>> compatible: >>> oneOf: >>> - 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 >>> - items: >>> - 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 >> >> That could be one way, but instead I propose to have only second part >> (so enum + generic-ahci) for all compatibles mentioned in >> ahci_platform.c, which do not customize the driver behavior for these >> compatibles.. > > tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) > > as far as i understand the logic it should be similar to this: > > properties: > compatible: > oneOf: > - items: > - enum: > - marvell,berlin2q-achi You need to extend this enum with all the entries I mentioned before. > - const: generic-ahci > - items: No items here, directly enum. > - 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 > > this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. > > dtbs_check found some more like > > 'brcm,iproc-ahci' > 'marvell,armada-8k-ahci' > and many more > > it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible > > regards Frank Best regards, Krzysztof _______________________________________________ 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] 42+ messages in thread
* Re: Aw: Re: Re: Re: [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml @ 2022-02-28 17:06 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 17:06 UTC (permalink / raw) To: Frank Wunderlich Cc: Frank Wunderlich, Rob Herring, Hans de Goede, Jens Axboe, linux-ide, linux-kernel, Heiko Stuebner, Peter Geis, Michael Riesch, linux-arm-kernel, linux-rockchip, devicetree@vger.kernel.org On 28/02/2022 18:01, Frank Wunderlich wrote: > Hi > >> Gesendet: Montag, 28. Februar 2022 um 15:35 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> >>>> Gesendet: Montag, 28. Februar 2022 um 13:38 Uhr >>>> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com> > >>>> No, this has to be oneOf. See for example >>>> Documentation/devicetree/bindings/gpio/gpio-vf610.yaml or many other files. > >>> compatible: >>> oneOf: >>> - 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 >>> - items: >>> - 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 >> >> That could be one way, but instead I propose to have only second part >> (so enum + generic-ahci) for all compatibles mentioned in >> ahci_platform.c, which do not customize the driver behavior for these >> compatibles.. > > tried many ways of defining it, but none passes with the examples. either to short (first example) or too long (second) > > as far as i understand the logic it should be similar to this: > > properties: > compatible: > oneOf: > - items: > - enum: > - marvell,berlin2q-achi You need to extend this enum with all the entries I mentioned before. > - const: generic-ahci > - items: No items here, directly enum. > - 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 > > this passes the dt-binding_check (examples) for me, but i guess there are many more compatibles defined with the generic. > > dtbs_check found some more like > > 'brcm,iproc-ahci' > 'marvell,armada-8k-ahci' > and many more > > it looks like these are also checked in the enum, so the yaml itself look correct, but needs some kind of wildcard instead of the "marvell,berlin2q-achi" as second for the generic-ahci compatible > > regards Frank Best regards, Krzysztof _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform 2022-02-27 18:27 ` Frank Wunderlich (?) @ 2022-02-27 18:27 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform @ 2022-02-27 18:27 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:27 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform 2022-02-27 18:27 ` Frank Wunderlich (?) @ 2022-02-28 10:09 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:09 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:27, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > 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 <frank-w@public-files.de> > --- > 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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform @ 2022-02-28 10:09 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:09 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:27, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > 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 <frank-w@public-files.de> > --- > 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 _______________________________________________ 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] 42+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform @ 2022-02-28 10:09 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 10:09 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:27, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > 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 <frank-w@public-files.de> > --- > 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 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x 2022-02-27 18:27 ` Frank Wunderlich (?) @ 2022-02-27 18:28 ` Frank Wunderlich -1 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:28 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 | 26 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi index 5b0f528d6818..2a2f65899d47 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi @@ -8,6 +8,19 @@ / { compatible = "rockchip,rk3568"; + sata0: sata@fc000000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc000000 0 0x1000>; + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, + <&cru CLK_SATA0_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>; @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { #power-domain-cells = <0>; }; }; + diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi index 7cdef800cb3c..484c5ace718a 100644 --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { }; }; + sata1: sata@fc400000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc400000 0 0x1000>; + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; + reg = <0 0xfc800000 0 0x1000>; + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, + <&cru CLK_SATA2_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] 42+ messages in thread
* [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-27 18:28 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:28 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 | 26 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi index 5b0f528d6818..2a2f65899d47 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi @@ -8,6 +8,19 @@ / { compatible = "rockchip,rk3568"; + sata0: sata@fc000000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc000000 0 0x1000>; + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, + <&cru CLK_SATA0_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>; @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { #power-domain-cells = <0>; }; }; + diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi index 7cdef800cb3c..484c5ace718a 100644 --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { }; }; + sata1: sata@fc400000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc400000 0 0x1000>; + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; + reg = <0 0xfc800000 0 0x1000>; + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, + <&cru CLK_SATA2_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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-27 18:28 ` Frank Wunderlich 0 siblings, 0 replies; 42+ messages in thread From: Frank Wunderlich @ 2022-02-27 18:28 UTC (permalink / raw) To: devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich 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 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 | 26 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi index 5b0f528d6818..2a2f65899d47 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi @@ -8,6 +8,19 @@ / { compatible = "rockchip,rk3568"; + sata0: sata@fc000000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc000000 0 0x1000>; + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, + <&cru CLK_SATA0_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>; @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { #power-domain-cells = <0>; }; }; + diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi index 7cdef800cb3c..484c5ace718a 100644 --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { }; }; + sata1: sata@fc400000 { + compatible = "snps,dwc-ahci"; + reg = <0 0xfc400000 0 0x1000>; + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; + reg = <0 0xfc800000 0 0x1000>; + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, + <&cru CLK_SATA2_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 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x 2022-02-27 18:28 ` Frank Wunderlich (?) @ 2022-02-28 9:32 ` Johan Jonker -1 siblings, 0 replies; 42+ messages in thread From: Johan Jonker @ 2022-02-28 9:32 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich Hi Frank, On 2/27/22 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci"; Compatible strings must be SoC orientated. By using a fall back string the driver doesn't have to be changed for every new SoC. Add binding in extra patch. > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; dito > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; dito > + reg = <0 0xfc800000 0 0x1000>; > + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, > + <&cru CLK_SATA2_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 */ ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-28 9:32 ` Johan Jonker 0 siblings, 0 replies; 42+ messages in thread From: Johan Jonker @ 2022-02-28 9:32 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich Hi Frank, On 2/27/22 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci"; Compatible strings must be SoC orientated. By using a fall back string the driver doesn't have to be changed for every new SoC. Add binding in extra patch. > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; dito > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; dito > + reg = <0 0xfc800000 0 0x1000>; > + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, > + <&cru CLK_SATA2_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 */ _______________________________________________ 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] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-28 9:32 ` Johan Jonker 0 siblings, 0 replies; 42+ messages in thread From: Johan Jonker @ 2022-02-28 9:32 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich Hi Frank, On 2/27/22 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci"; Compatible strings must be SoC orientated. By using a fall back string the driver doesn't have to be changed for every new SoC. Add binding in extra patch. > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; dito > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_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 = "snps,dwc-ahci"; dito > + reg = <0 0xfc800000 0 0x1000>; > + clocks = <&cru ACLK_SATA2>, <&cru CLK_SATA2_PMALIVE>, > + <&cru CLK_SATA2_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 */ _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x 2022-02-27 18:28 ` Frank Wunderlich (?) @ 2022-02-28 9:54 ` Krzysztof Kozlowski -1 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 9:54 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + No need for new line here, > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_RXOOB>; clock-names should be added. Also to bindings. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-28 9:54 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 9:54 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + No need for new line here, > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_RXOOB>; clock-names should be added. Also to bindings. Best regards, Krzysztof _______________________________________________ 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] 42+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x @ 2022-02-28 9:54 ` Krzysztof Kozlowski 0 siblings, 0 replies; 42+ messages in thread From: Krzysztof Kozlowski @ 2022-02-28 9:54 UTC (permalink / raw) To: Frank Wunderlich, devicetree @ vger . kernel . org 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 Cc: Frank Wunderlich On 27/02/2022 19:28, Frank Wunderlich wrote: > 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 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 | 26 ++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5b0f528d6818..2a2f65899d47 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -8,6 +8,19 @@ > / { > compatible = "rockchip,rk3568"; > > + sata0: sata@fc000000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc000000 0 0x1000>; > + clocks = <&cru ACLK_SATA0>, <&cru CLK_SATA0_PMALIVE>, > + <&cru CLK_SATA0_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>; > @@ -114,3 +127,4 @@ power-domain@RK3568_PD_PIPE { > #power-domain-cells = <0>; > }; > }; > + No need for new line here, > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > index 7cdef800cb3c..484c5ace718a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -230,6 +230,32 @@ scmi_shmem: sram@0 { > }; > }; > > + sata1: sata@fc400000 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0xfc400000 0 0x1000>; > + clocks = <&cru ACLK_SATA1>, <&cru CLK_SATA1_PMALIVE>, > + <&cru CLK_SATA1_RXOOB>; clock-names should be added. Also to bindings. Best regards, Krzysztof _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2022-02-28 17:08 UTC | newest] Thread overview: 42+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-27 18:27 [PATCH v3 0/3] Add sata nodes to rk356x Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-27 18:27 ` [PATCH v3 1/3] dt-bindings: Convert ahci-platform DT bindings to yaml Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-28 10:08 ` Krzysztof Kozlowski 2022-02-28 10:08 ` Krzysztof Kozlowski 2022-02-28 10:08 ` Krzysztof Kozlowski 2022-02-28 12:19 ` Aw: " Frank Wunderlich 2022-02-28 12:19 ` Frank Wunderlich 2022-02-28 12:19 ` Frank Wunderlich 2022-02-28 12:38 ` Krzysztof Kozlowski 2022-02-28 12:38 ` Krzysztof Kozlowski 2022-02-28 12:38 ` Krzysztof Kozlowski 2022-02-28 14:09 ` Aw: " Frank Wunderlich 2022-02-28 14:09 ` Frank Wunderlich 2022-02-28 14:09 ` Frank Wunderlich 2022-02-28 14:35 ` Krzysztof Kozlowski 2022-02-28 14:35 ` Krzysztof Kozlowski 2022-02-28 14:35 ` Krzysztof Kozlowski 2022-02-28 17:01 ` Aw: " Frank Wunderlich 2022-02-28 17:01 ` Frank Wunderlich 2022-02-28 17:01 ` Frank Wunderlich 2022-02-28 17:06 ` Krzysztof Kozlowski 2022-02-28 17:06 ` Krzysztof Kozlowski 2022-02-28 17:06 ` Krzysztof Kozlowski 2022-02-27 18:27 ` [PATCH v3 2/3] dt-bindings: Add power-domains property to ahci-platform Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-27 18:27 ` Frank Wunderlich 2022-02-28 10:09 ` Krzysztof Kozlowski 2022-02-28 10:09 ` Krzysztof Kozlowski 2022-02-28 10:09 ` Krzysztof Kozlowski 2022-02-27 18:28 ` [PATCH v3 3/3] arm64: dts: rockchip: Add sata nodes to rk356x Frank Wunderlich 2022-02-27 18:28 ` Frank Wunderlich 2022-02-27 18:28 ` Frank Wunderlich 2022-02-28 9:32 ` Johan Jonker 2022-02-28 9:32 ` Johan Jonker 2022-02-28 9:32 ` Johan Jonker 2022-02-28 9:54 ` Krzysztof Kozlowski 2022-02-28 9:54 ` Krzysztof Kozlowski 2022-02-28 9:54 ` Krzysztof Kozlowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.