Re: [PATCH 1/4] dt-bindings: spi: Add YAML schemas for the generic SPI options

[Rob Herring <robh+dt@kernel.org>]

On Tue, May 7, 2019 at 1:07 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Tue, May 07, 2019 at 09:35:28AM -0500, Rob Herring wrote:
> > On Tue, May 7, 2019 at 8:48 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > The SPI controllers have a bunch of generic options that are needed in a
> > > device tree. Add a YAML schemas for those.
> >
> > I'd started on this one, but was planning to move it to the schema
> > repository. The issue there is re-licensing (adding BSD 2 clause).
> > Maybe better to just move it later.
>
> I just found out that dt-doc-validate also chokes on the reference
> URI. Maybe I should just submit it to the repo then once that is
> settled?

I'm not really too excited about chasing down licensing on every file
we want to move and I'd like to avoid per file licenses, so I'd like
local $refs to work. I think I've got something figured out that will
work. It will need a small kernel side change though.

> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^spi(@[a-zA-Z0-9]+)?$"
> >
> > I think we want just "(@.*)". At a minimum, you need to allow for ','.
> > It would be the a bus schema for the parent which should validate unit
> > addresses, so we should pretty much just allow anything here.
>
> The issue with this is that it will also match any node starting with
> spi. In the Allwinner case, that also means the pinctrl nodes with spi
> pins in them, but I'm sure we can find more corner cases.

Maybe I wasn't clear, but I meant changing just the unit-address part. So:

"^spi(@.*)$"

>
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +  cs-gpios:
> > > +    description: |
> > > +      GPIOs used as chip selects.
> > > +      If that property is used, the number of chip selects will be
> > > +      increased automatically with max(cs-gpios, hardware chip selects).
> > > +
> > > +      So if, for example, the controller has 2 CS lines, and the
> > > +      cs-gpios looks like this
> > > +        cs-gpios = <&gpio1 0 0>, <0>, <&gpio1 1 0>, <&gpio1 2 0>;
> > > +
> > > +      Then it should be configured so that num_chipselect = 4, with
> > > +      the following mapping
> > > +        cs0 : &gpio1 0 0
> > > +        cs1 : native
> > > +        cs2 : &gpio1 1 0
> > > +        cs3 : &gpio1 2 0
> > > +
> > > +  num-cs:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      Total number of chip selects.
> > > +
> > > +  spi-slave:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> >
> > "type: boolean" is sufficient here. Maybe we should just remove
> > 'flag'. OTOH, maybe consistency with other types and the abstraction
> > is better as we could add to the flag schema.
>
> I was trying to be consistent. Do you want me to remove it?

No, it's fine.

Rob