[PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Linus Walleij]

I need to create subnodes for drives connected to PATA
or SATA host controllers, and this needs to be supported
generally, so create a common YAML binding for
"pata-controller" or "sata-controller" that will support
subnodes with drives.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../bindings/ata/pata-sata-common.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/pata-sata-common.yaml

diff --git a/Documentation/devicetree/bindings/ata/pata-sata-common.yaml b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
new file mode 100644
index 000000000000..d94aa20a29e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/pata-sata-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for Parallel and Serial AT attachment controllers
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  This document defines device tree properties common to most Parallel
+  (PATA) and Serial (SATA) AT attachment storage devices. It doesn't
+  constitue a device tree binding specification by itself but is meant to
+  be referenced by device tree bindings.
+
+  The PATA/SATA controller device tree bindings are responsible for
+  defining whether each property is required or optional.
+
+properties:
+  $nodename:
+    pattern: "^[ps]ata-controller(@.*)?$"
+    description:
+      Specifies the host controller node.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^drive@[0-1]$":
+    description: |
+      DT nodes for drives connected on the PATA or SATA host. The master drive
+      will have ID number 0 and the slave drive will have ID number 1.
+    type: object
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 1
+        description:
+          The ID number of the drive, 0 for the master and 1 for the slave.
+
+...
-- 
2.21.0

[PATCH 2/2] dt-bindings: Convert Faraday FTIDE010 to DT schema

[Linus Walleij]

This uses the new pata-sata-controller.yaml schema to
convert the Faraday FTIDE010 to DT schema.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: device@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../bindings/ata/faraday,ftide010.txt         | 38 --------
 .../bindings/ata/faraday,ftide010.yaml        | 89 +++++++++++++++++++
 2 files changed, 89 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/ata/faraday,ftide010.txt
 create mode 100644 Documentation/devicetree/bindings/ata/faraday,ftide010.yaml

diff --git a/Documentation/devicetree/bindings/ata/faraday,ftide010.txt b/Documentation/devicetree/bindings/ata/faraday,ftide010.txt
deleted file mode 100644
index a0c64a29104d..000000000000
--- a/Documentation/devicetree/bindings/ata/faraday,ftide010.txt
+++ /dev/null
@@ -1,38 +0,0 @@
-* Faraday Technology FTIDE010 PATA controller
-
-This controller is the first Faraday IDE interface block, used in the
-StorLink SL2312 and SL3516, later known as the Cortina Systems Gemini
-platform. The controller can do PIO modes 0 through 4, Multi-word DMA
-(MWDM)modes 0 through 2 and Ultra DMA modes 0 through 6.
-
-On the Gemini platform, this PATA block is accompanied by a PATA to
-SATA bridge in order to support SATA. This is why a phandle to that
-controller is compulsory on that platform.
-
-The timing properties are unique per-SoC, not per-board.
-
-Required properties:
-- compatible: should be one of
-  "cortina,gemini-pata", "faraday,ftide010"
-  "faraday,ftide010"
-- interrupts: interrupt for the block
-- reg: registers and size for the block
-
-Optional properties:
-- clocks: a SoC clock running the peripheral.
-- clock-names: should be set to "PCLK" for the peripheral clock.
-
-Required properties for "cortina,gemini-pata" compatible:
-- sata: a phande to the Gemini PATA to SATA bridge, see
-  cortina,gemini-sata-bridge.txt for details.
-
-Example:
-
-ata@63000000 {
-	compatible = "cortina,gemini-pata", "faraday,ftide010";
-	reg = <0x63000000 0x100>;
-	interrupts = <4 IRQ_TYPE_EDGE_RISING>;
-	clocks = <&gcc GEMINI_CLK_GATE_IDE>;
-	clock-names = "PCLK";
-	sata = <&sata>;
-};
diff --git a/Documentation/devicetree/bindings/ata/faraday,ftide010.yaml b/Documentation/devicetree/bindings/ata/faraday,ftide010.yaml
new file mode 100644
index 000000000000..65be218382e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/faraday,ftide010.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/faraday,ftide010.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Faraday Technology FTIDE010 PATA controller
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  This controller is the first Faraday IDE interface block, used in the
+  StorLink SL3512 and SL3516, later known as the Cortina Systems Gemini
+  platform. The controller can do PIO modes 0 through 4, Multi-word DMA
+  (MWDM) modes 0 through 2 and Ultra DMA modes 0 through 6.
+
+  On the Gemini platform, this PATA block is accompanied by a PATA to
+  SATA bridge in order to support SATA. This is why a phandle to that
+  controller is compulsory on that platform.
+
+  The timing properties are unique per-SoC, not per-board.
+
+properties:
+  compatible:
+    oneOf:
+      - const: faraday,ftide010
+      - items:
+        - const: cortina,gemini-pata
+        - const: faraday,ftide010
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+
+  clock-names:
+    const: PCLK
+
+  sata:
+    description:
+      phandle to the Gemini PATA to SATA bridge, if available
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - $ref: pata-sata-common.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: cortina,gemini-pata
+
+    then:
+      required:
+        - sata
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/cortina,gemini-clock.h>
+
+    pata-controller@63000000 {
+      compatible = "cortina,gemini-pata", "faraday,ftide010";
+      reg = <0x63000000 0x100>;
+      interrupts = <4 IRQ_TYPE_EDGE_RISING>;
+      clocks = <&gcc GEMINI_CLK_GATE_IDE>;
+      clock-names = "PCLK";
+      sata = <&sata>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      drive@0 {
+        reg = <0>;
+      };
+      drive@1 {
+        reg = <1>;
+      };
+    };
+
+...
-- 
2.21.0

Re: [PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Sergei Shtylyov]

Hello!

On 12/31/2019 08:24 PM, Linus Walleij wrote:

> I need to create subnodes for drives connected to PATA
> or SATA host controllers, and this needs to be supported
> generally, so create a common YAML binding for
> "pata-controller" or "sata-controller" that will support
> subnodes with drives.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/ata/pata-sata-common.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/pata-sata-common.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ata/pata-sata-common.yaml b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
> new file mode 100644
> index 000000000000..d94aa20a29e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/pata-sata-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for Parallel and Serial AT attachment controllers
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  This document defines device tree properties common to most Parallel
> +  (PATA) and Serial (SATA) AT attachment storage devices. It doesn't
> +  constitue a device tree binding specification by itself but is meant to
> +  be referenced by device tree bindings.
> +
> +  The PATA/SATA controller device tree bindings are responsible for
> +  defining whether each property is required or optional.
> +
> +properties:
> +  $nodename:
> +    pattern: "^[ps]ata-controller(@.*)?$"

   I thought the DT spec has long ago standardized the node name as "ide" and "sata"?

> +    description:
> +      Specifies the host controller node.
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
[...]

MBR, Sergei

Re: [PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Linus Walleij]

On Tue, Dec 31, 2019 at 6:46 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/31/2019 08:24 PM, Linus Walleij wrote:

> > +properties:
> > +  $nodename:
> > +    pattern: "^[ps]ata-controller(@.*)?$"
>
>    I thought the DT spec has long ago standardized the node name as "ide" and "sata"?

I'm not 100% sure (Rob can answer) but I have *half-guessed* that the process
so far has been that when we define a schema for some generic node name
we also invent some new generic node name like this, or else all the old DTS
files people have will generate (lots of) errors.

The DTS files can then opt in to the schema by simply renaming its nodes
in accordance: "ide" -> "pata-controller", "sata" -> "sata-controller"
(I have such a patch for the Gemini that I made this for.)

If I'm wrong I'll switch to to ide@ and sata@.

Rob?

Yours,
Linus Walleij

Re: [PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Rob Herring]

On Tue, Dec 31, 2019 at 10:56 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Dec 31, 2019 at 6:46 PM Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > On 12/31/2019 08:24 PM, Linus Walleij wrote:
>
> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^[ps]ata-controller(@.*)?$"
> >
> >    I thought the DT spec has long ago standardized the node name as "ide" and "sata"?

Correct.

> I'm not 100% sure (Rob can answer) but I have *half-guessed* that the process
> so far has been that when we define a schema for some generic node name
> we also invent some new generic node name like this, or else all the old DTS
> files people have will generate (lots of) errors.

The list of generic node names is a lot longer than what we have
schema for currently.

> The DTS files can then opt in to the schema by simply renaming its nodes
> in accordance: "ide" -> "pata-controller", "sata" -> "sata-controller"
> (I have such a patch for the Gemini that I made this for.)
>
> If I'm wrong I'll switch to to ide@ and sata@.
>
> Rob?
>
> Yours,
> Linus Walleij

Re: [PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Rob Herring]

On Tue, Dec 31, 2019 at 10:25 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> I need to create subnodes for drives connected to PATA
> or SATA host controllers, and this needs to be supported
> generally, so create a common YAML binding for
> "pata-controller" or "sata-controller" that will support
> subnodes with drives.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/ata/pata-sata-common.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/pata-sata-common.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/pata-sata-common.yaml b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
> new file mode 100644
> index 000000000000..d94aa20a29e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/pata-sata-common.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/pata-sata-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for Parallel and Serial AT attachment controllers
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  This document defines device tree properties common to most Parallel
> +  (PATA) and Serial (SATA) AT attachment storage devices. It doesn't
> +  constitue a device tree binding specification by itself but is meant to
> +  be referenced by device tree bindings.
> +
> +  The PATA/SATA controller device tree bindings are responsible for
> +  defining whether each property is required or optional.
> +
> +properties:
> +  $nodename:
> +    pattern: "^[ps]ata-controller(@.*)?$"
> +    description:
> +      Specifies the host controller node.
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^drive@[0-1]$":
> +    description: |
> +      DT nodes for drives connected on the PATA or SATA host. The master drive
> +      will have ID number 0 and the slave drive will have ID number 1.
> +    type: object

See ata/ahci-platform.txt. We already have child nodes defined as
ports which is essentially a drive (though IIRC SATA can have a mux).
Arguably, we could have both ports and then a drive child under that,
but I've never seen a case of a drive needing DT properties (that
would imply a non-standard connector). So either split this into
separate IDE/PATA and SATA schemas or extend this to work with SATA
ports.

Also, you might consider if you need a regulator property for IDE.

> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 1
> +        description:
> +          The ID number of the drive, 0 for the master and 1 for the slave.

Why do you need to describe the drives in DT for IDE? They are
discoverable, right? And unlike SATA, the power to master and slave is
shared.

Rob

Re: [PATCH 2/2] dt-bindings: Convert Faraday FTIDE010 to DT schema

[Rob Herring]

On Tue, Dec 31, 2019 at 10:25 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This uses the new pata-sata-controller.yaml schema to
> convert the Faraday FTIDE010 to DT schema.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: device@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/ata/faraday,ftide010.txt         | 38 --------
>  .../bindings/ata/faraday,ftide010.yaml        | 89 +++++++++++++++++++
>  2 files changed, 89 insertions(+), 38 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/ata/faraday,ftide010.txt
>  create mode 100644 Documentation/devicetree/bindings/ata/faraday,ftide010.yaml

LGTM. Will wait for new version of patch 1.

Rob

Re: [PATCH 1/2] dt-bindings: Create DT bindings for [PS]ATA controllers

[Linus Walleij]

Hi Rob,

thanks for the feedback, I am fixing up the patches with some
hammering and tinkering. Just answering this question:

On Tue, Dec 31, 2019 at 7:44 PM Rob Herring <robh+dt@kernel.org> wrote:

> Why do you need to describe the drives in DT for IDE? They are
> discoverable, right? And unlike SATA, the power to master and slave is
> shared.

It is because of a related patch set by Guenter Roeck where we
add thermal zones to the thermal sensors found in the drives.

To associate these with a thermal zone as defined in device tree,
I need a handle on the drive.

See my mail to hwmon for an example:
https://lore.kernel.org/linux-hwmon/CACRpkdb8rehAPKE2Zu-Jf4TSE2m6ks91vZdrVy+HitijabeVbg@mail.gmail.com/

Yours,
Linus Walleij