2020-08-24 15:13:18

by Lars Povlsen

[permalink] [raw]
Subject: [PATCH v4 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

The Sparx5 SDHCI controller is based on the Designware controller IP.

Signed-off-by: Lars Povlsen <[email protected]>
---
.../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 <[email protected]>
+
+# 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>;
+ };
--
2.27.0


2020-08-25 07:37:13

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <[email protected]> wrote:
>
> The Sparx5 SDHCI controller is based on the Designware controller IP.
>
> Signed-off-by: Lars Povlsen <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +# 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

2020-08-25 09:08:56

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
> On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <[email protected]> wrote:
> >
> > The Sparx5 SDHCI controller is based on the Designware controller IP.
> >
> > Signed-off-by: Lars Povlsen <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +# 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

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-08-25 09:37:03

by Lars Povlsen

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings


Alexandre Belloni writes:

> On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
>> On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <[email protected]> wrote:
>> >
>> > The Sparx5 SDHCI controller is based on the Designware controller IP.
>> >
>> > Signed-off-by: Lars Povlsen <[email protected]>
>> > ---
>> > .../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 <[email protected]>
>> > +
>> > +# 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

--
Lars Povlsen,
Microchip

2020-08-25 12:16:48

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

On Tue, 25 Aug 2020 at 10:47, Alexandre Belloni
<[email protected]> wrote:
>
> On 25/08/2020 09:33:45+0200, Ulf Hansson wrote:
> > On Mon, 24 Aug 2020 at 17:10, Lars Povlsen <[email protected]> wrote:
> > >
> > > The Sparx5 SDHCI controller is based on the Designware controller IP.
> > >
> > > Signed-off-by: Lars Povlsen <[email protected]>
> > > ---
> > > .../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 <[email protected]>
> > > +
> > > +# 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