Hi Pekon,
On 10/12/2013 04:58 PM, Gupta, Pekon wrote:
From: Brian Norris [mailto:computersforpe...@gmail.com]
On Fri, Oct 11, 2013 at 07:06:40PM +0530, Pekon Gupta wrote:
[...]
Why do you even need the #ifdef's for the #include's? It is not harmful
to include headers for stuff that is only conditionally used. In fact,
this creates a problem later when you try to use nand_bch_free(), and
you have to surround it with extra #ifdef's.
In short: these #ifdef's just breed more #ifdef's and cause the code to
become harder to read and less maintainable.
First off, I should clarify this: the above comment was speaking *only*
about surrounding the #include's with #ifdef's. Such #ifdef's only make
everything else harder.
Now, I recognize that there are still many cases where you may need
#ifdef's in the body of the code. I was only trying to hit the easiest ones.
There are 2 problems here:
(1)
I have removed #ifdef across headers in next version. But I cannot remove
#ifdef across some callbacks for cc.hwctl(), ecc.calculate(), ecc.correct(),
because then compilation would throw warnings for un-used functions.
The __maybe_unused attribute can be given to functions that will compile
but are unused, and the compiler will just remove unused ones from the
binary without warning. That is probably (weakly) preferable to
#ifdef's, if you can manage it. But I understand some #ifdef's are
necessary.
(2)
OMAP driver uses generic lib/bch.c which is compiled only when
CONFIG_MTD_NAND_ECC_BCH is enabled. So to avoid compilation
issues, I had to put #ifdefs on all wrapper functions which use lib/bch.c
or nand_bch.c
nand_bch.c (and its corresponding nand_bch.h) has stub implementations
so that extra #ifdef's often aren't needed. But include/linux/bch.h does
not, so you are correct.
I myself have tried in previous versions to avoid #ifdefs, but I ended
up in one or the other problem like (1) | (2) above.
And this is where randconfig test failed earlier when Arnd Bergmann
commented, so some #ifdef would hv to be carried till be deprecate
some legacy ecc-schemes.
However, with patch split many redundant #ifdefs are now removed.
Great, thanks for the effort.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
