[PATCH v5 0/3] Stacked/parallel memories bindings

[PATCH v5 0/3] Stacked/parallel memories bindings

[Miquel Raynal]

Hello Rob, Mark, Tudor & Pratyush,

Here is a fifth versions for these bindings, which applies on top of
Pratyush's work:
https://lore.kernel.org/all/20211109181911.2251-1-p.yadav@ti.com/

Cheers,
Miquèl

Changes in v5:
* Used the uint64-array instead of the matrix type.
* Updated the example as well to use a single "/bits/ 64" cast because
  doing it twice, despite being supported by the language itself, is not
  yet something that we can use for describing bindings.

Changes in v4:
* Changed the type of properties to uint64-arrays in order to be able to
  describe the size of each element in the array.
* Updated the example accordingly.

Changes in v3:
* Rebased on top of Pratyush's recent changes.
* Dropped the commit allowing to provide two reg entries on the node
  name.
* Dropped the commit referencing spi-controller.yaml from
  jedec,spi-nor.yaml, now replaced by spi-peripheral-props.yaml and
  already done in Pratyush's series.
* Added Rob's Ack.
* Enhanced a commit message.
* Moved the new properties to the new SPI peripheral binding file.

Changes in v2:
* Dropped the dtc changes for now.
* Moved the properties in the device's nodes, not the controller's.
* Dropped the useless #address-cells change.
* Added a missing "minItems".
* Moved the new properties in the spi-controller.yaml file.
* Added an example using two stacked memories in the
  spi-controller.yaml file.
* Renamed the properties to drop the Xilinx prefix.
* Added a patch to fix the spi-nor jedec yaml file.

Miquel Raynal (3):
  dt-bindings: mtd: spi-nor: Allow two CS per device
  spi: dt-bindings: Describe stacked/parallel memories modes
  spi: dt-bindings: Add an example with two stacked flashes

 .../bindings/mtd/jedec,spi-nor.yaml           |  3 ++-
 .../bindings/spi/spi-controller.yaml          |  7 ++++++
 .../bindings/spi/spi-peripheral-props.yaml    | 25 +++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.27.0

[PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device

[Miquel Raynal]

The Xilinx QSPI controller has two advanced modes which allow the
controller to behave differently and consider two flashes as one single
storage.

One of these two modes is quite complex to support from a binding point
of view and is the dual parallel memories. In this mode, each byte of
data is stored in both devices: the even bits in one, the odd bits in
the other. The split is automatically handled by the QSPI controller and
is transparent for the user.

The other mode is simpler to support, it is called dual stacked
memories. The controller shares the same SPI bus but each of the devices
contain half of the data. Once in this mode, the controller does not
follow CS requests but instead internally wires the two CS levels with
the value of the most significant address bit.

Supporting these two modes will involve core changes which include the
possibility of providing two CS for a single SPI device

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
index 39421f7233e4..4abfb4cfc157 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -47,7 +47,8 @@ properties:
       identified by the JEDEC READ ID opcode (0x9F).
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   spi-max-frequency: true
   spi-rx-bus-width: true
-- 
2.27.0

Re: [PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device

[Pratyush Yadav]

On 21/12/21 06:00PM, Miquel Raynal wrote:
> The Xilinx QSPI controller has two advanced modes which allow the
> controller to behave differently and consider two flashes as one single
> storage.
> 
> One of these two modes is quite complex to support from a binding point
> of view and is the dual parallel memories. In this mode, each byte of
> data is stored in both devices: the even bits in one, the odd bits in
> the other. The split is automatically handled by the QSPI controller and
> is transparent for the user.
> 
> The other mode is simpler to support, it is called dual stacked
> memories. The controller shares the same SPI bus but each of the devices
> contain half of the data. Once in this mode, the controller does not
> follow CS requests but instead internally wires the two CS levels with
> the value of the most significant address bit.
> 
> Supporting these two modes will involve core changes which include the
> possibility of providing two CS for a single SPI device
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index 39421f7233e4..4abfb4cfc157 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -47,7 +47,8 @@ properties:
>        identified by the JEDEC READ ID opcode (0x9F).
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

You allow up to 4 items in stacked-memories but only allow up to 2 CS, 
which would make the other 2 memories unusable. Should also change this 
to 4.

>  
>    spi-max-frequency: true
>    spi-rx-bus-width: true
> -- 
> 2.27.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device

[Miquel Raynal]

Hi Pratyush,

p.yadav@ti.com wrote on Wed, 22 Dec 2021 00:17:27 +0530:

> On 21/12/21 06:00PM, Miquel Raynal wrote:
> > The Xilinx QSPI controller has two advanced modes which allow the
> > controller to behave differently and consider two flashes as one single
> > storage.
> > 
> > One of these two modes is quite complex to support from a binding point
> > of view and is the dual parallel memories. In this mode, each byte of
> > data is stored in both devices: the even bits in one, the odd bits in
> > the other. The split is automatically handled by the QSPI controller and
> > is transparent for the user.
> > 
> > The other mode is simpler to support, it is called dual stacked
> > memories. The controller shares the same SPI bus but each of the devices
> > contain half of the data. Once in this mode, the controller does not
> > follow CS requests but instead internally wires the two CS levels with
> > the value of the most significant address bit.
> > 
> > Supporting these two modes will involve core changes which include the
> > possibility of providing two CS for a single SPI device
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > index 39421f7233e4..4abfb4cfc157 100644
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > @@ -47,7 +47,8 @@ properties:
> >        identified by the JEDEC READ ID opcode (0x9F).
> >  
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2  
> 
> You allow up to 4 items in stacked-memories but only allow up to 2 CS, 
> which would make the other 2 memories unusable. Should also change this 
> to 4.

Yes, I allowed "more" theoretical devices in the
stacked/parallel-memories properties because there is no real
limitation on this side so I didn't want to constrain it too much,
while still keeping a maximum value, hence 4 seemed a nice guess for a
"maximum but can be bigger value we don't really care it's just for
bounding". However on the SPI side this is a big change with deep
consequences and I don't want to rush things so it is on purpose that I
kept the limitation to 2. But we can change the maxItems to 2
everywhere if this appears to be the thing to do.

Thanks,
Miquèl

Re: [PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device

[Pratyush Yadav]

On 22/12/21 09:23AM, Miquel Raynal wrote:
> Hi Pratyush,
> 
> p.yadav@ti.com wrote on Wed, 22 Dec 2021 00:17:27 +0530:
> 
> > On 21/12/21 06:00PM, Miquel Raynal wrote:
> > > The Xilinx QSPI controller has two advanced modes which allow the
> > > controller to behave differently and consider two flashes as one single
> > > storage.
> > > 
> > > One of these two modes is quite complex to support from a binding point
> > > of view and is the dual parallel memories. In this mode, each byte of
> > > data is stored in both devices: the even bits in one, the odd bits in
> > > the other. The split is automatically handled by the QSPI controller and
> > > is transparent for the user.
> > > 
> > > The other mode is simpler to support, it is called dual stacked
> > > memories. The controller shares the same SPI bus but each of the devices
> > > contain half of the data. Once in this mode, the controller does not
> > > follow CS requests but instead internally wires the two CS levels with
> > > the value of the most significant address bit.
> > > 
> > > Supporting these two modes will involve core changes which include the
> > > possibility of providing two CS for a single SPI device
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > index 39421f7233e4..4abfb4cfc157 100644
> > > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > @@ -47,7 +47,8 @@ properties:
> > >        identified by the JEDEC READ ID opcode (0x9F).
> > >  
> > >    reg:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 2  
> > 
> > You allow up to 4 items in stacked-memories but only allow up to 2 CS, 
> > which would make the other 2 memories unusable. Should also change this 
> > to 4.
> 
> Yes, I allowed "more" theoretical devices in the
> stacked/parallel-memories properties because there is no real
> limitation on this side so I didn't want to constrain it too much,
> while still keeping a maximum value, hence 4 seemed a nice guess for a
> "maximum but can be bigger value we don't really care it's just for
> bounding". However on the SPI side this is a big change with deep
> consequences and I don't want to rush things so it is on purpose that I
> kept the limitation to 2. But we can change the maxItems to 2
> everywhere if this appears to be the thing to do.

Ok, sounds good to me.

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device

[Miquel Raynal]

miquel.raynal@bootlin.com wrote on Wed, 22 Dec 2021 09:23:24 +0100:

> Hi Pratyush,
> 
> p.yadav@ti.com wrote on Wed, 22 Dec 2021 00:17:27 +0530:
> 
> > On 21/12/21 06:00PM, Miquel Raynal wrote:  
> > > The Xilinx QSPI controller has two advanced modes which allow the
> > > controller to behave differently and consider two flashes as one single
> > > storage.
> > > 
> > > One of these two modes is quite complex to support from a binding point
> > > of view and is the dual parallel memories. In this mode, each byte of
> > > data is stored in both devices: the even bits in one, the odd bits in
> > > the other. The split is automatically handled by the QSPI controller and
> > > is transparent for the user.
> > > 
> > > The other mode is simpler to support, it is called dual stacked
> > > memories. The controller shares the same SPI bus but each of the devices
> > > contain half of the data. Once in this mode, the controller does not
> > > follow CS requests but instead internally wires the two CS levels with
> > > the value of the most significant address bit.
> > > 
> > > Supporting these two modes will involve core changes which include the
> > > possibility of providing two CS for a single SPI device
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > index 39421f7233e4..4abfb4cfc157 100644
> > > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > > @@ -47,7 +47,8 @@ properties:
> > >        identified by the JEDEC READ ID opcode (0x9F).
> > >  
> > >    reg:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 2    
> > 
> > You allow up to 4 items in stacked-memories but only allow up to 2 CS, 
> > which would make the other 2 memories unusable. Should also change this 
> > to 4.  
> 
> Yes, I allowed "more" theoretical devices in the
> stacked/parallel-memories properties because there is no real
> limitation on this side so I didn't want to constrain it too much,
> while still keeping a maximum value, hence 4 seemed a nice guess for a
> "maximum but can be bigger value we don't really care it's just for
> bounding". However on the SPI side this is a big change with deep
> consequences and I don't want to rush things so it is on purpose that I
> kept the limitation to 2. But we can change the maxItems to 2
> everywhere if this appears to be the thing to do.

I forgot to mention that the stacked/parallel-memories could
also certainly be considered "memory" properties (think about the
generic term), not necessarily bound to SPI. We could definitely have
the same pattern with other memory types as well and not be tight to
the number of SPI CS.

Thanks,
Miquèl

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

[Miquel Raynal]

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@bootlin.com>
---

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
+
+  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

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

[Pratyush Yadav]

On 21/12/21 06:00PM, Miquel Raynal wrote:
> 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@bootlin.com>

Acked-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

[Tudor.Ambarus]

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@bootlin.com>
> ---
> 
> 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?

> +
> +  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
> 

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

[Miquel Raynal]

Hello Tudor,

Tudor.Ambarus@microchip.com 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@bootlin.com>
> > ---
> > 
> > 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

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

[Tudor.Ambarus]

On 12/22/21 10:05 AM, Miquel Raynal wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello Tudor,

Hi!

> 
> Tudor.Ambarus@microchip.com 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@bootlin.com>
>>> ---
>>>
>>> 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

right, I saw it.

> fine to drop the maxItems property.

There's no such hardware limitation as far as I know, that's why I've
asked. Maybe Rob can advise.

Cheers,
ta

> 
>>> +
>>> +  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
> 

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

[Miquel Raynal]

Hi Tudor,

Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000:
> On 12/22/21 10:05 AM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hello Tudor,  
> 
> Hi!
> 
> > 
> > Tudor.Ambarus@microchip.com 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@bootlin.com>
> >>> ---
> >>>
> >>> 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  
> 
> right, I saw it.
> 
> > fine to drop the maxItems property.  
> 
> There's no such hardware limitation as far as I know, that's why I've
> asked. Maybe Rob can advise.

Yes, I'll follow what Rob thinks its best:
- keeping maxItems: 4 (as it is), which is a good enough value
- dropping the maxItems here because in the end no bounding is necessary
- using maxItems: 2 to match the SPI CS even though in theory these two
  numbers are not correlated (stacked-memories might very well be
  used by other non SPI memories as well).

BTW if you're fine with the proposal your Ack is welcome ;)

Thanks,
Miquèl

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

[Tudor.Ambarus]

On 12/22/21 10:35 AM, Miquel Raynal wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000:
>> On 12/22/21 10:05 AM, Miquel Raynal wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hello Tudor,
>>
>> Hi!
>>
>>>
>>> Tudor.Ambarus@microchip.com 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@bootlin.com>
>>>>> ---
>>>>>
>>>>> 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
>>
>> right, I saw it.
>>
>>> fine to drop the maxItems property.
>>
>> There's no such hardware limitation as far as I know, that's why I've
>> asked. Maybe Rob can advise.
> 
> Yes, I'll follow what Rob thinks its best:
> - keeping maxItems: 4 (as it is), which is a good enough value
> - dropping the maxItems here because in the end no bounding is necessary
Then I would drop maxItems for stacked-memories. For parallel-memories:
does the maxItems depend on the number of I/O lines?
 
> - using maxItems: 2 to match the SPI CS even though in theory these two
>   numbers are not correlated (stacked-memories might very well be
>   used by other non SPI memories as well).
> 
> BTW if you're fine with the proposal your Ack is welcome ;)
> 
> Thanks,
> Miquèl
> 

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

[Miquel Raynal]

Hi Tudor,

Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:44:16 +0000:

> On 12/22/21 10:35 AM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi Tudor,
> > 
> > Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000:  
> >> On 12/22/21 10:05 AM, Miquel Raynal wrote:  
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> Hello Tudor,  
> >>
> >> Hi!
> >>  
> >>>
> >>> Tudor.Ambarus@microchip.com 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@bootlin.com>
> >>>>> ---
> >>>>>
> >>>>> 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  
> >>
> >> right, I saw it.
> >>  
> >>> fine to drop the maxItems property.  
> >>
> >> There's no such hardware limitation as far as I know, that's why I've
> >> asked. Maybe Rob can advise.  
> > 
> > Yes, I'll follow what Rob thinks its best:
> > - keeping maxItems: 4 (as it is), which is a good enough value
> > - dropping the maxItems here because in the end no bounding is necessary  
> Then I would drop maxItems for stacked-memories. For parallel-memories:
> does the maxItems depend on the number of I/O lines?

Well, if you look into controller constraints, I bet most of them will
not support more than 8 data lines for now. For example, Xilinx QSPI
controller accepts up to two devices with 4 data lines on each. But I
believe it would be completely possible to use 4 devices with 2 data
lines or up to 8 with one as well. This is pure theory, I haven't
seen nor heard about such hosts so far, so let's wait for Rob advice on
that number and see what he has in mind.

> > - using maxItems: 2 to match the SPI CS even though in theory these two
> >   numbers are not correlated (stacked-memories might very well be
> >   used by other non SPI memories as well).
> > 
> > BTW if you're fine with the proposal your Ack is welcome ;)
> > 
> > Thanks,
> > Miquèl
> >   
> 

Thanks,
Miquèl

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

[Rob Herring]

On Wed, Dec 22, 2021 at 09:35:58AM +0100, Miquel Raynal wrote:
> Hi Tudor,
> 
> Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000:
> > On 12/22/21 10:05 AM, Miquel Raynal wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > > 
> > > Hello Tudor,  
> > 
> > Hi!
> > 
> > > 
> > > Tudor.Ambarus@microchip.com 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@bootlin.com>
> > >>> ---
> > >>>
> > >>> 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  
> > 
> > right, I saw it.
> > 
> > > fine to drop the maxItems property.  
> > 
> > There's no such hardware limitation as far as I know, that's why I've
> > asked. Maybe Rob can advise.
> 
> Yes, I'll follow what Rob thinks its best:
> - keeping maxItems: 4 (as it is), which is a good enough value

That's what I already suggested, though I would have expected a bigger 
value. For example, something more than the cost or electrical limit of 
what's practical. I don't know that is though. We don't want to be 
updating it frequently.

> - dropping the maxItems here because in the end no bounding is necessary

This will implicitly set the max to 2 based on minItems. That's because 
most of the time we want an exact number of entries.

> - using maxItems: 2 to match the SPI CS even though in theory these two
>   numbers are not correlated (stacked-memories might very well be
>   used by other non SPI memories as well).
> 
> BTW if you're fine with the proposal your Ack is welcome ;)
> 
> Thanks,
> Miquèl
> 

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

[Rob Herring]

On Tue, Dec 21, 2021 at 06:00:57PM +0100, Miquel Raynal wrote:
> 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@bootlin.com>
> ---
> 
> 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

I'm not seeing any of these. Are you up to date with dtschema?

Rob

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

[Rob Herring]

On Wed, Dec 22, 2021 at 03:08:59PM -0400, Rob Herring wrote:
> On Tue, Dec 21, 2021 at 06:00:57PM +0100, Miquel Raynal wrote:
> > 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@bootlin.com>
> > ---
> > 
> > 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
> 
> I'm not seeing any of these. Are you up to date with dtschema?

NM, I was missing a patch and now see it. It is indeed a tool problem. 
I'll work on a fix.

Rob

[PATCH v5 3/3] spi: dt-bindings: Add an example with two stacked flashes

[Miquel Raynal]

Provide an example of how to describe two flashes in eg. stacked mode.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
index 36b72518f565..0f4d40218400 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -139,4 +139,11 @@ examples:
             spi-max-frequency = <100000>;
             reg = <1>;
         };
+
+        flash@2 {
+          compatible = "jedec,spi-nor";
+          spi-max-frequency = <50000000>;
+          reg = <2>, <3>;
+          stacked-memories = /bits/ 64 <0x10000000 0x10000000>;
+        };
     };
-- 
2.27.0