[2.6.29-rc3] NAND: =?iso-8859-1?q?=A0fix_broken_debug?= messages

From: David Brownell <dbrownell@users.sourceforge.net>

Fix incorrect debug messages (*write* not read); someone
committed some cut'n'paste bugs.  There might be more,
I only noticed these since I was looking for nand_read
usage and landed in some very wrong functions.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/mtd/nand/nand_base.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

On Mon, Feb 02, 2009 at 06:13:20PM -0800, David Brownell wrote:
>From: David Brownell <dbrownell@users.sourceforge.net>
>
>Fix incorrect debug messages (*write* not read); someone
>committed some cut'n'paste bugs.  There might be more,
>I only noticed these since I was looking for nand_read
>usage and landed in some very wrong functions.
>
>Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
>---
> drivers/mtd/nand/nand_base.c |    8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>--- a/drivers/mtd/nand/nand_base.c
>+++ b/drivers/mtd/nand/nand_base.c
>@@ -1948,7 +1948,7 @@ static int nand_do_write_oob(struct mtd_
> 	}
> 
> 	if (unlikely(ops->ooboffs >= len)) {
>-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
>+		DEBUG(MTD_DEBUG_LEVEL0, "nand_do_write_oob: "
> 			"Attempt to start write outside oob\n");
> 		return -EINVAL;
> 	}
>@@ -1958,7 +1958,7 @@ static int nand_do_write_oob(struct mtd_
> 		     ops->ooboffs + ops->ooblen >
> 			((mtd->size >> chip->page_shift) -
> 			 (to >> chip->page_shift)) * len)) {
>-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
>+		DEBUG(MTD_DEBUG_LEVEL0, "do_nand_write_oob: "

This is wrong too.  The function is called 'nand_do_write_oob'.

Which leads me to wonder why we aren't doing something like:

	DEBUG(MTD_DEBUG_LEVEL0, "%s: <message>\n", __func__);

josh

On Tue, 3 Feb 2009, Josh Boyer wrote:
> On Mon, Feb 02, 2009 at 06:13:20PM -0800, David Brownell wrote:
> >From: David Brownell <dbrownell@users.sourceforge.net>
> >Fix incorrect debug messages (*write* not read); someone
> >committed some cut'n'paste bugs.  There might be more,
> >I only noticed these since I was looking for nand_read
> >usage and landed in some very wrong functions.
> >
> >Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> >---
> > drivers/mtd/nand/nand_base.c |    8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >--- a/drivers/mtd/nand/nand_base.c
> >+++ b/drivers/mtd/nand/nand_base.c
> >@@ -1948,7 +1948,7 @@ static int nand_do_write_oob(struct mtd_
> > 	}
> > 
> > 	if (unlikely(ops->ooboffs >= len)) {
> >-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
> >+		DEBUG(MTD_DEBUG_LEVEL0, "nand_do_write_oob: "
> > 			"Attempt to start write outside oob\n");
> > 		return -EINVAL;
> > 	}
> >@@ -1958,7 +1958,7 @@ static int nand_do_write_oob(struct mtd_
> > 		     ops->ooboffs + ops->ooblen >
> > 			((mtd->size >> chip->page_shift) -
> > 			 (to >> chip->page_shift)) * len)) {
> >-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
> >+		DEBUG(MTD_DEBUG_LEVEL0, "do_nand_write_oob: "
> 
> This is wrong too.  The function is called 'nand_do_write_oob'.
> 
> Which leads me to wonder why we aren't doing something like:
> 
> 	DEBUG(MTD_DEBUG_LEVEL0, "%s: <message>\n", __func__);

Or use

    #define pr_fmt(fmt)  "%s: " fmt, __func__

at the top of the file, and start using the official pr_*() routines?

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

On Tuesday 03 February 2009, Geert Uytterhoeven wrote:
> > >-            DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
> > >+            DEBUG(MTD_DEBUG_LEVEL0, "do_nand_write_oob: "
> > 
> > This is wrong too.  The function is called 'nand_do_write_oob'.

That's what happens sometimes when you edit patches by hand.
Originally I just changed "read" to "write", then I noticed
there was "do" in there ... obviously, it got inserted in
the wrong place.  ;)


> > Which leads me to wonder why we aren't doing something like:
> > 
> >       DEBUG(MTD_DEBUG_LEVEL0, "%s: <message>\n", __func__);

That would be sensible.

 
> Or use
> 
>     #define pr_fmt(fmt)  "%s: " fmt, __func__
> 
> at the top of the file, and start using the official pr_*() routines?

Yeah, DEBUG should be treated globally as a reserved
symbol.  Ever notice how dev_dbg() misbhaves in MTD
code?  Ditto pr_dbg(), but that's rarely the right thing
to use when there's a device node in hand.  Yeech.

"Now" (i.e. after rc1 and well before rc-last) would
be a good time to send a big patch changing all the
debug messaging to something less aggressively counter
to the core debug messaging convention of Linux...

- Dave