[PATCH v3 4/8] nand: spi: add basic operations support

[Arnaud Mouiche]

On 16/03/2017 07:47, Peter Pan wrote:
> [...]
> +
> +/*
> + * spinand_read_pages - read data from device to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operations description structure
> + * @max_bitflips: maximum bitflip count
> + */
> +static int spinand_read_pages(struct mtd_info *mtd, loff_t from,
> +			      struct mtd_oob_ops *ops,
> +			      unsigned int *max_bitflips)
> +{
> +	struct spinand_device *chip = mtd_to_spinand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	int size, ret;
> +	unsigned int corrected = 0;
> +	bool ecc_off = ops->mode == MTD_OPS_RAW;
> +	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
> +		     mtd->oobavail : mtd->oobsize;
> +	bool oob_only = !ops->datbuf;
> +	struct nand_page_iter iter;
> +
> +	ops->retlen = 0;
> +	ops->oobretlen = 0;
> +	*max_bitflips = 0;
> +

I'm stuck in a infinite bad block scan on the very first nand bad block 
mark read attempt.
Indeed, here I'm in the first page read attempt of scan_block_fast() 
where scan_block_fast() fills "struct mtd_oob_ops ops" the following way
     struct mtd_oob_ops ops;

     ops.ooblen = nand_per_page_oobsize(this);   <= 64
     ops.oobbuf = buf;
     ops.ooboffs = 0;
     ops.datbuf = NULL;
     ops.mode = MTD_OPS_PLACE_OOB;

It just forget to also set ops.len which is left to its uninitialized 
value, and is equal to 0xFFFFFFFF in my case.
Since we only try to read from oob, obviously retlen is never increased, 
and we never except the loop.
But more obviously, either ops.len should have been set to zero 
somewhere because we only read oob, either nand_for_each_page() should 
take in count this fact.

Arnaud
> +	nand_for_each_page(nand, from, ops->len, ops->ooboffs, ops->ooblen,
> +			   ooblen, &iter) {
> +		ret = spinand_do_read_page(mtd, iter.page, ecc_off,
> +					   &corrected, oob_only);
> +		if (ret)
> +			break;
> +		*max_bitflips = max(*max_bitflips, corrected);
> +		if (ops->datbuf) {
> +			size = min_t(int, from + ops->len - iter.offs,
> +				     nand_page_size(nand) - iter.pageoffs);
> +			memcpy(ops->datbuf + ops->retlen,
> +			       chip->buf + iter.pageoffs, size);
> +			ops->retlen += size;
> +		}
> +		if (ops->oobbuf) {
> +			size = min_t(int, iter.oobleft, ooblen);
> +			ret = spinand_transfer_oob(chip,
> +						   ops->oobbuf + ops->oobretlen,
> +						   ops, size);
> +			if (ret) {
> +				pr_err("Transfer oob error %d\n", ret);
> +				return ret;
> +			}
> +			ops->oobretlen += size;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * spinand_do_read_ops - read data from device to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operations description structure
> + */
> +static int spinand_do_read_ops(struct mtd_info *mtd, loff_t from,
> +			       struct mtd_oob_ops *ops)
> +{
> +	struct spinand_device *chip = mtd_to_spinand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	int ret;
> +	struct mtd_ecc_stats stats;
> +	unsigned int max_bitflips = 0;
> +	bool ecc_off = ops->mode == MTD_OPS_RAW;
> +
> +	if (!valid_nand_address(nand, from)) {
> +		pr_err("%s: invalid read address\n", __func__);
> +		return -EINVAL;
> +	}
> +	if ((ops->ooblen > 0) && !valid_nand_oob_ops(nand, from, ops)) {
> +		pr_err("%s: invalid oob operation input\n", __func__);
> +		return -EINVAL;
> +	}
> +	mutex_lock(&chip->lock);
> +	stats = mtd->ecc_stats;
> +	if (ecc_off)
> +		spinand_disable_ecc(chip);
> +	ret = spinand_read_pages(mtd, from, ops, &max_bitflips);
> +	if (ecc_off)
> +		spinand_enable_ecc(chip);
> +	if (ret)
> +		goto out;
> +
> +	if (mtd->ecc_stats.failed - stats.failed) {
> +		ret = -EBADMSG;
> +		goto out;
> +	}
> +	ret = max_bitflips;
> +
> +out:
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +