Skip to contentSkip to conversation

NAND: fix broken debug messages

PATCH In review David Brownell 3 participants 5 messages Feb 3, 2009 – Feb 5, 2009

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

David Brownell

author
#0 lore raw

PATCH 1/1 NAND: fix broken debug messages

From: David Brownell <[email protected]>

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 <[email protected]>
---
 drivers/mtd/nand/nand_base.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_base.c +4 -4
@@ -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: "
 			"Attempt write beyond end of device\n");
 		return -EINVAL;
 	}

@@ -2014,8 +2014,8 @@ static int nand_write_oob(struct mtd_inf
 
 	/* Do not allow writes past end of device */
 	if (ops->datbuf && (to + ops->len) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
-		      "Attempt read beyond end of device\n");
+		DEBUG(MTD_DEBUG_LEVEL0, "nand_write_oob: "
+		      "Attempt write beyond end of device\n");
 		return -EINVAL;
 	}
#1 lore raw
Quoted 24 lines · David Brownell

From: David Brownell <[email protected]>

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 <[email protected]>

---
drivers/mtd/nand/nand_base.c | 8 ++++----

1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_base.c +1 -1
@@ -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;
 	}

drivers/mtd/nand/nand_base.c static int nand_do_write_oob(struct mtd_ · line 1961

Quoted 5 lines
drivers/mtd/nand/nand_base.c +1 -1
@@ -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

#2 lore raw
Quoted 32 lines · Josh Boyer
Quoted 23 lines · David Brownell

From: David Brownell <[email protected]> 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 <[email protected]>

---
drivers/mtd/nand/nand_base.c | 8 ++++----

1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_base.c +1 -1
@@ -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;
 	}

drivers/mtd/nand/nand_base.c static int nand_do_write_oob(struct mtd_ · line 1961

Quoted 5 lines
drivers/mtd/nand/nand_base.c +1 -1
@@ -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'.

Quoted 3 lines

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:   [email protected]
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

author
#3 lore raw
Quoted 4 lines · Geert Uytterhoeven
Quoted 4 lines
Quoted 2 lines

-            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. ;)

Quoted 3 lines
Quoted 3 lines

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

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

That would be sensible.

Quoted 5 lines

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

author
#4 lore raw

PATCH 1/1 NAND: fix broken debug messages (v2)

In reply to David Brownell

From: David Brownell <[email protected]>

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.

IMO all MTD debugging message framework is goofed, anyway. It uses "DEBUG" in a way that's incompatible with usage most everywhere else in the kernel, and which prevents normal pr_dbg() and dev_dbg() calls from working right.

Signed-off-by David Brownell <[email protected]>

---
Fix a goofed string.

 drivers/mtd/nand/nand_base.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_base.c +4 -4
@@ -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, "nand_do_write_oob: "
 			"Attempt write beyond end of device\n");
 		return -EINVAL;
 	}

@@ -2014,8 +2014,8 @@ static int nand_write_oob(struct mtd_inf
 
 	/* Do not allow writes past end of device */
 	if (ops->datbuf && (to + ops->len) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
-		      "Attempt read beyond end of device\n");
+		DEBUG(MTD_DEBUG_LEVEL0, "nand_write_oob: "
+		      "Attempt write beyond end of device\n");
 		return -EINVAL;
 	}