Re: [PATCH 7/7] nand-bb: implement lseek in readonly mode

[Robert Jarzmik <robert.jarzmik@free.fr>]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> +static off_t nand_bb_lseek(struct cdev *cdev, off_t __offset)
> +{
> +	struct nand_bb *bb = cdev->priv;
> +	unsigned long raw_pos = 0;
> +	uint32_t offset = __offset;
> +	int ret;
> +
> +	/* lseek only in readonly mode */
> +	if (bb->flags & O_ACCMODE)
> +		return -ENOSYS;
> +
> +	while (raw_pos < bb->raw_size) {
> +		off_t now = min(offset, bb->info.erasesize);
> +
> +		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
> +		if (ret < 0)
> +			return ret;
> +		if (!ret)
> +			offset -= now;
> +		raw_pos += now;
> +		if (!offset) {
> +			bb->offset = raw_pos;
> +			return __offset;
> +		}
> +	}
Are you sure of this algorithm ?
I tried to check it with:
 - erasesize=16 (silly I know, but it's simpler for my mind)
 - bb->raw_size = +oo
 - offset = 34

Let's assume we have eraseblock B0, B1, B2 and B3 of 16 bytes.
B0, B1 and B3 are good, B2 is a bad block.

+--------+--------+--------+--------+
|   B0   |   B1   |xxxB2xxx|   B3   |
+--------+--------+--------+--------+

If I unroll the while loop:
 - loop1:
     now=16
     ret=0 (B0 good)
     offset = 18
     raw_pos = 16
 - loop2:
     now=16
     ret=0 (B1 good)
     offset = 2
     raw_pos = 32
 - loop3:
     now=2
     ret=1 (B2 bad)
     offset = 2
     raw_pos = 34
 - loop4:
     now=2
     ret=0 (B3 good)
     offset = 0
     raw_pos = 36
     bb->offset = 36
     return 34

So we end up with bb->offset = 36, which seems incorrect to me. I would have
understood a value of 50, but 36 ... I don't

Cheers.

-- 
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox