[RFC PATCH 0/3] Dual stacked/parallel memories bindings

[RFC PATCH 0/3] Dual stacked/parallel memories bindings

[Miquel Raynal]

Hello Rob, Mark, Tudor & Pratyush,

Here is an RFC to open the discussion about the sensitive task of
supporting specific SPI controller modes like Xilinx's where the
controller can highly abstract the hardware and provide access to a
single bigger device instead. I'll let you go through the series and
tell me what you think.

I think there are two possible approaches:
1- Describe the two devices as being a single one which is what we will
   get from the controller anyway (implies supporting two CS per SPI
   device)
or
2- Describe the two devices in the device tree and then by software hack
   into the MTD core to simulate a single device to talk to.

I have looked at the code, there is no good solution, but #2 definitely
looks horribly complicated and subject to a lot of corner cases to
handle, hence this proposal to go for solution #1.

Cheers,
Miquèl

Miquel Raynal (3):
  spi: dt-bindings: Allow describing flashes with two CS
  dt-binding: mtd: spi-nor: Allow two CS per device
  spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories
    modes

 .../bindings/mtd/jedec,spi-nor.yaml           |  2 +-
 .../bindings/spi/spi-controller.yaml          |  6 ++--
 .../bindings/spi/spi-zynqmp-qspi.yaml         | 31 +++++++++++++++++++
 scripts/dtc/checks.c                          | 24 ++++++++------
 4 files changed, 50 insertions(+), 13 deletions(-)

-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[RFC PATCH 1/3] spi: dt-bindings: Allow describing flashes with two CS

[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 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>
---
 .../bindings/spi/spi-controller.yaml          |  6 ++---
 scripts/dtc/checks.c                          | 24 ++++++++++++-------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
index 8246891602e7..51877b637bfe 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -20,7 +20,7 @@ properties:
     pattern: "^spi(@.*|-[0-9a-f])*$"
 
   "#address-cells":
-    enum: [0, 1]
+    enum: [0, 1, 2]
 
   "#size-cells":
     const: 0
@@ -91,7 +91,7 @@ properties:
       - compatible
 
 patternProperties:
-  "^.*@[0-9a-f]+$":
+  "^.*@[0-9a-f]+,?[0-9a-f]*$":
     type: object
 
     properties:
@@ -174,7 +174,7 @@ allOf:
     then:
       properties:
         "#address-cells":
-          const: 1
+          enum: [1, 2]
     else:
       properties:
         "#address-cells":
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);
 
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 1/3] spi: dt-bindings: Allow describing flashes with two CS

[Rob Herring]

On Fri, Nov 12, 2021 at 9:24 AM Miquel Raynal <miquel.raynal@bootlin.com> 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 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>
> ---
>  .../bindings/spi/spi-controller.yaml          |  6 ++---

Needs to go to dt list...

>  scripts/dtc/checks.c                          | 24 ++++++++++++-------

We don't take changes to dtc in the kernel. It must go to upstream
first and then we sync it.

>  2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 8246891602e7..51877b637bfe 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -20,7 +20,7 @@ properties:
>      pattern: "^spi(@.*|-[0-9a-f])*$"
>
>    "#address-cells":
> -    enum: [0, 1]
> +    enum: [0, 1, 2]

A single 'device' with 2 chip-selects would be 2 'reg' entries, not 2
address cells. 2 address cells would be a chip-select plus some other
data needed to access the device. Sounds like your case is the former.

Rob

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 1/3] spi: dt-bindings: Allow describing flashes with two CS

[Miquel Raynal]

Hi Rob,

robh+dt@kernel.org wrote on Fri, 12 Nov 2021 10:52:44 -0600:

> On Fri, Nov 12, 2021 at 9:24 AM Miquel Raynal <miquel.raynal@bootlin.com> 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 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>
> > ---
> >  .../bindings/spi/spi-controller.yaml          |  6 ++---  
> 
> Needs to go to dt list...
> 
> >  scripts/dtc/checks.c                          | 24 ++++++++++++-------  
> 
> We don't take changes to dtc in the kernel. It must go to upstream
> first and then we sync it.
> 
> >  2 files changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > index 8246891602e7..51877b637bfe 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > @@ -20,7 +20,7 @@ properties:
> >      pattern: "^spi(@.*|-[0-9a-f])*$"
> >
> >    "#address-cells":
> > -    enum: [0, 1]
> > +    enum: [0, 1, 2]  
> 
> A single 'device' with 2 chip-selects would be 2 'reg' entries, not 2
> address cells. 2 address cells would be a chip-select plus some other
> data needed to access the device. Sounds like your case is the former.

Oh yeah I got confused while playing with the different tools, indeed
this change should not be needed, it's the number of 'reg' entries that
we should extend, not its internal size.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[RFC PATCH 2/3] dt-binding: mtd: spi-nor: Allow two CS per device

[Miquel Raynal]

This will be needed in order to be able to describe Xilinx QSPI
controller stacked and parallel modes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 2/3] dt-binding: mtd: spi-nor: Allow two CS per device

[Rob Herring]

On Fri, Nov 12, 2021 at 9:24 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> This will be needed in order to be able to describe Xilinx QSPI
> controller stacked and parallel modes.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> index ed590d7c6e37..dc99629e40db 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> @@ -46,7 +46,7 @@ properties:
>        identified by the JEDEC READ ID opcode (0x9F).
>
>    reg:
> -    maxItems: 1
> +    maxItems: 2

You need minItems here.

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[RFC PATCH 3/3] spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories modes

[Miquel Raynal]

Describe the two dual memories modes which are available on the ZynqMP
QSPI controller: stacked and parallel.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/spi/spi-zynqmp-qspi.yaml         | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
index ea72c8001256..959e008602f5 100644
--- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
@@ -30,6 +30,28 @@ properties:
   clocks:
     maxItems: 2
 
+  xlnx,dual-stacked-memories:
+    type: boolean
+    description: The QSPI controller can be wired to two SPI memorie in stacked
+      mode. This basically means that the two devices share the same bus but
+      have their own chip-select. From the controller perspective, it is like
+      having a single big device, the most significant addressing bit
+      automatically driving one chip-select or the other. This basically doubles
+      the total address space with only a single additional wire. Note that when
+      configured in dual-stacked mode, the controller does not support XIP nor
+      linear addressing.
+
+  xlnx,dual-parallel-memories:
+    type: boolean
+    description: The QSPI controller can be wired to two SPI memorie in parallel
+      mode. The two devices physically are on different buses but will always
+      act synchronously as for each byte, the even bits are stored in one memory
+      and the odd bits are stored in the other. From the controller perspective,
+      it is like having a single big device with a much higher throughput. This
+      basically uses two quad devices as if it was a single octal device. Note
+      that when configured in dual-parallel mode, the controller does not
+      support XIP nor linear addressing.
+
 unevaluatedProperties: false
 
 examples:
@@ -47,5 +69,14 @@ examples:
         interrupt-parent = <&gic>;
         reg = <0x0 0xff0f0000 0x0 0x1000>,
               <0x0 0xc0000000 0x0 0x8000000>;
+        #address-cells = <2>;
+        #size-cells = <0>;
+        xlnx,dual-stacked-memories;
+
+        flash@0,1 {
+          compatible = "jedec,spi-nor";
+          spi-max-frequency = <50000000>;
+          reg = <0>, <1>;
+        };
       };
     };
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 3/3] spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories modes

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 261 bytes --]

On Fri, Nov 12, 2021 at 04:24:11PM +0100, Miquel Raynal wrote:

> Describe the two dual memories modes which are available on the ZynqMP
> QSPI controller: stacked and parallel.

Shouldn't this be a property of the controlled device rather than the
controller?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 3/3] spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories modes

[Miquel Raynal]

Hi Mark,

broonie@kernel.org wrote on Fri, 12 Nov 2021 15:42:14 +0000:

> On Fri, Nov 12, 2021 at 04:24:11PM +0100, Miquel Raynal wrote:
> 
> > Describe the two dual memories modes which are available on the ZynqMP
> > QSPI controller: stacked and parallel.  
> 
> Shouldn't this be a property of the controlled device rather than the
> controller?

I had a hard time picking the right location, I believe we could
live with these properties being flash based. I just proposed to drop
one of these two properties entirely (or make it a generic property) in
the discussion with Pratyush in the cover letter, let us know what you
think.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 0/3] Dual stacked/parallel memories bindings

[Pratyush Yadav]

On 12/11/21 04:24PM, Miquel Raynal wrote:
> Hello Rob, Mark, Tudor & Pratyush,
> 
> Here is an RFC to open the discussion about the sensitive task of
> supporting specific SPI controller modes like Xilinx's where the
> controller can highly abstract the hardware and provide access to a
> single bigger device instead. I'll let you go through the series and
> tell me what you think.
> 
> I think there are two possible approaches:
> 1- Describe the two devices as being a single one which is what we will
>    get from the controller anyway (implies supporting two CS per SPI
>    device)
> or
> 2- Describe the two devices in the device tree and then by software hack
>    into the MTD core to simulate a single device to talk to.

Approach 1 makes more sense to me since once we implement it you can 
also use such multi-CS flashes with "dumber" controllers as well like 
spi-cadence-quadspi. There, the driver would have to manually set the 
chip select instead of it being done automatically by looking at the top 
bit. This would at least work for the dual-stacked memories.

How I envision this being implemented is that SPI NOR would be aware of 
the number of Chip Selects and when to use which one, and it would 
specify the CS value in the SPI MEM op. The controller driver can then 
execute this op as needed. One point to note here is that the entire 
memory won't be read in a single transaction. There would be 2 
transactions: one with CS=0 and one with CS=1. Is this fine for you? Do 
you have something else in mind?

I am not sure how this model would work for a dual-parallel memory 
though.

> 
> I have looked at the code, there is no good solution, but #2 definitely
> looks horribly complicated and subject to a lot of corner cases to
> handle, hence this proposal to go for solution #1.
> 
> Cheers,
> Miquèl
> 
> Miquel Raynal (3):
>   spi: dt-bindings: Allow describing flashes with two CS
>   dt-binding: mtd: spi-nor: Allow two CS per device
>   spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories
>     modes
> 
>  .../bindings/mtd/jedec,spi-nor.yaml           |  2 +-
>  .../bindings/spi/spi-controller.yaml          |  6 ++--
>  .../bindings/spi/spi-zynqmp-qspi.yaml         | 31 +++++++++++++++++++
>  scripts/dtc/checks.c                          | 24 ++++++++------
>  4 files changed, 50 insertions(+), 13 deletions(-)
> 
> -- 
> 2.27.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 0/3] Dual stacked/parallel memories bindings

[Miquel Raynal]

Hi Pratyush,

p.yadav@ti.com wrote on Mon, 15 Nov 2021 15:53:10 +0530:

> On 12/11/21 04:24PM, Miquel Raynal wrote:
> > Hello Rob, Mark, Tudor & Pratyush,
> > 
> > Here is an RFC to open the discussion about the sensitive task of
> > supporting specific SPI controller modes like Xilinx's where the
> > controller can highly abstract the hardware and provide access to a
> > single bigger device instead. I'll let you go through the series and
> > tell me what you think.
> > 
> > I think there are two possible approaches:
> > 1- Describe the two devices as being a single one which is what we will
> >    get from the controller anyway (implies supporting two CS per SPI
> >    device)
> > or
> > 2- Describe the two devices in the device tree and then by software hack
> >    into the MTD core to simulate a single device to talk to.  
> 
> Approach 1 makes more sense to me since once we implement it you can 
> also use such multi-CS flashes with "dumber" controllers as well like 
> spi-cadence-quadspi. There, the driver would have to manually set the 
> chip select instead of it being done automatically by looking at the top 
> bit. This would at least work for the dual-stacked memories.

I believe it would. But in that case we should think about a more
generic binding for the stacked mode. So far I've proposed:
- xlnx,dual-stacked-memories
- xlnx,dual-parallel-memories

It actually looks like the former might be a generic binding. What do
you think is best between:
- 'dual-stacked-memories'
- 'stacked-memories' ('dual' is encoded in the reg property)
- no specific property, it's just a memory with two CS, again 'reg'
  gives us the information.

Then we could keep only the latter property, which looks more specific
to Xilinx and use it as a flash node property instead (as advised
by Mark).

> How I envision this being implemented is that SPI NOR would be aware of 
> the number of Chip Selects and when to use which one, and it would 
> specify the CS value in the SPI MEM op.

Yes, this is the approach I had in mind to. This fits both the purpose
of SPI-NOR and SPI-NAND which will both need to be updated as well tu
support multi-CS.

> The controller driver can then 
> execute this op as needed. One point to note here is that the entire 
> memory won't be read in a single transaction. There would be 2 
> transactions: one with CS=0 and one with CS=1. Is this fine for you? Do 
> you have something else in mind?

I believe this should be let to the controller's discretion and appear
like a single op in the upper layers.

> I am not sure how this model would work for a dual-parallel memory 
> though.

If the controller is aware of the two CS and knows about the full
request we can hope that integration won't be difficult (last famous
words).

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 0/3] Dual stacked/parallel memories bindings

[Pratyush Yadav]

On 16/11/21 09:19AM, Miquel Raynal wrote:
> Hi Pratyush,
> 
> p.yadav@ti.com wrote on Mon, 15 Nov 2021 15:53:10 +0530:
> 
> > On 12/11/21 04:24PM, Miquel Raynal wrote:
> > > Hello Rob, Mark, Tudor & Pratyush,
> > > 
> > > Here is an RFC to open the discussion about the sensitive task of
> > > supporting specific SPI controller modes like Xilinx's where the
> > > controller can highly abstract the hardware and provide access to a
> > > single bigger device instead. I'll let you go through the series and
> > > tell me what you think.
> > > 
> > > I think there are two possible approaches:
> > > 1- Describe the two devices as being a single one which is what we will
> > >    get from the controller anyway (implies supporting two CS per SPI
> > >    device)
> > > or
> > > 2- Describe the two devices in the device tree and then by software hack
> > >    into the MTD core to simulate a single device to talk to.  
> > 
> > Approach 1 makes more sense to me since once we implement it you can 
> > also use such multi-CS flashes with "dumber" controllers as well like 
> > spi-cadence-quadspi. There, the driver would have to manually set the 
> > chip select instead of it being done automatically by looking at the top 
> > bit. This would at least work for the dual-stacked memories.
> 
> I believe it would. But in that case we should think about a more
> generic binding for the stacked mode. So far I've proposed:
> - xlnx,dual-stacked-memories
> - xlnx,dual-parallel-memories
> 
> It actually looks like the former might be a generic binding. What do
> you think is best between:
> - 'dual-stacked-memories'
> - 'stacked-memories' ('dual' is encoded in the reg property)

I think this works best. This would also allow "triple" and "quad" 
stacked flashes.

> - no specific property, it's just a memory with two CS, again 'reg'
>   gives us the information.
> 
> Then we could keep only the latter property, which looks more specific
> to Xilinx and use it as a flash node property instead (as advised
> by Mark).

Even if the parallel mode is only implemented by the Xilinx controller, 
we would need to support it in the core, right? So we need to figure out 
how that case would work as well.

> 
> > How I envision this being implemented is that SPI NOR would be aware of 
> > the number of Chip Selects and when to use which one, and it would 
> > specify the CS value in the SPI MEM op.
> 
> Yes, this is the approach I had in mind to. This fits both the purpose
> of SPI-NOR and SPI-NAND which will both need to be updated as well tu
> support multi-CS.
> 
> > The controller driver can then 
> > execute this op as needed. One point to note here is that the entire 
> > memory won't be read in a single transaction. There would be 2 
> > transactions: one with CS=0 and one with CS=1. Is this fine for you? Do 
> > you have something else in mind?
> 
> I believe this should be let to the controller's discretion and appear
> like a single op in the upper layers.

But then how do you tell the controller when to change the CS if all it 
sees is a single large transaction that spans across multiple flashes? 
You mention in patch 1 that your controller automatically switches CS 
based on most significant address bit, but that would only work if you 
have two 2 GiB flashes wired in. In case someone uses two 1 GiB flashes, 
the MSB always remains 0. And what about controllers that can't switch 
the CS automatically?

> 
> > I am not sure how this model would work for a dual-parallel memory 
> > though.
> 
> If the controller is aware of the two CS and knows about the full
> request we can hope that integration won't be difficult (last famous
> words).

For your specific controller this might work but if we want this feature 
implemented generically I think it would need some more thought.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 0/3] Dual stacked/parallel memories bindings

[Miquel Raynal]

Hi Pratyush,

p.yadav@ti.com wrote on Sat, 20 Nov 2021 00:30:25 +0530:

> On 16/11/21 09:19AM, Miquel Raynal wrote:
> > Hi Pratyush,
> > 
> > p.yadav@ti.com wrote on Mon, 15 Nov 2021 15:53:10 +0530:
> >   
> > > On 12/11/21 04:24PM, Miquel Raynal wrote:  
> > > > Hello Rob, Mark, Tudor & Pratyush,
> > > > 
> > > > Here is an RFC to open the discussion about the sensitive task of
> > > > supporting specific SPI controller modes like Xilinx's where the
> > > > controller can highly abstract the hardware and provide access to a
> > > > single bigger device instead. I'll let you go through the series and
> > > > tell me what you think.
> > > > 
> > > > I think there are two possible approaches:
> > > > 1- Describe the two devices as being a single one which is what we will
> > > >    get from the controller anyway (implies supporting two CS per SPI
> > > >    device)
> > > > or
> > > > 2- Describe the two devices in the device tree and then by software hack
> > > >    into the MTD core to simulate a single device to talk to.    
> > > 
> > > Approach 1 makes more sense to me since once we implement it you can 
> > > also use such multi-CS flashes with "dumber" controllers as well like 
> > > spi-cadence-quadspi. There, the driver would have to manually set the 
> > > chip select instead of it being done automatically by looking at the top 
> > > bit. This would at least work for the dual-stacked memories.  
> > 
> > I believe it would. But in that case we should think about a more
> > generic binding for the stacked mode. So far I've proposed:
> > - xlnx,dual-stacked-memories
> > - xlnx,dual-parallel-memories
> > 
> > It actually looks like the former might be a generic binding. What do
> > you think is best between:
> > - 'dual-stacked-memories'
> > - 'stacked-memories' ('dual' is encoded in the reg property)  
> 
> I think this works best. This would also allow "triple" and "quad" 
> stacked flashes.

Agreed.

> > - no specific property, it's just a memory with two CS, again 'reg'
> >   gives us the information.
> > 
> > Then we could keep only the latter property, which looks more specific
> > to Xilinx and use it as a flash node property instead (as advised
> > by Mark).  
> 
> Even if the parallel mode is only implemented by the Xilinx controller, 
> we would need to support it in the core, right? So we need to figure out 
> how that case would work as well.


> 
> >   
> > > How I envision this being implemented is that SPI NOR would be aware of 
> > > the number of Chip Selects and when to use which one, and it would 
> > > specify the CS value in the SPI MEM op.  
> > 
> > Yes, this is the approach I had in mind to. This fits both the purpose
> > of SPI-NOR and SPI-NAND which will both need to be updated as well tu
> > support multi-CS.
> >   
> > > The controller driver can then 
> > > execute this op as needed. One point to note here is that the entire 
> > > memory won't be read in a single transaction. There would be 2 
> > > transactions: one with CS=0 and one with CS=1. Is this fine for you? Do 
> > > you have something else in mind?  
> > 
> > I believe this should be let to the controller's discretion and appear
> > like a single op in the upper layers.  
> 
> But then how do you tell the controller when to change the CS if all it 
> sees is a single large transaction that spans across multiple flashes?

Actually after changing my mind a couple of times I think we agree on
the fact that the core should be aware of all of that, and:
- in parallel mode let the controller handle a single big op,
- in stacked mode split the request into two ops.

> You mention in patch 1 that your controller automatically switches CS 
> based on most significant address bit, but that would only work if you 
> have two 2 GiB flashes wired in. In case someone uses two 1 GiB flashes, 
> the MSB always remains 0. And what about controllers that can't switch 
> the CS automatically?
> 
> >   
> > > I am not sure how this model would work for a dual-parallel memory 
> > > though.  
> > 
> > If the controller is aware of the two CS and knows about the full
> > request we can hope that integration won't be difficult (last famous
> > words).  
> 
> For your specific controller this might work but if we want this feature 
> implemented generically I think it would need some more thought.
> 


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/