Hi Amit,
amit.kumar-mahapatra@xxxxxxx wrote on Thu, 10 Oct 2024 10:35:06 +0000:
> Hello Miquel,
>
> > -----Original Message-----
> > From: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > Sent: Thursday, October 10, 2024 2:58 PM
> > To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@xxxxxxx>
> > Cc: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>; michael@xxxxxxxx;
> > broonie@xxxxxxxxxx; pratyush@xxxxxxxxxx; richard@xxxxxx; vigneshr@xxxxxx; Rob
> > Herring <robh@xxxxxxxxxx>; cornor+dt@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; linux-
> > spi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-mtd@xxxxxxxxxxxxxxxxxxx;
> > nicolas.ferre@xxxxxxxxxxxxx; alexandre.belloni@xxxxxxxxxxx;
> > claudiu.beznea@xxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; linux-arm-
> > kernel@xxxxxxxxxxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx;
> > patches@xxxxxxxxxxxxxxxxxxxxx; linux-sound@xxxxxxxxxxxxxxx; git (AMD-Xilinx)
> > <git@xxxxxxx>; amitrkcian2002@xxxxxxxxx; Conor Dooley
> > <conor.dooley@xxxxxxxxxxxxx>; beanhuo@xxxxxxxxxx
> > Subject: Re: Add stacked and parallel memories support in spi-nor
> >
> > Hi Amit,
> >
> > amit.kumar-mahapatra@xxxxxxx wrote on Thu, 10 Oct 2024 09:17:58 +0000:
> >
> > > Hello Miquel,
> > >
> > > > > - The stacked-memories DT bindings will contain the phandles of
> > > > > the flash nodes
> > > > connected in stacked mode.
> > > > >
> > > > > - The first flash node will contain the mtd partition that would
> > > > > have the cross over memory staring at a memory location in the
> > > > > first flash and ending at some memory location of the 2nd flash
> > > >
> > > > I don't like that much. Describing partitions past the actual device
> > > > sounds wrong. If you look into [1] there is a suggestion from Rob to
> > > > handle this case using a property that tells us there is a
> > > > continuation, so from a software perspective we can easily make the link, but on
> > the hardware description side the information are correct.
> > >
> > > I reviewed Rob's suggestions in [1], and I need to examine the MTD
> > > layer to determine how this can be implemented from a software perspective.
> > > For reference, here is Rob's suggestion:
> > >
> > > Describe each device and partition separately and add link(s) from one
> > > partition to the next
> > >
> > > flash0 {
> > > partitions {
> > > compatible = "fixed-partitions";
> > > concat-partition = <&flash1_partitions>;
> > > ...
> > > };
> > > };
> > >
> > > flash1 {
> > > flash1_partition: partitions {
> > > compatible = "fixed-partitions";
> > > ...
> > > };
> > > };
> > >
> > > >
> > > > If this description is accepted, then fine, you can deprecate the "stacked-
> > memories"
> > > > property.
> > >
> > > I believe that in addition to Rob's description, we should also
> > > include the 'stacked-memories' property. This property helps us
> > > identify which flashes are stacked, while Rob's suggestion explains
> > > how the partitions within the stacked flashes are connected.
> > >
> > > For example, if we have three flashes connected to an SPI host, with
> > > flash@0 and flash@1 operating in stacked mode and flash@2 functioning
> > > as a standalone flash, the Device Tree binding might look something like this:
> > > Please share your thoughts on this.
> > >
> > > spi@0 {
> > > ...
> > > flash@0 {
> > > compatible = "jedec,spi-nor"
> > > reg = <0x00>;
> > > stacked-memories = <&flash@0 &flash@1>;
> > > spi-max-frequency = <50000000>;
> > > ...
> > > flash0_partition: partitions {
> > > compatible = "fixed-partitions";
> > > concat-partition = <&flash1_partitions>;
> > > partition@0 {
> > > label = "Stacked-Flash-1";
> > > reg = <0x0 0x800000>;
> > > }
> > > }
> > > }
> > > flash@1 {
> > > compatible = "jedec,spi-nor"
> > > reg = <0x01>;
> > > spi-max-frequency = <50000000>;
> > > ...
> > > flash1_partition: partitions {
> > > compatible = "fixed-partitions";
> > > concat-partition = <&flash0_partitions>;
> > > partition@0 {
> > > label = " Stacked-Flash-2";
> > > reg = <0x0 0x800000>;
> > > }
> > > }
> > > }
> > >
> > > flash@2 {
> > > compatible = "jedec,spi-nor"
> > > reg = <0x01>;
> > > spi-max-frequency = <50000000>;
> > > ...
> > > partitions {
> > > compatible = "fixed-partitions";
> > > concat-partition = <&flash0_partitions>;
> > > partition@0 {
> > > label = "Single-Flash";
> > > reg = <0x0 0x800000>;
> > > }
> > > }
> > > }
> >
> > I'm sorry but this is pretty messed up. The alignments are wrong, I believe the labels
> > are wrong, the reg properties as well. Can you please work on this example and
> > send an updated version?
>
> Apologies for that. Here's the updated version along with the explanation.
Thanks for the update.
> spi@0 {
> ...
> flash@0 {
> compatible = "jedec,spi-nor"
> reg = <0x00>;
> stacked-memories = <&flash@0 &flash@1>;
The same property should, IMHO, also be expected...
> spi-max-frequency = <50000000>;
> ...
> partitions {
> compatible = "fixed-partitions";
> concat-partition = <&flash1_partition>; /* Link to the flash@1 partition@0 */
> flash0_partition: partition@0 {
> label = "part0_0";
> reg = <0x0 0x800000>;
> }
> }
> }
> flash@1 {
> compatible = "jedec,spi-nor"
> reg = <0x01>;
... here.
> spi-max-frequency = <50000000>;
> ...
> partitions {
> compatible = "fixed-partitions";
> concat-partition = <&flash0_partition>; /* Link to the flash@0 partition@0 */
> flash1_partition: partition@0 {
> label = "part0_1";
> reg = <0x0 0x800000>;
> }
> }
> }
>
> flash@2 {
> compatible = "jedec,spi-nor"
> reg = <0x02>;
> spi-max-frequency = <50000000>;
> ...
> partitions {
> compatible = "fixed-partitions";
> partition@0 {
> label = "part1_0";
> reg = <0x0 0x800000>;
> }
> }
> }
> }
Otherwise, okay for me.
Thanks,
Miquèl
