[PATCH] Random fixes for the ixp4xx mapping driver

[Alexey Zaytsev]

On 4/20/06, Jörn Engel <joern at wohnheim.fh-wedel.de> wrote:
> > @@ -151,7 +151,9 @@
> >        struct resource *res;
> >  };
> >
> > -static const char *probes[] = { "RedBoot", "cmdlinepart", NULL };
> > +#ifdef CONFIG_MTD_PARTITIONS
> > +static const char *probes[] __initdata = { "RedBoot", "cmdlinepart", NULL };
> > +#endif
>
> All this #ifdef mess doesn't make me happy.  In this case, it saves 32
> bytes for people using this driver but not having
> CONFIG_MTD_PARTITIONS enabled.  That's not a very small gain for a
> very small group.  Just make it unconditional.

The gain is not big, but the price is small too. The thing I'm not
sure about is __initdata.
Now I see that it may be needed after initialization too, not sure.

>
> >   static int ixp4xx_flash_remove(struct platform_device *dev)
> >  {
> > @@ -164,7 +166,9 @@
> >                return 0;
> >
> >        if (info->mtd) {
> > +#ifdef CONFIG_MTD_PARTITIONS
> >                del_mtd_partitions(info->mtd);
> > +#endif
>
> In this case, del_mtd_partitions() should be defined to a noop if
> CONFIG_MTD_PARTITIONS is not defined.
>
> > +#ifdef CONFIG_MTD_PARTITIONS
> >        err = parse_mtd_partitions(info->mtd, probes, &info->partitions, 0);
> >        if (err > 0) {
> >                err = add_mtd_partitions(info->mtd, info->partitions, err);
> > -               if(err)
> > -                       printk(KERN_ERR "Could not parse partitions\n");
> > +
> >        }
> > +       if (err < 0)
> > +               printk(KERN_ERR "Could not parse partitions\n");
> > +#endif
>
> Same here.  parse_mtd_partitions() should be a noop, the rest can just
> stay.  Gcc should be smart enough to optimize it away.
>
> The rest looks fine.

Why do you think nooping functions is better than having #ifdefs? I
agree that lots of
#ifdefs make code hard to follow, but if you noop a function, defined
in some other place,
someone reading the code may not notice it, and refer to the un-nooped function.

> Jörn
>
> --
> Everything should be made as simple as possible, but not simpler.
> -- Albert Einstein
>