On 07 Sep 02:40 AM, pekon wrote:
> + linux-omap, as this patch touches arch/arm/mach-omap2/gpmc.c
>
> Hi Ezequiel,
>
> On Sunday 07 September 2014 01:26 AM, Ezequiel Garcia wrote:
> >The current code abuses ifdefs to determine if the selected ECC scheme
> >is supported by the running kernel. As a result the code is hard to read,
> >and it also fails to load as a module.
> >
> >This commit removes all the ifdefs and instead introduces a function
> >omap2_nand_ecc_check() to check if the ECC is supported by using
> >IS_ENABLED(CONFIG_xxx).
> >
> Yup, true.
> Thanks for cleaning up OMAP2 NAND driver, but few feedbacks.
>
First of all, this is no cleaning. The driver fails to load as a module
over here. Unless I'm missed something, ifdef seemed false when CONFIG_xxx=m.
> Just for record, current version of omap2.c NAND driver has so
> many #ifdef because:
>
[..]
I can't think of any valid excuses for cluttering the code with ifdefs like
this. There are several ways of doing things cleaner, by grouping code
differently.
[..]
> >+ }
> >+ if (ecc_needs_elm && !is_elm_present(info, pdata->elm_of_node)) {
> >+ dev_err(&info->pdev->dev, "ELM not available\n");
> >+ return false;
> >+ }
> >+
> >+ return true;
> > }
>
> Actually this new function is not required.
[snip]
> If I'm not wrong, this is all you need in this patch
> empty functions for CONFIG_MTD_NAND_ECC_BCH exposed symbols are already
> handled in /include/linux/mtd/nand_bch.h
>
> So, after this change, you can most of #ifdefs and IS_ENABLED()
> and this patch should be simplified.
>
OK, I'll take a look. Thanks for the suggestion,
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
