linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: nand: make more readable panic_nand_wait_ready() and nand_wait_ready()
@ 2014-01-07 23:19 Eunbong Song
  2014-01-20 19:19 ` Brian Norris
  0 siblings, 1 reply; 2+ messages in thread
From: Eunbong Song @ 2014-01-07 23:19 UTC (permalink / raw)
  To: computersforpeace, dwmw2; +Cc: linux-mtd, linux-kernel


panic_nand_wait_ready() and nand_wait_ready() calls dev_ready() without checking if it exists.
This patch add check routine dev_ready() before run dev_ready()
and this makes the code more readable

Signed-off-by: Eunbong Song <eunb.song@samsung.com>
---
 drivers/mtd/nand/nand_base.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index bd39f7b..110db78 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -473,8 +473,10 @@ static void panic_nand_wait_ready(struct mtd_info *mtd, unsigned long timeo)
 
 	/* Wait for the device to get ready */
 	for (i = 0; i < timeo; i++) {
-		if (chip->dev_ready(mtd))
-			break;
+		if(chip->dev_ready){
+			if (chip->dev_ready(mtd))
+				break;
+		}
 		touch_softlockup_watchdog();
 		mdelay(1);
 	}
@@ -493,8 +495,10 @@ void nand_wait_ready(struct mtd_info *mtd)
 	led_trigger_event(nand_led_trigger, LED_FULL);
 	/* Wait until command is processed or timeout occurs */
 	do {
-		if (chip->dev_ready(mtd))
-			break;
+		if(chip->dev_ready){
+			if (chip->dev_ready(mtd))
+				break;
+		}
 		touch_softlockup_watchdog();
 	} while (time_before(jiffies, timeo));
 	led_trigger_event(nand_led_trigger, LED_OFF);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mtd: nand: make more readable panic_nand_wait_ready() and nand_wait_ready()
  2014-01-07 23:19 [PATCH] mtd: nand: make more readable panic_nand_wait_ready() and nand_wait_ready() Eunbong Song
@ 2014-01-20 19:19 ` Brian Norris
  0 siblings, 0 replies; 2+ messages in thread
From: Brian Norris @ 2014-01-20 19:19 UTC (permalink / raw)
  To: Eunbong Song; +Cc: linux-mtd, dwmw2, linux-kernel

On Tue, Jan 07, 2014 at 11:19:56PM +0000, Eunbong Song wrote:
> 
> panic_nand_wait_ready() and nand_wait_ready() calls dev_ready() without checking if it exists.

There are many occasions where we call a function pointer without
(locally) checking that it is non-NULL. That is by design.

If you see a problem with this behavior, please fix this properly rather
than covering it up. This patch will just make faulty drivers spin for
20ms rather than hitting a bug. Instead, you should aim to fix buggy
drivers that call dev_ready() without assigning it; or even better,
defensively improve nand_base to detect those drivers which are
utilizing nand_command_lp() or nand_command() without assigning
dev_ready().

> This patch add check routine dev_ready() before run dev_ready()
> and this makes the code more readable

This patch does not make the code more readable, nor does it improve the
state of the original code much. Please solve and document a real
problem.

Thanks,
Brian

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-01-20 19:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-07 23:19 [PATCH] mtd: nand: make more readable panic_nand_wait_ready() and nand_wait_ready() Eunbong Song
2014-01-20 19:19 ` Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).