diff mbox series

[v4,1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

Message ID 20200824151035.31093-2-lars.povlsen@microchip.com (mailing list archive)
State Superseded
Headers show
Series mmc: Adding support for Microchip Sparx5 SoC | expand

Commit Message

Lars Povlsen Aug. 24, 2020, 3:10 p.m. UTC 
The Sparx5 SDHCI controller is based on the Designware controller IP.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../mmc/microchip,dw-sparx5-sdhci.yaml        | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml

--
2.27.0

Comments

Ulf Hansson Aug. 25, 2020, 7:33 a.m. UTC | #1 
On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <lars.povlsen@microchip.com> wrote:
>
> The Sparx5 SDHCI controller is based on the Designware controller IP.
>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> new file mode 100644
> index 0000000000000..55883290543b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip Sparx5 Mobile Storage Host Controller Binding
> +
> +allOf:
> +  - $ref: "mmc-controller.yaml"
> +
> +maintainers:
> +  - Lars Povlsen <lars.povlsen@microchip.com>
> +
> +# Everything else is described in the common file
> +properties:
> +  compatible:
> +    const: microchip,dw-sparx5-sdhci
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Handle to "core" clock for the sdhci controller.
> +
> +  clock-names:
> +    items:
> +      - const: core
> +
> +  microchip,clock-delay:
> +    description: Delay clock to card to meet setup time requirements.
> +      Each step increase by 1.25ns.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 1
> +    maximum: 15
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/microchip,sparx5.h>
> +    sdhci0: mmc@600800000 {

Nitpick:

I think we should use solely "mmc[n]" here. So:

mmc0@600800000 {

Please update patch3/3 accordingly as well.

> +        compatible = "microchip,dw-sparx5-sdhci";
> +        reg = <0x00800000 0x1000>;
> +        pinctrl-0 = <&emmc_pins>;
> +        pinctrl-names = "default";
> +        clocks = <&clks CLK_ID_AUX1>;
> +        clock-names = "core";
> +        assigned-clocks = <&clks CLK_ID_AUX1>;
> +        assigned-clock-rates = <800000000>;
> +        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +        bus-width = <8>;
> +        microchip,clock-delay = <10>;
> +    };

Kind regards
Uffe
Alexandre Belloni Aug. 25, 2020, 8:47 a.m. UTC | #2 
On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
> On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <lars.povlsen@microchip.com> wrote:
> >
> > The Sparx5 SDHCI controller is based on the Designware controller IP.
> >
> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> > ---
> >  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 65 +++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> > new file mode 100644
> > index 0000000000000..55883290543b9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip Sparx5 Mobile Storage Host Controller Binding
> > +
> > +allOf:
> > +  - $ref: "mmc-controller.yaml"
> > +
> > +maintainers:
> > +  - Lars Povlsen <lars.povlsen@microchip.com>
> > +
> > +# Everything else is described in the common file
> > +properties:
> > +  compatible:
> > +    const: microchip,dw-sparx5-sdhci
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description:
> > +      Handle to "core" clock for the sdhci controller.
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
> > +
> > +  microchip,clock-delay:
> > +    description: Delay clock to card to meet setup time requirements.
> > +      Each step increase by 1.25ns.
> > +    $ref: "/schemas/types.yaml#/definitions/uint32"
> > +    minimum: 1
> > +    maximum: 15
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/microchip,sparx5.h>
> > +    sdhci0: mmc@600800000 {
> 
> Nitpick:
> 
> I think we should use solely "mmc[n]" here. So:
> 
> mmc0@600800000 {
> 
> Please update patch3/3 accordingly as well.

This is not what the devicetree specification says. 2.2.2 says that the
generic name is mmc, not mmc[n]. Since there is a proper unit-address, I
don't see the need for an index here.

> 
> > +        compatible = "microchip,dw-sparx5-sdhci";
> > +        reg = <0x00800000 0x1000>;
> > +        pinctrl-0 = <&emmc_pins>;
> > +        pinctrl-names = "default";
> > +        clocks = <&clks CLK_ID_AUX1>;
> > +        clock-names = "core";
> > +        assigned-clocks = <&clks CLK_ID_AUX1>;
> > +        assigned-clock-rates = <800000000>;
> > +        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> > +        bus-width = <8>;
> > +        microchip,clock-delay = <10>;
> > +    };
> 
> Kind regards
> Uffe
Ulf Hansson Aug. 25, 2020, 9:25 a.m. UTC | #3 
On Tue, 25 Aug 2020 at 10:47, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
> > On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <lars.povlsen@microchip.com> wrote:
> > >
> > > The Sparx5 SDHCI controller is based on the Designware controller IP.
> > >
> > > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> > > ---
> > >  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 65 +++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> > > new file mode 100644
> > > index 0000000000000..55883290543b9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> > > @@ -0,0 +1,65 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Microchip Sparx5 Mobile Storage Host Controller Binding
> > > +
> > > +allOf:
> > > +  - $ref: "mmc-controller.yaml"
> > > +
> > > +maintainers:
> > > +  - Lars Povlsen <lars.povlsen@microchip.com>
> > > +
> > > +# Everything else is described in the common file
> > > +properties:
> > > +  compatible:
> > > +    const: microchip,dw-sparx5-sdhci
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +    description:
> > > +      Handle to "core" clock for the sdhci controller.
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: core
> > > +
> > > +  microchip,clock-delay:
> > > +    description: Delay clock to card to meet setup time requirements.
> > > +      Each step increase by 1.25ns.
> > > +    $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +    minimum: 1
> > > +    maximum: 15
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    #include <dt-bindings/clock/microchip,sparx5.h>
> > > +    sdhci0: mmc@600800000 {
> >
> > Nitpick:
> >
> > I think we should use solely "mmc[n]" here. So:
> >
> > mmc0@600800000 {
> >
> > Please update patch3/3 accordingly as well.
>
> This is not what the devicetree specification says. 2.2.2 says that the
> generic name is mmc, not mmc[n]. Since there is a proper unit-address, I
> don't see the need for an index here.

You are absolutely right, thanks!

My apologies for the noise!

>
> >
> > > +        compatible = "microchip,dw-sparx5-sdhci";
> > > +        reg = <0x00800000 0x1000>;
> > > +        pinctrl-0 = <&emmc_pins>;
> > > +        pinctrl-names = "default";
> > > +        clocks = <&clks CLK_ID_AUX1>;
> > > +        clock-names = "core";
> > > +        assigned-clocks = <&clks CLK_ID_AUX1>;
> > > +        assigned-clock-rates = <800000000>;
> > > +        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> > > +        bus-width = <8>;
> > > +        microchip,clock-delay = <10>;
> > > +    };



Kind regards
Uffe
Lars Povlsen Aug. 25, 2020, 9:35 a.m. UTC | #4 
Alexandre Belloni writes:

> On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
>> On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <lars.povlsen@microchip.com> wrote:
>> >
>> > The Sparx5 SDHCI controller is based on the Designware controller IP.
>> >
>> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> > ---
>> >  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 65 +++++++++++++++++++
>> >  1 file changed, 65 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>> >
>> > diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>> > new file mode 100644
>> > index 0000000000000..55883290543b9
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>> > @@ -0,0 +1,65 @@
>> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> > +%YAML 1.2
>> > +---
>> > +$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
>> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > +
>> > +title: Microchip Sparx5 Mobile Storage Host Controller Binding
>> > +
>> > +allOf:
>> > +  - $ref: "mmc-controller.yaml"
>> > +
>> > +maintainers:
>> > +  - Lars Povlsen <lars.povlsen@microchip.com>
>> > +
>> > +# Everything else is described in the common file
>> > +properties:
>> > +  compatible:
>> > +    const: microchip,dw-sparx5-sdhci
>> > +
>> > +  reg:
>> > +    maxItems: 1
>> > +
>> > +  interrupts:
>> > +    maxItems: 1
>> > +
>> > +  clocks:
>> > +    maxItems: 1
>> > +    description:
>> > +      Handle to "core" clock for the sdhci controller.
>> > +
>> > +  clock-names:
>> > +    items:
>> > +      - const: core
>> > +
>> > +  microchip,clock-delay:
>> > +    description: Delay clock to card to meet setup time requirements.
>> > +      Each step increase by 1.25ns.
>> > +    $ref: "/schemas/types.yaml#/definitions/uint32"
>> > +    minimum: 1
>> > +    maximum: 15
>> > +
>> > +required:
>> > +  - compatible
>> > +  - reg
>> > +  - interrupts
>> > +  - clocks
>> > +  - clock-names
>> > +
>> > +examples:
>> > +  - |
>> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> > +    #include <dt-bindings/clock/microchip,sparx5.h>
>> > +    sdhci0: mmc@600800000 {
>>
>> Nitpick:
>>
>> I think we should use solely "mmc[n]" here. So:
>>
>> mmc0@600800000 {
>>
>> Please update patch3/3 accordingly as well.
>
> This is not what the devicetree specification says. 2.2.2 says that the
> generic name is mmc, not mmc[n]. Since there is a proper unit-address, I
> don't see the need for an index here.
>

Alex,

Yeah, I thought so as well - and the existing DTs have practically all
variations..

Nevertheless, I followed suit since I had to refresh the patch set
anyhow.

Cheers,

---Lars
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
new file mode 100644
index 0000000000000..55883290543b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
@@ -0,0 +1,65 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Sparx5 Mobile Storage Host Controller Binding
+
+allOf:
+  - $ref: "mmc-controller.yaml"
+
+maintainers:
+  - Lars Povlsen <lars.povlsen@microchip.com>
+
+# Everything else is described in the common file
+properties:
+  compatible:
+    const: microchip,dw-sparx5-sdhci
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Handle to "core" clock for the sdhci controller.
+
+  clock-names:
+    items:
+      - const: core
+
+  microchip,clock-delay:
+    description: Delay clock to card to meet setup time requirements.
+      Each step increase by 1.25ns.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    minimum: 1
+    maximum: 15
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/microchip,sparx5.h>
+    sdhci0: mmc@600800000 {
+        compatible = "microchip,dw-sparx5-sdhci";
+        reg = <0x00800000 0x1000>;
+        pinctrl-0 = <&emmc_pins>;
+        pinctrl-names = "default";
+        clocks = <&clks CLK_ID_AUX1>;
+        clock-names = "core";
+        assigned-clocks = <&clks CLK_ID_AUX1>;
+        assigned-clock-rates = <800000000>;
+        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+        bus-width = <8>;
+        microchip,clock-delay = <10>;
+    };