Re: [PATCH v2 1/2] dt-bindings: mmc: Document Aspeed SD controller

[Andrew Jeffery]

On Wed, 17 Jul 2019, at 00:27, Rob Herring wrote:
> On Mon, Jul 15, 2019 at 6:36 PM Andrew Jeffery <andrew@xxxxxxxx> wrote:
> >
> >
> >
> > On Tue, 16 Jul 2019, at 07:47, Rob Herring wrote:
> > > On Thu, Jul 11, 2019 at 9:32 PM Andrew Jeffery <andrew@xxxxxxxx> 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@xxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxx>
> > > > +  - Ryan Chen <ryanchen.aspeed@xxxxxxxxx>
> > > > +
> > > > +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