Re: [PATCH 041/104] mtd: prepare to convert of_mtd_parse_partitions to partition parser

[Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>]

On 06.06.2011 11:57, Artem Bityutskiy wrote:
> On Thu, 2011-06-02 at 18:51 +0400, Dmitry Eremin-Solenikov wrote:
>> Prepare to convert of_mtd_parse_partitions() to usual partitions parser:
>> 1) Register ofpart parser
>> 2) Internally don't use passed device for error printing
>> 3) Add device_node to mtd_info struct
>> 4) Move of_mtd_parse_partitions from __devinit to common text section
>> 5) add ofpart to the default list of partition parsers
>>
>> Signed-off-by: Dmitry Eremin-Solenikov<dbaryshkov@gmail.com>
>
> Good idea, but
>
>>   #include<linux/mtd/mtd.h>
>>   #include<linux/mtd/partitions.h>
>> @@ -446,6 +447,10 @@ int mtd_device_register(struct mtd_info *master,
>>   			const struct mtd_partition *parts,
>>   			int nr_parts)
>>   {
>> +#ifdef CONFIG_OF
>> +	if (master->node)
>> +		of_node_get(master->node);
>> +#endif
>
> Could all the OF-specific things be done in the ofpart.c ?

Answering both of your questions here:
My idea is to have an of node connected to mtd device, if there is any.
It can be used by ofpart.c, ofoldpart.c (extract from physmap_of),
maybe some other parsers/drivers/etc.

The field is populated by the creator of mtd_info. Then the function
dedicated to registration of the mtd should get the node and other 
functions can just use it. Maybe this should be moved to add_mtd_device,
but this shouldn't be handled in ofpart.c

Another way can be to populate ->node pointer before parsing,
then drop it at the end of parsing/registration. Thus we won't have to
handle getting/putting the of node, but it won't be available during
mtd lifecycle.

>> --- a/include/linux/mtd/mtd.h
>> +++ b/include/linux/mtd/mtd.h
>> @@ -171,6 +171,9 @@ struct mtd_info {
>>   	// Kernel-only stuff starts here.
>>   	const char *name;
>>   	int index;
>> +#ifdef CONFIG_OF
>> +	struct device_node *node;
>> +#endif
>
> And designe-wise this does not look like a good idea to have such fields
> in mtd_info ... Who initializes this "node" field?
>


-- 
With best wishes
Dmitry