Re: [PATCH v5 2/3] spi: dt-bindings: Describe stacked/parallel memories modes

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hello Tudor,

Tudor.Ambarus@xxxxxxxxxxxxx wrote on Wed, 22 Dec 2021 07:52:44 +0000:

> On 12/21/21 7:00 PM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Describe two new memories modes:
> > - A stacked mode when the bus is common but the address space extended
> >   with an additinals wires.
> > - A parallel mode with parallel busses accessing parallel flashes where
> >   the data is spread.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > ---
> > 
> > Hello Rob,
> > 
> > I know the below does not pass the tests (at least the example patch 3
> > does not pass) but I believe the issue is probably on the tooling side
> > because the exact same thing with uing32-array instead is accepted. The
> > problem comes from the minItems/maxItems lines. Without them, this is
> > okay. The maxItems btw matches the "good enough value for now" idea.
> > 
> > The errors I get are:
> > 
> > $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/spi-controller.yaml
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> >   DTEX    Documentation/devicetree/bindings/spi/spi-controller.example.dts
> >   DTC     Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> >   CHECK   Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'display@0', 'sensor@1', 'flash@2' were unexpected)
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: flash@2: stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > 
> > 
> >  .../bindings/spi/spi-peripheral-props.yaml    | 25 +++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 5dd209206e88..fedb7ae98ff6 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -82,6 +82,31 @@ properties:
> >      description:
> >        Delay, in microseconds, after a write transfer.
> > 
> > +  stacked-memories:
> > +    description: Several SPI memories can be wired in stacked mode.
> > +      This basically means that either a device features several chip
> > +      selects, or that different devices must be seen as a single
> > +      bigger chip. This basically doubles (or more) the total address
> > +      space with only a single additional wire, while still needing
> > +      to repeat the commands when crossing a chip boundary. The size of
> > +      each chip should be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4  
> 
> Why do we define maxItems? Can't we remove this restriction?

Rob usually prefers to bound properties, that's why we often see "good
enough values for now" in the bindings. If it's no longer the case it's
fine to drop the maxItems property.

> > +
> > +  parallel-memories:
> > +    description: Several SPI memories can be wired in parallel mode.
> > +      The devices are physically on a different buses but will always
> > +      act synchronously as each data word is spread across the
> > +      different memories (eg. even bits are stored in one memory, odd
> > +      bits in the other). This basically doubles the address space and
> > +      the throughput while greatly complexifying the wiring because as
> > +      many busses as devices must be wired. The size of each chip should
> > +      be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4
> > +
> >  # The controller specific properties go here.
> >  allOf:
> >    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > --
> > 2.27.0
> >   
> 


Thanks,
Miquèl