[PATCH 0/2] Add SEAMA partition types

[PATCH 0/2] Add SEAMA partition types

[Linus Walleij]

This type of firmware partition appear in some devices in
NAND flash, so we need to be able to tag the partitions
with the appropriate type.

The origin of the "SEAttle iMAge" is unknown.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (2):
      dt-bindings: mtd: Add SEAMA partition bindings
      ARM: dts: bcm5301x: Add SEAMA compatibles

 .../devicetree/bindings/mtd/partitions/seama.yaml  | 50 ++++++++++++++++++++++
 arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts      |  1 +
 arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts      |  1 +
 3 files changed, 52 insertions(+)
---
base-commit: caad71e7d226920623d78be2e6283516decdc502
change-id: 20230506-seama-partitions-b620117b9985

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/2] dt-bindings: mtd: Add SEAMA partition bindings

[Linus Walleij]

This types of NAND partitions appear in OpenWrt and
U-Boot.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/mtd/partitions/seama.yaml  | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/partitions/seama.yaml b/Documentation/devicetree/bindings/mtd/partitions/seama.yaml
new file mode 100644
index 000000000000..352f008ffdb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/seama.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/seama.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Seattle Image Partitions
+
+description: The SEAttle iMAge (SEAMA) partition is a type of partition
+  used for NAND flash devices. This type of flash image is found in some
+  D-Link routers such as DIR-645, DIR-842, DIR-859, DIR-860L, DIR-885L,
+  DIR890L and DCH-M225, as well as in WD and NEC routers on the ath79
+  (MIPS), Broadcom BCM53xx, and RAMIPS platforms.
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+select: false
+
+properties:
+  compatible:
+    const: seama
+
+  '#address-cells': false
+
+  '#size-cells': false
+
+patternProperties:
+  "@[0-9a-f]+$":
+    $ref: partition.yaml#
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    flash {
+      partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+        firmware@0 {
+          compatible = "seama";
+          label = "firmware";
+          reg = <0x00000000 0x08000000>;
+        };
+      };
+    };

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: mtd: Add SEAMA partition bindings

[Krzysztof Kozlowski]

On 06/05/2023 17:29, Linus Walleij wrote:
> This types of NAND partitions appear in OpenWrt and
> U-Boot.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/mtd/partitions/seama.yaml  | 50 ++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/seama.yaml b/Documentation/devicetree/bindings/mtd/partitions/seama.yaml
> new file mode 100644
> index 000000000000..352f008ffdb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/seama.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/seama.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Seattle Image Partitions
> +
> +description: The SEAttle iMAge (SEAMA) partition is a type of partition
> +  used for NAND flash devices. This type of flash image is found in some
> +  D-Link routers such as DIR-645, DIR-842, DIR-859, DIR-860L, DIR-885L,
> +  DIR890L and DCH-M225, as well as in WD and NEC routers on the ath79
> +  (MIPS), Broadcom BCM53xx, and RAMIPS platforms.
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +select: false

You miss update to partitions.yaml. Anyway, for your testing,
temporarily change it to true, to see errors.

> +
> +properties:
> +  compatible:
> +    const: seama
> +
> +  '#address-cells': false
> +
> +  '#size-cells': false

You have children, so these must be true.

> +
> +patternProperties:
> +  "@[0-9a-f]+$":
> +    $ref: partition.yaml#
> +
> +required:
> +  - compatible

address/size-cells as well

> +
> +additionalProperties: false
> +
> +examples:

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mtd: Add SEAMA partition bindings

[Linus Walleij]

On Wed, May 10, 2023 at 4:57 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> > +select: false
>
> You miss update to partitions.yaml. Anyway, for your testing,
> temporarily change it to true, to see errors.

This "select" just confuses me. I can set it to true and then I get
a flood of irrelevant errors, and the same if I change it to true for
any of the other childless partitions such as arm,arm-firmware-suite.yaml

So I do a best effort here...

> > +properties:
> > +  compatible:
> > +    const: seama
> > +
> > +  '#address-cells': false
> > +
> > +  '#size-cells': false
>
> You have children, so these must be true.

Not really, seama is childless.

I guess the example with fixed-partitions
was confusion, seama can be a subpartition in a set
of fixed-partions.

I'll try to write up something so it's clear.

Yours,
Linus Walleij

[PATCH 2/2] ARM: dts: bcm5301x: Add SEAMA compatibles

[Linus Walleij]

This adds SEAMA compatibles to the firmware partition of these
two D-Link devices.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts | 1 +
 arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
index 555fbe41dd8f..cf3000b07eea 100644
--- a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
+++ b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
@@ -33,6 +33,7 @@ partitions {
 				#size-cells = <1>;
 
 				partition@0 {
+					compatible = "seama";
 					label = "firmware";
 					reg = <0x00000000 0x08000000>;
 				};
diff --git a/arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts b/arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts
index d945a20b06e0..3551f855ff20 100644
--- a/arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts
+++ b/arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts
@@ -149,6 +149,7 @@ partitions {
 		 * partitions: this device uses SEAMA.
 		 */
 		firmware@0 {
+			compatible = "seama";
 			label = "firmware";
 			reg = <0x00000000 0x08000000>;
 		};

-- 
2.34.1

Re: [PATCH 0/2] Add SEAMA partition types

[Miquel Raynal]

Hi Linus,

linus.walleij@linaro.org wrote on Sat, 06 May 2023 17:29:43 +0200:

> This type of firmware partition appear in some devices in
> NAND flash, so we need to be able to tag the partitions
> with the appropriate type.
> 
> The origin of the "SEAttle iMAge" is unknown.

I don't see any kernel changes, why do we need an additional binding?

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Linus Walleij (2):
>       dt-bindings: mtd: Add SEAMA partition bindings
>       ARM: dts: bcm5301x: Add SEAMA compatibles
> 
>  .../devicetree/bindings/mtd/partitions/seama.yaml  | 50 ++++++++++++++++++++++
>  arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts      |  1 +
>  arch/arm/boot/dts/bcm47094-dlink-dir-890l.dts      |  1 +
>  3 files changed, 52 insertions(+)
> ---
> base-commit: caad71e7d226920623d78be2e6283516decdc502
> change-id: 20230506-seama-partitions-b620117b9985
> 
> Best regards,


Thanks,
Miquèl

Re: [PATCH 0/2] Add SEAMA partition types

[Linus Walleij]

On Tue, May 9, 2023 at 9:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> linus.walleij@linaro.org wrote on Sat, 06 May 2023 17:29:43 +0200:
>
> > This type of firmware partition appear in some devices in
> > NAND flash, so we need to be able to tag the partitions
> > with the appropriate type.
> >
> > The origin of the "SEAttle iMAge" is unknown.
>
> I don't see any kernel changes, why do we need an additional binding?

The bindings are not strictly bound to Linux, it's more like all OS:es
uses the Linux DT binding repo because it is the biggest project.
Also we actually merge a bunch of bindings just to describe hardware
(or things like partitions), in the hope of making use of them in the
long run.

Anyways, for the record, the full story:

Currently this binding is used in out-of-tree OpenWrt code, where it
is used as magic for splitting partitions with mtdsplit.

I guess you might be familiar with mtdsplit. It is a software partition
splitter that makes it possible to split a big partition into smaller
partitions dynamically, using magic block identifiers.

The typical usecase is to put the kernel in the first flash blocks,
then pad up to the nearest even erase block, and then add a
JFFS2 or UBI filesystem immediately there.

This way it avoids using static partitioning, the tools rebuilding the
firmware can dynamically split off more flash as the kernel image
grows.

The mtdsplit code uses different magic numbers to identify where
the different partitions start.

One such type of partition is seama, so the code needs to know
that it should look for seama magic to determine the size and
split this partition in a kernel and rootfs part. This is the code:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/generic/files/drivers/mtd/mtdsplit;h=3e0df856713a84b1decf17190f171cb10ce7a757;hb=HEAD

It is a bit sad that no-one has the energy to propose mtdsplit
upstream, I think it is quite generic and generally useful. I started
to make an upstream patch but got exhausted with the task.

Yours,
Linus Walleij

Re: [PATCH 0/2] Add SEAMA partition types

[Miquel Raynal]

Hi Linus,

linus.walleij@linaro.org wrote on Tue, 9 May 2023 20:30:33 +0200:

> On Tue, May 9, 2023 at 9:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > linus.walleij@linaro.org wrote on Sat, 06 May 2023 17:29:43 +0200:
> >  
> > > This type of firmware partition appear in some devices in
> > > NAND flash, so we need to be able to tag the partitions
> > > with the appropriate type.
> > >
> > > The origin of the "SEAttle iMAge" is unknown.  
> >
> > I don't see any kernel changes, why do we need an additional binding?  
> 
> The bindings are not strictly bound to Linux, it's more like all OS:es
> uses the Linux DT binding repo because it is the biggest project.

Yes, that's why I wanted more context :-)

> Also we actually merge a bunch of bindings just to describe hardware
> (or things like partitions), in the hope of making use of them in the
> long run.

It's always problematic to do it this way because no user == no precise
requirement. As binding are supposed to remain stable, and because we,
human are far from perfect when it comes to read the future, I of
course prefer when there is an implementation that uses the new binding
so we can more easily spot the issues.

> Anyways, for the record, the full story:
> 
> Currently this binding is used in out-of-tree OpenWrt code, where it
> is used as magic for splitting partitions with mtdsplit.
> 
> I guess you might be familiar with mtdsplit. It is a software partition
> splitter that makes it possible to split a big partition into smaller
> partitions dynamically, using magic block identifiers.
> 
> The typical usecase is to put the kernel in the first flash blocks,
> then pad up to the nearest even erase block, and then add a
> JFFS2 or UBI filesystem immediately there.
> 
> This way it avoids using static partitioning, the tools rebuilding the
> firmware can dynamically split off more flash as the kernel image
> grows.
> 
> The mtdsplit code uses different magic numbers to identify where
> the different partitions start.

Is mtdsplit acting on a device or on a partition? Right now you define
a partition to be compatible with seama, I would have imagined the
partitions container should be compatible with seama instead of
fixed-partitions, but I haven't looked at the whole implementation, so
maybe my comment is just wrong.

> One such type of partition is seama, so the code needs to know
> that it should look for seama magic to determine the size and
> split this partition in a kernel and rootfs part. This is the code:
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/generic/files/drivers/mtd/mtdsplit;h=3e0df856713a84b1decf17190f171cb10ce7a757;hb=HEAD

That's very informative, thanks for all the context. I believe this
could actually be part of the binding description (not the "this is an
openWRT stuff", of course).

> It is a bit sad that no-one has the energy to propose mtdsplit
> upstream, I think it is quite generic and generally useful. I started
> to make an upstream patch but got exhausted with the task.

:-)

Thanks,
Miquèl

Re: [PATCH 0/2] Add SEAMA partition types

[Linus Walleij]

On Mon, May 22, 2023 at 4:46 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> > The mtdsplit code uses different magic numbers to identify where
> > the different partitions start.
>
> Is mtdsplit acting on a device or on a partition?

It acts on a partition, usually you use a fixed-partitition scheme to point
out the different chunks in the flash and then mtdsplit comes in to
do its job.

> Right now you define
> a partition to be compatible with seama, I would have imagined the
> partitions container should be compatible with seama instead of
> fixed-partitions, but I haven't looked at the whole implementation, so
> maybe my comment is just wrong.

The NAND flash on my device needs it to be a partition, it looks
like so:

&nandcs {
        /* Spansion S34ML01G2, 128MB with 128KB erase blocks */
        partitions {
                compatible = "fixed-partitions";
                #address-cells = <1>;
                #size-cells = <1>;

                firmware@0 {
                        compatible = "seama";
                        label = "firmware";
                        reg = <0x00000000 0x08000000>;
                };
        };
};

The reason is mainly that other devices may put eraseblocks
aside for other things, and the SEAMA format itself does not
know its extents (it needs to be told where the end of the
partition is).

> > One such type of partition is seama, so the code needs to know
> > that it should look for seama magic to determine the size and
> > split this partition in a kernel and rootfs part. This is the code:
> > https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/generic/files/drivers/mtd/mtdsplit;h=3e0df856713a84b1decf17190f171cb10ce7a757;hb=HEAD
>
> That's very informative, thanks for all the context. I believe this
> could actually be part of the binding description (not the "this is an
> openWRT stuff", of course).

Hm I'll think about what I can put in there...

Yours,
Linus Walleij