Re: [PATCH v2 4/4] Freescale enhanced Local Bus Controller FCM NAND support.

[Anton Vorontsov <avorontsov@ru.mvista.com>]

On Fri, Dec 14, 2007 at 01:33:41PM -0600, Scott Wood wrote:
> Anton Vorontsov wrote:
> >On Fri, Dec 14, 2007 at 12:56:24PM -0600, Scott Wood wrote:
> >>+#if 0
> >>+#define ELBC_NAND_DEBUG_LVL 6
> >>+#endif
> >>+
> >>+#ifdef ELBC_NAND_DEBUG_LVL
> >>+static int fcm_debug_level = ELBC_NAND_DEBUG_LVL;
> >>+#define FCM_DEBUG(n, args...)		\
> >>+	do {					\
> >>+		if (n <= fcm_debug_level)	\
> >>+			pr_dbg(args);	\
> >>+	} while(0)
> >>+#else
> >>+#define FCM_DEBUG(n, dev, args...) do { } while(0)
> >>+#endif
> >
> >Only 1, 2 and 5 debug levels are used. Maybe better use dev_dbg
> >and dev_vdbg instead?
> 
> Yeah, probably...
> 
> >Btw, checkpatch result:
> >total: 69 errors, 14 warnings, 1236 lines checked
> 
> Most of those are errors in checkpatch, wherein it fails to understand 
> the difference between aligning with spaces (good) and indenting with 
> spaces (bad).  Checkpatch spits out so many of those that other things 
> get lost in the noise, so I don't usually bother to run it.
> 
> I also do not understand the allergy to C99 comments in the Linux 
> community, though I'll change the few that slipped in by accident.
> 
> I'll fix the few legitimate ones.
> 
> >This isn't very friendly to the people going to look into.
> 
> You should have seen what it looked like before I touched it. :-P

I have seen that. ;-) Its previous name was fsl_fcm, and I shuddered
from it, honestly. I must admit that you've done great work on this!

> >Maybe this desires its own header?
> 
> It can be factored out if anything else ever uses it.

It's just confusing to parse lbc-specific and nand-specific code
placed in the same file.


Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2