[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230611204541.ml3ekvsj4qnlmc2s@mobilestation>
Date: Sun, 11 Jun 2023 23:45:41 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
linux-ide@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH v3 2/5] dt-bindings: ata: dwc-ahci: add Rockchip RK3588
On Thu, Jun 08, 2023 at 06:22:35PM +0200, Sebastian Reichel wrote:
> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
> allowed clocks without bloating the generic binding, the description
> of Rockchip's AHCI controllers has been moved to its own file.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.com>
> ---
> .../bindings/ata/rockchip,dwc-ahci.yaml | 114 ++++++++++++++++++
> .../bindings/ata/snps,dwc-ahci.yaml | 17 ++-
> 2 files changed, 125 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
> new file mode 100644
> index 000000000000..86da9bd594a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/rockchip,dwc-ahci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DWC AHCI SATA controller for Rockchip devices
> +
> +maintainers:
> + - Serge Semin <fancer.lancer@...il.com>
> +
> +description:
> + This document defines device tree bindings for the Synopsys DWC
> + implementation of the AHCI SATA controller found in Rockchip
> + devices.
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + - const: snps,dwc-ahci
> +
> + ports-implemented:
> + const: 1
> +
> +patternProperties:
> + "^sata-port@[0-9a-e]$":
> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> +
What about adding the "reg" property constraints? Seeing the
"ports-implemented" property is supposed to be "1" the "reg" property
should be zero for all the Rockchip AHCI SATA controllers. Right?
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports-implemented
> +
> +allOf:
> + - $ref: snps,dwc-ahci-common.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3588-dwc-ahci
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: sata
> + - const: pmalive
> + - const: rxoob
> + - const: ref
> + - const: asic
The "clocks" property constraints could have been added too. Right?
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3568-dwc-ahci
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: sata
> + - const: pmalive
> + - const: rxoob
ditto
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/ata/ahci.h>
> + #include <dt-bindings/phy/phy.h>
> +
> + sata@...10000 {
> + compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
> + reg = <0xfe210000 0x1000>;
> + clocks = <&cru ACLK_SATA0>, <&cru CLK_PMALIVE0>,
> + <&cru CLK_RXOOB0>, <&cru CLK_PIPEPHY0_REF>,
> + <&cru CLK_PIPEPHY0_PIPE_ASIC_G>;
> + clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
> + interrupts = <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH 0>;
> + ports-implemented = <0x1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata-port@0 {
> + reg = <0>;
> + hba-port-cap = <HBA_PORT_FBSCP>;
> + phys = <&combphy0_ps PHY_TYPE_SATA>;
> + phy-names = "sata-phy";
> + snps,rx-ts-max = <32>;
> + snps,tx-ts-max = <32>;
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> index 5afa4b57ce20..55a4bdfa3d9a 100644
> --- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> @@ -13,8 +13,14 @@ description:
> This document defines device tree bindings for the generic Synopsys DWC
> implementation of the AHCI SATA controller.
>
> -allOf:
> - - $ref: snps,dwc-ahci-common.yaml#
> +select:
> + properties:
> + compatible:
> + enum:
> + - snps,dwc-ahci
> + - snps,spear-ahci
> + required:
> + - compatible
Please don't move the allOf property. It's ok to have it defined above
the object properties clause if it has just a few ref-statements.
* Also make sure please that the "select" property is defined above
the "allOf" schema composition.
-Serge(y)
>
> properties:
> compatible:
> @@ -23,10 +29,6 @@ properties:
> const: snps,dwc-ahci
> - description: SPEAr1340 AHCI SATA device
> const: snps,spear-ahci
> - - description: Rockhip RK3568 AHCI controller
> - items:
> - - const: rockchip,rk3568-dwc-ahci
> - - const: snps,dwc-ahci
>
> patternProperties:
> "^sata-port@[0-9a-e]$":
> @@ -39,6 +41,9 @@ required:
> - reg
> - interrupts
>
> +allOf:
> + - $ref: snps,dwc-ahci-common.yaml#
> +
> unevaluatedProperties: false
>
> examples:
> --
> 2.39.2
>
Powered by blists - more mailing lists