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

[Yogesh Narayan Gaur]

> -----Original Message-----
> From: Tudor.Ambarus@xxxxxxxxxxxxx [mailto:Tudor.Ambarus@xxxxxxxxxxxxx]
> Sent: Friday, November 9, 2018 10:27 PM
> To: boris.brezillon@xxxxxxxxxxx; marek.vasut@xxxxxxxxx;
> dwmw2@xxxxxxxxxxxxx; computersforpeace@xxxxxxxxx; richard@xxxxxx
> Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Yogesh
> Narayan Gaur <yogeshnarayan.gaur@xxxxxxx>; cyrille.pitchen@xxxxxxxxxx;
> Tudor.Ambarus@xxxxxxxxxxxxx
> 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@xxxxxxx>
Tested-by: Yogesh Gaur <yogeshnarayan.gaur@xxxxxxx>

> Suggested-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
> ---
> 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