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

Dmitry Eremin-Solenikov
2011-06-06T08:15:08Z

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