Re: [RFC PATCH 1/2] dt-bindings: mtd: Add bindings for describing concatinated MTD devices

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

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

> This approach was suggested by Rob [1] during a discussion on Miquel's
> initial approach [2] to extend the MTD-CONCAT driver to support stacked
> memories.
> Define each flash node separately with its respective partitions, and add
> a 'concat-parts' binding to link the partitions of the two flash nodes that
> need to be concatenated.
>
> flash@0 {
>         compatible = "jedec,spi-nor"
>         ...
>                 partitions {

Wrong indentation here and below which makes the example hard to read.

>                 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"
>         ...
>                 partitions {
>                 compatible = "fixed-partitions";
>                         concat-partition = <&flash0_partition &flash1_partition>;
>                         flash1_partition: partition@0 {
>                                 label = "part0_1";
>                                 reg = <0x0 0x800000>;
>                         }
>                 }
> }

This approach has a limitation I didn't think about before: you cannot
use anything else than fixed partitions as partition parser.

> Based on the bindings the MTD-CONCAT driver need to be updated to create
> virtual mtd-concat devices.
>
> [1] https://lore.kernel.org/all/20191118221341.GA30937@bogus/
> [2] https://lore.kernel.org/all/20191113171505.26128-4-miquel.raynal@xxxxxxxxxxx/
>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xxxxxxx>
> ---
>  .../mtd/partitions/fixed-partitions.yaml       | 18 ++++++++++++++++++
>  .../bindings/mtd/partitions/partitions.yaml    |  6 ++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> index 058253d6d889..df4ccb3dfeba 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -183,3 +183,21 @@ examples:
>              read-only;
>          };
>      };
> +
> +  - |
> +    partitions {
> +        compatible = "fixed-partitions";
> +        #address-cells = <1>;
> +        #size-cells = <1>;

This is not strictly related but I believe we will soon have issues with
these, as we will soon cross the 4GiB boundary.

> +        concat-parts = <&part0 &part1>;
> +
> +        part0: partition@0 {
> +            label = "flash0-part0";
> +            reg = <0x0000000 0x100000>;
> +        };
> +
> +        part1: partition@100000 {
> +            label = "flash1-part0";
> +            reg = <0x0100000 0x200000>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> index 1dda2c80747b..86bbd83c3f6d 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> @@ -32,6 +32,12 @@ properties:
>    '#size-cells':
>      enum: [1, 2]
>  
> +  concat-parts:
> +    description: List of MTD partitions phandles that should be concatenated.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 2
> +    maxItems: 4
> +
>  patternProperties:
>    "^partition(-.+|@[0-9a-f]+)$":
>      $ref: partition.yaml

Fine by me otherwise.

Thanks,
Miquèl