[PATCH 2/3] dt-bindings: mtd: Add Documentation for Airoha fixed-partitions

[Miquel Raynal]

Hi Christian,

ansuelsmth at gmail.com wrote on Wed, 25 Sep 2024 13:35:38 +0200:

> On Wed, Sep 25, 2024 at 01:30:03PM +0200, Miquel Raynal wrote:
> > Hi Christian,
> > 
> > ansuelsmth at gmail.com wrote on Wed, 25 Sep 2024 12:13:58 +0200:
> >   
> > > Add Documentation for Airoha fixed-partitions compatibles.
> > > 
> > > Airoha based SoC declare a dedicated partition at the end of the flash to
> > > store calibration and device specific data, in addition to fixed
> > > partitions.
> > > 
> > > The offset of this special partition is not well defined as a custom bad
> > > block management driver is used that reserve space at the end of the flash.
> > > 
> > > This binding allows defining all fixed partitions and marking the last one
> > > to detect the correct offset.
> > > 
> > > Signed-off-by: Christian Marangi <ansuelsmth at gmail.com>
> > > ---
> > >  .../partitions/airoha,fixed-partitions.yaml   | 80 +++++++++++++++++++
> > >  .../bindings/mtd/partitions/partitions.yaml   |  1 +
> > >  2 files changed, 81 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/airoha,fixed-partitions.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/airoha,fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/airoha,fixed-partitions.yaml
> > > new file mode 100644
> > > index 000000000000..a45df51065af
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/partitions/airoha,fixed-partitions.yaml
> > > @@ -0,0 +1,80 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mtd/partitions/airoha,fixed-partitions.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Airoha SoC partitioning
> > > +
> > > +description: |
> > > +  Airoha based SoC declare a dedicated partition at the end of the flash to
> > > +  store calibration and device specific data, in addition to fixed partitions.
> > > +
> > > +  The offset of this special partition is not well defined as a custom bad block
> > > +  management driver is used that reserve space at the end of the flash.
> > > +
> > > +  This binding allows defining all fixed partitions and marking the last one to
> > > +  detect the correct offset from the new end of the flash.
> > > +
> > > +maintainers:
> > > +  - Christian Marangi <ansuelsmth at gmail.com>
> > > +
> > > +select: false
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: airoha,fixed-partitions
> > > +
> > > +  "#address-cells":
> > > +    enum: [ 1, 2 ]
> > > +
> > > +  "#size-cells":
> > > +    enum: [ 1, 2 ]
> > > +
> > > +patternProperties:
> > > +  "^partition@[0-9a-f]+$":
> > > +    $ref: partition.yaml#
> > > +    properties:
> > > +      compatible:
> > > +        const: airoha,dynamic-art
> > > +    unevaluatedProperties: false
> > > +
> > > +required:
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    partitions {
> > > +        compatible = "airoha,fixed-partitions";
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +
> > > +        partition at 0 {
> > > +          label = "bootloader";
> > > +          reg = <0x00000000 0x00080000>;
> > > +        };
> > > +
> > > +        partition at 80000 {
> > > +          label = "tclinux";
> > > +          reg = <0x00080000 0x02800000>;
> > > +        };
> > > +
> > > +        partition at 2880000 {
> > > +          label = "tclinux_slave";
> > > +          reg = <0x02880000 0x02800000>;
> > > +        };
> > > +
> > > +        partition at 5080000 {
> > > +          label = "rootfs_data";
> > > +          reg = <0x5080000 0x00800000>;
> > > +        };
> > > +
> > > +        partition at ffffffff {
> > > +          compatible = "airoha,dynamic-art";
> > > +          label = "art";
> > > +          reg = <0xffffffff 0x00300000>;  
> > 
> > I'm a little bit puzzled by this kind of information which is known to
> > be wrong. As the partition offset and size must be dynamically
> > calculated, this reg property (as well as the size parameter of the
> > previous one) are notably wrong. I guess we are not fully constrained
> > by the fixed-partitions schema here, so could we avoid the reg property
> > in the airoha,dynamic-art partition? Maybe we also need a #define for a
> > specific placeholder in the penultimate reg property too (for the size).
> >  
> 
> Maybe instead of reg we can use a property like size?
> 
> Can you better elaborate the suggestion about the #define?
> 
> Do you mean for case where the last partition might overlap
> with the penultimate? Honestly in such case I would error hard, that
> case happen when too much space is reserved and that is a
> misconfiguration of the system (developer error)

That's not what I mean.

In the above case you say partition "partition at 5080000" is 0x800000
bytes long. This is obviously wrong otherwise you would know where the
art partition starts. And right after you're saying partition
"partition at ffffffff" starts at 0xffffffff and is 0x300000 bytes long.
This is also wrong because 0xffffffff is not a valid start address and
IIUC 0x300000 is also unknown and dynamically derived.

So for the art partition my advise if you know nothing about the
start/length is to just skip the reg property. For the previous
partition I'd maybe use a definition (whose name is to discuss) instead
of the wrong size argument (the start offset being correct on his side).

Thanks, Miquèl