Re: [PATCH 1/6] mtd: nand: Check length of ID before reading bits per cell

[Brian Norris <computersforpeace@gmail.com>]

A little tangential to the big-picture discussion, but I'll get this out
there:

On Tue, Dec 02, 2014 at 09:58:51AM -0300, Ezequiel Garcia wrote:
> The table-based NAND identification currently reads the number
> of bits per cell from the 3rd byte of the extended ID. This is done
> for the so-called 'full ID' devices; i.e. devices that have a known
> length ID.
> 
> However, if the ID length is shorter than three, there's no 3rd byte,
> and so it's wrong to read the bits per cell from there. Fix this by
> adding a check for the ID length.

You're right about the problem, but I'm not sure about the fix. I
actually don't know why we are using the ID bytes to guess at the number
of bits per cell here; the point of the full-id checks is that with some
NAND, we just can't decode all the information reliably from the ID.

I'd kinda rather just see bits-per-cell represented as a separate bit;
either a new field, or in the 'options' field.

> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> ---
>  drivers/mtd/nand/nand_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 5b5c627..a4c9cee 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3589,7 +3589,8 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>  		mtd->erasesize = type->erasesize;
>  		mtd->oobsize = type->oobsize;
>  
> -		chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
> +		if (type->id_len > 2)
> +			chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
>  		chip->chipsize = (uint64_t)type->chipsize << 20;
>  		chip->options |= type->options;
>  		chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);

Brian