On Thu, May 12, 2022 at 08:19:34AM +0200, Hannes Reinecke wrote: > On 5/12/22 01:17, 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. > > > > Changelog v3: > > - Replace Jens's email address with Damien's one in the list of the > > schema maintainers. (@Damien) > > --- > > .../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..620042ca12e7 > > --- /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> > > + - Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> > > + > > +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#" > > What happened to 'sata-common.yaml' ? Nothing. It's still relevant for some devices. > Not needed anymore? Included via other means? Included by the SATA-compatible devices. Mainly that schema is relevant to the devices which aren't AHCI-compatible. > > Please clarify. sata-common.yaml is a common schema for the SATA devices (including AHCI-devices), while ahci-common.yaml is a common schema for the AHCI-compatible devices. So the later is more restrictive, than the former one. The SATA DT-indings can be used by the AHCI-incompatible devices for instance by the ones handled in the drivers drivers/ata/sata_*.c. The AHCI DT-schema can be used by the AHCI-enabled platforms like described in the LLDDs drivers/ata/ahci_*. This means if your device is based on AHCI, then its bindings should refer to the ahci-common.yaml schema and specify the platform-specific DT-bindings: new properties and common properties constraints. If it isn't AHCI-compatible but is a Serial ATA device, then it's bindings should refer to the sata-common.yaml schema (which is much less restrictive and defines just some small set of the properties). For instance the brcm,sata-brcm.yaml DT-bindings can be converted to using the ahci-common.yaml schema instead of the sata-common.yaml schema, while the renesas,rcar-sata.yaml bindings can refer to the sata-common.yaml schema thus for instance restricting the DT-node name to be 'sata' for which in the current RCAR SATA bindings there is no relevant constraints. -Sergey > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Kernel Storage Architect > hare@xxxxxxx +49 911 74053 688 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg > HRB 36809 (AG Nürnberg), GF: Felix Imendörffer