Improve mtdpart robustness

Hello,

I hit a problem with ofpart getting confused by dt nodes unrelated to parttioning and refusing the dt which in turn led to mtd refusing access to the device altogether.

Compiling in a partitioning scheme should not prevent you access to the unpartitioned device.

Please apply at least the functional patches.

Thanks

Michal

Michal Suchanek (3):
mtd: mtdpart: add debug prints to partition parser.
mtd: ofpart: do not fail probe when no partitions exist
mtd: mtdpart: Do not fail mtd probe when parsing partitions fails.

 drivers/mtd/mtdpart.c | 9 ++++++---
 drivers/mtd/ofpart.c  | 4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)

2.1.4

The probe of a mtd device can fail when a partition parser returns error. The failure due to partition parsing can be quite mysterious when multiple partitioning schemes are comiled in and any of them can fail the probe.

Add debug prints which show what parsers were tried and what they returned.

---
 drivers/mtd/mtdpart.c | 4 ++++
 1 file changed, 4 insertions(+)

index cafdb88..a79c4f7 100644
@@ -759,12 +759,16 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		types = default_mtd_part_types;
 
 	for ( ; ret <= 0 && *types; types++) {
+		pr_debug("Parsing partitions %s on %s", *types, master->name);
 		parser = get_partition_parser(*types);
 		if (!parser && !request_module("%s", *types))
 			parser = get_partition_parser(*types);
+		pr_debug("%s got parser %s", master->name,
+			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
 		ret = (*parser->parse_fn)(master, pparts, data);
+		pr_debug("%s parser %s: %i", master->name, parser->name, ret);
 		put_partition_parser(parser);
 		if (ret > 0) {
 			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",

2.1.4

On Exynos it is necessary to set SPI controller parameters that apply to a SPI slave in a DT subnode of the slave device. Example:

flash: m25p80@0 {
        #address-cells = <1>;
        #size-cells = <1>;
        compatible = "jedec,spi-nor";
        reg = <0>;
        spi-max-frequency = <40000000>;
        m25p,fast-read;
        controller-data {
            samsung,spi-feedback-delay = <0>;
        };
};

The ofpart code checks if there are any subnodes on the MTD device DT node and if so it tries to parse them as MTD partitions (example below)

flash@0 {
        #address-cells = <1>;
        #size-cells = <1>;

        partition@0 {
                label = "u-boot";
                reg = <0x0000000 0x100000>;
                read-only;
        };

        uimage@100000 {
                reg = <0x0100000 0x200000>;
        };
};

The controller-data node contains no partition information and no other subnodes with partition information exist.

The ofpart code returns an error when there are subnodes of the flash DT node but no partitions are found. This error is then propagated to mtdpart which propagetes it to MTD probe which fails probing the flash device.

Change this condition to a warning so that flash without partitions can be accessed on Exynos with ofpart support compiled in.

--
- add more verbose explanation

---
 drivers/mtd/ofpart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index aa26c32..a29d29f 100644
@@ -94,10 +94,10 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 
 	if (!i) {
 		of_node_put(pp);
-		pr_err("No valid partition found on %s\n", node->full_name);
+		pr_warn("No valid partition found on %s\n", node->full_name);
 		kfree(*pparts);
 		*pparts = NULL;
-		return -EINVAL;
+		return 0;
 	}
 
 	return nr_parts;

2.1.4

On Mon, Jul 27, 2015 at 08:30:43PM -0000, Michal Suchanek wrote:
...

You never replied to my suggestion here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/352206.html

Particularly, "just define a proper compatibile property for [the 'controller-data'] subnode, and ofpart.c will naturally handle this".

Ok, so I don't find it reasonable to have one driver handle devicetree nodes without compatible property and insist that no other driver ever defines nodes without a compatible property.

So either drivers that parse nodes without a compatible property should handle nodes that are meant to be used by other drivers or *every* driver has to use a compatible property including ofpart and then no collision can happen.

So this goes both ways - either deal with nodes that have no compatible but are not for your driver or define a compatible for your driver. The s3c64xx driver is ahead of ofpart here since it just tries to read a property from a subnode of particular name and falls back to default if not present.

All in all there is no 'foo-bar devicetree'. The fact that your driver does not understand a particular part of a devicetree does not mean the part is incorrect. You cannot know what drivers will emerge in the future and what bindings will they use. In fact this very issue shows that you make wrong assumptions about that.

Thanks

Michal

Due to wrong assumption in ofpart ofpart fails on Exynos on SPI chips with no partitions because the subnode containing controller data confuses the ofpart parser.

Thus compiling in ofpart support automatically fails probing any SPI NOR flash without partitions on Exynos. Compiling in a partitioning scheme should not cause probe of otherwise valid device to fail.

Remove that failure possibility.

---
 drivers/mtd/mtdpart.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

index a79c4f7..37a2b79 100644
@@ -743,7 +743,6 @@ static const char * const default_mtd_part_types[] = {
  * partitions parsed out by the first parser.
  *
  * This function may return:
- * o a negative error code in case of failure
  * o zero if no partitions were found
  * o a positive number of found partitions, in which case on exit @pparts will
  *   point to an array containing this number of &struct mtd_info objects.

@@ -773,10 +772,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret > 0) {
 			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
 			       ret, parser->name, master->name);
-			break;
+			return ret;
 		}
 	}
-	return ret;
+	return 0;
 }
 
 int mtd_is_partition(const struct mtd_info *mtd)

2.1.4