On Wed, Oct 07, 2009 at 23:05:48, Mark A. Greer wrote:
> On Mon, Oct 05, 2009 at 11:12:17AM +0530, Nori, Sekhar wrote:
> > On Fri, Oct 02, 2009 at 02:24:17, Mark A. Greer wrote:
>
> > > I don't know Sekhar...  This looks like a step backwards to me with all
> > > the #ifdef-ery.  Why not keep the code as it is and change the #ifdefs
> > > that are already there (e.g., change the CONFIG_UI ones to CONFIG_FB,
> > > etc)?
> >
> > Mark,
> >
> > The only new #ifdef the patch introduces is for MMC/SD. That is
> > to make sure plugging in UI card does not get MMC to stop working.
>
> You have taken #ifdef's that included/excluded code and replaced them with
> #ifdef's that are a part of the runtime logic.  That's typically frowned
> upon.  That's my issue.
>
> > I think the patch actually improves readability by removing the
> > existing #ifdefs present inside function body.
>
> I disagree plus the existing ifdef's do what #ifdef's are supposed to do,
> include or exclude chunks of code.

Existing ifdefs also cause the warning:

arch/arm/mach-davinci/board-da830-evm.c:371: warning: 'da830_evm_devices' 
defined but not used

with the default config. Removing ifdefs also eliminates this warning.

Thanks,
Sekhar

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to