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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/24/2017 05:31 PM, Jonas Gorski wrote:
On 24 April 2017 at 14:41, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
From: Brian Norris <computersforpeace@xxxxxxxxx>

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@xxxxxxxxx>
Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
Acked-by: Brian Norris <computersforpeac@xxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux