diff mbox series

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

Message ID 20211206095921.33302-3-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Headers show
Series Stacked/parallel memories bindings | expand

Commit Message

Miquel Raynal Dec. 6, 2021, 9:59 a.m. UTC
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(+)

Comments

Rob Herring (Arm) Dec. 6, 2021, 9:22 p.m. UTC | #1
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>
Pratyush Yadav Dec. 7, 2021, 7:14 a.m. UTC | #2
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
>
Tudor Ambarus Dec. 7, 2021, 7:35 a.m. UTC | #3
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.
>
Tudor Ambarus Dec. 7, 2021, 7:43 a.m. UTC | #4
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.
Tudor Ambarus Dec. 7, 2021, 7:47 a.m. UTC | #5
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
Pratyush Yadav Dec. 7, 2021, 7:57 a.m. UTC | #6
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.
> > 
>
Miquel Raynal Dec. 7, 2021, 8:37 a.m. UTC | #7
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
Miquel Raynal Dec. 10, 2021, 8:07 p.m. UTC | #8
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
diff mbox series

Patch

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#