[PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[Christian Marangi]

This small series handle 2 problems.

It does try to ""standardize"" the usage of block2mtd module with
MTD OF nodes.

It is very easy to add support for MTD parser by just adding an
OF node to the mtd created for block2mtd.

This apply only if the root block is used for block2mtd to allow
scenario where the full eMMC or an NVME is used for MTD and it doesn't
have any partition table.

To also support NVME, similar to how it's done with eMMC, we introduce
a subnode to the NVME controller that needs to have the "nvme-card"
compatible where a dev can define fixed-paritions for MTD parser usage.

This series also add support for the Airoha partition table where
the last partition is always ART and is placed at the end of the flash.

This require dynamic calculation of the offset as some dedicated
driver for bad block management might be used that reserve some space
at the end of the flash for block accounting.

New aarch64 Airoha SoC make use of this partition table and use block2mtd
for eMMC to treat them as MTD with custom bad block management and block
tracking.

Changes v4:
- Add additional patch for ofpart kmod with of_update_property
  not exported
Changes v3:
- Fix compilation error for missing slab.h header
- Add compatible to partitions.yaml
Changes v2:
- Fix typo in DT patch
- Fix compilation error for non-OF platform
- Fix compilation error due to recent changes in block2mtd module

Christian Marangi (7):
  dt-bindings: nvme: Document nvme-card compatible
  nvme: assign of_node to nvme device
  dt-bindings: mmc: add property for partitions node in mmc-card node
  block2mtd: attach device OF node to MTD device
  of: also export of_update_property
  dt-bindings: mtd: Add Documentation for Airoha fixed-partitions
  mtd: parser: add support for Airoha parser

 .../devicetree/bindings/mmc/mmc-card.yaml     | 40 ++++++++++
 .../partitions/airoha,fixed-partitions.yaml   | 80 +++++++++++++++++++
 .../bindings/mtd/partitions/partitions.yaml   |  1 +
 .../devicetree/bindings/nvme/nvme-card.yaml   | 78 ++++++++++++++++++
 drivers/mtd/devices/block2mtd.c               | 12 +++
 drivers/mtd/parsers/Kconfig                   | 10 +++
 drivers/mtd/parsers/Makefile                  |  1 +
 drivers/mtd/parsers/ofpart_airoha.c           | 57 +++++++++++++
 drivers/mtd/parsers/ofpart_airoha.h           | 18 +++++
 drivers/mtd/parsers/ofpart_core.c             |  6 ++
 drivers/nvme/host/core.c                      |  4 +
 drivers/of/base.c                             |  1 +
 12 files changed, 308 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/airoha,fixed-partitions.yaml
 create mode 100644 Documentation/devicetree/bindings/nvme/nvme-card.yaml
 create mode 100644 drivers/mtd/parsers/ofpart_airoha.c
 create mode 100644 drivers/mtd/parsers/ofpart_airoha.h

-- 
2.45.2

[PATCH v4 1/7] dt-bindings: nvme: Document nvme-card compatible

[Christian Marangi]

Document new nvme-card compatible to permit defining fixed-partition in
DT by the use of the block2mtd module to use block devices as MTD.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/nvme/nvme-card.yaml   | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvme/nvme-card.yaml

diff --git a/Documentation/devicetree/bindings/nvme/nvme-card.yaml b/Documentation/devicetree/bindings/nvme/nvme-card.yaml
new file mode 100644
index 000000000000..20e9a877fac4
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvme/nvme-card.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvme/nvme-card.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVME Card
+
+maintainers:
+  - Christian Marangi <ansuelsmth@gmail.com>
+
+description: |
+  This documents describes the devicetree bindings for a NVME controller
+  child node describing a nvme-card.
+
+properties:
+  compatible:
+    const: nvme-card
+
+  partitions:
+    $ref: /schemas/mtd/partitions/partitions.yaml
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        bridge@0,0 {
+            reg = <0x00000000 0 0 0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            nvme@1,0 {
+                compatible = "pci144d,a809";
+
+                reg = <0x00010000 0 0 0 0>;
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                card {
+                    compatible = "nvme-card";
+
+                    partitions {
+                        compatible = "fixed-partitions";
+                        #address-cells = <1>;
+                        #size-cells = <1>;
+
+                        bootloader@0 {
+                          label = "bootloader";
+                          reg = <0x00000000 0x00080000>;
+                        };
+
+                        tclinux@80000 {
+                          label = "tclinux";
+                          reg = <0x00080000 0x02800000>;
+                        };
+
+                        tclinux_slave@2880000 {
+                          label = "tclinux_slave";
+                          reg = <0x02880000 0x02800000>;
+                        };
+
+                        rootfs_data@5080000 {
+                          label = "rootfs_data";
+                          reg = <0x5080000 0x00800000>;
+                        };
+                    };
+                };
+            };
+        };
+    };
-- 
2.45.2

[PATCH v4 2/7] nvme: assign of_node to nvme device

[Christian Marangi]

Introduce support for a dedicated node for a nvme card. This will be a
subnode of the nvme controller node that will have the "nvme-card"
compatible.

This follow a similar implementation done for mmc where the specific mmc
card have a dedicated of_node.

This can be used for scenario where block2mtd module is used to declare
partition in DT and block2mtd is called on the root block of the nvme
card, permitting the usage of fixed-partition parser or alternative
ones.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/nvme/host/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 053d5b4909cd..344523274d1b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/backing-dev.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/pr.h>
@@ -4651,6 +4652,7 @@ void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 	nvme_hwmon_exit(ctrl);
 	nvme_fault_inject_fini(&ctrl->fault_inject);
 	dev_pm_qos_hide_latency_tolerance(ctrl->device);
+	of_node_put(ctrl->device->of_node);
 	cdev_device_del(&ctrl->cdev, ctrl->device);
 	nvme_put_ctrl(ctrl);
 }
@@ -4771,6 +4773,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	else
 		ctrl->device->groups = nvme_dev_attr_groups;
 	ctrl->device->release = nvme_free_ctrl;
+	ctrl->device->of_node = of_get_compatible_child(ctrl->dev->of_node,
+							"nvme-card");
 	dev_set_drvdata(ctrl->device, ctrl);
 
 	return ret;
-- 
2.45.2

[PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Christian Marangi]

Add property for defining partitions node in mmc-card node to define
partitions in DT by the use of the block2mtd module to use block
devices as MTD.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/mmc/mmc-card.yaml     | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-card.yaml b/Documentation/devicetree/bindings/mmc/mmc-card.yaml
index fd347126449a..0f32d24417bc 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-card.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-card.yaml
@@ -26,6 +26,9 @@ properties:
       Use this to indicate that the mmc-card has a broken hpi
       implementation, and that hpi should not be used.
 
+  partitions:
+    $ref: /schemas/mtd/partitions/partitions.yaml
+
 required:
   - compatible
   - reg
@@ -45,4 +48,41 @@ examples:
         };
     };
 
+    mmc1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        card@0 {
+            compatible = "mmc-card";
+            reg = <0>;
+            broken-hpi;
+
+            partitions {
+                compatible = "fixed-partitions";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                bootloader@0 {
+                  label = "bootloader";
+                  reg = <0x00000000 0x00080000>;
+                };
+
+                tclinux@80000 {
+                  label = "tclinux";
+                  reg = <0x00080000 0x02800000>;
+                };
+
+                tclinux_slave@2880000 {
+                  label = "tclinux_slave";
+                  reg = <0x02880000 0x02800000>;
+                };
+
+                rootfs_data@5080000 {
+                  label = "rootfs_data";
+                  reg = <0x5080000 0x00800000>;
+                };
+            };
+        };
+    };
+
 ...
-- 
2.45.2

[PATCH v4 4/7] block2mtd: attach device OF node to MTD device

[Christian Marangi]

Attach device OF node to MTD device if defined and the root blockdev is
being used to add support for partitions defined in DT node.

This permits the usage of fixed-partition or alternative parser with the
use of block2mtd module.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/mtd/devices/block2mtd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index b06c8dd51562..8ff9787edc24 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -265,6 +265,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size,
 	struct file *bdev_file;
 	struct block_device *bdev;
 	struct block2mtd_dev *dev;
+	struct device *ddev;
 	loff_t size;
 	char *name;
 
@@ -324,6 +325,15 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size,
 	dev->mtd.priv = dev;
 	dev->mtd.owner = THIS_MODULE;
 
+	/*
+	 * Check if we are using root blockdev.
+	 * If it's the case, connect the MTD of_node to the ddev parent
+	 * to support providing partition in DT node.
+	 */
+	ddev = disk_to_dev(bdev->bd_disk);
+	if (ddev == &bdev->bd_device)
+		dev->mtd.dev.of_node = of_node_get(ddev->parent->of_node);
+
 	if (mtd_device_register(&dev->mtd, NULL, 0)) {
 		/* Device didn't get added, so free the entry */
 		goto err_destroy_mutex;
@@ -337,6 +347,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size,
 	return dev;
 
 err_destroy_mutex:
+	of_node_put(dev->mtd.dev.of_node);
 	mutex_destroy(&dev->write_mutex);
 err_free_block2mtd:
 	block2mtd_free_device(dev);
@@ -515,6 +526,7 @@ static void block2mtd_exit(void)
 		struct block2mtd_dev *dev = list_entry(pos, typeof(*dev), list);
 		block2mtd_sync(&dev->mtd);
 		mtd_device_unregister(&dev->mtd);
+		of_node_put(dev->mtd.dev.of_node);
 		mutex_destroy(&dev->write_mutex);
 		pr_info("mtd%d: [%s] removed\n",
 			dev->mtd.index,
-- 
2.45.2

[PATCH v4 5/7] of: also export of_update_property

[Christian Marangi]

Similar to how is done for of_add_property and of_remove_property, also
export of_update_property to permit kmod to use this additional function
and correctly update entry in DT.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/of/base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 20603d3c9931..b2d523cf4925 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1746,6 +1746,7 @@ int of_update_property(struct device_node *np, struct property *newprop)
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(of_update_property);
 
 static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 			 int id, const char *stem, int stem_len)
-- 
2.45.2

[PATCH v4 6/7] dt-bindings: mtd: Add Documentation for Airoha fixed-partitions

[Christian Marangi]

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 it depends on
flash bad block management that might require reserving additional 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@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..8d7221561f51
--- /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 it depends on
+  flash bad block management that might require reserving additional space at the
+  end of the flash.
+
+  This binding allows defining all fixed partitions and marking the last one to
+  detect the correct offset.
+
+maintainers:
+  - Christian Marangi <ansuelsmth@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@0 {
+          label = "bootloader";
+          reg = <0x00000000 0x00080000>;
+        };
+
+        partition@80000 {
+          label = "tclinux";
+          reg = <0x00080000 0x02800000>;
+        };
+
+        partition@2880000 {
+          label = "tclinux_slave";
+          reg = <0x02880000 0x02800000>;
+        };
+
+        partition@5080000 {
+          label = "rootfs_data";
+          reg = <0x5080000 0x00800000>;
+        };
+
+        partition@ffffffff {
+          compatible = "airoha,dynamic-art";
+          label = "art";
+          reg = <0xffffffff 0x00300000>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
index 1dda2c80747b..ec254e03adf0 100644
--- a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
@@ -14,6 +14,7 @@ maintainers:
   - Miquel Raynal <miquel.raynal@bootlin.com>
 
 oneOf:
+  - $ref: airoha,fixed-partitions.yaml
   - $ref: arm,arm-firmware-suite.yaml
   - $ref: brcm,bcm4908-partitions.yaml
   - $ref: brcm,bcm947xx-cfe-partitions.yaml
-- 
2.45.2

[PATCH v4 7/7] mtd: parser: add support for Airoha parser

[Christian Marangi]

Add support for Airoha parser based on a post parse ofpart function.

Airoha partition table follow normal fixed-partition implementation
with a special implementation for the ART partition. This is always the
last partition and is placed from the end of the flash - the partition
size.

To enable this special implementation for ART partition, the relevant
node require the "airoha,dynamic-art" compatible. With that declared,
offset value is ignored and real offset is updated with the calculated
value.

Due to usage of specific bad block management driver, the MTD size might
vary hence the ART partition offset needs to be dynamically parsed and
can't be declared statically.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/mtd/parsers/Kconfig         | 10 +++++
 drivers/mtd/parsers/Makefile        |  1 +
 drivers/mtd/parsers/ofpart_airoha.c | 57 +++++++++++++++++++++++++++++
 drivers/mtd/parsers/ofpart_airoha.h | 18 +++++++++
 drivers/mtd/parsers/ofpart_core.c   |  6 +++
 5 files changed, 92 insertions(+)
 create mode 100644 drivers/mtd/parsers/ofpart_airoha.c
 create mode 100644 drivers/mtd/parsers/ofpart_airoha.h

diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig
index da03ab6efe04..d6c53aa16ea6 100644
--- a/drivers/mtd/parsers/Kconfig
+++ b/drivers/mtd/parsers/Kconfig
@@ -72,6 +72,16 @@ config MTD_OF_PARTS
 	  flash memory node, as described in
 	  Documentation/devicetree/bindings/mtd/mtd.yaml.
 
+config MTD_OF_PARTS_AIROHA
+	bool "Airoha EN7815 partitioning support"
+	depends on MTD_OF_PARTS && (ARCH_AIROHA || COMPILE_TEST)
+	default ARCH_AIROHA
+	help
+	  This provides partitions parser for Airoha EN7815 family devices
+	  that can have dynamic "ART" partition at the end of the flash.
+	  It takes care of finding the correct offset and update property
+	  with it.
+
 config MTD_OF_PARTS_BCM4908
 	bool "BCM4908 partitioning support"
 	depends on MTD_OF_PARTS && (ARCH_BCMBCA || COMPILE_TEST)
diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile
index 9b00c62b837a..d67f9b4d39ac 100644
--- a/drivers/mtd/parsers/Makefile
+++ b/drivers/mtd/parsers/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_MTD_BRCM_U_BOOT)		+= brcm_u-boot.o
 obj-$(CONFIG_MTD_CMDLINE_PARTS)		+= cmdlinepart.o
 obj-$(CONFIG_MTD_OF_PARTS)		+= ofpart.o
 ofpart-y				+= ofpart_core.o
+ofpart-$(CONFIG_MTD_OF_PARTS_AIROHA)	+= ofpart_airoha.o
 ofpart-$(CONFIG_MTD_OF_PARTS_BCM4908)	+= ofpart_bcm4908.o
 ofpart-$(CONFIG_MTD_OF_PARTS_LINKSYS_NS)+= ofpart_linksys_ns.o
 obj-$(CONFIG_MTD_PARSER_IMAGETAG)	+= parser_imagetag.o
diff --git a/drivers/mtd/parsers/ofpart_airoha.c b/drivers/mtd/parsers/ofpart_airoha.c
new file mode 100644
index 000000000000..5e2cc7b6376b
--- /dev/null
+++ b/drivers/mtd/parsers/ofpart_airoha.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Christian Marangi <ansuelsmth@gmail.com>
+ */
+
+#include <linux/mtd/mtd.h>
+#include <linux/slab.h>
+#include <linux/mtd/partitions.h>
+
+#include "ofpart_airoha.h"
+
+int airoha_partitions_post_parse(struct mtd_info *mtd,
+				 struct mtd_partition *parts,
+				 int nr_parts)
+{
+	struct mtd_partition *part;
+	int len, a_cells, s_cells;
+	struct device_node *pp;
+	struct property *prop;
+	const __be32 *reg;
+	__be32 *new_reg;
+
+	part = &parts[nr_parts - 1];
+	pp = part->of_node;
+
+	/* Skip if ART partition have a valid offset instead of a dynamic one */
+	if (!of_device_is_compatible(pp, "airoha,dynamic-art"))
+		return 0;
+
+	/* ART partition is set at the end of flash - size */
+	part->offset = mtd->size - part->size;
+
+	/* Update the offset with the new calculate value in DT */
+	prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+	if (!prop)
+		return -ENOMEM;
+
+	/* Reg already validated by fixed-partition parser */
+	reg = of_get_property(pp, "reg", &len);
+
+	/* Fixed partition */
+	a_cells = of_n_addr_cells(pp);
+	s_cells = of_n_size_cells(pp);
+
+	prop->name = "reg";
+	prop->length = (a_cells + s_cells) * sizeof(__be32);
+	prop->value = kmemdup(reg, (a_cells + s_cells) * sizeof(__be32),
+			      GFP_KERNEL);
+	new_reg = prop->value;
+	memset(new_reg, 0, a_cells * sizeof(__be32));
+	new_reg[a_cells - 1] = cpu_to_be32(part->offset);
+	if (a_cells > 1)
+		new_reg[0] = cpu_to_be32(part->offset >> 32);
+	of_update_property(pp, prop);
+
+	return 0;
+}
diff --git a/drivers/mtd/parsers/ofpart_airoha.h b/drivers/mtd/parsers/ofpart_airoha.h
new file mode 100644
index 000000000000..3e8a8456c13a
--- /dev/null
+++ b/drivers/mtd/parsers/ofpart_airoha.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __OFPART_AIROHA_H
+#define __OFPART_AIROHA_H
+
+#ifdef CONFIG_MTD_OF_PARTS_AIROHA
+int airoha_partitions_post_parse(struct mtd_info *mtd,
+				 struct mtd_partition *parts,
+				 int nr_parts);
+#else
+static inline int airoha_partitions_post_parse(struct mtd_info *mtd,
+					       struct mtd_partition *parts,
+					       int nr_parts)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
+#endif
diff --git a/drivers/mtd/parsers/ofpart_core.c b/drivers/mtd/parsers/ofpart_core.c
index e7b8e9d0a910..9e078636d158 100644
--- a/drivers/mtd/parsers/ofpart_core.c
+++ b/drivers/mtd/parsers/ofpart_core.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/mtd/partitions.h>
 
+#include "ofpart_airoha.h"
 #include "ofpart_bcm4908.h"
 #include "ofpart_linksys_ns.h"
 
@@ -23,6 +24,10 @@ struct fixed_partitions_quirks {
 	int (*post_parse)(struct mtd_info *mtd, struct mtd_partition *parts, int nr_parts);
 };
 
+static struct fixed_partitions_quirks airoha_partitions_quirks = {
+	.post_parse = airoha_partitions_post_parse,
+};
+
 static struct fixed_partitions_quirks bcm4908_partitions_quirks = {
 	.post_parse = bcm4908_partitions_post_parse,
 };
@@ -192,6 +197,7 @@ static const struct of_device_id parse_ofpart_match_table[] = {
 	/* Generic */
 	{ .compatible = "fixed-partitions" },
 	/* Customized */
+	{ .compatible = "airoha,fixed-partitions", .data = &airoha_partitions_quirks, },
 	{ .compatible = "brcm,bcm4908-partitions", .data = &bcm4908_partitions_quirks, },
 	{ .compatible = "linksys,ns-partitions", .data = &linksys_ns_partitions_quirks, },
 	{},
-- 
2.45.2

Re: [PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[Miquel Raynal]

Hi Christian,

ansuelsmth@gmail.com wrote on Fri,  9 Aug 2024 19:20:58 +0200:

> This small series handle 2 problems.
> 
> It does try to ""standardize"" the usage of block2mtd module with
> MTD OF nodes.
> 
> It is very easy to add support for MTD parser by just adding an
> OF node to the mtd created for block2mtd.
> 
> This apply only if the root block is used for block2mtd to allow
> scenario where the full eMMC or an NVME is used for MTD and it doesn't
> have any partition table.
> 
> To also support NVME, similar to how it's done with eMMC, we introduce
> a subnode to the NVME controller that needs to have the "nvme-card"
> compatible where a dev can define fixed-paritions for MTD parser usage.
> 
> This series also add support for the Airoha partition table where
> the last partition is always ART and is placed at the end of the flash.
> 
> This require dynamic calculation of the offset as some dedicated
> driver for bad block management might be used that reserve some space
> at the end of the flash for block accounting.

Who is reserving this space? And this is not reflected anywhere in the
partition table?

> New aarch64 Airoha SoC make use of this partition table and use block2mtd
> for eMMC to treat them as MTD with custom bad block management and block
> tracking.

I am sorry, I am not used to such use cases, and I really fail getting
why you would like to use mtd with an eMMC. Can you explain a little
bit more what is not available in the block world that you really need
from mtd?

Also, did you consider nvmem layouts instead to detect and define the
ART area? (just asking).

Thanks,
Miquèl

Re: [PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[Christian Marangi]

On Mon, Aug 12, 2024 at 10:49:54AM +0200, Miquel Raynal wrote:
> Hi Christian,
> 
> ansuelsmth@gmail.com wrote on Fri,  9 Aug 2024 19:20:58 +0200:
> 
> > This small series handle 2 problems.
> > 
> > It does try to ""standardize"" the usage of block2mtd module with
> > MTD OF nodes.
> > 
> > It is very easy to add support for MTD parser by just adding an
> > OF node to the mtd created for block2mtd.
> > 
> > This apply only if the root block is used for block2mtd to allow
> > scenario where the full eMMC or an NVME is used for MTD and it doesn't
> > have any partition table.
> > 
> > To also support NVME, similar to how it's done with eMMC, we introduce
> > a subnode to the NVME controller that needs to have the "nvme-card"
> > compatible where a dev can define fixed-paritions for MTD parser usage.
> > 
> > This series also add support for the Airoha partition table where
> > the last partition is always ART and is placed at the end of the flash.
> > 
> > This require dynamic calculation of the offset as some dedicated
> > driver for bad block management might be used that reserve some space
> > at the end of the flash for block accounting.
> 
> Who is reserving this space? And this is not reflected anywhere in the
> partition table?
>

To be more precise Mediatek use a custom way to handle bad blocks called
BMT where they reserve and store data at the end of the nand. This is
loaded before the flash driver controller so when MTD is init, the size
is already reduced. The reserved space can change and it really depends
on the tuned values hence it may change.

> > New aarch64 Airoha SoC make use of this partition table and use block2mtd
> > for eMMC to treat them as MTD with custom bad block management and block
> > tracking.
> 
> I am sorry, I am not used to such use cases, and I really fail getting
> why you would like to use mtd with an eMMC. Can you explain a little
> bit more what is not available in the block world that you really need
> from mtd?

Since vendor needs more space and doesn't want to adapt to block world,
they are starting to use eMMC or block devices in general unpartitioned
and raw and using block2mtd to simulate it. They don't care about the
performance penalities as it's something read at boot time and only new
firmware or some config files are written.

Is it more clear now?

> 
> Also, did you consider nvmem layouts instead to detect and define the
> ART area? (just asking).
> 

They still need a MTD partition and most of the time userspace tool are
used on the ART partition. Using block2mtd and DT support will permit
the use of nvmem cell as a side effect (and that is a missive bonus
point of this honestly)

-- 
	Ansuel

Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Christoph Hellwig]

On Fri, Aug 09, 2024 at 07:21:00PM +0200, Christian Marangi wrote:
> Introduce support for a dedicated node for a nvme card. This will be a
> subnode of the nvme controller node that will have the "nvme-card"
> compatible.

FYI, there really is no such thing as an NVMe card.  There is an
NVMe Namespace, which is the entity that contains the block data,
the Controller which corresponds to the pci_dev for NVMe-PCIe, and
the NVMe Subsystem, which contains Controllers and Namespaces.

> This follow a similar implementation done for mmc where the specific mmc
> card have a dedicated of_node.

That's not a good explanation to be honest.  Most eMMC host controllers
are OF probed devices, so of course they'll have an of_node.

Binding PCIe functions to of_nodes seems completely weird to me, and
you'll need to explain what this totally non-obvious thing makes sense.
Maybe it does, but it needs to be backed up with a very good rationale
that is very clearly documented.

Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Christian Marangi]

On Mon, Aug 12, 2024 at 01:12:05PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 09, 2024 at 07:21:00PM +0200, Christian Marangi wrote:
> > Introduce support for a dedicated node for a nvme card. This will be a
> > subnode of the nvme controller node that will have the "nvme-card"
> > compatible.
> 
> FYI, there really is no such thing as an NVMe card.  There is an
> NVMe Namespace, which is the entity that contains the block data,
> the Controller which corresponds to the pci_dev for NVMe-PCIe, and
> the NVMe Subsystem, which contains Controllers and Namespaces.
>

The chosen name was arbritrary just to follow eMMC ones. Can totally
change if problematic.

> > This follow a similar implementation done for mmc where the specific mmc
> > card have a dedicated of_node.
> 
> That's not a good explanation to be honest.  Most eMMC host controllers
> are OF probed devices, so of course they'll have an of_node.
> 
> Binding PCIe functions to of_nodes seems completely weird to me, and
> you'll need to explain what this totally non-obvious thing makes sense.
> Maybe it does, but it needs to be backed up with a very good rationale
> that is very clearly documented.
> 

But support of OF for PCIe is already a thing for a long time. (it all
works by setting the compatible of the PCIe ID card) and used in wifi
card at assign MAC address, calibration data, disable frequency.

In this context we would do a similar thing with declaring the NVMe with
the PCIe ID card (already supported) and we add support for defining an
additional subnode for usage of block2mtd.

The subnode is needed to keep consistency in how the disk struct are
defined with all the parenting levels.

Just to stress it more... This is really for consistency as PCIe OF node
are already a thing and on block2mtd (for example) the same thing can be
done with something like

disk->parent->parent.of_node (as it would point, if present, to the OF node
of the PCIe card (the NVMe))

With eMMC with the mmc-card subnode we would have to use

disk->parent.of_node

Not having this well organized and consistent schema in DT will result
in additional condition in the drivers...

Also consider that if the card is not detected, nothing is probed so
those additional node won't cause any harm.

If these 2 patch are problematic I can totally drop from the series but
it was really to add consistency in NVMe and eMMC. The real important
part is eMMC that is becoming the de-facto replacement for NAND/NOR on
high tier devices (mostly wifi6/7 consumer router)

-- 
	Ansuel

Re: [PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[Miquel Raynal]

Hi Christian,

ansuelsmth@gmail.com wrote on Mon, 12 Aug 2024 12:10:03 +0200:

> On Mon, Aug 12, 2024 at 10:49:54AM +0200, Miquel Raynal wrote:
> > Hi Christian,
> > 
> > ansuelsmth@gmail.com wrote on Fri,  9 Aug 2024 19:20:58 +0200:
> >   
> > > This small series handle 2 problems.
> > > 
> > > It does try to ""standardize"" the usage of block2mtd module with
> > > MTD OF nodes.
> > > 
> > > It is very easy to add support for MTD parser by just adding an
> > > OF node to the mtd created for block2mtd.
> > > 
> > > This apply only if the root block is used for block2mtd to allow
> > > scenario where the full eMMC or an NVME is used for MTD and it doesn't
> > > have any partition table.
> > > 
> > > To also support NVME, similar to how it's done with eMMC, we introduce
> > > a subnode to the NVME controller that needs to have the "nvme-card"
> > > compatible where a dev can define fixed-paritions for MTD parser usage.
> > > 
> > > This series also add support for the Airoha partition table where
> > > the last partition is always ART and is placed at the end of the flash.
> > > 
> > > This require dynamic calculation of the offset as some dedicated
> > > driver for bad block management might be used that reserve some space
> > > at the end of the flash for block accounting.  
> > 
> > Who is reserving this space? And this is not reflected anywhere in the
> > partition table?
> >  
> 
> To be more precise Mediatek use a custom way to handle bad blocks called
> BMT where they reserve and store data at the end of the nand. This is
> loaded before the flash driver controller so when MTD is init, the size
> is already reduced. The reserved space can change and it really depends
> on the tuned values hence it may change.

Is this supported in mainline Linux? MTD handles the bad blocks and the
bad block tables, so I don't understand how this hardware feature can
live together with MTD.

Anyway, you are talking about MMCs, I don't understand why there are
bad blocks, nor what is checking them and when. This is all still very
fuzzy to me, I'm sorry.

> > > New aarch64 Airoha SoC make use of this partition table and use block2mtd
> > > for eMMC to treat them as MTD with custom bad block management and block
> > > tracking.  
> > 
> > I am sorry, I am not used to such use cases, and I really fail getting
> > why you would like to use mtd with an eMMC. Can you explain a little
> > bit more what is not available in the block world that you really need
> > from mtd?  
> 
> Since vendor needs more space and doesn't want to adapt to block world,
> they are starting to use eMMC or block devices in general unpartitioned
> and raw 

Okay, why not, it's easier for ROMs to access it I guess.

> and using block2mtd to simulate it.

This is what I don't understand. You can very well access your block
device by offset, why do you need the mtd interface at all?

> They don't care about the
> performance penalities as it's something read at boot time and only new
> firmware or some config files are written.
> 
> Is it more clear now?

I still don't understand the need for going through MTD tbh.

> > Also, did you consider nvmem layouts instead to detect and define the
> > ART area? (just asking).
> >   
> 
> They still need a MTD partition

Why?

> and most of the time userspace tool are
> used on the ART partition. Using block2mtd and DT support will permit
> the use of nvmem cell as a side effect (and that is a missive bonus
> point of this honestly)

MTD also registers into NVMEM, so this is nice if you need both, but if
what you need is some NVMEM area derived dynamically and you register
into MTD for that, then no, I'm sorry, that's not the correct approach.

Thanks,
Miquèl

Re: [PATCH v4 0/7] mtd: improve block2mtd + airoha parser

[Christoph Hellwig]

On Mon, Aug 12, 2024 at 03:17:55PM +0200, Miquel Raynal wrote:
> Is this supported in mainline Linux? MTD handles the bad blocks and the
> bad block tables, so I don't understand how this hardware feature can
> live together with MTD.
> 
> Anyway, you are talking about MMCs, I don't understand why there are
> bad blocks, nor what is checking them and when. This is all still very
> fuzzy to me, I'm sorry.

Yes.  The idea of using block2mtd for anything but development seems
a bit odd to say it politely.  Using it to reinvent bad block management
on top of a block device that needs to do that as one of it's fundamental
functions seems extremely odd.

Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Christoph Hellwig]

On Mon, Aug 12, 2024 at 02:10:28PM +0200, Christian Marangi wrote:
> The chosen name was arbritrary just to follow eMMC ones. Can totally
> change if problematic.

NVMe namespaces are dynamic and can be created and deleted at will
at runtime.  I just don't see how they would even fit into OF
concepts.

There is a huge impedance mismatch here, to the point where I completely
fail to understand what you are trying to do.

> But support of OF for PCIe is already a thing for a long time. (it all
> works by setting the compatible of the PCIe ID card) and used in wifi
> card at assign MAC address, calibration data, disable frequency.

Please point to a document describing how, but more importantly why
this is done.  I've worked with and maintained Linux PCI(e) drivers for
about 20 years and never seen it.  And the concept simply doesn't make
sense in terms of a dynamically probed bus.

> Not having this well organized and consistent schema in DT will result
> in additional condition in the drivers...

NVMe Controllers are PCI functions (or virtual entities over the network).
Defining them in a static DT scheme does not make sense.  NVMe Namespaces
which are what contains the block data are dynamically discoverred and
can be created and deleted at runtime, so refering to them in DT is even
more broken.  I really don't see how any of this could remotely work.

> If these 2 patch are problematic I can totally drop from the series but
> it was really to add consistency in NVMe and eMMC. The real important
> part is eMMC that is becoming the de-facto replacement for NAND/NOR on
> high tier devices (mostly wifi6/7 consumer router)

If you aren't dealing with raw(ish) NAND don't use mtd.  MTD is designed
to deal with the nitty gritty details of NOR and NAND flash.  If you
already have an FTL running in the device there is absolutely no reason
to use it.

Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Markus Elfring]

…
> This follow a similar implementation done for mmc …

       follows?

Regards,
Markus

Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Rob Herring]

On Mon, Aug 12, 2024 at 03:31:28PM +0200, Christoph Hellwig wrote:
> On Mon, Aug 12, 2024 at 02:10:28PM +0200, Christian Marangi wrote:
> > The chosen name was arbritrary just to follow eMMC ones. Can totally
> > change if problematic.

The purpose of those eMMC nodes was to describe various non-standard 
stuff you get when discoverable devices get soldered on a board. Power 
rails, reset GPIOs, etc. It was not to put partition info there, but I 
suppose having the node opens it to such abuse.

> 
> NVMe namespaces are dynamic and can be created and deleted at will
> at runtime.  I just don't see how they would even fit into OF
> concepts.
> 
> There is a huge impedance mismatch here, to the point where I completely
> fail to understand what you are trying to do.
> 
> > But support of OF for PCIe is already a thing for a long time. (it all
> > works by setting the compatible of the PCIe ID card) and used in wifi
> > card at assign MAC address, calibration data, disable frequency.
> 
> Please point to a document describing how, but more importantly why
> this is done.  I've worked with and maintained Linux PCI(e) drivers for
> about 20 years and never seen it.  And the concept simply doesn't make
> sense in terms of a dynamically probed bus.

With OpenFirmware systems, the firmware will probe PCI and populate 
nodes with what it discovered and with how it configured things. IBM 
PSeries uses DT for PCI hotplug as well, AIUI.

For Flattened DT, PCI devices are typically only present if there are 
extra non-discoverable things (again, happens when devices are soldered 
down and not in standard slots). Another example is a NIC where they 
cheaped out and didn't put an EEPROM to store the MAC address, so you 
store it in the DT. Now we're starting to see non-discoverable things 
sitting behind PCI devices, so we need the PCI device in DT to describe 
those downstream devices.

> > Not having this well organized and consistent schema in DT will result
> > in additional condition in the drivers...
> 
> NVMe Controllers are PCI functions (or virtual entities over the network).
> Defining them in a static DT scheme does not make sense.  NVMe Namespaces
> which are what contains the block data are dynamically discoverred and
> can be created and deleted at runtime, so refering to them in DT is even
> more broken.  I really don't see how any of this could remotely work.
> 
> > If these 2 patch are problematic I can totally drop from the series but
> > it was really to add consistency in NVMe and eMMC. The real important
> > part is eMMC that is becoming the de-facto replacement for NAND/NOR on
> > high tier devices (mostly wifi6/7 consumer router)
> 
> If you aren't dealing with raw(ish) NAND don't use mtd.  MTD is designed
> to deal with the nitty gritty details of NOR and NAND flash.  If you
> already have an FTL running in the device there is absolutely no reason
> to use it.

Yes, agreed.

Rob

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Rob Herring]

On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:
> Add property for defining partitions node in mmc-card node to define
> partitions in DT by the use of the block2mtd module to use block
> devices as MTD.

You justified patch 1 saying eMMC already supported this, but then here 
you add support.

Both are a NAK for me as both already have a way to describe partitions 
with GPT.

> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/mmc/mmc-card.yaml     | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Christian Marangi]

On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:
> > Add property for defining partitions node in mmc-card node to define
> > partitions in DT by the use of the block2mtd module to use block
> > devices as MTD.
> 
> You justified patch 1 saying eMMC already supported this, but then here 
> you add support.
> 
> Both are a NAK for me as both already have a way to describe partitions 
> with GPT.
>

I think this got a bit confused and hope we can find a way to add
support for this.

What is "already supported" is assigning an OF node so driver can
reference it. This patch was just adding the nodes in the schema to say
that partitions can be defined.

I think what is not clear is that block devices might be used as raw
devices without a partition table defined in the device. In such case
it's the kernel that define a fixed partition table.

One example is [1] where the partition table is provided by cmdline.
Similar to cmdlinepart MTD parser.

The use of block2mtd is just to make use of the MTD parser system.

Considering
- eMMC is soldered to the device (no dynamic scan)
- cmdline might be not tunable and hardcoding it might also be
  problematic (as some cmdline needs to be used)
- concept of fixed partition for block device is already a thing and
  used a lot (android AFAIK)

I think it should be acceptable to introduce in DT support for defining
fixed partition for block devices and some kind of parser system similar
to MTD. What do you think? Would this be more acceptable? Idea is to
just have a DT schema that makes use of the values that can be set in
[1].

Hope we can find a solution to this, I'm totally OK for dropping NVMe as
I understand it's PCIe stuff and very dynamic but OEM are making lots of
use of eMMC and are starting to use these strange way (block2mtd) as we
currently don't give a proper and easy solution for the task.

[1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst

> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/mmc/mmc-card.yaml     | 40 +++++++++++++++++++
> >  1 file changed, 40 insertions(+)

-- 
	Ansuel

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Christian Marangi]

On Wed, Aug 21, 2024 at 03:12:49PM +0200, Miquel Raynal wrote:
> Hi Christian,
> 
> ansuelsmth@gmail.com wrote on Tue, 20 Aug 2024 22:20:59 +0200:
> 
> > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:  
> > > > Add property for defining partitions node in mmc-card node to define
> > > > partitions in DT by the use of the block2mtd module to use block
> > > > devices as MTD.  
> > > 
> > > You justified patch 1 saying eMMC already supported this, but then here 
> > > you add support.
> > > 
> > > Both are a NAK for me as both already have a way to describe partitions 
> > > with GPT.
> > >  
> > 
> > I think this got a bit confused and hope we can find a way to add
> > support for this.
> > 
> > What is "already supported" is assigning an OF node so driver can
> > reference it. This patch was just adding the nodes in the schema to say
> > that partitions can be defined.
> > 
> > I think what is not clear is that block devices might be used as raw
> > devices without a partition table defined in the device. In such case
> > it's the kernel that define a fixed partition table.
> > 
> > One example is [1] where the partition table is provided by cmdline.
> > Similar to cmdlinepart MTD parser.
> > 
> > The use of block2mtd is just to make use of the MTD parser system.
> > 
> > Considering
> > - eMMC is soldered to the device (no dynamic scan)
> > - cmdline might be not tunable and hardcoding it might also be
> >   problematic (as some cmdline needs to be used)
> > - concept of fixed partition for block device is already a thing and
> >   used a lot (android AFAIK)
> > 
> > I think it should be acceptable to introduce in DT support for defining
> > fixed partition for block devices and some kind of parser system similar
> > to MTD. What do you think? Would this be more acceptable? Idea is to
> > just have a DT schema that makes use of the values that can be set in
> > [1].
> > 
> > Hope we can find a solution to this, I'm totally OK for dropping NVMe as
> > I understand it's PCIe stuff and very dynamic but OEM are making lots of
> > use of eMMC and are starting to use these strange way (block2mtd) as we
> > currently don't give a proper and easy solution for the task.
> 
> Thanks for the summary. I believe I now have a better understanding of
> what you want to do, but I am still convinced you should not abuse the
> mtd layer for that. If blocks can be partitioned based on the cmdline
> (or from the DT) then the partitioning logic should be specific to
> the block layer. However, the parsing logic is probably very similar
> and could be extracted into a lib/, provided that the mtd bits are
> moved away cleverly. You only need the fixed parser anyway, and you
> probably don't want all the weird corner cases we keep supporting for
> backward compatibility reasons.
>

Yes I also agree abusing MTD layer for this is wrong and I also love
introducing and actually "stabilize" support for fixed-partitions for
block devices. So what it needs to be done is creating support in DT.
(interesting that cmdline is supported but nobody cared of adding
support in DT as that opens to lots of possibilities and is generally
better)

Yes I will check how the current fixed-partition parser for MTD can be
generalized and moved to MTD. In theory the DT schema should be the
same. (it's just offset, label and size after all)

> > [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst
> > 
> > > > 
> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/mmc/mmc-card.yaml     | 40 +++++++++++++++++++
> > > >  1 file changed, 40 insertions(+)  
> > 
> 
> Thanks,
> Miquèl

-- 
	Ansuel

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Christian Marangi]

On Wed, Aug 21, 2024 at 03:14:29PM +0200, Ulf Hansson wrote:
> On Wed, 21 Aug 2024 at 11:52, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:
> > > > Add property for defining partitions node in mmc-card node to define
> > > > partitions in DT by the use of the block2mtd module to use block
> > > > devices as MTD.
> > >
> > > You justified patch 1 saying eMMC already supported this, but then here
> > > you add support.
> > >
> > > Both are a NAK for me as both already have a way to describe partitions
> > > with GPT.
> > >
> >
> > I think this got a bit confused and hope we can find a way to add
> > support for this.
> >
> > What is "already supported" is assigning an OF node so driver can
> > reference it. This patch was just adding the nodes in the schema to say
> > that partitions can be defined.
> >
> > I think what is not clear is that block devices might be used as raw
> > devices without a partition table defined in the device. In such case
> > it's the kernel that define a fixed partition table.
> >
> > One example is [1] where the partition table is provided by cmdline.
> > Similar to cmdlinepart MTD parser.
> >
> > The use of block2mtd is just to make use of the MTD parser system.
> >
> > Considering
> > - eMMC is soldered to the device (no dynamic scan)
> > - cmdline might be not tunable and hardcoding it might also be
> >   problematic (as some cmdline needs to be used)
> > - concept of fixed partition for block device is already a thing and
> >   used a lot (android AFAIK)
> 
> Sorry for sidestepping your discussion, but I just wanted to add a few comments.
> 
> It's not clear to me why we need something different than what we
> already have today.

It's really adding a missing feature. We have cmdline but we don't have
DT. And in emebedded many times cmdline can't be changed.

> 
> If it's a partuuid/uuid/label or a fixed block-partition from the
> command line, we still need to know what partition we shall use for
> what. So why is this problem different from how we manage filesystem
> mounts, for example?

In the context of eMMC there isn't any partition table and the cmdline
is hardcoded by the bootloader. The cmdline might provide the root
partition to use but doesn't declare the partition table (with cmdline)

And the bootloader with the hardcoded cmdline might provide a different
root partition in dual-boot-partition implementation on switching the
boot partition. (this is used a lot with bootloader using a env variable
and a different cmdline passed based on the variable to signal what
partition to use for root)

> 
> >
> > I think it should be acceptable to introduce in DT support for defining
> > fixed partition for block devices and some kind of parser system similar
> > to MTD. What do you think? Would this be more acceptable? Idea is to
> > just have a DT schema that makes use of the values that can be set in
> > [1].
> 
> In DT we can describe that there is an eMMC card soldered to the
> board, because it's a description of the HW. But describing what
> stored inside the eMMC-flash doesn't belong in DT.
> 

Why? This conflicts with how it's done on MTD. Also consider the fact
that eMMC might contain calibration partition used for NVMEM.

Describing fixed partition on a soldered eMMC that the bootloader
expects at a fixed offset and won't ever change (or it will result in a
brick) sounds like describing the HW to me. (it's the same principle of
MTD just applied to block devices)

(I know it sound strange as block devices are expected to have a
partition table in MBR or GPT but reality is that it's not always the
case)

> >
> > Hope we can find a solution to this, I'm totally OK for dropping NVMe as
> > I understand it's PCIe stuff and very dynamic but OEM are making lots of
> > use of eMMC and are starting to use these strange way (block2mtd) as we
> > currently don't give a proper and easy solution for the task.
> 
> I certainly appreciate that you are trying to solve the fragmentation
> issue around this, but it looks like we need a different approach than
> using DT to describe partitions.
> 

Well it's either DT that can be tweaked and is part of the image or
cmdline that have tons of limitation due to bootloader having fun
hardcoding it or also actually requiring the bootloader cmdline and
adding overlay on it. Honestly as I said adding DT support is really
compleating the feature of the cmdline implementation.

> >
> > [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst
> >
> 
> Kind regards
> Uffe

-- 
	Ansuel

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Miquel Raynal]

Hi Christian,

ansuelsmth@gmail.com wrote on Tue, 20 Aug 2024 22:20:59 +0200:

> On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:  
> > > Add property for defining partitions node in mmc-card node to define
> > > partitions in DT by the use of the block2mtd module to use block
> > > devices as MTD.  
> > 
> > You justified patch 1 saying eMMC already supported this, but then here 
> > you add support.
> > 
> > Both are a NAK for me as both already have a way to describe partitions 
> > with GPT.
> >  
> 
> I think this got a bit confused and hope we can find a way to add
> support for this.
> 
> What is "already supported" is assigning an OF node so driver can
> reference it. This patch was just adding the nodes in the schema to say
> that partitions can be defined.
> 
> I think what is not clear is that block devices might be used as raw
> devices without a partition table defined in the device. In such case
> it's the kernel that define a fixed partition table.
> 
> One example is [1] where the partition table is provided by cmdline.
> Similar to cmdlinepart MTD parser.
> 
> The use of block2mtd is just to make use of the MTD parser system.
> 
> Considering
> - eMMC is soldered to the device (no dynamic scan)
> - cmdline might be not tunable and hardcoding it might also be
>   problematic (as some cmdline needs to be used)
> - concept of fixed partition for block device is already a thing and
>   used a lot (android AFAIK)
> 
> I think it should be acceptable to introduce in DT support for defining
> fixed partition for block devices and some kind of parser system similar
> to MTD. What do you think? Would this be more acceptable? Idea is to
> just have a DT schema that makes use of the values that can be set in
> [1].
> 
> Hope we can find a solution to this, I'm totally OK for dropping NVMe as
> I understand it's PCIe stuff and very dynamic but OEM are making lots of
> use of eMMC and are starting to use these strange way (block2mtd) as we
> currently don't give a proper and easy solution for the task.

Thanks for the summary. I believe I now have a better understanding of
what you want to do, but I am still convinced you should not abuse the
mtd layer for that. If blocks can be partitioned based on the cmdline
(or from the DT) then the partitioning logic should be specific to
the block layer. However, the parsing logic is probably very similar
and could be extracted into a lib/, provided that the mtd bits are
moved away cleverly. You only need the fixed parser anyway, and you
probably don't want all the weird corner cases we keep supporting for
backward compatibility reasons.

> [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst
> 
> > > 
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  .../devicetree/bindings/mmc/mmc-card.yaml     | 40 +++++++++++++++++++
> > >  1 file changed, 40 insertions(+)  
> 

Thanks,
Miquèl

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Ulf Hansson]

On Wed, 21 Aug 2024 at 11:52, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:
> > > Add property for defining partitions node in mmc-card node to define
> > > partitions in DT by the use of the block2mtd module to use block
> > > devices as MTD.
> >
> > You justified patch 1 saying eMMC already supported this, but then here
> > you add support.
> >
> > Both are a NAK for me as both already have a way to describe partitions
> > with GPT.
> >
>
> I think this got a bit confused and hope we can find a way to add
> support for this.
>
> What is "already supported" is assigning an OF node so driver can
> reference it. This patch was just adding the nodes in the schema to say
> that partitions can be defined.
>
> I think what is not clear is that block devices might be used as raw
> devices without a partition table defined in the device. In such case
> it's the kernel that define a fixed partition table.
>
> One example is [1] where the partition table is provided by cmdline.
> Similar to cmdlinepart MTD parser.
>
> The use of block2mtd is just to make use of the MTD parser system.
>
> Considering
> - eMMC is soldered to the device (no dynamic scan)
> - cmdline might be not tunable and hardcoding it might also be
>   problematic (as some cmdline needs to be used)
> - concept of fixed partition for block device is already a thing and
>   used a lot (android AFAIK)

Sorry for sidestepping your discussion, but I just wanted to add a few comments.

It's not clear to me why we need something different than what we
already have today.

If it's a partuuid/uuid/label or a fixed block-partition from the
command line, we still need to know what partition we shall use for
what. So why is this problem different from how we manage filesystem
mounts, for example?

>
> I think it should be acceptable to introduce in DT support for defining
> fixed partition for block devices and some kind of parser system similar
> to MTD. What do you think? Would this be more acceptable? Idea is to
> just have a DT schema that makes use of the values that can be set in
> [1].

In DT we can describe that there is an eMMC card soldered to the
board, because it's a description of the HW. But describing what
stored inside the eMMC-flash doesn't belong in DT.

>
> Hope we can find a solution to this, I'm totally OK for dropping NVMe as
> I understand it's PCIe stuff and very dynamic but OEM are making lots of
> use of eMMC and are starting to use these strange way (block2mtd) as we
> currently don't give a proper and easy solution for the task.

I certainly appreciate that you are trying to solve the fragmentation
issue around this, but it looks like we need a different approach than
using DT to describe partitions.

>
> [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst
>

Kind regards
Uffe

Re: [PATCH v4 3/7] dt-bindings: mmc: add property for partitions node in mmc-card node

[Ulf Hansson]

On Wed, 21 Aug 2024 at 20:09, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Wed, Aug 21, 2024 at 03:14:29PM +0200, Ulf Hansson wrote:
> > On Wed, 21 Aug 2024 at 11:52, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > >
> > > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote:
> > > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote:
> > > > > Add property for defining partitions node in mmc-card node to define
> > > > > partitions in DT by the use of the block2mtd module to use block
> > > > > devices as MTD.
> > > >
> > > > You justified patch 1 saying eMMC already supported this, but then here
> > > > you add support.
> > > >
> > > > Both are a NAK for me as both already have a way to describe partitions
> > > > with GPT.
> > > >
> > >
> > > I think this got a bit confused and hope we can find a way to add
> > > support for this.
> > >
> > > What is "already supported" is assigning an OF node so driver can
> > > reference it. This patch was just adding the nodes in the schema to say
> > > that partitions can be defined.
> > >
> > > I think what is not clear is that block devices might be used as raw
> > > devices without a partition table defined in the device. In such case
> > > it's the kernel that define a fixed partition table.
> > >
> > > One example is [1] where the partition table is provided by cmdline.
> > > Similar to cmdlinepart MTD parser.
> > >
> > > The use of block2mtd is just to make use of the MTD parser system.
> > >
> > > Considering
> > > - eMMC is soldered to the device (no dynamic scan)
> > > - cmdline might be not tunable and hardcoding it might also be
> > >   problematic (as some cmdline needs to be used)
> > > - concept of fixed partition for block device is already a thing and
> > >   used a lot (android AFAIK)
> >
> > Sorry for sidestepping your discussion, but I just wanted to add a few comments.
> >
> > It's not clear to me why we need something different than what we
> > already have today.
>
> It's really adding a missing feature. We have cmdline but we don't have
> DT. And in emebedded many times cmdline can't be changed.
>
> >
> > If it's a partuuid/uuid/label or a fixed block-partition from the
> > command line, we still need to know what partition we shall use for
> > what. So why is this problem different from how we manage filesystem
> > mounts, for example?
>
> In the context of eMMC there isn't any partition table and the cmdline
> is hardcoded by the bootloader. The cmdline might provide the root
> partition to use but doesn't declare the partition table (with cmdline)
>
> And the bootloader with the hardcoded cmdline might provide a different
> root partition in dual-boot-partition implementation on switching the
> boot partition. (this is used a lot with bootloader using a env variable
> and a different cmdline passed based on the variable to signal what
> partition to use for root)
>
> >
> > >
> > > I think it should be acceptable to introduce in DT support for defining
> > > fixed partition for block devices and some kind of parser system similar
> > > to MTD. What do you think? Would this be more acceptable? Idea is to
> > > just have a DT schema that makes use of the values that can be set in
> > > [1].
> >
> > In DT we can describe that there is an eMMC card soldered to the
> > board, because it's a description of the HW. But describing what
> > stored inside the eMMC-flash doesn't belong in DT.
> >
>
> Why? This conflicts with how it's done on MTD. Also consider the fact
> that eMMC might contain calibration partition used for NVMEM.

Because what is stored in the flash (eMMC) can be dynamically updated.
It's not a description of the HW as such, but I guess it depends on
how you see it. No matter what, you need to convince the DT
maintainers that this is the way to do it.

In my opinion, I think it would be better to invest our time to try a
different path.

>
> Describing fixed partition on a soldered eMMC that the bootloader
> expects at a fixed offset and won't ever change (or it will result in a
> brick) sounds like describing the HW to me. (it's the same principle of
> MTD just applied to block devices)

I guess it's somewhat a greyzone in this kind of case, assuming you
are referring to a bootloader that is stored in some ROM code that
can't be updated.

>
> (I know it sound strange as block devices are expected to have a
> partition table in MBR or GPT but reality is that it's not always the
> case)

I fully understand that but there are different ways to deal with that
too. Maybe enforce to write an MBR/GPT when flashing, assuming it
doesn't overwrite some data that is needed.

Another option is to let the bootloader parse some platform specific
data in the flash (unless it's hard coded in there too) and then make
it update the block-devparts for the kernel command line, for example.

>
> > >
> > > Hope we can find a solution to this, I'm totally OK for dropping NVMe as
> > > I understand it's PCIe stuff and very dynamic but OEM are making lots of
> > > use of eMMC and are starting to use these strange way (block2mtd) as we
> > > currently don't give a proper and easy solution for the task.
> >
> > I certainly appreciate that you are trying to solve the fragmentation
> > issue around this, but it looks like we need a different approach than
> > using DT to describe partitions.
> >
>
> Well it's either DT that can be tweaked and is part of the image or
> cmdline that have tons of limitation due to bootloader having fun
> hardcoding it or also actually requiring the bootloader cmdline and
> adding overlay on it. Honestly as I said adding DT support is really
> compleating the feature of the cmdline implementation.

Another option is also to parse the platform specific data in some way
and create partitions by using initramfs.

Kind regards
Uffe