diff mbox series

[v4,3/4] dt-bindings: mtd: Describe mtd-concat devices

Message ID 20191113171505.26128-4-miquel.raynal@bootlin.com
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series MTD concat | expand

Commit Message

Miquel Raynal Nov. 13, 2019, 5:15 p.m. UTC
From: Bernhard Frauendienst <kernel@nospam.obeliks.de>

The main use case to concatenate MTD devices is probably SPI-NOR
flashes where the number of address bits is limited to 24, which can
access a range of 16MiB. Board manufacturers might want to double the
SPI storage size by adding a second flash asserted thanks to a second
chip selects which enhances the addressing capabilities to 25 bits,
32MiB. Having two devices for twice the size is great but without more
glue, we cannot define partition boundaries spread across the two
devices. This is the gap mtd-concat intends to address.

There are two options to describe concatenated devices:
1/ One flash chip is described in the DT with two CS;
2/ Two flash chips are described in the DT with one CS each, a virtual
device is also created to describe the concatenation.

Solution 1/ presents at least 3 issues:
* The hardware description is abused;
* The concatenation only works for SPI devices (while it could be
  helpful for any MTD);
* It would require a lot of rework in the SPI core as most of the
  logic assumes there is and there always will be only one CS per
  chip.

Solution 2/ also has caveats:
* The virtual device has no hardware reality;
* Possible optimizations at the hardware level will be hard to enable
  efficiently (ie. a common direct mapping abstracted by a SPI
  memories oriented controller).

There is no easy and perfect answer to this need but it feels more
reasonable to address the problem with solution 2, with the
information/needs we have today.

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
[<miquel.raynal@bootlin.com>:
Wrote a commit message explaining what mtd-concat is.
Explained the implementation details.
Switched to yaml schema.]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mtd/mtd-concat.yaml   | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.yaml

Comments

Rob Herring Nov. 18, 2019, 10:13 p.m. UTC | #1
On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> 
> The main use case to concatenate MTD devices is probably SPI-NOR
> flashes where the number of address bits is limited to 24, which can
> access a range of 16MiB. Board manufacturers might want to double the
> SPI storage size by adding a second flash asserted thanks to a second
> chip selects which enhances the addressing capabilities to 25 bits,
> 32MiB. Having two devices for twice the size is great but without more
> glue, we cannot define partition boundaries spread across the two
> devices. This is the gap mtd-concat intends to address.
> 
> There are two options to describe concatenated devices:
> 1/ One flash chip is described in the DT with two CS;
> 2/ Two flash chips are described in the DT with one CS each, a virtual
> device is also created to describe the concatenation.
> 
> Solution 1/ presents at least 3 issues:
> * The hardware description is abused;
> * The concatenation only works for SPI devices (while it could be
>   helpful for any MTD);
> * It would require a lot of rework in the SPI core as most of the
>   logic assumes there is and there always will be only one CS per
>   chip.

This seems ok if all the devices are identical.

> Solution 2/ also has caveats:
> * The virtual device has no hardware reality;
> * Possible optimizations at the hardware level will be hard to enable
>   efficiently (ie. a common direct mapping abstracted by a SPI
>   memories oriented controller).

Something like this may be necessary if data is interleaved rather than 
concatinated.


Solution 3
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";
    ...
  };
};

Maybe a link back to the previous paritions too or a boolean to mark as 
a continuation.

No idea how well this works or not for the kernel, but that really 
shouldn't matter for the binding design.

Rob
Miquel Raynal Nov. 25, 2019, 2:15 p.m. UTC | #2
Hi Rob,

Rob Herring <robh@kernel.org> wrote on Mon, 18 Nov 2019 16:13:41 -0600:

> On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> > From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> > 
> > The main use case to concatenate MTD devices is probably SPI-NOR
> > flashes where the number of address bits is limited to 24, which can
> > access a range of 16MiB. Board manufacturers might want to double the
> > SPI storage size by adding a second flash asserted thanks to a second
> > chip selects which enhances the addressing capabilities to 25 bits,
> > 32MiB. Having two devices for twice the size is great but without more
> > glue, we cannot define partition boundaries spread across the two
> > devices. This is the gap mtd-concat intends to address.
> > 
> > There are two options to describe concatenated devices:
> > 1/ One flash chip is described in the DT with two CS;
> > 2/ Two flash chips are described in the DT with one CS each, a virtual
> > device is also created to describe the concatenation.
> > 
> > Solution 1/ presents at least 3 issues:
> > * The hardware description is abused;
> > * The concatenation only works for SPI devices (while it could be
> >   helpful for any MTD);
> > * It would require a lot of rework in the SPI core as most of the
> >   logic assumes there is and there always will be only one CS per
> >   chip.  
> 
> This seems ok if all the devices are identical.

This is not an option for Mark and I agree with him as we are faking
the reality: the two devices we want to virtually concatenate may be
two physically different devices. Binding them as one is lying.

> > Solution 2/ also has caveats:
> > * The virtual device has no hardware reality;
> > * Possible optimizations at the hardware level will be hard to enable
> >   efficiently (ie. a common direct mapping abstracted by a SPI
> >   memories oriented controller).  
> 
> Something like this may be necessary if data is interleaved rather than 
> concatinated.

This is something that is gonna happen too, it is called "dual
parallel".

> Solution 3
> 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";
>     ...
>   };
> };

I honestly don't see how this is different as solution 2/? In one case
we describe the partition concatenation in one subnode as a "link", in
the other we create a separate node to describe the link. Are you
strongly opposed as solution 2/? From a pure conceptual point of view,
is it really different than 3/?
 

Thanks,
Miquèl
Rob Herring Dec. 2, 2019, 4:26 p.m. UTC | #3
On Mon, Nov 25, 2019 at 8:15 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Rob,
>
> Rob Herring <robh@kernel.org> wrote on Mon, 18 Nov 2019 16:13:41 -0600:
>
> > On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> > > From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> > >
> > > The main use case to concatenate MTD devices is probably SPI-NOR
> > > flashes where the number of address bits is limited to 24, which can
> > > access a range of 16MiB. Board manufacturers might want to double the
> > > SPI storage size by adding a second flash asserted thanks to a second
> > > chip selects which enhances the addressing capabilities to 25 bits,
> > > 32MiB. Having two devices for twice the size is great but without more
> > > glue, we cannot define partition boundaries spread across the two
> > > devices. This is the gap mtd-concat intends to address.
> > >
> > > There are two options to describe concatenated devices:
> > > 1/ One flash chip is described in the DT with two CS;
> > > 2/ Two flash chips are described in the DT with one CS each, a virtual
> > > device is also created to describe the concatenation.
> > >
> > > Solution 1/ presents at least 3 issues:
> > > * The hardware description is abused;
> > > * The concatenation only works for SPI devices (while it could be
> > >   helpful for any MTD);
> > > * It would require a lot of rework in the SPI core as most of the
> > >   logic assumes there is and there always will be only one CS per
> > >   chip.
> >
> > This seems ok if all the devices are identical.
>
> This is not an option for Mark and I agree with him as we are faking
> the reality: the two devices we want to virtually concatenate may be
> two physically different devices. Binding them as one is lying.
>
> > > Solution 2/ also has caveats:
> > > * The virtual device has no hardware reality;
> > > * Possible optimizations at the hardware level will be hard to enable
> > >   efficiently (ie. a common direct mapping abstracted by a SPI
> > >   memories oriented controller).
> >
> > Something like this may be necessary if data is interleaved rather than
> > concatinated.
>
> This is something that is gonna happen too, it is called "dual
> parallel".

Then it would be good to think about how that should look. Maybe
there's overlap or maybe not.

> > Solution 3
> > 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";
> >     ...
> >   };
> > };
>
> I honestly don't see how this is different as solution 2/?

It's a single new property rather than a whole binding for a virtual
device. It's a minimal change to the DT. It would work with existing
bootloaders (and other OSs and older kernels) without change except
for the one concatenated partition.

> In one case
> we describe the partition concatenation in one subnode as a "link", in
> the other we create a separate node to describe the link. Are you
> strongly opposed as solution 2/?

I'd prefer to not have virtual devices without good reason.

> From a pure conceptual point of view,
> is it really different than 3/?
>
>
> Thanks,
> Miquèl
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/mtd-concat.yaml b/Documentation/devicetree/bindings/mtd/mtd-concat.yaml
new file mode 100644
index 000000000000..7341198575cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtd-concat.yaml
@@ -0,0 +1,56 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mtd-concat.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual MTD concatenation device bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: "mtd-concat"
+
+  devices:
+    minItems: 2
+    description: |
+      List of phandles to MTD nodes that should be concatenated (in
+      order).
+
+required:
+  - compatible
+  - devices
+
+examples:
+  - |
+    &spi {
+            flash0: flash@0 {
+	            reg = <0>;
+            };
+            flash1: flash@1 {
+	            reg = <1>;
+            };
+    };
+
+    flash {
+            compatible = "mtd-concat";
+            devices = <&flash0 &flash1>;
+
+            partitions {
+                    compatible = "fixed-partitions";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    partition@0 {
+                            label = "boot";
+                            reg = <0x0000000 0x0040000>;
+                            read-only;
+                    };
+                    partition@40000 {
+                            label = "firmware";
+                            reg = <0x0040000 0x1fc0000>;
+                    };
+            };
+    };