Skip to contentSkip to conversation

Fix oops on reboot in physmap

PATCH In review Arnaud Patard (Rtp) 2 participants 4 messages Mar 13, 2008 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix oops on reboot in physmap

Hi,

I sent this patch to David Woodhouse, but I forgot to put the list in
CC: (and I'm not even sure that I used the right email address :( ). So, I'm sending the patch here so everyone can have my fix.

Thanks,
Arnaud

Commit df66e7167ac756baf14d2b8ea7a2cfa056600a93 is adding support for multiple resources in physmap. On shutdown/supend/resume, it's suspending all resources by calling mtd[]->suspend() without checking the mtd[] is not null. This makes oopsing my kernel.

---
arch/arm/mach-iop32x/em7210.c | 10 6 + 4 - 0 !
drivers/mtd/maps/physmap.c | 8 5 + 3 - 0 !

 2 files changed, 11 insertions(+), 7 deletions(-)
drivers/mtd/maps/physmap.c +5 -3
Index: linux-2.6/drivers/mtd/maps/physmap.c
===================================================================
@@ -204,7 +204,8 @@ static int physmap_flash_suspend(struct 
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			ret |= info->mtd[i]->suspend(info->mtd[i]);
+			if (info && info->mtd[i])
+				ret |= info->mtd[i]->suspend(info->mtd[i]);
 
 	return ret;
 }
@@ -216,7 +217,8 @@ static int physmap_flash_resume(struct p
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			info->mtd[i]->resume(info->mtd[i]);
+			if (info && info->mtd[i])
+				info->mtd[i]->resume(info->mtd[i]);
 	return 0;
 }
 
@@ -226,7 +228,7 @@ static void physmap_flash_shutdown(struc
 	int i;
 
 	for (i = 0; i < MAX_RESOURCES; i++)
-		if (info && info->mtd[i]->suspend(info->mtd[i]) == 0)
+		if (info && info->mtd[i] &&  info->mtd[i]->suspend(info->mtd[i]) == 0)
 			info->mtd[i]->resume(info->mtd[i]);
 }
 #else
#1 lore raw
Quoted 4 lines · Arnaud Patard

I sent this patch to David Woodhouse, but I forgot to put the list in
CC: (and I'm not even sure that I used the right email address :( ). So, I'm sending the patch here so everyone can have my fix.

There were various small errors in it, afaics. Does this patch achieve the same result for you?

Added dwmw2 on Cc:. This could be material for -stable.

Jörn

Signature · 10 lines
Prosperity makes friends, adversity tries them.
-- Publilius Syrus

Commit df66e7167ac756baf14d2b8ea7a2cfa056600a93 is adding support for multiple
resources in physmap. On shutdown/supend/resume, it's suspending all resources
by calling mtd[]->suspend() without checking the mtd[] is not null.
This makes oopsing my kernel.

Signed-off-by: Arnaud Patard <[email protected]>
Signed-Off-By: Joern Engel <[email protected]>
drivers/mtd/maps/physmap.c +8 -5
index f00e04e..1abecd5 100644
@@ -204,7 +204,8 @@ static int physmap_flash_suspend(struct platform_device *dev, pm_message_t state
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			ret |= info->mtd[i]->suspend(info->mtd[i]);
+			if (info->mtd[i])
+				ret |= info->mtd[i]->suspend(info->mtd[i]);
 
 	return ret;
 }
@@ -216,7 +217,8 @@ static int physmap_flash_resume(struct platform_device *dev)
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			info->mtd[i]->resume(info->mtd[i]);
+			if (info->mtd[i])
+				info->mtd[i]->resume(info->mtd[i]);
 	return 0;
 }
 
@@ -225,9 +227,10 @@ static void physmap_flash_shutdown(struct platform_device *dev)
 	struct physmap_flash_info *info = platform_get_drvdata(dev);
 	int i;
 
-	for (i = 0; i < MAX_RESOURCES; i++)
-		if (info && info->mtd[i]->suspend(info->mtd[i]) == 0)
-			info->mtd[i]->resume(info->mtd[i]);
+	if (info)
+		for (i = 0; i < MAX_RESOURCES; i++)
+			if (info->mtd[i] && info->mtd[i]->suspend(info->mtd[i]) == 0)
+				info->mtd[i]->resume(info->mtd[i]);
 }
 #else
 #define physmap_flash_suspend NULL
#2 lore raw

Jörn Engel <[email protected]> writes:

Hi,

Quoted 6 lines · Arnaud Patard
Quoted 4 lines

I sent this patch to David Woodhouse, but I forgot to put the list in
CC: (and I'm not even sure that I used the right email address :( ). So, I'm sending the patch here so everyone can have my fix.

Quoted 7 lines

There were various small errors in it, afaics. Does this patch achieve the same result for you?

From what I can see, this patch is fine. I can't test it now but will do asap.

Added dwmw2 on Cc:. This could be material for -stable.

-stable ? why ? iirc, the faulty commit has been merged for 25-rc..something so 2.6.24 is not affected. Are you talking about 2.6.25?

Arnaud

#3 lore raw
Quoted 5 lines · Arnaud Patard
Quoted 2 lines

Added dwmw2 on Cc:. This could be material for -stable.

-stable ? why ? iirc, the faulty commit has been merged for 25-rc..something so 2.6.24 is not affected. Are you talking about 2.6.25?

No, I simply didn't check when the commit happened. But yes, this is .25 material.

Jörn

Signature · 2 lines
He that composes himself is wiser than he that composes a book.
-- B. Franklin