Re: [PATCH v2 3/6] nand: spi: Add bad block support

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Wed, 1 Mar 2017 16:52:07 +0800
Peter Pan <peterpandong@micron.com> wrote:

> Add isbad and markbad support for SPI NAND. And do not
> erase bad blocks in spi_nand_erase.

Hm, can you put patch 4 before patch 3 (or maybe merge the 2 patches?).

> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/spinand_base.c | 116 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/spinand_base.c b/drivers/mtd/nand/spi/spinand_base.c
> index 1bc57e9..442997d 100644
> --- a/drivers/mtd/nand/spi/spinand_base.c
> +++ b/drivers/mtd/nand/spi/spinand_base.c
> @@ -1003,6 +1003,113 @@ static int spinand_write_oob(struct mtd_info *mtd, loff_t to,
>  	return ret;
>  }
>  
> +/**
> + * spinand_block_bad - Check if block at offset is bad
> + * @mtd: MTD device structure
> + * @offs: offset relative to mtd start
> + * @getchip: 0, if the chip is already selected
> + */
> +static int spinand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)

Please don't drop the getchip parameter and let the caller take the
spi NAND lock. If you want to make it clear that the function should be
called with the lock held, you can rename it spinand_block_bad_locked()
and/or document this expectation in the function doc header.

> +{
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	struct mtd_oob_ops ops = {0};
> +	u32 block_addr;
> +	u8 bad[2] = {0, 0};
> +	u8 ret = 0;
> +
> +	block_addr = nand_offs_to_eraseblock(nand, ofs);
> +	ops.mode = MTD_OPS_PLACE_OOB;
> +	ops.ooblen = 2;
> +	ops.oobbuf = bad;
> +
> +	if (getchip)
> +		spinand_get_device(mtd_to_spinand(mtd));
> +	spinand_do_read_ops(mtd, nand_eraseblock_to_offs(nand, block_addr), &ops);
> +	if (getchip)
> +		spinand_release_device(mtd_to_spinand(mtd));
> +	if (bad[0] != 0xFF || bad[1] != 0xFF)
> +		ret =  1;
> +
> +	return ret;
> +}