Message-ID: <87y11gwtij.fsf@bootlin.com>
Date: Mon, 18 Nov 2024 14:39:32 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>
Cc: <tudor.ambarus@...aro.org>,  <michael@...le.cc>,  <broonie@...nel.org>,
  <pratyush@...nel.org>,  <richard@....at>,  <vigneshr@...com>,
  <robh@...nel.org>,  <conor+dt@...nel.org>,  <krzk+dt@...nel.org>,
  <venkatesh.abbarapu@....com>,  <linux-spi@...r.kernel.org>,
  <linux-kernel@...r.kernel.org>,  <linux-mtd@...ts.infradead.org>,
  <nicolas.ferre@...rochip.com>,  <alexandre.belloni@...tlin.com>,
  <claudiu.beznea@...on.dev>,  <michal.simek@....com>,
  <linux-arm-kernel@...ts.infradead.org>,  <alsa-devel@...a-project.org>,
  <patches@...nsource.cirrus.com>,  <git@....com>,
  <amitrkcian2002@...il.com>,  <beanhuo@...ron.com>
Subject: Re: [RFC PATCH 2/2] dt-bindings: spi: Update stacked and parallel
 bindings

Hi Amit,

On 26/10/2024 at 13:23:47 +0530, Amit Kumar Mahapatra <amit.kumar-mahapatra@....com> wrote:

> For implementing the proposed solution the current 'stacked-memories' &
> 'parallel-memories' bindings need to be updated as follow.
>
> stacked-memories binding changes:
> - Each flash will have its own flash node. This approach allows flashes of
>   different makes and sizes to be stacked together, as each flash will be
>   probed individually.
> - Each of the flash node will have its own “reg” property that will contain
>   its physical CS.
> - Remove the size information from the bindings as it can be retrived
>   drirectly from the flash.
> - The stacked-memories DT bindings will contain the phandles of the flash
>   nodes connected in stacked mode.
>
> The new layer will update the mtd->size and other mtd_info parameters after
> both the flashes are probed and will call mtd_device_register with the
> combined information.
>
> spi@0 {
>         ...
>         flash@0 {
>                 compatible = "jedec,spi-nor"
>                 reg = <0x00>;
>                 stacked-memories = <&flash@0 &flash@1>;
>                 spi-max-frequency = <50000000>;
>                 ...
>                         partitions {
>                         compatible = "fixed-partitions";
>                                 concat-partition = <&flash0_partition &flash1_partition>;
>                                 flash0_partition: partition@0 {
>                                         label = "part0_0";
>                                         reg = <0x0 0x800000>;
>                                 }
>                         }
>         }
>         flash@1 {
>                 compatible = "jedec,spi-nor"
>                 reg = <0x01>;
>                 stacked-memories = <&flash@0 &flash@1>;
>                 spi-max-frequency = <50000000>;
>                 ...
>                         partitions {

Same comment as before here.

>                         compatible = "fixed-partitions";
>                                 concat-partition = <&flash0_partition &flash1_partition>;
>                                 flash1_partition: partition@0 {
>                                         label = "part0_1";
>                                         reg = <0x0 0x800000>;
>                                 }
>                         }
>         }
>
> }
>
> parallel-memories binding changes:
> - Remove the size information from the bindings and change the type to
>   boolen.
> - Each flash connected in parallel mode should be identical and will have
>   one flash node for both the flash devices.
> - The “reg” prop will contain the physical CS number for both the connected
>   flashes.
>
> The new layer will double the mtd-> size and register it with the mtd
> layer.

Not so sure about that, you'll need a new mtd device to capture the
whole device. But this is implementation related, not relevant for
binding.

>
> spi@1 {
>         ...
>         flash@3 {
>                 compatible = "jedec,spi-nor"
>                 reg = <0x00 0x01>;
>                 paralle-memories ;

Please fix the typos and the spacing (same above).

>                 spi-max-frequency = <50000000>;
>                 ...
>                         partitions {
>                         compatible = "fixed-partitions";
>                                 flash0_partition: partition@0 {
>                                         label = "part0_0";
>                                         reg = <0x0 0x800000>;
>                                 }
>                         }
>         }
> }
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>
> ---
>  .../bindings/spi/spi-controller.yaml          | 23 +++++++++++++++++--
>  .../bindings/spi/spi-peripheral-props.yaml    |  9 +++-----
>  2 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 093150c0cb87..2d300f98dd72 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -185,7 +185,26 @@ examples:
>          flash@2 {
>              compatible = "jedec,spi-nor";
>              spi-max-frequency = <50000000>;
> -            reg = <2>, <3>;
> -            stacked-memories = /bits/ 64 <0x10000000 0x10000000>;
> +            reg = <2>;
> +            stacked-memories = <&flash0 &flash1>;
>          };

I'm sorry but this is not what you've talked about in this series.
Either you have flash0 and flash1 and use the stacked-memories property
in both of them (which is what you described) or you create a third
virtual device which points to two other flashes. This example allows
for an easier use of the partitions mechanism on top of a virtual mtd
device but, heh, you're now describing a virtual mtd device, which is
not a physical device as it "should" be.

Thanks,
Miquèl

Powered by blists - more mailing lists