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

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

[Miquel Raynal]

Hello Rob, Mark, Tudor & Pratyush,

Here is a third 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 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    | 21 +++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

-- 
2.27.0

[PATCH v3 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

[PATCH v3 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>
---
 .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 5dd209206e88..13aa6a2374c9 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -82,6 +82,27 @@ properties:
     description:
       Delay, in microseconds, after a write transfer.
 
+  stacked-memories:
+    type: boolean
+    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. XIP is
+      usually not supported in this mode.
+
+  parallel-memories:
+    type: boolean
+    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. XIP is usually not
+      supported in this mode.
+
 # The controller specific properties go here.
 allOf:
   - $ref: cdns,qspi-nor-peripheral-props.yaml#
-- 
2.27.0

[PATCH v3 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>
---
 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..35743448fdd2 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;
+        };
     };
-- 
2.27.0

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

[Rob Herring]

On Mon, Dec 06, 2021 at 10:59:20AM +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>
> ---
>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 0/3] Stacked/parallel memories bindings

[Rob Herring]

On Mon, Dec 06, 2021 at 10:59:18AM +0100, Miquel Raynal wrote:
> Hello Rob, Mark, Tudor & Pratyush,
> 
> Here is a third versions for these bindings, which applies on top of
> Pratyush's work:
> https://lore.kernel.org/all/20211109181911.2251-1-p.yadav@ti.com/

Mark, can you either provide a stable branch with this or apply the 
series? Note that there's going to be some other patches needing 
spi-peripheral-props.yaml, so providing a branch might be better if you 
don't want to collect the patches.

Rob

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

[Rob Herring]

On Mon, 06 Dec 2021 10:59:21 +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(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Pratyush Yadav]

On 06/12/21 10:59AM, 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>
> ---
>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 5dd209206e88..13aa6a2374c9 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -82,6 +82,27 @@ properties:
>      description:
>        Delay, in microseconds, after a write transfer.
>  
> +  stacked-memories:
> +    type: boolean

I don't think a boolean is enough to completely describe the memory. 
Sure, you say the memories are stacked, but where do you specify when to 
switch the CS? They could be two 512 MiB memories, two 1 GiB memories, 
or one 512 MiB and one 256 MiB.

> +    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. XIP is
> +      usually not supported in this mode.
> +
> +  parallel-memories:
> +    type: boolean

With this I assume both memories have to be the same size?

> +    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. XIP is usually not
> +      supported in this mode.
> +
>  # The controller specific properties go here.
>  allOf:
>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> -- 
> 2.27.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

[Tudor.Ambarus]

Hi, Miquel,

On 12/6/21 11:59 AM, Miquel Raynal wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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.

The stacked mode that you describe seems particular to a specific
vendor. There are multi die NOR flashes which do not require any
controller intervention, the logic is held at the flash level:
https://media-www.micron.com/-/media/client/global/documents/products/technical-note/nor-flash/tn2505_n25q_mt25q_stacked_devices.pdf?rev=7a23cc95238e46f7b22e2a9f6bc736b7

Can you point us to which kind of memories you're willing to add
support for? Some datasheets will be best.

Cheers,
ta

> 
> 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 v3 2/3] spi: dt-bindings: Describe stacked/parallel memories modes

[Tudor.Ambarus]

On 12/7/21 9:14 AM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 06/12/21 10:59AM, 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>
>> ---
>>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> index 5dd209206e88..13aa6a2374c9 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> @@ -82,6 +82,27 @@ properties:
>>      description:
>>        Delay, in microseconds, after a write transfer.
>>
>> +  stacked-memories:
>> +    type: boolean
> 
> I don't think a boolean is enough to completely describe the memory.
> Sure, you say the memories are stacked, but where do you specify when to
> switch the CS? They could be two 512 MiB memories, two 1 GiB memories,
> or one 512 MiB and one 256 MiB.

If the multi-die flash contains identical dies then the die boundary can be
determined with flash_size / number_of_cs. Are there any multi die flashes
with different types of dies?

> 
>> +    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. XIP is
>> +      usually not supported in this mode.
>> +
>> +  parallel-memories:
>> +    type: boolean
> 
> With this I assume both memories have to be the same size?

It looks like the assumption for both cases is that the dies are identical.

> 
>> +    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. XIP is usually not
>> +      supported in this mode.
>> +
>>  # The controller specific properties go here.
>>  allOf:
>>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
>> --
>> 2.27.0
>>
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.
> 

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

[Tudor.Ambarus]

On 12/7/21 9:35 AM, Tudor Ambarus wrote:
> On 12/7/21 9:14 AM, Pratyush Yadav wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 06/12/21 10:59AM, 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>
>>> ---
>>>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>> index 5dd209206e88..13aa6a2374c9 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>> @@ -82,6 +82,27 @@ properties:
>>>      description:
>>>        Delay, in microseconds, after a write transfer.
>>>
>>> +  stacked-memories:
>>> +    type: boolean
>>
>> I don't think a boolean is enough to completely describe the memory.
>> Sure, you say the memories are stacked, but where do you specify when to
>> switch the CS? They could be two 512 MiB memories, two 1 GiB memories,
>> or one 512 MiB and one 256 MiB.
> 
> If the multi-die flash contains identical dies then the die boundary can be
> determined with flash_size / number_of_cs. Are there any multi die flashes

but the problem is there, yes, there is still the case where there are stacked
devices with a single cs. We'll need to describe the size of the die in some
way.

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

[Tudor.Ambarus]

On 12/7/21 9:43 AM, Tudor Ambarus wrote:
> On 12/7/21 9:35 AM, Tudor Ambarus wrote:
>> On 12/7/21 9:14 AM, Pratyush Yadav wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 06/12/21 10:59AM, 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>
>>>> ---
>>>>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
>>>>  1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>>> index 5dd209206e88..13aa6a2374c9 100644
>>>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>>> @@ -82,6 +82,27 @@ properties:
>>>>      description:
>>>>        Delay, in microseconds, after a write transfer.
>>>>
>>>> +  stacked-memories:
>>>> +    type: boolean
>>>
>>> I don't think a boolean is enough to completely describe the memory.
>>> Sure, you say the memories are stacked, but where do you specify when to
>>> switch the CS? They could be two 512 MiB memories, two 1 GiB memories,
>>> or one 512 MiB and one 256 MiB.
>>
>> If the multi-die flash contains identical dies then the die boundary can be
>> determined with flash_size / number_of_cs. Are there any multi die flashes
> 
> but the problem is there, yes, there is still the case where there are stacked
> devices with a single cs. We'll need to describe the size of the die in some
> way.
> 

Even better, winbond stacks a NOR and a NAND:
https://www.winbond.com/hq/product/code-storage-flash-memory/spistack-flash/?__locale=en

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

[Pratyush Yadav]

On 07/12/21 07:35AM, Tudor.Ambarus@microchip.com wrote:
> On 12/7/21 9:14 AM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 06/12/21 10:59AM, 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>
> >> ---
> >>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
> >>  1 file changed, 21 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> >> index 5dd209206e88..13aa6a2374c9 100644
> >> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> >> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> >> @@ -82,6 +82,27 @@ properties:
> >>      description:
> >>        Delay, in microseconds, after a write transfer.
> >>
> >> +  stacked-memories:
> >> +    type: boolean
> > 
> > I don't think a boolean is enough to completely describe the memory.
> > Sure, you say the memories are stacked, but where do you specify when to
> > switch the CS? They could be two 512 MiB memories, two 1 GiB memories,
> > or one 512 MiB and one 256 MiB.
> 
> If the multi-die flash contains identical dies then the die boundary can be
> determined with flash_size / number_of_cs. Are there any multi die flashes
> with different types of dies?

The way I see it, a multi-die flash is not much different from 2 
independent flashes attached to the same SPI bus. So if we are going to 
implement this feature, I want it to be generic enough to allow 
supporting this type of hardware setup as well.

I am not aware of any flashes with a different CS for each die (that 
isn't handled by the flash internally), let alone with different types 
of dies. IIRC from our IRC conversation, Miquel's use case was using 2 
smaller identical flashes connected to the same SPI bus with 1 CS each. 
Do I remember this right Miquel?

> 
> > 
> >> +    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. XIP is
> >> +      usually not supported in this mode.
> >> +
> >> +  parallel-memories:
> >> +    type: boolean
> > 
> > With this I assume both memories have to be the same size?
> 
> It looks like the assumption for both cases is that the dies are identical.

I would like to _not_ assume that for stacked-memories, unless 
implementing that becomes too complicated.

> 
> > 
> >> +    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. XIP is usually not
> >> +      supported in this mode.
> >> +
> >>  # The controller specific properties go here.
> >>  allOf:
> >>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> >> --
> >> 2.27.0
> >>
> > 
> > --
> > Regards,
> > Pratyush Yadav
> > Texas Instruments Inc.
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

[Miquel Raynal]

Hi Pratyush & Tudor,

p.yadav@ti.com wrote on Tue, 7 Dec 2021 13:27:23 +0530:

> On 07/12/21 07:35AM, Tudor.Ambarus@microchip.com wrote:
> > On 12/7/21 9:14 AM, Pratyush Yadav wrote:  
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > > 
> > > On 06/12/21 10:59AM, 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>
> > >> ---
> > >>  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
> > >>  1 file changed, 21 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > >> index 5dd209206e88..13aa6a2374c9 100644
> > >> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > >> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > >> @@ -82,6 +82,27 @@ properties:
> > >>      description:
> > >>        Delay, in microseconds, after a write transfer.
> > >>
> > >> +  stacked-memories:
> > >> +    type: boolean  
> > > 
> > > I don't think a boolean is enough to completely describe the memory.
> > > Sure, you say the memories are stacked, but where do you specify when to
> > > switch the CS? They could be two 512 MiB memories, two 1 GiB memories,
> > > or one 512 MiB and one 256 MiB.  
> > 
> > If the multi-die flash contains identical dies then the die boundary can be
> > determined with flash_size / number_of_cs. Are there any multi die flashes
> > with different types of dies?  
> 
> The way I see it, a multi-die flash is not much different from 2 
> independent flashes attached to the same SPI bus. So if we are going to 
> implement this feature, I want it to be generic enough to allow 
> supporting this type of hardware setup as well.
> 
> I am not aware of any flashes with a different CS for each die (that 
> isn't handled by the flash internally), let alone with different types 
> of dies. IIRC from our IRC conversation, Miquel's use case was using 2 
> smaller identical flashes connected to the same SPI bus with 1 CS each. 
> Do I remember this right Miquel?

I made the assumption that dies would be identical in order to use this
mode. However, if you think this is too risky I see two alternatives:
* Keep the bindings as I proposed and if we ever have the case, add
  another property, something like:
	stacked-memories;
	stacked-sizes = <x>, <y>;
* Merge these two properties into one:
	stacked-memories = <x>, <y>;

But TBH I prefer the former solution for these two reasons:
1/ You need to know the devices exact geometry when writing the
   bindings while this is something that is usually let to the core and
   the hardware designers.
2/ I am not sure this is really a valid use case. If we ever need to
   concatenate two devices, in particular if they are different, I
   would prefer reviving the mtd-concat series which, besides lacking a
   dynamic discovery feature, is almost ready to be used. Plus, adding
   too much complexity to the core logic (such as handling different
   die sizes) might impact negatively the overall performances even for
   simpler devices.

> > >> +    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. XIP is
> > >> +      usually not supported in this mode.
> > >> +
> > >> +  parallel-memories:
> > >> +    type: boolean  
> > > 
> > > With this I assume both memories have to be the same size?  
> > 
> > It looks like the assumption for both cases is that the dies are identical.  
> 
> I would like to _not_ assume that for stacked-memories, unless 
> implementing that becomes too complicated.
> 
> >   
> > >   
> > >> +    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. XIP is usually not
> > >> +      supported in this mode.
> > >> +
> > >>  # The controller specific properties go here.
> > >>  allOf:
> > >>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > >> --
> > >> 2.27.0
> > >>  
> > > 
> > > --
> > > Regards,
> > > Pratyush Yadav
> > > Texas Instruments Inc.
> > >   
> >   
> 


Thanks,
Miquèl

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

[Miquel Raynal]

Hello Tudor,

Tudor.Ambarus@microchip.com wrote on Tue, 7 Dec 2021 07:16:11 +0000:

> Hi, Miquel,
> 
> On 12/6/21 11:59 AM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > 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.  
> 
> The stacked mode that you describe seems particular to a specific
> vendor. There are multi die NOR flashes which do not require any
> controller intervention, the logic is held at the flash level:
> https://media-www.micron.com/-/media/client/global/documents/products/technical-note/nor-flash/tn2505_n25q_mt25q_stacked_devices.pdf?rev=7a23cc95238e46f7b22e2a9f6bc736b7
> 
> Can you point us to which kind of memories you're willing to add
> support for? Some datasheets will be best.

Unfortunately I don't have any datasheets to propose, I think this mode
must be seen like a controller abstraction of any device with several
physical cs.

The question: "should these properties be common to all devices" is
still open, I don't have a strong opinion, whether we should make these
Xilinx specific, or not. For now they are, but in the near future, it
is not so sure (and my crystal ball is under maintenance ;) ).

Thanks,
Miquèl

Re: [PATCH v3 0/3] Stacked/parallel memories bindings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Mon, Dec 06, 2021 at 03:31:32PM -0600, Rob Herring wrote:

> Mark, can you either provide a stable branch with this or apply the 
> series? Note that there's going to be some other patches needing 
> spi-peripheral-props.yaml, so providing a branch might be better if you 
> don't want to collect the patches.

I'll apply the series but there's still debate about it so it looks like
there will be further revisions.  Unfortunately the patch is buried in
the middle of history so it's hard to pull out a sensible stable branch,
Linus gets upset about topic branches which makes doing this sort of
thing retroactively painful.

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

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

[Miquel Raynal]

Hi Rob,

robh@kernel.org wrote on Mon, 6 Dec 2021 15:22:02 -0600:

> On Mon, Dec 06, 2021 at 10:59:20AM +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>
> > ---
> >  .../bindings/spi/spi-peripheral-props.yaml    | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)  
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

I am sending a new version of this series so that I can get feedback on
other way of describing the flashes, so I'll drop your tag because I'll
need you to re-check that I'm not doing anything silly (it took me a
while to understand the array vs. matrix logic).

Thanks,
Miquèl