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

[Yogesh Narayan Gaur]

> -----Original Message-----
> From: tudor.amba...@microchip.com [mailto:tudor.amba...@microchip.com]
> Sent: Friday, November 9, 2018 10:27 PM
> To: boris.brezil...@bootlin.com; marek.va...@gmail.com;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at
> Cc: linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; Yogesh
> Narayan Gaur <yogeshnarayan.g...@nxp.com>; cyrille.pitc...@wedev4u.fr;
> tudor.amba...@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.g...@nxp.com>
Tested-by: Yogesh Gaur <yogeshnarayan.g...@nxp.com>

> Suggested-by: Boris Brezillon <boris.brezil...@bootlin.com>
> Signed-off-by: Tudor Ambarus <tudor.amba...@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