On Wed, May 04, 2022 at 05:29:23AM +0900, Damien Le Moal wrote: > On 2022/05/04 5:09, Serge Semin wrote: > > In order to create a more sophisticated AHCI controller DT bindings let's > > divide the already available generic AHCI platform YAML schema into the > > platform part and a set of the common AHCI properties. The former part > > will be used to evaluate the AHCI DT nodes mainly compatible with the > > generic AHCI controller while the later schema will be used for more > > thorough AHCI DT nodes description. For instance such YAML schemas design > > will be useful for our DW AHCI SATA controller derivative with four clock > > sources, two reset lines, one system controller reference and specific > > max Rx/Tx DMA xfers size constraints. > > > > Note the phys and target-supply property requirement is preserved in the > > generic AHCI platform bindings because some platforms can lack of the > > explicitly specified PHYs or target device power regulators. > > > > Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> > > > > --- > > > > Folks, I don't really see why the phys/target-supply requirement has been > > added to the generic AHCI DT schema in the first place. Probably just to > > imply some meaning for the sub-nodes definition. Anyway in one of the > > further patches I am adding the DW AHCI SATA controller DT bindings which > > won't require having these properties specified in the sub-nodes, but will > > describe additional port-specific properties. That's why I get to keep the > > constraints in the ahci-platform.yaml schema instead of moving them to the > > common schema. > > > > Changelog v2: > > - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel. > > --- > > .../devicetree/bindings/ata/ahci-common.yaml | 117 ++++++++++++++++++ > > .../bindings/ata/ahci-platform.yaml | 68 +--------- > > 2 files changed, 123 insertions(+), 62 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/ata/ahci-common.yaml > > > > diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml > > new file mode 100644 > > index 000000000000..72e24b246040 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml > > @@ -0,0 +1,117 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/ata/ahci-common.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Common Properties for Serial ATA AHCI controllers > > + > > +maintainers: > > + - Hans de Goede <hdegoede@xxxxxxxxxx> > > + - Jens Axboe <axboe@xxxxxxxxx> > > This should probably be me rather than Jens since Jens is no longer maintaining > libata/ahci. Ok. I'll replace his email address with yours in v3. -Sergey > > > + > > +description: > > + This document defines device tree properties for a common AHCI SATA > > + controller implementation. It's hardware interface is supposed to > > + conform to the technical standard defined by Intel (see Serial ATA > > + Advanced Host Controller Interface specification for details). The > > + document doesn't constitute a DT-node binding by itself but merely > > + defines a set of common properties for the AHCI-compatible devices. > > + > > +select: false > > + > > +allOf: > > + - $ref: sata-common.yaml# > > + > > +properties: > > + reg: > > + description: > > + Generic AHCI registers space conforming to the Serial ATA AHCI > > + specification. > > + > > + reg-names: > > + description: CSR space IDs > > + > > + interrupts: > > + description: > > + Generic AHCI state change interrupt. Can be implemented either as a > > + single line attached to the controller as a set of the dedicated signals > > + for the global and particular port events. > > + > > + clocks: > > + description: > > + List of all the reference clocks connected to the controller. > > + > > + clock-names: > > + description: Reference clocks IDs > > + > > + resets: > > + description: > > + List of the reset control lines to reset the controller clock > > + domains. > > + > > + reset-names: > > + description: Reset line IDs > > + > > + power-domains: > > + description: > > + List of the power domain the AHCI controller being a part of. > > + > > + ahci-supply: > > + description: Power regulator for AHCI controller > > + > > + target-supply: > > + description: Power regulator for SATA target device > > + > > + phy-supply: > > + description: Power regulator for SATA PHY > > + > > + phys: > > + description: Reference to the SATA PHY node > > + maxItems: 1 > > + > > + phy-names: > > + maxItems: 1 > > + > > + ports-implemented: > > + $ref: '/schemas/types.yaml#/definitions/uint32' > > + description: > > + Mask that indicates which ports the HBA supports. Useful if PI is not > > + programmed by the BIOS, which is true for some embedded SoC's. > > + maximum: 0x1f > > + > > +patternProperties: > > + "^sata-port@[0-9a-f]+$": > > + type: object > > + description: > > + It is optionally possible to describe the ports as sub-nodes so > > + to enable each port independently when dealing with multiple PHYs. > > + > > + properties: > > + reg: > > + description: AHCI SATA port identifier > > + maxItems: 1 > > + > > + phys: > > + description: Individual AHCI SATA port PHY > > + maxItems: 1 > > + > > + phy-names: > > + description: AHCI SATA port PHY ID > > + maxItems: 1 > > + > > + target-supply: > > + description: Power regulator for SATA port target device > > + > > + required: > > + - reg > > + > > + additionalProperties: true > > + > > +required: > > + - reg > > + - interrupts > > + > > +additionalProperties: true > > + > > +... > > diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.yaml b/Documentation/devicetree/bindings/ata/ahci-platform.yaml > > index 9304e4731965..76075d3c8987 100644 > > --- a/Documentation/devicetree/bindings/ata/ahci-platform.yaml > > +++ b/Documentation/devicetree/bindings/ata/ahci-platform.yaml > > @@ -36,8 +36,7 @@ select: > > - compatible > > > > allOf: > > - - $ref: "sata-common.yaml#" > > - > > + - $ref: "ahci-common.yaml#" > > > > properties: > > compatible: > > @@ -69,90 +68,35 @@ properties: > > maxItems: 1 > > > > clocks: > > - description: > > - Clock IDs array as required by the controller. > > minItems: 1 > > maxItems: 3 > > > > clock-names: > > - description: > > - Names of clocks corresponding to IDs in the clock property. > > minItems: 1 > > maxItems: 3 > > > > interrupts: > > maxItems: 1 > > > > - ahci-supply: > > - description: > > - regulator for AHCI controller > > - > > - phy-supply: > > - description: > > - regulator for PHY power > > - > > - phys: > > - description: > > - List of all PHYs on this controller > > - maxItems: 1 > > - > > - phy-names: > > - description: > > - Name specifier for the PHYs > > - maxItems: 1 > > - > > - ports-implemented: > > - $ref: '/schemas/types.yaml#/definitions/uint32' > > - description: | > > - Mask that indicates which ports that the HBA supports > > - are available for software to use. Useful if PORTS_IMPL > > - is not programmed by the BIOS, which is true with > > - some embedded SoCs. > > - maximum: 0x1f > > - > > power-domains: > > maxItems: 1 > > > > resets: > > maxItems: 1 > > > > - target-supply: > > - description: > > - regulator for SATA target power > > - > > -required: > > - - compatible > > - - reg > > - - interrupts > > - > > patternProperties: > > "^sata-port@[0-9a-f]+$": > > type: object > > - additionalProperties: false > > - description: > > - Subnode with configuration of the Ports. > > - > > - properties: > > - reg: > > - maxItems: 1 > > - > > - phys: > > - maxItems: 1 > > - > > - phy-names: > > - maxItems: 1 > > - > > - target-supply: > > - description: > > - regulator for SATA target power > > - > > - required: > > - - reg > > > > anyOf: > > - required: [ phys ] > > - required: [ target-supply ] > > > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > unevaluatedProperties: false > > > > examples: > > > -- > Damien Le Moal > Western Digital Research