diff mbox series

[v2,1/5] mtd: spi-nor: don't drop sfdp data if optional parsers fail

Message ID 20181109165644.30534-2-tudor.ambarus@microchip.com
State Accepted
Delegated to: Boris Brezillon
Headers show
Series mtd: spi-nor: fixes found when debugging smpt | expand

Commit Message

Tudor Ambarus Nov. 9, 2018, 4:56 p.m. UTC
JESD216C states that just the Basic Flash Parameter Table is mandatory.
Already defined (or future) additional parameter headers and tables are
optional.

Don't drop already collected sfdp data in case an optional table
parser fails. In case of failing, each optional parser is responsible
to roll back to the previously known spi_nor data.

Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
Reported-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: update Fixes tag to point to correct commit

 drivers/mtd/spi-nor/spi-nor.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Yogesh Narayan Gaur Nov. 13, 2018, 6:29 a.m. UTC | #1
> -----Original Message-----
> From: Tudor.Ambarus@microchip.com [mailto:Tudor.Ambarus@microchip.com]
> Sent: Friday, November 9, 2018 10:27 PM
> To: boris.brezillon@bootlin.com; marek.vasut@gmail.com;
> dwmw2@infradead.org; computersforpeace@gmail.com; richard@nod.at
> Cc: linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Yogesh
> Narayan Gaur <yogeshnarayan.gaur@nxp.com>; cyrille.pitchen@wedev4u.fr;
> Tudor.Ambarus@microchip.com
> Subject: [PATCH v2 1/5] mtd: spi-nor: don't drop sfdp data if optional parsers fail
> 
> JESD216C states that just the Basic Flash Parameter Table is mandatory.
> Already defined (or future) additional parameter headers and tables are optional.
> 
> Don't drop already collected sfdp data in case an optional table parser fails. In
> case of failing, each optional parser is responsible to roll back to the previously
> known spi_nor data.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
Tested-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>

> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2: update Fixes tag to point to correct commit
> 
>  drivers/mtd/spi-nor/spi-nor.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
> 4a96ee719e5a..2cdf96013689 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -3130,7 +3130,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  	if (err)
>  		goto exit;
> 
> -	/* Parse other parameter headers. */
> +	/* Parse optional parameter tables. */
>  	for (i = 0; i < header.nph; i++) {
>  		param_header = &param_headers[i];
> 
> @@ -3143,8 +3143,17 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  			break;
>  		}
> 
> -		if (err)
> -			goto exit;
> +		if (err) {
> +			dev_warn(dev, "Failed to parse optional parameter
> table: %04x\n",
> +				 SFDP_PARAM_HEADER_ID(param_header));
> +			/*
> +			 * Let's not drop all information we extracted so far
> +			 * if optional table parsers fail. In case of failing,
> +			 * each optional parser is responsible to roll back to
> +			 * the previously known spi_nor data.
> +			 */
> +			err = 0;
> +		}
>  	}
> 
>  exit:
> --
> 2.9.4
Boris Brezillon Nov. 15, 2018, 1:25 p.m. UTC | #2
On Fri, 2018-11-09 at 16:56:48 UTC,  wrote:
> JESD216C states that just the Basic Flash Parameter Table is mandatory.
> Already defined (or future) additional parameter headers and tables are
> optional.
> 
> Don't drop already collected sfdp data in case an optional table
> parser fails. In case of failing, each optional parser is responsible
> to roll back to the previously known spi_nor data.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks.

Boris
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4a96ee719e5a..2cdf96013689 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -3130,7 +3130,7 @@  static int spi_nor_parse_sfdp(struct spi_nor *nor,
 	if (err)
 		goto exit;
 
-	/* Parse other parameter headers. */
+	/* Parse optional parameter tables. */
 	for (i = 0; i < header.nph; i++) {
 		param_header = &param_headers[i];
 
@@ -3143,8 +3143,17 @@  static int spi_nor_parse_sfdp(struct spi_nor *nor,
 			break;
 		}
 
-		if (err)
-			goto exit;
+		if (err) {
+			dev_warn(dev, "Failed to parse optional parameter table: %04x\n",
+				 SFDP_PARAM_HEADER_ID(param_header));
+			/*
+			 * Let's not drop all information we extracted so far
+			 * if optional table parsers fail. In case of failing,
+			 * each optional parser is responsible to roll back to
+			 * the previously known spi_nor data.
+			 */
+			err = 0;
+		}
 	}
 
 exit: