Re: [PATCH v2 5/5] spi: dt-bindings: Add an example with two stacked flashes

[Miquel Raynal <miquel.raynal@bootlin.com>]

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);