[v2,0/5] Stacked/parallel memories bindings

Hello Rob, Mark, Tudor & Pratyush,

Now that the discussion has move forward, let met propose a second
version for these bindings.

Cheers,
Miquèl

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 (5):
  spi: dt-bindings: Allow describing flashes with two CS
  dt-bindings: mtd: spi-nor: Allow external properties
  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           |  5 ++--
 .../bindings/spi/spi-controller.yaml          | 30 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 3 deletions(-)

On Fri, 26 Nov 2021 17:34:50 +0100, Miquel Raynal wrote:
> Provide an example of how to describe two flashes in eg. stacked mode.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/spi-controller.example.dts:40.23-45.15: Warning (spi_bus_reg): /example-0/spi@80010000/flash@2,3: SPI bus unit address format error, expected "2"

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1560255

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

On Sat, Nov 27, 2021 at 04:13:22PM -0700, Rob Herring wrote:
> On Fri, 26 Nov 2021 17:34:50 +0100, Miquel Raynal wrote:
> > Provide an example of how to describe two flashes in eg. stacked mode.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/spi/spi-controller.example.dts:40.23-45.15: Warning (spi_bus_reg): /example-0/spi@80010000/flash@2,3: SPI bus unit address format error, expected "2"

Unit-addresses are based on the first reg entry.

Hi Rob,

robh@kernel.org wrote on Sun, 28 Nov 2021 10:55:06 -0600:

> On Sat, Nov 27, 2021 at 04:13:22PM -0700, Rob Herring wrote:
> > On Fri, 26 Nov 2021 17:34:50 +0100, Miquel Raynal wrote:  
> > > Provide an example of how to describe two flashes in eg. stacked mode.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >   
> > 
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > Documentation/devicetree/bindings/spi/spi-controller.example.dts:40.23-45.15: Warning (spi_bus_reg): /example-0/spi@80010000/flash@2,3: SPI bus unit address format error, expected "2"  
> 
> Unit-addresses are based on the first reg entry.

Yes, I believe this error is expected since dtc has not been yet
updated. Below the patch for adapting dtc to this new situation and
keep the robots happy.

How should we proceed?

Thanks,
Miquèl

---

Author: Miquel Raynal <miquel.raynal@bootlin.com>
Date:   Fri Nov 26 16:08:27 2021 +0100

    dtc: checks: spi: Allow describing flashes with two CS
    
    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 CSlevels 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>

diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
index 781ba1129a8e..4eaa925c3442 100644
--- a/scripts/dtc/checks.c
+++ b/scripts/dtc/checks.c
@@ -1094,7 +1094,7 @@ static const struct bus_type spi_bus = {
 
 static void check_spi_bus_bridge(struct check *c, struct dt_info *dti, struct node *node)
 {
-       int spi_addr_cells = 1;
+       int spi_addr_cells = 2;
 
        if (strprefixeq(node->name, node->basenamelen, "spi")) {
                node->bus = &spi_bus;
@@ -1125,7 +1125,7 @@ static void check_spi_bus_bridge(struct check *c, struct dt_info *dti, struct no
 
        if (get_property(node, "spi-slave"))
                spi_addr_cells = 0;
-       if (node_addr_cells(node) != spi_addr_cells)
+       if (node_addr_cells(node) > spi_addr_cells)
                FAIL(c, dti, node, "incorrect #address-cells for SPI bus");
        if (node_size_cells(node) != 0)
                FAIL(c, dti, node, "incorrect #size-cells for SPI bus");
@@ -1137,8 +1137,8 @@ static void check_spi_bus_reg(struct check *c, struct dt_info *dti, struct node
 {
        struct property *prop;
        const char *unitname = get_unitname(node);
-       char unit_addr[9];
-       uint32_t reg = 0;
+       char unit_addr[18];
+       uint32_t reg0 = 0, reg1 = 0;
        cell_t *cells = NULL;
 
        if (!node->parent || (node->parent->bus != &spi_bus))
@@ -1156,11 +1156,17 @@ static void check_spi_bus_reg(struct check *c, struct dt_info *dti, struct node
                return;
        }
 
-       reg = fdt32_to_cpu(*cells);
-       snprintf(unit_addr, sizeof(unit_addr), "%x", reg);
-       if (!streq(unitname, unit_addr))
-               FAIL(c, dti, node, "SPI bus unit address format error, expected \"%s\"",
-                    unit_addr);
+       reg0 = fdt32_to_cpu(cells[0]);
+       snprintf(unit_addr, sizeof(unit_addr), "%x", reg0);
+       if (!streq(unitname, unit_addr)) {
+               reg1 = fdt32_to_cpu(cells[1]);
+               snprintf(unit_addr, sizeof(unit_addr), "%x,%x", reg0, reg1);
+               if (!streq(unitname, unit_addr)) {
+                       FAIL(c, dti, node,
+                            "SPI bus unit address format error, expected \"%s\"",
+                            unit_addr);
+               }
+       }
 }
 WARNING(spi_bus_reg, check_spi_bus_reg, NULL, &reg_format, &spi_bus_bridge);

On Mon, Nov 29, 2021 at 10:23:19AM +0100, Miquel Raynal wrote:
> Hi Rob,
> 
> robh@kernel.org wrote on Sun, 28 Nov 2021 10:55:06 -0600:
> 
> > On Sat, Nov 27, 2021 at 04:13:22PM -0700, Rob Herring wrote:
> > > On Fri, 26 Nov 2021 17:34:50 +0100, Miquel Raynal wrote:  
> > > > Provide an example of how to describe two flashes in eg. stacked mode.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >   
> > > 
> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > > 
> > > yamllint warnings/errors:
> > > 
> > > dtschema/dtc warnings/errors:
> > > Documentation/devicetree/bindings/spi/spi-controller.example.dts:40.23-45.15: Warning (spi_bus_reg): /example-0/spi@80010000/flash@2,3: SPI bus unit address format error, expected "2"  
> > 
> > Unit-addresses are based on the first reg entry.
> 
> Yes, I believe this error is expected since dtc has not been yet
> updated. Below the patch for adapting dtc to this new situation and
> keep the robots happy.
> 
> How should we proceed?

No, I'm saying you have this wrong. A unit-address is composed of 
different fields, not different entries of the same field. For 
example, an external parallel bus has a chip select plus address, so the 
unit-address is '<cs>,<addr>'. If you have 2 SPI chip selects, that's 2 
entries of the same thing. The SPI bus is not 2 address cells, but 1 
cell with 2 entries.

Rob

Hi Rob,

robh@kernel.org wrote on Wed, 1 Dec 2021 17:57:15 -0600:

> On Mon, Nov 29, 2021 at 10:23:19AM +0100, Miquel Raynal wrote:
> > Hi Rob,
> > 
> > robh@kernel.org wrote on Sun, 28 Nov 2021 10:55:06 -0600:
> >   
> > > On Sat, Nov 27, 2021 at 04:13:22PM -0700, Rob Herring wrote:  
> > > > On Fri, 26 Nov 2021 17:34:50 +0100, Miquel Raynal wrote:    
> > > > > Provide an example of how to describe two flashes in eg. stacked mode.
> > > > > 
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/spi/spi-controller.yaml | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >     
> > > > 
> > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > > > 
> > > > yamllint warnings/errors:
> > > > 
> > > > dtschema/dtc warnings/errors:
> > > > Documentation/devicetree/bindings/spi/spi-controller.example.dts:40.23-45.15: Warning (spi_bus_reg): /example-0/spi@80010000/flash@2,3: SPI bus unit address format error, expected "2"    
> > > 
> > > Unit-addresses are based on the first reg entry.  
> > 
> > Yes, I believe this error is expected since dtc has not been yet
> > updated. Below the patch for adapting dtc to this new situation and
> > keep the robots happy.
> > 
> > How should we proceed?  
> 
> No, I'm saying you have this wrong. A unit-address is composed of 
> different fields, not different entries of the same field. For 
> example, an external parallel bus has a chip select plus address, so the 
> unit-address is '<cs>,<addr>'. If you have 2 SPI chip selects, that's 2 
> entries of the same thing. The SPI bus is not 2 address cells, but 1 
> cell with 2 entries.

My bad, now I get it, thanks.

Cheers,
Miquèl