[U-Boot] [PATCH V2] mtd: uboot: Fix hanging during mtd list command

[U-Boot] [PATCH V2] mtd: uboot: Fix hanging during mtd list command

[Adam Ford]

Some boards using simple NAND drivers (like omap3_logic) hang
when executing the new 'mtd list' command.  This patch enhances
the checks for conditions that would preclude mtd_probe_devices()
from operating.

Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")

Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  Move the check for NULL mtdparts or mtdids until after the
     partitions have been removed.

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 6a211d52ff..7d7a11c990 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -104,7 +104,10 @@ int mtd_probe_devices(void)
 	mtd_probe_uclass_mtd_devs();
 
 	/* Check if mtdparts/mtdids changed since last call, otherwise: exit */
-	if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
+	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
+	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
+	     !strcmp(mtdparts, old_mtdparts) &&
+	     !strcmp(mtdids, old_mtdids)))
 		return 0;
 
 	/* Update the local copy of mtdparts */
@@ -140,6 +143,10 @@ int mtd_probe_devices(void)
 		}
 	}
 
+	/* If either mtdparts or mtdids is empty, then exit */
+	if (!mtdparts || !mtdids)
+		return 0;
+
 	/* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
 	if (strstr(mtdparts, "mtdparts="))
 		mtdparts += 9;
-- 
2.17.1

[U-Boot] [PATCH V2] mtd: uboot: Fix hanging during mtd list command

[Boris Brezillon]

On Mon,  8 Oct 2018 13:39:31 -0500
Adam Ford <aford173@gmail.com> wrote:

> Some boards using simple NAND drivers (like omap3_logic) hang
> when executing the new 'mtd list' command.  This patch enhances

					    ^One too many spaces here.

Also, i don't think it has to do with simple/old NAND drivers vs new
ones, it's more related to how the platform behaves when you access
address 0 (NULL). I guess on Miquel's CPU address 0 is accessible, on
yours it's not.

> the checks for conditions that would preclude mtd_probe_devices()
> from operating.
> 
> Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
> 

You can drop the blank line here.

> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Once my comments on the commit message are addressed, you can add

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
> V2:  Move the check for NULL mtdparts or mtdids until after the
>      partitions have been removed.
> 
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index 6a211d52ff..7d7a11c990 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -104,7 +104,10 @@ int mtd_probe_devices(void)
>  	mtd_probe_uclass_mtd_devs();
>  
>  	/* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> -	if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> +	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
> +	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
> +	     !strcmp(mtdparts, old_mtdparts) &&
> +	     !strcmp(mtdids, old_mtdids)))
>  		return 0;
>  
>  	/* Update the local copy of mtdparts */
> @@ -140,6 +143,10 @@ int mtd_probe_devices(void)
>  		}
>  	}
>  
> +	/* If either mtdparts or mtdids is empty, then exit */
> +	if (!mtdparts || !mtdids)
> +		return 0;
> +
>  	/* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
>  	if (strstr(mtdparts, "mtdparts="))
>  		mtdparts += 9;