diff mbox series

[v2,02/23] dt-bindings: ata: ahci-platform: Detach common AHCI bindings

Message ID 20220503200938.18027-3-Sergey.Semin@baikalelectronics.ru
State New
Headers show
Series ata: ahci: Add DWC/Baikal-T1 AHCI SATA support | expand

Commit Message

Serge Semin May 3, 2022, 8:09 p.m. UTC
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@baikalelectronics.ru>

---

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

Comments

Damien Le Moal May 3, 2022, 8:29 p.m. UTC | #1
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@baikalelectronics.ru>
> 
> ---
> 
> 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@redhat.com>
> +  - Jens Axboe <axboe@kernel.dk>

This should probably be me rather than Jens since Jens is no longer maintaining
libata/ahci.

> +
> +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:
Serge Semin May 3, 2022, 8:43 p.m. UTC | #2
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@baikalelectronics.ru>
> > 
> > ---
> > 
> > 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@redhat.com>
> > +  - Jens Axboe <axboe@kernel.dk>
> 

> 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
diff mbox series

Patch

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@redhat.com>
+  - Jens Axboe <axboe@kernel.dk>
+
+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: