[PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Stefan Roese]

The patch 9a310d21196f38f6ad0ad146057548653e495c09 ("[MTD] Factor out OF
partition support from the NOR driver.") introduced an problem. Now the
physmap_of driver returns with error upon the first unsuccessful
partition scan (parse_mtd_partitions()). This is not wanted, since even
when the RedBoot/cmdlinepart partition scan is unsuccessful, the other
scan's (of_mtd_parse_partitions(), parse_obsolete_partitions()) should be
done nevertheless.

This patch fixes this problem.

Signed-off-by: Stefan Roese <sr@denx.de>
---

Scott, could you please take a look at this patch and add you ACK if you
think it is ok?

Thanks.

 drivers/mtd/maps/physmap_of.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 49acd41..9808c0d 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -224,24 +224,16 @@ static int __devinit of_flash_probe(struct of_device *dev,
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
 	err = parse_mtd_partitions(info->mtd, part_probe_types,
-	                           &info->parts, 0);
-	if (err < 0)
-		return err;
+				   &info->parts, 0);
 
 #ifdef CONFIG_MTD_OF_PARTS
-	if (err == 0) {
+	if (err < 0)
 		err = of_mtd_parse_partitions(&dev->dev, info->mtd,
-		                              dp, &info->parts);
-		if (err < 0)
-			return err;
-	}
+					      dp, &info->parts);
 #endif
 
-	if (err == 0) {
+	if (err < 0)
 		err = parse_obsolete_partitions(dev, info, dp);
-		if (err < 0)
-			return err;
-	}
 
 	if (err > 0)
 		add_mtd_partitions(info->mtd, info->parts, err);
-- 
1.5.4.1

Re: [PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Scott Wood]

Stefan Roese wrote:
> The patch 9a310d21196f38f6ad0ad146057548653e495c09 ("[MTD] Factor out OF
> partition support from the NOR driver.") introduced an problem. Now the
> physmap_of driver returns with error upon the first unsuccessful
> partition scan (parse_mtd_partitions()). This is not wanted, since even
> when the RedBoot/cmdlinepart partition scan is unsuccessful, the other
> scan's (of_mtd_parse_partitions(), parse_obsolete_partitions()) should be
> done nevertheless.
> 
> This patch fixes this problem.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
> 
> Scott, could you please take a look at this patch and add you ACK if you
> think it is ok?

I was under the impression -- and looking at the code seems to back that 
up -- that parse_mtd_partitions() returns zero if the scan was 
unsuccessful due to the lack of a partition table, and negative only if 
there's a real error.

Under what conditions are you actually seeing this fail?

-Scott

Re: [PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Stefan Roese]

On Tuesday 12 February 2008, Scott Wood wrote:
> Stefan Roese wrote:
> > The patch 9a310d21196f38f6ad0ad146057548653e495c09 ("[MTD] Factor out OF
> > partition support from the NOR driver.") introduced an problem. Now the
> > physmap_of driver returns with error upon the first unsuccessful
> > partition scan (parse_mtd_partitions()). This is not wanted, since even
> > when the RedBoot/cmdlinepart partition scan is unsuccessful, the other
> > scan's (of_mtd_parse_partitions(), parse_obsolete_partitions()) should be
> > done nevertheless.
> >
> > This patch fixes this problem.
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > ---
> >
> > Scott, could you please take a look at this patch and add you ACK if you
> > think it is ok?
>
> I was under the impression -- and looking at the code seems to back that
> up -- that parse_mtd_partitions() returns zero if the scan was
> unsuccessful due to the lack of a partition table, and negative only if
> there's a real error.
>
> Under what conditions are you actually seeing this fail?

When CONFIG_MTD_REDBOOT_PARTS is not defined for example it returns with -22 
(EINVAL).

Best regards,
Stefan

Re: [PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Scott Wood]

Stefan Roese wrote:
> On Tuesday 12 February 2008, Scott Wood wrote:
>> Under what conditions are you actually seeing this fail?
> 
> When CONFIG_MTD_REDBOOT_PARTS is not defined for example it returns with -22 
> (EINVAL).

Ah, I see -- it seems the cmdline partition code behaves differently 
than the redboot code.

Your patch changes it to treat zero as success, however -- which breaks 
some other cases.  The test should be "err <= 0", which is what 
parse_mtd_partiitions() itself uses in its loop.

-Scott

Re: [PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Stefan Roese]

On Tuesday 12 February 2008, Scott Wood wrote:
> >> Under what conditions are you actually seeing this fail?
> >
> > When CONFIG_MTD_REDBOOT_PARTS is not defined for example it returns with
> > -22 (EINVAL).
>
> Ah, I see -- it seems the cmdline partition code behaves differently
> than the redboot code.
>
> Your patch changes it to treat zero as success, however -- which breaks
> some other cases.  The test should be "err <= 0", which is what
> parse_mtd_partiitions() itself uses in its loop.

OK, I'll fixup another version of this patch tomorrow.

Thanks.

Best regards,
Stefan

Re: [PATCH] mtd: Fix physmap_of to not exit upon unsuccessful partition scan

[Peter Korsgaard]

>>>>> "Stefan" == Stefan Roese <sr@denx.de> writes:

 Stefan> On Tuesday 12 February 2008, Scott Wood wrote:
 >> >> Under what conditions are you actually seeing this fail?
 >> >
 >> > When CONFIG_MTD_REDBOOT_PARTS is not defined for example it returns with
 >> > -22 (EINVAL).
 >> 
 >> Ah, I see -- it seems the cmdline partition code behaves differently
 >> than the redboot code.
 >> 
 >> Your patch changes it to treat zero as success, however -- which breaks
 >> some other cases.  The test should be "err <= 0", which is what
 >> parse_mtd_partiitions() itself uses in its loop.

 Stefan> OK, I'll fixup another version of this patch tomorrow.

I would prefer to fix up cmdlinepart.c instead, as missing cmdline
data isn't really an error.

>From 7cc1d55f9704a9df1053aefd21fd5db98ac4c983 Mon Sep 17 00:00:00 2001
From: Peter Korsgaard <jacmet@sunsite.dk>
Date: Thu, 14 Feb 2008 13:46:13 +0100
Subject: [PATCH] cmdlinepart: Missing partition info is not an error

Return 0 partitions instead of -EINVAL on no mtdpart= argument on kernel
cmdline or missing partition info for device.
---
 drivers/mtd/cmdlinepart.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index b44292a..08b82c9 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -308,9 +308,6 @@ static int parse_cmdline_partitions(struct mtd_info *master,
 	struct cmdline_mtd_partition *part;
 	char *mtd_id = master->name;
 
-	if(!cmdline)
-		return -EINVAL;
-
 	/* parse command line */
 	if (!cmdline_parsed)
 		mtdpart_setup_real(cmdline);
@@ -341,7 +338,7 @@ static int parse_cmdline_partitions(struct mtd_info *master,
 			return part->num_parts;
 		}
 	}
-	return -EINVAL;
+	return 0;
 }
 
 
-- 
debian.1.5.3.7.1-dirty


-- 
Bye, Peter Korsgaard