[PATCH] MTD: fix partition scan control logic in physmap_of and fsl_elbc_nand

[PATCH] MTD: fix partition scan control logic in physmap_of and fsl_elbc_nand

[Li Yang]

The generic rule for partition scan is to try all supported
partition types before an usable partition table is found.
However the original code returns unsuccessful when there is
an error in any of the partition types.

Signed-off-by: Li Yang <leoli@freescale.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Peter Korsgaard <jacmet@sunsite.dk>
---
I later found that Stefan has proposed a similar patch for physmap_of
and Peter proposed another patch to fix cmdlinepart instead.
I'd say that even after cmdlinepart patch is applied the scan control
logic still needs to be fixed.

 drivers/mtd/maps/physmap_of.c    |   12 ++----------
 drivers/mtd/nand/fsl_elbc_nand.c |    7 +------
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 49acd41..5ebbbc4 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -225,23 +225,15 @@ static int __devinit of_flash_probe(struct of_device *dev,
 	 * 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;
 
 #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;
-	}
 #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);
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index b025dfe..72e9bd9 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -1054,16 +1054,11 @@ static int fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
 	ret = parse_mtd_partitions(&priv->mtd, part_probe_types, &parts, 0);
-	if (ret < 0)
-		goto err;
 
 #ifdef CONFIG_MTD_OF_PARTS
-	if (ret == 0) {
+	if (ret <= 0)
 		ret = of_mtd_parse_partitions(priv->dev, &priv->mtd,
 		                              node, &parts);
-		if (ret < 0)
-			goto err;
-	}
 #endif
 
 	if (ret > 0)
-- 
1.5.4.rc4

Re: [PATCH] MTD: fix partition scan control logic in physmap_of and fsl_elbc_nand

[David Woodhouse]

On Thu, 2008-02-28 at 20:17 +0800, Li Yang wrote:
> The generic rule for partition scan is to try all supported
> partition types before an usable partition table is found.
> However the original code returns unsuccessful when there is
> an error in any of the partition types.
> 
> Signed-off-by: Li Yang <leoli@freescale.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> ---
> I later found that Stefan has proposed a similar patch for physmap_of
> and Peter proposed another patch to fix cmdlinepart instead.
> I'd say that even after cmdlinepart patch is applied the scan control
> logic still needs to be fixed.

I'm not convinced. I think the partition code should return an error if
it hits a hard error, like a failure to read from the device. And return
zero when it doesn't find anything.

-- 
dwmw2

RE: [PATCH] MTD: fix partition scan control logic in physmap_ofand fsl_elbc_nand

[Li Yang]

> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org]=20
> Sent: Wednesday, April 23, 2008 2:49 AM
> To: Li Yang
> Cc: linux-mtd@lists.infradead.org; Stefan Roese;=20
> linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] MTD: fix partition scan control logic in=20
> physmap_ofand fsl_elbc_nand
>=20
> On Thu, 2008-02-28 at 20:17 +0800, Li Yang wrote:
> > The generic rule for partition scan is to try all supported=20
> partition=20
> > types before an usable partition table is found.
> > However the original code returns unsuccessful when there=20
> is an error=20
> > in any of the partition types.
> >=20
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Cc: Stefan Roese <sr@denx.de>
> > Cc: Peter Korsgaard <jacmet@sunsite.dk>
> > ---
> > I later found that Stefan has proposed a similar patch for=20
> physmap_of=20
> > and Peter proposed another patch to fix cmdlinepart instead.
> > I'd say that even after cmdlinepart patch is applied the=20
> scan control=20
> > logic still needs to be fixed.
>=20
> I'm not convinced. I think the partition code should return=20
> an error if it hits a hard error, like a failure to read from=20
> the device. And return zero when it doesn't find anything.

The error can also be soft error like a typo in the cmdline or device
tree.  I could be better to try other ways to see if we can find a sane
partition table than just to fail.

- Leo

Re: [PATCH] MTD: fix partition scan control logic in physmap_ofand fsl_elbc_nand

[Scott Wood]

On Fri, Apr 25, 2008 at 07:09:59PM +0800, Li Yang wrote:
> The error can also be soft error like a typo in the cmdline or device
> tree.  I could be better to try other ways to see if we can find a sane
> partition table than just to fail.

If cmdline partition information exists, typo or not, then obviously
there was intent for the cmdline to be used.  It's better to let the user
know there's a problem than silently fall back to some other, likely
wrong partition information.

-Scott