[PATCH 0/4] mtd: extend support for "fixed-partitions"

[PATCH 0/4] mtd: extend support for "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings easier in the future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "cfe";
			reg = <0x0000000 0x40000>;
			read-only;
		};

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

Brian: I took your patches so I preserved your Signed-off-by, but please review
these changes to make sure I didn't do anything nasty there.

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 28 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 101 insertions(+), 15 deletions(-)

-- 
2.11.0

[PATCH 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 81a224da63be..27593adc45c2 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,45 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g. "boot
+code", "nvram", "kernel").
+
+Different devices may be partitioned in a different ways. Some may use a fixed
+flash layout set at production time. Some may use on-flash table that describes
+the geometry and naming/purpose of each functional region. It is also possible
+to see these methods mixed.
+
+To assist system software in locating partitions, we provide a binding to
+describe which method is used for a given flash.
+
+
+Fixed Partitions
+================
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "fixed-partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
-- 
2.11.0

[PATCH 2/4] mtd: partitions: factor out code calling parser

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

This code is going to be reused for parsers matched using OF so let's
factor it out to make this easier.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 81e0b80237df..73c52f1a2e4c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n", ret,
+		  parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.11.0

[PATCH 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

This support is currently limited to built-in parsers as it uses
request_module() and friends. This should be sufficient for most cases
though as compiling parsers as modules isn't a common choice.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 73c52f1a2e4c..d0cb1a892ed2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
 	return ret;
 }
 
+static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+				    struct mtd_part_parser *parser)
+{
+	struct device_node *np;
+	bool ret;
+
+	np = mtd_get_of_node(mtd);
+	np = of_get_child_by_name(np, "partitions");
+
+	ret = !!of_match_node(parser->of_match_table, np);
+
+	of_node_put(np);
+
+	return ret;
+}
+
+static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_get_parser_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2787e76c030f..073e1d8d5d17 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -77,6 +77,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
-- 
2.11.0

[PATCH 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This allows using this parser with any flash driver that takes care of
setting of_node (using mtd_set_of_node helper) correctly. Up to now
support for "fixed-partitions" DT compatibility string was working only
with flash drivers that were specifying "ofpart" (manually or by letting
mtd use the default set of parsers).

This matches existing bindings documentation.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/ofpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 2861c7079d7b..fb6f3df40e94 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	return ret;
 }
 
+static const struct of_device_id parse_ofpart_match_table[] = {
+	{ .compatible = "fixed-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
+
 static struct mtd_part_parser ofpart_parser = {
 	.parse_fn = parse_ofpart_partitions,
 	.name = "ofpart",
+	.of_match_table = parse_ofpart_match_table,
 };
 
 static int parse_ofoldpart_partitions(struct mtd_info *master,
-- 
2.11.0

[PATCH RESEND 0/4] mtd: extend support for "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

RESENDING with DT guys included, sorry for the mistake.

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings easier in the future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "cfe";
			reg = <0x0000000 0x40000>;
			read-only;
		};

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

Brian: I took your patches so I preserved your Signed-off-by, but please review
these changes to make sure I didn't do anything nasty there.

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 28 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 101 insertions(+), 15 deletions(-)

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 0/4] mtd: extend support for "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

RESENDING with DT guys included, sorry for the mistake.

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings easier in the future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "cfe";
			reg = <0x0000000 0x40000>;
			read-only;
		};

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

Brian: I took your patches so I preserved your Signed-off-by, but please review
these changes to make sure I didn't do anything nasty there.

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 28 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 101 insertions(+), 15 deletions(-)

-- 
2.11.0

[PATCH RESEND 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 81a224da63be..27593adc45c2 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,45 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g. "boot
+code", "nvram", "kernel").
+
+Different devices may be partitioned in a different ways. Some may use a fixed
+flash layout set at production time. Some may use on-flash table that describes
+the geometry and naming/purpose of each functional region. It is also possible
+to see these methods mixed.
+
+To assist system software in locating partitions, we provide a binding to
+describe which method is used for a given flash.
+
+
+Fixed Partitions
+================
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "fixed-partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 81a224da63be..27593adc45c2 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,45 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g. "boot
+code", "nvram", "kernel").
+
+Different devices may be partitioned in a different ways. Some may use a fixed
+flash layout set at production time. Some may use on-flash table that describes
+the geometry and naming/purpose of each functional region. It is also possible
+to see these methods mixed.
+
+To assist system software in locating partitions, we provide a binding to
+describe which method is used for a given flash.
+
+
+Fixed Partitions
+================
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "fixed-partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
-- 
2.11.0

[PATCH RESEND 2/4] mtd: partitions: factor out code calling parser

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This code is going to be reused for parsers matched using OF so let's
factor it out to make this easier.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 81e0b80237df..73c52f1a2e4c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n", ret,
+		  parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 2/4] mtd: partitions: factor out code calling parser

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

This code is going to be reused for parsers matched using OF so let's
factor it out to make this easier.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 81e0b80237df..73c52f1a2e4c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n", ret,
+		  parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.11.0

[PATCH RESEND 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

This support is currently limited to built-in parsers as it uses
request_module() and friends. This should be sufficient for most cases
though as compiling parsers as modules isn't a common choice.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 73c52f1a2e4c..d0cb1a892ed2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
 	return ret;
 }
 
+static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+				    struct mtd_part_parser *parser)
+{
+	struct device_node *np;
+	bool ret;
+
+	np = mtd_get_of_node(mtd);
+	np = of_get_child_by_name(np, "partitions");
+
+	ret = !!of_match_node(parser->of_match_table, np);
+
+	of_node_put(np);
+
+	return ret;
+}
+
+static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_get_parser_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2787e76c030f..073e1d8d5d17 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -77,6 +77,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

This support is currently limited to built-in parsers as it uses
request_module() and friends. This should be sufficient for most cases
though as compiling parsers as modules isn't a common choice.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 73c52f1a2e4c..d0cb1a892ed2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
 	return ret;
 }
 
+static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+				    struct mtd_part_parser *parser)
+{
+	struct device_node *np;
+	bool ret;
+
+	np = mtd_get_of_node(mtd);
+	np = of_get_child_by_name(np, "partitions");
+
+	ret = !!of_match_node(parser->of_match_table, np);
+
+	of_node_put(np);
+
+	return ret;
+}
+
+static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_get_parser_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2787e76c030f..073e1d8d5d17 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -77,6 +77,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
-- 
2.11.0

[PATCH RESEND 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This allows using this parser with any flash driver that takes care of
setting of_node (using mtd_set_of_node helper) correctly. Up to now
support for "fixed-partitions" DT compatibility string was working only
with flash drivers that were specifying "ofpart" (manually or by letting
mtd use the default set of parsers).

This matches existing bindings documentation.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 drivers/mtd/ofpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 2861c7079d7b..fb6f3df40e94 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	return ret;
 }
 
+static const struct of_device_id parse_ofpart_match_table[] = {
+	{ .compatible = "fixed-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
+
 static struct mtd_part_parser ofpart_parser = {
 	.parse_fn = parse_ofpart_partitions,
 	.name = "ofpart",
+	.of_match_table = parse_ofpart_match_table,
 };
 
 static int parse_ofoldpart_partitions(struct mtd_info *master,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This allows using this parser with any flash driver that takes care of
setting of_node (using mtd_set_of_node helper) correctly. Up to now
support for "fixed-partitions" DT compatibility string was working only
with flash drivers that were specifying "ofpart" (manually or by letting
mtd use the default set of parsers).

This matches existing bindings documentation.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/ofpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 2861c7079d7b..fb6f3df40e94 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	return ret;
 }
 
+static const struct of_device_id parse_ofpart_match_table[] = {
+	{ .compatible = "fixed-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
+
 static struct mtd_part_parser ofpart_parser = {
 	.parse_fn = parse_ofpart_partitions,
 	.name = "ofpart",
+	.of_match_table = parse_ofpart_match_table,
 };
 
 static int parse_ofoldpart_partitions(struct mtd_info *master,
-- 
2.11.0

Re: [PATCH RESEND 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Brian Norris]

+ others

Hi Rafał,

Thanks for drudging up my old work. This is something that's been stuck
too far down in my stack of TODOs that it essentially timed out...

On Thu, Apr 20, 2017 at 03:57:28PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---

I mentioned the missing changelog on IRC, but to fill in the gaps:
essentially you've dropped the partition parser and associated bindings
I had in my series (for the 'Google FMAP' format). That's fine, but I
just wanted to note it.

And with that, I think you've not quite nailed the purpose of my
original patch. This now seems to suggest there are other potential
bindings here, but then you leave the reader hanging. One note to that
effect below. If that's the only objection, then I can make the
additions myself when applying.

>  .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..27593adc45c2 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,45 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>  
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g. "boot
> +code", "nvram", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.


We've suggested above that there may be "different ways" (fixed vs.
on-flash tables) to partition, but then we still only describe one way.
Maybe we can just paste something like this as a caveat before moving
on?

"We currently only document a binding for fixed layouts."

We can delete that in the presumed follow-up that proposes the
"on-flash" parser bindings. Sound OK?

Brian

> +
> +
> +Fixed Partitions
> +================
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>  
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "fixed-partitions"
>  Partitions are then defined in subnodes of the partitions node.
>  
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>  
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>  
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>  
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Brian Norris]

+ others

Hi Rafał,

Thanks for drudging up my old work. This is something that's been stuck
too far down in my stack of TODOs that it essentially timed out...

On Thu, Apr 20, 2017 at 03:57:28PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace@gmail.com>
> 
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

I mentioned the missing changelog on IRC, but to fill in the gaps:
essentially you've dropped the partition parser and associated bindings
I had in my series (for the 'Google FMAP' format). That's fine, but I
just wanted to note it.

And with that, I think you've not quite nailed the purpose of my
original patch. This now seems to suggest there are other potential
bindings here, but then you leave the reader hanging. One note to that
effect below. If that's the only objection, then I can make the
additions myself when applying.

>  .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..27593adc45c2 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,45 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>  
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g. "boot
> +code", "nvram", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.


We've suggested above that there may be "different ways" (fixed vs.
on-flash tables) to partition, but then we still only describe one way.
Maybe we can just paste something like this as a caveat before moving
on?

"We currently only document a binding for fixed layouts."

We can delete that in the presumed follow-up that proposes the
"on-flash" parser bindings. Sound OK?

Brian

> +
> +
> +Fixed Partitions
> +================
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>  
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "fixed-partitions"
>  Partitions are then defined in subnodes of the partitions node.
>  
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>  
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>  
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>  
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> -- 
> 2.11.0
> 

Re: [PATCH RESEND 2/4] mtd: partitions: factor out code calling parser

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:29PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This code is going to be reused for parsers matched using OF so let's
> factor it out to make this easier.
> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---

Could have used a changelog to note dropping the
mtd_part_parser_get_by_name() refactoring. Also the corresponding change
in subject (from "mtd: partitions: factor out "match by name"
handling").

But this seems fine to me.

Acked-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

>  drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 81e0b80237df..73c52f1a2e4c 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
>  	NULL
>  };
>  
> +static int mtd_part_do_parse(struct mtd_part_parser *parser,
> +			     struct mtd_info *master,
> +			     struct mtd_partitions *pparts,
> +			     struct mtd_part_parser_data *data)
> +{
> +	int ret;
> +
> +	ret = (*parser->parse_fn)(master, &pparts->parts, data);
> +	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
> +	if (ret <= 0)
> +		return ret;
> +
> +	pr_notice("%d %s partitions found on MTD device %s\n", ret,
> +		  parser->name, master->name);
> +
> +	pparts->nr_parts = ret;
> +	pparts->parser = parser;
> +
> +	return ret;
> +}
> +
>  /**
>   * parse_mtd_partitions - parse MTD partitions
>   * @master: the master partition (describes whole MTD device)
> @@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>  			 parser ? parser->name : NULL);
>  		if (!parser)
>  			continue;
> -		ret = (*parser->parse_fn)(master, &pparts->parts, data);
> -		pr_debug("%s: parser %s: %i\n",
> -			 master->name, parser->name, ret);
> -		if (ret > 0) {
> -			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
> -			       ret, parser->name, master->name);
> -			pparts->nr_parts = ret;
> -			pparts->parser = parser;
> +		ret = mtd_part_do_parse(parser, master, pparts, data);
> +		/* Found partitions! */
> +		if (ret > 0)
>  			return 0;
> -		}
>  		mtd_part_parser_put(parser);
>  		/*
>  		 * Stash the first error we see; only report it if no parser
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 2/4] mtd: partitions: factor out code calling parser

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:29PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace@gmail.com>
> 
> This code is going to be reused for parsers matched using OF so let's
> factor it out to make this easier.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Could have used a changelog to note dropping the
mtd_part_parser_get_by_name() refactoring. Also the corresponding change
in subject (from "mtd: partitions: factor out "match by name"
handling").

But this seems fine to me.

Acked-by: Brian Norris <computersforpeace@gmail.com>

>  drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 81e0b80237df..73c52f1a2e4c 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
>  	NULL
>  };
>  
> +static int mtd_part_do_parse(struct mtd_part_parser *parser,
> +			     struct mtd_info *master,
> +			     struct mtd_partitions *pparts,
> +			     struct mtd_part_parser_data *data)
> +{
> +	int ret;
> +
> +	ret = (*parser->parse_fn)(master, &pparts->parts, data);
> +	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
> +	if (ret <= 0)
> +		return ret;
> +
> +	pr_notice("%d %s partitions found on MTD device %s\n", ret,
> +		  parser->name, master->name);
> +
> +	pparts->nr_parts = ret;
> +	pparts->parser = parser;
> +
> +	return ret;
> +}
> +
>  /**
>   * parse_mtd_partitions - parse MTD partitions
>   * @master: the master partition (describes whole MTD device)
> @@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>  			 parser ? parser->name : NULL);
>  		if (!parser)
>  			continue;
> -		ret = (*parser->parse_fn)(master, &pparts->parts, data);
> -		pr_debug("%s: parser %s: %i\n",
> -			 master->name, parser->name, ret);
> -		if (ret > 0) {
> -			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
> -			       ret, parser->name, master->name);
> -			pparts->nr_parts = ret;
> -			pparts->parser = parser;
> +		ret = mtd_part_do_parse(parser, master, pparts, data);
> +		/* Found partitions! */
> +		if (ret > 0)
>  			return 0;
> -		}
>  		mtd_part_parser_put(parser);
>  		/*
>  		 * Stash the first error we see; only report it if no parser
> -- 
> 2.11.0
> 

Re: [PATCH RESEND 3/4] mtd: partitions: add of_match_table parser matching

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:30PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Partition parsers can now provide an of_match_table to enable
> flash<-->parser matching via device tree.
> 
> This support is currently limited to built-in parsers as it uses
> request_module() and friends. This should be sufficient for most cases
> though as compiling parsers as modules isn't a common choice.
> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/partitions.h |  1 +
>  2 files changed, 48 insertions(+)

Seems like a valid refactoring of my original work, esp. given that
of_mtd.c went away since then:

Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 73c52f1a2e4c..d0cb1a892ed2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>  	return ret;
>  }
>  
> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +				    struct mtd_part_parser *parser)
> +{
> +	struct device_node *np;
> +	bool ret;
> +
> +	np = mtd_get_of_node(mtd);
> +	np = of_get_child_by_name(np, "partitions");
> +
> +	ret = !!of_match_node(parser->of_match_table, np);
> +
> +	of_node_put(np);
> +
> +	return ret;
> +}
> +
> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
> +{
> +	struct mtd_part_parser *p, *ret = NULL;
> +
> +	spin_lock(&part_parser_lock);
> +
> +	list_for_each_entry(p, &part_parsers, list) {
> +		if (of_mtd_match_mtd_parser(mtd, p) &&
> +				try_module_get(p->owner)) {
> +			ret = p;
> +			break;
> +		}
> +	}
> +
> +	spin_unlock(&part_parser_lock);
> +
> +	return ret;
> +}
> +
>  /**
>   * parse_mtd_partitions - parse MTD partitions
>   * @master: the master partition (describes whole MTD device)
> @@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>  		if (ret < 0 && !err)
>  			err = ret;
>  	}
> +
> +	parser = mtd_part_get_parser_by_of(master);
> +	if (!parser)
> +		return err;
> +
> +	ret = mtd_part_do_parse(parser, master, pparts, data);
> +	if (ret > 0)
> +		return 0;
> +	mtd_part_parser_put(parser);
> +	if (ret < 0 && !err)
> +		err = ret;
> +
>  	return err;
>  }
>  
> diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
> index 2787e76c030f..073e1d8d5d17 100644
> --- a/include/linux/mtd/partitions.h
> +++ b/include/linux/mtd/partitions.h
> @@ -77,6 +77,7 @@ struct mtd_part_parser {
>  	struct list_head list;
>  	struct module *owner;
>  	const char *name;
> +	const struct of_device_id *of_match_table;
>  	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
>  			struct mtd_part_parser_data *);
>  	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 3/4] mtd: partitions: add of_match_table parser matching

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:30PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace@gmail.com>
> 
> Partition parsers can now provide an of_match_table to enable
> flash<-->parser matching via device tree.
> 
> This support is currently limited to built-in parsers as it uses
> request_module() and friends. This should be sufficient for most cases
> though as compiling parsers as modules isn't a common choice.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/partitions.h |  1 +
>  2 files changed, 48 insertions(+)

Seems like a valid refactoring of my original work, esp. given that
of_mtd.c went away since then:

Acked-by: Brian Norris <computersforpeac@gmail.com>

> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 73c52f1a2e4c..d0cb1a892ed2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>  	return ret;
>  }
>  
> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +				    struct mtd_part_parser *parser)
> +{
> +	struct device_node *np;
> +	bool ret;
> +
> +	np = mtd_get_of_node(mtd);
> +	np = of_get_child_by_name(np, "partitions");
> +
> +	ret = !!of_match_node(parser->of_match_table, np);
> +
> +	of_node_put(np);
> +
> +	return ret;
> +}
> +
> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
> +{
> +	struct mtd_part_parser *p, *ret = NULL;
> +
> +	spin_lock(&part_parser_lock);
> +
> +	list_for_each_entry(p, &part_parsers, list) {
> +		if (of_mtd_match_mtd_parser(mtd, p) &&
> +				try_module_get(p->owner)) {
> +			ret = p;
> +			break;
> +		}
> +	}
> +
> +	spin_unlock(&part_parser_lock);
> +
> +	return ret;
> +}
> +
>  /**
>   * parse_mtd_partitions - parse MTD partitions
>   * @master: the master partition (describes whole MTD device)
> @@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>  		if (ret < 0 && !err)
>  			err = ret;
>  	}
> +
> +	parser = mtd_part_get_parser_by_of(master);
> +	if (!parser)
> +		return err;
> +
> +	ret = mtd_part_do_parse(parser, master, pparts, data);
> +	if (ret > 0)
> +		return 0;
> +	mtd_part_parser_put(parser);
> +	if (ret < 0 && !err)
> +		err = ret;
> +
>  	return err;
>  }
>  
> diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
> index 2787e76c030f..073e1d8d5d17 100644
> --- a/include/linux/mtd/partitions.h
> +++ b/include/linux/mtd/partitions.h
> @@ -77,6 +77,7 @@ struct mtd_part_parser {
>  	struct list_head list;
>  	struct module *owner;
>  	const char *name;
> +	const struct of_device_id *of_match_table;
>  	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
>  			struct mtd_part_parser_data *);
>  	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
> -- 
> 2.11.0
> 

Re: [PATCH RESEND 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:31PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> This allows using this parser with any flash driver that takes care of
> setting of_node (using mtd_set_of_node helper) correctly. Up to now
> support for "fixed-partitions" DT compatibility string was working only
> with flash drivers that were specifying "ofpart" (manually or by letting
> mtd use the default set of parsers).
> 
> This matches existing bindings documentation.
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---

Seems OK to me. I think this means we can even get away without putting
"ofpart" in the "probe type" arrays in drivers now, if we don't care
about the legacy binding. Probably not gonna happen for existing
drivers, but at least it helps going forward. And of course, it provides
a starting example for new partition-parser compatible properties, if
that's the way we go.

Reviewed-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

>  drivers/mtd/ofpart.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
> index 2861c7079d7b..fb6f3df40e94 100644
> --- a/drivers/mtd/ofpart.c
> +++ b/drivers/mtd/ofpart.c
> @@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
>  	return ret;
>  }
>  
> +static const struct of_device_id parse_ofpart_match_table[] = {
> +	{ .compatible = "fixed-partitions" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
> +
>  static struct mtd_part_parser ofpart_parser = {
>  	.parse_fn = parse_ofpart_partitions,
>  	.name = "ofpart",
> +	.of_match_table = parse_ofpart_match_table,
>  };
>  
>  static int parse_ofoldpart_partitions(struct mtd_info *master,
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Brian Norris]

On Thu, Apr 20, 2017 at 03:57:31PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This allows using this parser with any flash driver that takes care of
> setting of_node (using mtd_set_of_node helper) correctly. Up to now
> support for "fixed-partitions" DT compatibility string was working only
> with flash drivers that were specifying "ofpart" (manually or by letting
> mtd use the default set of parsers).
> 
> This matches existing bindings documentation.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Seems OK to me. I think this means we can even get away without putting
"ofpart" in the "probe type" arrays in drivers now, if we don't care
about the legacy binding. Probably not gonna happen for existing
drivers, but at least it helps going forward. And of course, it provides
a starting example for new partition-parser compatible properties, if
that's the way we go.

Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Tested-by: Brian Norris <computersforpeace@gmail.com>

>  drivers/mtd/ofpart.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
> index 2861c7079d7b..fb6f3df40e94 100644
> --- a/drivers/mtd/ofpart.c
> +++ b/drivers/mtd/ofpart.c
> @@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
>  	return ret;
>  }
>  
> +static const struct of_device_id parse_ofpart_match_table[] = {
> +	{ .compatible = "fixed-partitions" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
> +
>  static struct mtd_part_parser ofpart_parser = {
>  	.parse_fn = parse_ofpart_partitions,
>  	.name = "ofpart",
> +	.of_match_table = parse_ofpart_match_table,
>  };
>  
>  static int parse_ofoldpart_partitions(struct mtd_info *master,
> -- 
> 2.11.0
> 

[PATCH V2 0/4] mtd: extend support for "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

Original patches:
(not picked) [RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/
(not picked) [RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu
(partially)  [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
(picked)     [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
(partially)  [RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling
(picked)     [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
(not picked) [RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings (like Google's FMAP) easier in the
future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "cfe";
			reg = <0x0000000 0x40000>;
			read-only;
		};

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

V2: Modify patch 1/4
    Include list of original patches in 0/4
    Include changelog in every patch
    Add Brian's tags (Acked/Reviewed/Tested)

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 30 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 103 insertions(+), 15 deletions(-)

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 0/4] mtd: extend support for "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

Original patches:
(not picked) [RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/
(not picked) [RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu
(partially)  [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
(picked)     [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
(partially)  [RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling
(picked)     [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
(not picked) [RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings (like Google's FMAP) easier in the
future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "cfe";
			reg = <0x0000000 0x40000>;
			read-only;
		};

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

V2: Modify patch 1/4
    Include list of original patches in 0/4
    Include changelog in every patch
    Add Brian's tags (Acked/Reviewed/Tested)

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 30 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 103 insertions(+), 15 deletions(-)

-- 
2.11.0

[PATCH V2 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
This is based on Brian's patch:
[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

V1: Dropped "Section B: On-Flash Partition Tables" with Google's FMAP as this
    patchset doesn't add that new parser.
V2: Add "We currently only document a binding for fixed layouts." part
---
 .../devicetree/bindings/mtd/partition.txt          | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 81a224da63be..b5de311b967a 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,47 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g. "boot
+code", "nvram", "kernel").
+
+Different devices may be partitioned in a different ways. Some may use a fixed
+flash layout set at production time. Some may use on-flash table that describes
+the geometry and naming/purpose of each functional region. It is also possible
+to see these methods mixed.
+
+To assist system software in locating partitions, we provide a binding to
+describe which method is used for a given flash.
+
+We currently only document a binding for fixed layouts.
+
+
+Fixed Partitions
+================
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "fixed-partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This is based on Brian's patch:
[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

V1: Dropped "Section B: On-Flash Partition Tables" with Google's FMAP as this
    patchset doesn't add that new parser.
V2: Add "We currently only document a binding for fixed layouts." part
---
 .../devicetree/bindings/mtd/partition.txt          | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 81a224da63be..b5de311b967a 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,47 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g. "boot
+code", "nvram", "kernel").
+
+Different devices may be partitioned in a different ways. Some may use a fixed
+flash layout set at production time. Some may use on-flash table that describes
+the geometry and naming/purpose of each functional region. It is also possible
+to see these methods mixed.
+
+To assist system software in locating partitions, we provide a binding to
+describe which method is used for a given flash.
+
+We currently only document a binding for fixed layouts.
+
+
+Fixed Partitions
+================
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "fixed-partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
-- 
2.11.0

[PATCH V2 2/4] mtd: partitions: factor out code calling parser

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This code is going to be reused for parsers matched using OF so let's
factor it out to make this easier.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Acked-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
This is based on Brian's patch:
[RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling

V1: Do not factor out mtd_part_parser_get_by_name as it's not required by this
    patchset.
---
 drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 81e0b80237df..73c52f1a2e4c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n", ret,
+		  parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/4] mtd: partitions: factor out code calling parser

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

This code is going to be reused for parsers matched using OF so let's
factor it out to make this easier.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Brian Norris <computersforpeace@gmail.com>
---
This is based on Brian's patch:
[RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling

V1: Do not factor out mtd_part_parser_get_by_name as it's not required by this
    patchset.
---
 drivers/mtd/mtdpart.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 81e0b80237df..73c52f1a2e4c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -840,6 +840,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n", ret,
+		  parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -880,16 +901,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.11.0

[PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

This support is currently limited to built-in parsers as it uses
request_module() and friends. This should be sufficient for most cases
though as compiling parsers as modules isn't a common choice.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
This is based on Brian's patches:
[RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
[RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching

V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
    Merge helpers into a single of_mtd_match_mtd_parser
---
 drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 73c52f1a2e4c..d0cb1a892ed2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
 	return ret;
 }
 
+static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+				    struct mtd_part_parser *parser)
+{
+	struct device_node *np;
+	bool ret;
+
+	np = mtd_get_of_node(mtd);
+	np = of_get_child_by_name(np, "partitions");
+
+	ret = !!of_match_node(parser->of_match_table, np);
+
+	of_node_put(np);
+
+	return ret;
+}
+
+static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_get_parser_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2787e76c030f..073e1d8d5d17 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -77,6 +77,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

From: Brian Norris <computersforpeace@gmail.com>

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

This support is currently limited to built-in parsers as it uses
request_module() and friends. This should be sufficient for most cases
though as compiling parsers as modules isn't a common choice.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Brian Norris <computersforpeac@gmail.com>
---
This is based on Brian's patches:
[RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
[RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching

V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
    Merge helpers into a single of_mtd_match_mtd_parser
---
 drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 73c52f1a2e4c..d0cb1a892ed2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
 	return ret;
 }
 
+static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+				    struct mtd_part_parser *parser)
+{
+	struct device_node *np;
+	bool ret;
+
+	np = mtd_get_of_node(mtd);
+	np = of_get_child_by_name(np, "partitions");
+
+	ret = !!of_match_node(parser->of_match_table, np);
+
+	of_node_put(np);
+
+	return ret;
+}
+
+static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -913,6 +948,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_get_parser_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2787e76c030f..073e1d8d5d17 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -77,6 +77,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
-- 
2.11.0

[PATCH V2 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This allows using this parser with any flash driver that takes care of
setting of_node (using mtd_set_of_node helper) correctly. Up to now
support for "fixed-partitions" DT compatibility string was working only
with flash drivers that were specifying "ofpart" (manually or by letting
mtd use the default set of parsers).

This matches existing bindings documentation.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Reviewed-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/mtd/ofpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 2861c7079d7b..fb6f3df40e94 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	return ret;
 }
 
+static const struct of_device_id parse_ofpart_match_table[] = {
+	{ .compatible = "fixed-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
+
 static struct mtd_part_parser ofpart_parser = {
 	.parse_fn = parse_ofpart_partitions,
 	.name = "ofpart",
+	.of_match_table = parse_ofpart_match_table,
 };
 
 static int parse_ofoldpart_partitions(struct mtd_info *master,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 4/4] mtd: ofpart: add of_match_table with "fixed-partitions"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This allows using this parser with any flash driver that takes care of
setting of_node (using mtd_set_of_node helper) correctly. Up to now
support for "fixed-partitions" DT compatibility string was working only
with flash drivers that were specifying "ofpart" (manually or by letting
mtd use the default set of parsers).

This matches existing bindings documentation.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Tested-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/ofpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 2861c7079d7b..fb6f3df40e94 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -140,9 +140,16 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 	return ret;
 }
 
+static const struct of_device_id parse_ofpart_match_table[] = {
+	{ .compatible = "fixed-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, parse_ofpart_match_table);
+
 static struct mtd_part_parser ofpart_parser = {
 	.parse_fn = parse_ofpart_partitions,
 	.name = "ofpart",
+	.of_match_table = parse_ofpart_match_table,
 };
 
 static int parse_ofoldpart_partitions(struct mtd_info *master,
-- 
2.11.0

Re: [PATCH V2 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Jonas Gorski]

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
>
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> This is based on Brian's patch:
> [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
>
> V1: Dropped "Section B: On-Flash Partition Tables" with Google's FMAP as this
>     patchset doesn't add that new parser.
> V2: Add "We currently only document a binding for fixed layouts." part
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..b5de311b967a 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,47 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g. "boot
> +code", "nvram", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.

Since patch 3 adds specifying methods through the compatible of the
partitions subnode, maybe we should document that here? Something
along

"To assist system software in locating partitions, we allow describing
which method is used for a given flash device. To describe the method
there should be a subnode of the flash device that is named
'partitions'. It must have a 'compatible' property, which is used to
identify the method to use."


Regards
Jonas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/4] dt-bindings: mtd: make partitions doc a bit more generic

[Jonas Gorski]

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Brian Norris <computersforpeace@gmail.com>
>
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> This is based on Brian's patch:
> [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
>
> V1: Dropped "Section B: On-Flash Partition Tables" with Google's FMAP as this
>     patchset doesn't add that new parser.
> V2: Add "We currently only document a binding for fixed layouts." part
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..b5de311b967a 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,47 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g. "boot
> +code", "nvram", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.

Since patch 3 adds specifying methods through the compatible of the
partitions subnode, maybe we should document that here? Something
along

"To assist system software in locating partitions, we allow describing
which method is used for a given flash device. To describe the method
there should be a subnode of the flash device that is named
'partitions'. It must have a 'compatible' property, which is used to
identify the method to use."


Regards
Jonas

Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Jonas Gorski]

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Partition parsers can now provide an of_match_table to enable
> flash<-->parser matching via device tree.
>
> This support is currently limited to built-in parsers as it uses
> request_module() and friends. This should be sufficient for most cases
> though as compiling parsers as modules isn't a common choice.
>
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> This is based on Brian's patches:
> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>
> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>     Merge helpers into a single of_mtd_match_mtd_parser
> ---
>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/partitions.h |  1 +
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 73c52f1a2e4c..d0cb1a892ed2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>         return ret;
>  }
>
> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +                                   struct mtd_part_parser *parser)
> +{
> +       struct device_node *np;
> +       bool ret;
> +
> +       np = mtd_get_of_node(mtd);
> +       np = of_get_child_by_name(np, "partitions");
> +
> +       ret = !!of_match_node(parser->of_match_table, np);
> +
> +       of_node_put(np);
> +
> +       return ret;
> +}
> +
> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
> +{
> +       struct mtd_part_parser *p, *ret = NULL;
> +
> +       spin_lock(&part_parser_lock);
> +
> +       list_for_each_entry(p, &part_parsers, list) {
> +               if (of_mtd_match_mtd_parser(mtd, p) &&
> +                               try_module_get(p->owner)) {
> +                       ret = p;
> +                       break;
> +               }
> +       }


Hm, maybe iterate over the compatibles, so parsers matching the most
specific compatible get precedence in case there is more than one
compatible? Currently it will match the first one that matches any
compatible, and registration order of parsers can change that. I'm
thinking of parsers that partially rely on fixed, unprobable layouts,
so can use "fixed-partitions" as a fallback compatible.

E.g. having something like this

partitions {
        compatible = "sample,custom-layout", "fixed-partitions";

        bootloader@0 { ...  };

        firmware@10000 { .... }; /* will be split by the parser */

        extra@780000 { .... }; /* partition the on-flash format can't specify */
};

Where you will still be able to write an image raw to the image
partition even if the "custom-layout"-parser isn't present/enabled,
but if it is present, it should always be used.


Regards
Jonas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Jonas Gorski]

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Brian Norris <computersforpeace@gmail.com>
>
> Partition parsers can now provide an of_match_table to enable
> flash<-->parser matching via device tree.
>
> This support is currently limited to built-in parsers as it uses
> request_module() and friends. This should be sufficient for most cases
> though as compiling parsers as modules isn't a common choice.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Acked-by: Brian Norris <computersforpeac@gmail.com>
> ---
> This is based on Brian's patches:
> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>
> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>     Merge helpers into a single of_mtd_match_mtd_parser
> ---
>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/partitions.h |  1 +
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 73c52f1a2e4c..d0cb1a892ed2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>         return ret;
>  }
>
> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +                                   struct mtd_part_parser *parser)
> +{
> +       struct device_node *np;
> +       bool ret;
> +
> +       np = mtd_get_of_node(mtd);
> +       np = of_get_child_by_name(np, "partitions");
> +
> +       ret = !!of_match_node(parser->of_match_table, np);
> +
> +       of_node_put(np);
> +
> +       return ret;
> +}
> +
> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
> +{
> +       struct mtd_part_parser *p, *ret = NULL;
> +
> +       spin_lock(&part_parser_lock);
> +
> +       list_for_each_entry(p, &part_parsers, list) {
> +               if (of_mtd_match_mtd_parser(mtd, p) &&
> +                               try_module_get(p->owner)) {
> +                       ret = p;
> +                       break;
> +               }
> +       }


Hm, maybe iterate over the compatibles, so parsers matching the most
specific compatible get precedence in case there is more than one
compatible? Currently it will match the first one that matches any
compatible, and registration order of parsers can change that. I'm
thinking of parsers that partially rely on fixed, unprobable layouts,
so can use "fixed-partitions" as a fallback compatible.

E.g. having something like this

partitions {
        compatible = "sample,custom-layout", "fixed-partitions";

        bootloader@0 { ...  };

        firmware@10000 { .... }; /* will be split by the parser */

        extra@780000 { .... }; /* partition the on-flash format can't specify */
};

Where you will still be able to write an image raw to the image
partition even if the "custom-layout"-parser isn't present/enabled,
but if it is present, it should always be used.


Regards
Jonas

Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

On 04/24/2017 05:31 PM, Jonas Gorski wrote:
> On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Partition parsers can now provide an of_match_table to enable
>> flash<-->parser matching via device tree.
>>
>> This support is currently limited to built-in parsers as it uses
>> request_module() and friends. This should be sufficient for most cases
>> though as compiling parsers as modules isn't a common choice.
>>
>> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> This is based on Brian's patches:
>> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
>> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>>
>> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>>     Merge helpers into a single of_mtd_match_mtd_parser
>> ---
>>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/partitions.h |  1 +
>>  2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 73c52f1a2e4c..d0cb1a892ed2 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>>         return ret;
>>  }
>>
>> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
>> +                                   struct mtd_part_parser *parser)
>> +{
>> +       struct device_node *np;
>> +       bool ret;
>> +
>> +       np = mtd_get_of_node(mtd);
>> +       np = of_get_child_by_name(np, "partitions");
>> +
>> +       ret = !!of_match_node(parser->of_match_table, np);
>> +
>> +       of_node_put(np);
>> +
>> +       return ret;
>> +}
>> +
>> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
>> +{
>> +       struct mtd_part_parser *p, *ret = NULL;
>> +
>> +       spin_lock(&part_parser_lock);
>> +
>> +       list_for_each_entry(p, &part_parsers, list) {
>> +               if (of_mtd_match_mtd_parser(mtd, p) &&
>> +                               try_module_get(p->owner)) {
>> +                       ret = p;
>> +                       break;
>> +               }
>> +       }
>
>
> Hm, maybe iterate over the compatibles, so parsers matching the most
> specific compatible get precedence in case there is more than one
> compatible? Currently it will match the first one that matches any
> compatible, and registration order of parsers can change that. I'm
> thinking of parsers that partially rely on fixed, unprobable layouts,
> so can use "fixed-partitions" as a fallback compatible.
>
> E.g. having something like this
>
> partitions {
>         compatible = "sample,custom-layout", "fixed-partitions";
>
>         bootloader@0 { ...  };
>
>         firmware@10000 { .... }; /* will be split by the parser */
>
>         extra@780000 { .... }; /* partition the on-flash format can't specify */
> };
>
> Where you will still be able to write an image raw to the image
> partition even if the "custom-layout"-parser isn't present/enabled,
> but if it is present, it should always be used.

I see the point, but I'm afraid we're lacking some DT helper for this. See
below for the function I wrote (and I'm not proud of) - compile tested only.

I think we would need a new helper similar to the of_match_node:
1) Taking const struct of_device_id *matches
2) Taking const struct device_node *node
but returning a score of the best match.

DT guys: any comment on this? Rob?

Would this be acceptable to:
1) Take this patch as is as Linux current doesn't support other bindings
2) Work on DT helper + mtd modification in a separated patchset?

static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
{
	struct mtd_part_parser *p, *ret = NULL;
	struct device_node *np;
	struct property *prop;
	const char *cp;

	np = mtd_get_of_node(mtd);
	np = of_get_child_by_name(np, "partitions");
	if (!np)
		return NULL;

	spin_lock(&part_parser_lock);

	of_property_for_each_string(np, "compatible", prop, cp) {
		list_for_each_entry(p, &part_parsers, list) {
			const struct of_device_id *matches;

			for (matches = p->of_match_table;
			     matches->name[0] || matches->type[0] || matches->compatible[0];
			     matches++) {
				if (!of_compat_cmp(cp, matches->compatible, strlen(matches->compatible)) &&
				    try_module_get(p->owner)) {
					ret = p;
					break;
				}
			}

			if (ret)
				break;
		}

		if (ret)
			break;
	}

	spin_unlock(&part_parser_lock);

	of_node_put(np);

	return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching

[Rafał Miłecki]

On 04/24/2017 05:31 PM, Jonas Gorski wrote:
> On 24 April 2017 at 14:41, Rafał Miłecki <zajec5@gmail.com> wrote:
>> From: Brian Norris <computersforpeace@gmail.com>
>>
>> Partition parsers can now provide an of_match_table to enable
>> flash<-->parser matching via device tree.
>>
>> This support is currently limited to built-in parsers as it uses
>> request_module() and friends. This should be sufficient for most cases
>> though as compiling parsers as modules isn't a common choice.
>>
>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> Acked-by: Brian Norris <computersforpeac@gmail.com>
>> ---
>> This is based on Brian's patches:
>> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
>> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>>
>> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>>     Merge helpers into a single of_mtd_match_mtd_parser
>> ---
>>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/partitions.h |  1 +
>>  2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 73c52f1a2e4c..d0cb1a892ed2 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>>         return ret;
>>  }
>>
>> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
>> +                                   struct mtd_part_parser *parser)
>> +{
>> +       struct device_node *np;
>> +       bool ret;
>> +
>> +       np = mtd_get_of_node(mtd);
>> +       np = of_get_child_by_name(np, "partitions");
>> +
>> +       ret = !!of_match_node(parser->of_match_table, np);
>> +
>> +       of_node_put(np);
>> +
>> +       return ret;
>> +}
>> +
>> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
>> +{
>> +       struct mtd_part_parser *p, *ret = NULL;
>> +
>> +       spin_lock(&part_parser_lock);
>> +
>> +       list_for_each_entry(p, &part_parsers, list) {
>> +               if (of_mtd_match_mtd_parser(mtd, p) &&
>> +                               try_module_get(p->owner)) {
>> +                       ret = p;
>> +                       break;
>> +               }
>> +       }
>
>
> Hm, maybe iterate over the compatibles, so parsers matching the most
> specific compatible get precedence in case there is more than one
> compatible? Currently it will match the first one that matches any
> compatible, and registration order of parsers can change that. I'm
> thinking of parsers that partially rely on fixed, unprobable layouts,
> so can use "fixed-partitions" as a fallback compatible.
>
> E.g. having something like this
>
> partitions {
>         compatible = "sample,custom-layout", "fixed-partitions";
>
>         bootloader@0 { ...  };
>
>         firmware@10000 { .... }; /* will be split by the parser */
>
>         extra@780000 { .... }; /* partition the on-flash format can't specify */
> };
>
> Where you will still be able to write an image raw to the image
> partition even if the "custom-layout"-parser isn't present/enabled,
> but if it is present, it should always be used.

I see the point, but I'm afraid we're lacking some DT helper for this. See
below for the function I wrote (and I'm not proud of) - compile tested only.

I think we would need a new helper similar to the of_match_node:
1) Taking const struct of_device_id *matches
2) Taking const struct device_node *node
but returning a score of the best match.

DT guys: any comment on this? Rob?

Would this be acceptable to:
1) Take this patch as is as Linux current doesn't support other bindings
2) Work on DT helper + mtd modification in a separated patchset?

static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
{
	struct mtd_part_parser *p, *ret = NULL;
	struct device_node *np;
	struct property *prop;
	const char *cp;

	np = mtd_get_of_node(mtd);
	np = of_get_child_by_name(np, "partitions");
	if (!np)
		return NULL;

	spin_lock(&part_parser_lock);

	of_property_for_each_string(np, "compatible", prop, cp) {
		list_for_each_entry(p, &part_parsers, list) {
			const struct of_device_id *matches;

			for (matches = p->of_match_table;
			     matches->name[0] || matches->type[0] || matches->compatible[0];
			     matches++) {
				if (!of_compat_cmp(cp, matches->compatible, strlen(matches->compatible)) &&
				    try_module_get(p->owner)) {
					ret = p;
					break;
				}
			}

			if (ret)
				break;
		}

		if (ret)
			break;
	}

	spin_unlock(&part_parser_lock);

	of_node_put(np);

	return ret;
}