From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mail.bootlin.com ([62.4.15.54])
 by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux))
 id 1fBjzu-0008W8-9a
 for linux-mtd@lists.infradead.org; Thu, 26 Apr 2018 16:43:56 +0000
Date: Thu, 26 Apr 2018 18:43:41 +0200
From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Sam Lefebvre <sam.lefebvre@essensium.com>
Cc: linux-mtd@lists.infradead.org, Arnout Vandecapelle <arnout@mind.be>,
 Dries Staelens <dries.staelens@barco.com>
Subject: Re: [PATCH 08/13] mtd: rawnand: gpmi: return generated errors in
 gpmi_ecc_read_oob()
Message-ID: <20180426184341.21e36b86@bbrezillon>
In-Reply-To: <20180426154134.8270-9-sam.lefebvre@essensium.com>
References: <20180426154134.8270-1-sam.lefebvre@essensium.com>
 <20180426154134.8270-9-sam.lefebvre@essensium.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
List-Id: Linux MTD discussion mailing list <linux-mtd.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-mtd>,
 <mailto:linux-mtd-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-mtd/>
List-Post: <mailto:linux-mtd@lists.infradead.org>
List-Help: <mailto:linux-mtd-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-mtd>,
 <mailto:linux-mtd-request@lists.infradead.org?subject=subscribe>

Hi Sam,

On Thu, 26 Apr 2018 17:41:29 +0200
Sam Lefebvre <sam.lefebvre@essensium.com> wrote:

> Errors can be generated by nand_read_page_op(). It's important
> to check the error flags and pass them upwards.
> 
> Signed-off-by: Sam Lefebvre <sam.lefebvre@essensium.com>
> 
> ---
> 
> v2: also bail out before calling ->read_byte() (Boris)
> 
> When handling errors we need to bail out before calling
> ->read_byte().  
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 7ba00b8ab288..60bc1bac7741 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -1334,13 +1334,18 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  				int page)
>  {
>  	struct gpmi_nand_data *this = nand_get_controller_data(chip);
> +	int ret = 0;

No need to initialize ret to 0 here.

>  
>  	dev_dbg(this->dev, "page number is %d\n", page);
>  	/* clear the OOB buffer */
>  	memset(chip->oob_poi, ~0, mtd->oobsize);
>  
>  	/* Read out the conventional OOB. */
> -	nand_read_page_op(chip, page, mtd->writesize, NULL, 0);
> +	ret = nand_read_page_op(chip, page, mtd->writesize, NULL, 0);
> +

Please drop this empty line.

> +	if (ret)
> +		return ret;
> +
>  	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>  
>  	/*
> @@ -1350,11 +1355,15 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  	 */
>  	if (GPMI_IS_MX23(this)) {
>  		/* Read the block mark into the first byte of the OOB buffer. */
> -		nand_read_page_op(chip, page, 0, NULL, 0);
> +		ret = nand_read_page_op(chip, page, 0, NULL, 0);
> +

Here as well.

> +		if (ret)
> +			return ret;
> +
>  		chip->oob_poi[0] = chip->read_byte(mtd);
>  	}
>  
> -	return 0;
> +	return ret;

You can keep return 0 here.

>  }
>  
>  static int

Regards,

Boris