Re: [PATCH 01/17] mtd: prepare to convert of_mtd_parse_partitions to partition parser

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

On 09.06.2011 18:26, Artem Bityutskiy wrote:
> On Thu, 2011-06-09 at 18:22 +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>
>> ---
>>   drivers/mtd/mtdcore.c          |   19 +++++++++++++++++++
>>   drivers/mtd/mtdpart.c          |    8 ++++++--
>>   drivers/mtd/ofpart.c           |   30 ++++++++++++++++++++++++++++--
>>   include/linux/mtd/mtd.h        |    5 +++++
>>   include/linux/mtd/partitions.h |    2 +-
>>   5 files changed, 59 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>> index 1326747..2d5b865 100644
>> --- a/drivers/mtd/mtdcore.c
>> +++ b/drivers/mtd/mtdcore.c
>> @@ -36,6 +36,7 @@
>>   #include<linux/idr.h>
>>   #include<linux/backing-dev.h>
>>   #include<linux/gfp.h>
>> +#include<linux/of.h>
>>
>>   #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
>
> These ifdefs are not very nice, do you have ideas how to avoid them?
> Ideally, mtdcore should not know or bother about OF things. All
> OF-specific things should be done in ofpart.c...

I know they aren't nice. OTOH ofpart.c also seems a bit non-logical: one 
can have of node in the MTD, but doesn't (strangely) want to compile in 
ofpart.c. Of course I can add separate small of-handling functions (to 
do OF handling) to mtdcore.c to be replaced by empty functions in the 
absence of CONFIG_OF, but this also look like overhead for me.

-- 
With best wishes
Dmitry