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

[Artem Bityutskiy <dedekind1@gmail.com>]

On Thu, 2011-06-09 at 18:37 +0400, Dmitry Eremin-Solenikov wrote:
> 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.

How about turning the "origin" argument into "void *private" and
declaring that this is "parser-specific info". It then can become
"origin" for the RedBoot parser and the OF node pointer for the ofpart
parser?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)