| Message ID | 20190712033214.24713-2-andrew@aj.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: Add support for the ASPEED SD controller | expand |
On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > data bus if only a single slot is enabled. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > In v2: > > * Rename to aspeed,sdhci.yaml > * Rename sd-controller compatible > * Add `maxItems: 1` for reg properties > * Move sdhci subnode description to patternProperties > * Drop sdhci compatible requirement > * #address-cells and #size-cells are required > * Prevent additional properties > * Implement explicit ranges in example > * Remove slot property > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml Reviewed-by: Rob Herring <robh@kernel.org>
Hi, On Fri, Jul 12, 2019 at 01:02:13PM +0930, Andrew Jeffery wrote: > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > data bus if only a single slot is enabled. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > In v2: > > * Rename to aspeed,sdhci.yaml > * Rename sd-controller compatible > * Add `maxItems: 1` for reg properties > * Move sdhci subnode description to patternProperties > * Drop sdhci compatible requirement > * #address-cells and #size-cells are required > * Prevent additional properties > * Implement explicit ranges in example > * Remove slot property > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > new file mode 100644 > index 000000000000..67a691c3348c > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ASPEED SD/SDIO/eMMC Controller > + > +maintainers: > + - Andrew Jeffery <andrew@aj.id.au> > + - Ryan Chen <ryanchen.aspeed@gmail.com> > + > +description: |+ > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > + only a single slot is enabled. > + > + The two slots are supported by a common configuration area. As the SDHCIs for > + the slots are dependent on the common configuration area, they are described > + as child nodes. > + > +properties: > + compatible: > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > + reg: > + maxItems: 1 > + description: Common configuration registers > + ranges: true > + clocks: > + maxItems: 1 > + description: The SD/SDIO controller clock gate #address-cells and #size-cells have not been described here. > +patternProperties: > + "^sdhci@[0-9a-f]+$": > + type: object > + properties: > + compatible: > + enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ] > + reg: > + maxItems: 1 > + description: The SDHCI registers > + clocks: > + maxItems: 1 > + description: The SD bus clock > + interrupts: > + maxItems: 1 > + description: The SD interrupt shared between both slots > + required: > + - compatible > + - reg > + - clocks > + - interrupts > + > +additionalProperties: false But that means that it will generate a warning in your DT if you ever use them. > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + - ranges > + - clocks > + > +examples: > + - | > + #include <dt-bindings/clock/aspeed-clock.h> > + sdc@1e740000 { > + compatible = "aspeed,ast2500-sd-controller"; > + reg = <0x1e740000 0x100>; > + #address-cells = <1>; > + #size-cells = <1>; Starting with your example. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Fri, 12 Jul 2019, at 22:41, Maxime Ripard wrote: > Hi, > > On Fri, Jul 12, 2019 at 01:02:13PM +0930, Andrew Jeffery wrote: > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > data bus if only a single slot is enabled. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > In v2: > > > > * Rename to aspeed,sdhci.yaml > > * Rename sd-controller compatible > > * Add `maxItems: 1` for reg properties > > * Move sdhci subnode description to patternProperties > > * Drop sdhci compatible requirement > > * #address-cells and #size-cells are required > > * Prevent additional properties > > * Implement explicit ranges in example > > * Remove slot property > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > 1 file changed, 90 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > new file mode 100644 > > index 000000000000..67a691c3348c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > @@ -0,0 +1,90 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: ASPEED SD/SDIO/eMMC Controller > > + > > +maintainers: > > + - Andrew Jeffery <andrew@aj.id.au> > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > + > > +description: |+ > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > + only a single slot is enabled. > > + > > + The two slots are supported by a common configuration area. As the SDHCIs for > > + the slots are dependent on the common configuration area, they are described > > + as child nodes. > > + > > +properties: > > + compatible: > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > + reg: > > + maxItems: 1 > > + description: Common configuration registers > > + ranges: true > > + clocks: > > + maxItems: 1 > > + description: The SD/SDIO controller clock gate > > #address-cells and #size-cells have not been described here. > > > +patternProperties: > > + "^sdhci@[0-9a-f]+$": > > + type: object > > + properties: > > + compatible: > > + enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ] > > + reg: > > + maxItems: 1 > > + description: The SDHCI registers > > + clocks: > > + maxItems: 1 > > + description: The SD bus clock > > + interrupts: > > + maxItems: 1 > > + description: The SD interrupt shared between both slots > > + required: > > + - compatible > > + - reg > > + - clocks > > + - interrupts > > + > > +additionalProperties: false > > But that means that it will generate a warning in your DT if you ever > use them. > > > +required: > > + - compatible > > + - reg > > + - "#address-cells" > > + - "#size-cells" > > + - ranges > > + - clocks > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/aspeed-clock.h> > > + sdc@1e740000 { > > + compatible = "aspeed,ast2500-sd-controller"; > > + reg = <0x1e740000 0x100>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > Starting with your example. Heh, right. Thanks. I was inspecting the output of the `dt_binding_check` and `dtbs_check` make targets, though maybe I overlooked this. The aspeed dtsis do generate a quite a number of warnings which make it hard to parse, so I'm going to send a series to clean that up too. Andrew > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > Attachments: > * signature.asc
On Sun, Jul 14, 2019 at 8:30 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > On Fri, 12 Jul 2019, at 22:41, Maxime Ripard wrote: > > Hi, > > > > On Fri, Jul 12, 2019 at 01:02:13PM +0930, Andrew Jeffery wrote: > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > > data bus if only a single slot is enabled. > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > --- > > > In v2: > > > > > > * Rename to aspeed,sdhci.yaml > > > * Rename sd-controller compatible > > > * Add `maxItems: 1` for reg properties > > > * Move sdhci subnode description to patternProperties > > > * Drop sdhci compatible requirement > > > * #address-cells and #size-cells are required > > > * Prevent additional properties > > > * Implement explicit ranges in example > > > * Remove slot property > > > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > > 1 file changed, 90 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > new file mode 100644 > > > index 000000000000..67a691c3348c > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > @@ -0,0 +1,90 @@ > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: ASPEED SD/SDIO/eMMC Controller > > > + > > > +maintainers: > > > + - Andrew Jeffery <andrew@aj.id.au> > > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > > + > > > +description: |+ > > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > > + only a single slot is enabled. > > > + > > > + The two slots are supported by a common configuration area. As the SDHCIs for > > > + the slots are dependent on the common configuration area, they are described > > > + as child nodes. > > > + > > > +properties: > > > + compatible: > > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > > + reg: > > > + maxItems: 1 > > > + description: Common configuration registers > > > + ranges: true > > > + clocks: > > > + maxItems: 1 > > > + description: The SD/SDIO controller clock gate > > > > #address-cells and #size-cells have not been described here. > > > > > +patternProperties: > > > + "^sdhci@[0-9a-f]+$": > > > + type: object > > > + properties: > > > + compatible: > > > + enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ] > > > + reg: > > > + maxItems: 1 > > > + description: The SDHCI registers > > > + clocks: > > > + maxItems: 1 > > > + description: The SD bus clock > > > + interrupts: > > > + maxItems: 1 > > > + description: The SD interrupt shared between both slots > > > + required: > > > + - compatible > > > + - reg > > > + - clocks > > > + - interrupts > > > + > > > +additionalProperties: false > > > > But that means that it will generate a warning in your DT if you ever > > use them. > > > > > +required: > > > + - compatible > > > + - reg > > > + - "#address-cells" > > > + - "#size-cells" > > > + - ranges > > > + - clocks > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/clock/aspeed-clock.h> > > > + sdc@1e740000 { > > > + compatible = "aspeed,ast2500-sd-controller"; > > > + reg = <0x1e740000 0x100>; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > > Starting with your example. > > Heh, right. Thanks. I was inspecting the output of the `dt_binding_check` and > `dtbs_check` make targets, though maybe I overlooked this. The aspeed dtsis > do generate a quite a number of warnings which make it hard to parse, so I'm > going to send a series to clean that up too. FYI, This will run checks with only the schema file you specify: make dtbs_check DT_SCHEMA_FILES=path/to/schema/file Rob
On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > data bus if only a single slot is enabled. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > In v2: > > * Rename to aspeed,sdhci.yaml > * Rename sd-controller compatible > * Add `maxItems: 1` for reg properties > * Move sdhci subnode description to patternProperties > * Drop sdhci compatible requirement > * #address-cells and #size-cells are required > * Prevent additional properties > * Implement explicit ranges in example > * Remove slot property > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > new file mode 100644 > index 000000000000..67a691c3348c > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ASPEED SD/SDIO/eMMC Controller > + > +maintainers: > + - Andrew Jeffery <andrew@aj.id.au> > + - Ryan Chen <ryanchen.aspeed@gmail.com> > + > +description: |+ > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > + only a single slot is enabled. > + > + The two slots are supported by a common configuration area. As the SDHCIs for > + the slots are dependent on the common configuration area, they are described > + as child nodes. > + > +properties: > + compatible: > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] This is actually a list of 4 strings. Please reformat to 1 per line. Rob
On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote: > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > data bus if only a single slot is enabled. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > In v2: > > > > * Rename to aspeed,sdhci.yaml > > * Rename sd-controller compatible > > * Add `maxItems: 1` for reg properties > > * Move sdhci subnode description to patternProperties > > * Drop sdhci compatible requirement > > * #address-cells and #size-cells are required > > * Prevent additional properties > > * Implement explicit ranges in example > > * Remove slot property > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > 1 file changed, 90 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > new file mode 100644 > > index 000000000000..67a691c3348c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > @@ -0,0 +1,90 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: ASPEED SD/SDIO/eMMC Controller > > + > > +maintainers: > > + - Andrew Jeffery <andrew@aj.id.au> > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > + > > +description: |+ > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > + only a single slot is enabled. > > + > > + The two slots are supported by a common configuration area. As the SDHCIs for > > + the slots are dependent on the common configuration area, they are described > > + as child nodes. > > + > > +properties: > > + compatible: > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > This is actually a list of 4 strings. Please reformat to 1 per line. On reflection that's obvious, but also a somewhat subtle interaction with the preference for no quotes (the obvious caveat being "except where required"). Thanks for pointing it out. I have been running `make dt_binding_check` and `make dtbs_check` over these, looks like I need to up my game a bit though. Do you do additional things in your workflow? Andrew
On Mon, Jul 15, 2019 at 6:36 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote: > > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > > data bus if only a single slot is enabled. > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > --- > > > In v2: > > > > > > * Rename to aspeed,sdhci.yaml > > > * Rename sd-controller compatible > > > * Add `maxItems: 1` for reg properties > > > * Move sdhci subnode description to patternProperties > > > * Drop sdhci compatible requirement > > > * #address-cells and #size-cells are required > > > * Prevent additional properties > > > * Implement explicit ranges in example > > > * Remove slot property > > > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > > 1 file changed, 90 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > new file mode 100644 > > > index 000000000000..67a691c3348c > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > @@ -0,0 +1,90 @@ > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: ASPEED SD/SDIO/eMMC Controller > > > + > > > +maintainers: > > > + - Andrew Jeffery <andrew@aj.id.au> > > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > > + > > > +description: |+ > > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > > + only a single slot is enabled. > > > + > > > + The two slots are supported by a common configuration area. As the SDHCIs for > > > + the slots are dependent on the common configuration area, they are described > > > + as child nodes. > > > + > > > +properties: > > > + compatible: > > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > > > This is actually a list of 4 strings. Please reformat to 1 per line. > > On reflection that's obvious, but also a somewhat subtle interaction with the > preference for no quotes (the obvious caveat being "except where required"). It wasn't something I'd run into before. I'm working on a check, but unfortunately we can only check for quotes not needed and can't check for missing quotes. > Thanks for pointing it out. > > I have been running `make dt_binding_check` and `make dtbs_check` over > these, looks like I need to up my game a bit though. Do you do additional things > in your workflow? That should have thrown the warnings. If you aren't seeing those, do you have dtschema package installed (see Documentation/devicetree/writing-schema.md)? Or it could be erroring out on something else first. There's a few breakages that I'm trying to fix. Rob
On Wed, 17 Jul 2019, at 00:27, Rob Herring wrote: > On Mon, Jul 15, 2019 at 6:36 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote: > > > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > > > data bus if only a single slot is enabled. > > > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > > --- > > > > In v2: > > > > > > > > * Rename to aspeed,sdhci.yaml > > > > * Rename sd-controller compatible > > > > * Add `maxItems: 1` for reg properties > > > > * Move sdhci subnode description to patternProperties > > > > * Drop sdhci compatible requirement > > > > * #address-cells and #size-cells are required > > > > * Prevent additional properties > > > > * Implement explicit ranges in example > > > > * Remove slot property > > > > > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > > > 1 file changed, 90 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > new file mode 100644 > > > > index 000000000000..67a691c3348c > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > @@ -0,0 +1,90 @@ > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: ASPEED SD/SDIO/eMMC Controller > > > > + > > > > +maintainers: > > > > + - Andrew Jeffery <andrew@aj.id.au> > > > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > > > + > > > > +description: |+ > > > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > > > + only a single slot is enabled. > > > > + > > > > + The two slots are supported by a common configuration area. As the SDHCIs for > > > > + the slots are dependent on the common configuration area, they are described > > > > + as child nodes. > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > > > > > This is actually a list of 4 strings. Please reformat to 1 per line. > > > > On reflection that's obvious, but also a somewhat subtle interaction with the > > preference for no quotes (the obvious caveat being "except where required"). > > It wasn't something I'd run into before. I'm working on a check, but > unfortunately we can only check for quotes not needed and can't check > for missing quotes. > > > Thanks for pointing it out. > > > > I have been running `make dt_binding_check` and `make dtbs_check` over > > these, looks like I need to up my game a bit though. Do you do additional things > > in your workflow? > > That should have thrown the warnings. If you aren't seeing those, do > you have dtschema package installed (see > Documentation/devicetree/writing-schema.md)? I do have it installed, but as mentioned previously there's a fair few warnings emitted currently by the Aspeed devicetrees, so it might have got lost in the noise. I've started to clean that up, though probably need some direction there too. Separately I'm currently trying to track down an issue where I get errors on the Aspeed dts cpu nodes about failing to match the riscv CPU compatibles, it seems dt-validate isn't finding the ARM CPU compatible strings. It feels more annoying to track down that I'd like. > Or it could be erroring > out on something else first. There's a few breakages that I'm trying > to fix. Okay. I'll keep an eye on the dt-schema repo. Cheers, Andrew
On Tue, Jul 16, 2019 at 9:58 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > On Wed, 17 Jul 2019, at 00:27, Rob Herring wrote: > > On Mon, Jul 15, 2019 at 6:36 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > > > > > On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote: > > > > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > > > > data bus if only a single slot is enabled. > > > > > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > > > --- > > > > > In v2: > > > > > > > > > > * Rename to aspeed,sdhci.yaml > > > > > * Rename sd-controller compatible > > > > > * Add `maxItems: 1` for reg properties > > > > > * Move sdhci subnode description to patternProperties > > > > > * Drop sdhci compatible requirement > > > > > * #address-cells and #size-cells are required > > > > > * Prevent additional properties > > > > > * Implement explicit ranges in example > > > > > * Remove slot property > > > > > > > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > > > > 1 file changed, 90 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > new file mode 100644 > > > > > index 000000000000..67a691c3348c > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > @@ -0,0 +1,90 @@ > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: ASPEED SD/SDIO/eMMC Controller > > > > > + > > > > > +maintainers: > > > > > + - Andrew Jeffery <andrew@aj.id.au> > > > > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > > > > + > > > > > +description: |+ > > > > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > > > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > > > > + only a single slot is enabled. > > > > > + > > > > > + The two slots are supported by a common configuration area. As the SDHCIs for > > > > > + the slots are dependent on the common configuration area, they are described > > > > > + as child nodes. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > > > > > > > This is actually a list of 4 strings. Please reformat to 1 per line. > > > > > > On reflection that's obvious, but also a somewhat subtle interaction with the > > > preference for no quotes (the obvious caveat being "except where required"). > > > > It wasn't something I'd run into before. I'm working on a check, but > > unfortunately we can only check for quotes not needed and can't check > > for missing quotes. > > > > > Thanks for pointing it out. > > > > > > I have been running `make dt_binding_check` and `make dtbs_check` over > > > these, looks like I need to up my game a bit though. Do you do additional things > > > in your workflow? > > > > That should have thrown the warnings. If you aren't seeing those, do > > you have dtschema package installed (see > > Documentation/devicetree/writing-schema.md)? > > I do have it installed, but as mentioned previously there's a fair few > warnings emitted currently by the Aspeed devicetrees, so it might have > got lost in the noise. I've started to clean that up, though probably need > some direction there too. > > Separately I'm currently trying to track down an issue where I get errors > on the Aspeed dts cpu nodes about failing to match the riscv CPU > compatibles, it seems dt-validate isn't finding the ARM CPU compatible > strings. It feels more annoying to track down that I'd like. There's a fix in today's linux-next for that and it should be in Linus' tree in a few days. Rob
On Wed, 17 Jul 2019, at 23:13, Rob Herring wrote: > On Tue, Jul 16, 2019 at 9:58 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > On Wed, 17 Jul 2019, at 00:27, Rob Herring wrote: > > > On Mon, Jul 15, 2019 at 6:36 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > > > > > > > > > On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote: > > > > > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > > > > > > > > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the > > > > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit > > > > > > data bus if only a single slot is enabled. > > > > > > > > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > > > > --- > > > > > > In v2: > > > > > > > > > > > > * Rename to aspeed,sdhci.yaml > > > > > > * Rename sd-controller compatible > > > > > > * Add `maxItems: 1` for reg properties > > > > > > * Move sdhci subnode description to patternProperties > > > > > > * Drop sdhci compatible requirement > > > > > > * #address-cells and #size-cells are required > > > > > > * Prevent additional properties > > > > > > * Implement explicit ranges in example > > > > > > * Remove slot property > > > > > > > > > > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ > > > > > > 1 file changed, 90 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..67a691c3348c > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > > > > @@ -0,0 +1,90 @@ > > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: ASPEED SD/SDIO/eMMC Controller > > > > > > + > > > > > > +maintainers: > > > > > > + - Andrew Jeffery <andrew@aj.id.au> > > > > > > + - Ryan Chen <ryanchen.aspeed@gmail.com> > > > > > > + > > > > > > +description: |+ > > > > > > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO > > > > > > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if > > > > > > + only a single slot is enabled. > > > > > > + > > > > > > + The two slots are supported by a common configuration area. As the SDHCIs for > > > > > > + the slots are dependent on the common configuration area, they are described > > > > > > + as child nodes. > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] > > > > > > > > > > This is actually a list of 4 strings. Please reformat to 1 per line. > > > > > > > > On reflection that's obvious, but also a somewhat subtle interaction with the > > > > preference for no quotes (the obvious caveat being "except where required"). > > > > > > It wasn't something I'd run into before. I'm working on a check, but > > > unfortunately we can only check for quotes not needed and can't check > > > for missing quotes. > > > > > > > Thanks for pointing it out. > > > > > > > > I have been running `make dt_binding_check` and `make dtbs_check` over > > > > these, looks like I need to up my game a bit though. Do you do additional things > > > > in your workflow? > > > > > > That should have thrown the warnings. If you aren't seeing those, do > > > you have dtschema package installed (see > > > Documentation/devicetree/writing-schema.md)? > > > > I do have it installed, but as mentioned previously there's a fair few > > warnings emitted currently by the Aspeed devicetrees, so it might have > > got lost in the noise. I've started to clean that up, though probably need > > some direction there too. > > > > Separately I'm currently trying to track down an issue where I get errors > > on the Aspeed dts cpu nodes about failing to match the riscv CPU > > compatibles, it seems dt-validate isn't finding the ARM CPU compatible > > strings. It feels more annoying to track down that I'd like. > > There's a fix in today's linux-next for that and it should be in > Linus' tree in a few days. > Thanks, I'll take a look. Andrew
diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml new file mode 100644 index 000000000000..67a691c3348c --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ASPEED SD/SDIO/eMMC Controller + +maintainers: + - Andrew Jeffery <andrew@aj.id.au> + - Ryan Chen <ryanchen.aspeed@gmail.com> + +description: |+ + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if + only a single slot is enabled. + + The two slots are supported by a common configuration area. As the SDHCIs for + the slots are dependent on the common configuration area, they are described + as child nodes. + +properties: + compatible: + enum: [ aspeed,ast2400-sd-controller, aspeed,ast2500-sd-controller ] + reg: + maxItems: 1 + description: Common configuration registers + ranges: true + clocks: + maxItems: 1 + description: The SD/SDIO controller clock gate + +patternProperties: + "^sdhci@[0-9a-f]+$": + type: object + properties: + compatible: + enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ] + reg: + maxItems: 1 + description: The SDHCI registers + clocks: + maxItems: 1 + description: The SD bus clock + interrupts: + maxItems: 1 + description: The SD interrupt shared between both slots + required: + - compatible + - reg + - clocks + - interrupts + +additionalProperties: false + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + - ranges + - clocks + +examples: + - | + #include <dt-bindings/clock/aspeed-clock.h> + sdc@1e740000 { + compatible = "aspeed,ast2500-sd-controller"; + reg = <0x1e740000 0x100>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e740000 0x10000>; + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>; + + sdhci0: sdhci@100 { + compatible = "aspeed,ast2500-sdhci"; + reg = <0x100 0x100>; + interrupts = <26>; + sdhci,auto-cmd12; + clocks = <&syscon ASPEED_CLK_SDIO>; + }; + + sdhci1: sdhci@200 { + compatible = "aspeed,ast2500-sdhci"; + reg = <0x200 0x100>; + interrupts = <26>; + sdhci,auto-cmd12; + clocks = <&syscon ASPEED_CLK_SDIO>; + }; + };
The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if only a single slot is enabled. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- In v2: * Rename to aspeed,sdhci.yaml * Rename sd-controller compatible * Add `maxItems: 1` for reg properties * Move sdhci subnode description to patternProperties * Drop sdhci compatible requirement * #address-cells and #size-cells are required * Prevent additional properties * Implement explicit ranges in example * Remove slot property .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml