[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 at gmail.com> wrote:
>> From: Brian Norris <computersforpeace at 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 at gmail.com>
>> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
>> Acked-by: Brian Norris <computersforpeac at 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 at 0 { ...  };
>
>         firmware at 10000 { .... }; /* will be split by the parser */
>
>         extra at 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;
}