Dear Boris,
thanks for pointing this out again.
I'm on the same topic too, using iMX6 (I'll try to test you patch on the
next days, if I found some spare time, unfortunately I got a 3.10
kernel, so I think the patch will not apply cleanly :-( ).
See my comment below (and on the next mail too)
Il 31/07/2015 09:10, Boris Brezillon ha scritto:
On Thu, 30 Jul 2015 19:34:53 +0200
Boris Brezillon <[email protected]> wrote:
Add two helper functions to help NAND controller drivers test whether a
specific NAND region is erased or not.
Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/nand_base.c | 104 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/nand.h | 8 ++++
2 files changed, 112 insertions(+)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ceb68ca..1542ea7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1101,6 +1101,110 @@ out:
EXPORT_SYMBOL(nand_lock);
/**
+ * nand_check_erased_buf - check if a buffer contains (almost) only 0xff data
+ * @buf: buffer to test
+ * @len: buffer length
+ * @bitflips_threshold:maximum number of bitflips
+ *
+ * Check if a buffer contains only 0xff, which means the underlying region
+ * has been erased and is ready to be programmed.
+ * The bitflips_threshold specify the maximum number of bitflips before
+ * considering the region is not erased.
+ * Note: The logic of this function has been extracted from the memweight
+ * implementation, except that nand_check_erased_buf function exit before
+ * testing the whole buffer if the number of bitflips exceed the
+ * bitflips_threshold value.
+ *
+ * Returns a positive number of bitflips or -ERROR_CODE.
+ */
+int nand_check_erased_buf(void *buf, int len, int bitflips_threshold)
+{
+ const unsigned char *bitmap = buf;
+ int bitflips = 0;
+ int weight;
+ int longs;
+
+ for (; len && ((unsigned long)bitmap) % sizeof(long);
+ len--, bitmap++) {
+ weight = hweight8(*bitmap);
+
+ bitflips += sizeof(u8) - weight;
+ if (bitflips > bitflips_threshold)
+ return -EINVAL;
I think it's better to do something like:
if (UNLIKELY(bitflips > bitflips_threshold))
return -EINVAL;
isn't it? :-)
(the same for the other if)
+ }
+
+
+ for (longs = len / sizeof(long); longs;
+ longs--, bitmap += sizeof(long)) {
+ BUG_ON(longs >= INT_MAX / BITS_PER_LONG);
+ weight = hweight_long(*((unsigned long *)bitmap));
+
+ bitflips += sizeof(long) - weight;
+ if (bitflips > bitflips_threshold)
+ return -EINVAL;
+ }
+
+ len %= sizeof(long);
+
+ for (; len > 0; len--, bitmap++) {
+ weight = hweight8(*bitmap);
+ bitflips += sizeof(u8) - weight;
+ }
+
+ return bitflips;
+}
+EXPORT_SYMBOL(nand_check_erased_buf);
+
+/**
+ * nand_check_erased_ecc_chunk - check if an ECC chunk contains (almost) only
+ * 0xff data
+ * @data: data buffer to test
+ * @datalen: data length
+ * @ecc: ECC buffer
+ * @ecclen: ECC length
+ * @extraoob: extra OOB buffer
+ * @extraooblen: extra OOB length
+ * @bitflips_threshold: maximum number of bitflips
+ *
+ * Check if a data buffer and its associated ECC and OOB data contains only
+ * 0xff pattern, which means the underlying region has been erased and is
+ * ready to be programmed.
+ * The bitflips_threshold specify the maximum number of bitflips before
+ * considering the region as not erased.
+ *
+ * Returns a positive number of bitflips or -ERROR_CODE.
+ */
+int nand_check_erased_ecc_chunk(void *data, int datalen,
+ void *ecc, int ecclen,
+ void *extraoob, int extraooblen,
+ int bitflips_threshold)
+{
+ int bitflips = 0;
+ int ret;
+
+ ret = nand_check_erased_buf(data, datalen, bitflips_threshold);
+ if (ret < 0)
+ return ret;
+
+ bitflips += ret;
+ bitflips_threshold -= ret;
+
+ ret = nand_check_erased_buf(ecc, ecclen, bitflips_threshold);
+ if (ret < 0)
+ return ret;
+
+ bitflips += ret;
+ bitflips_threshold -= ret;
+
+ ret = nand_check_erased_buf(extraoob, extraooblen, bitflips_threshold);
+ if (ret < 0)
+ return ret;
+
Forgot the memset operations here:
memset(data, 0xff, datalen);
memset(ecc, 0xff, ecclen);
memset(extraoob, 0xff, extraooblen);
Yes, you're right.. I did the same mistake on my first implementation
too ;-)
As additional optimization you may also check if the lower layer already
did the check for you (e.g. if you have an iMXQP as we saw in latest
days), but I think it's a minor one, because you'll face this situation
very very unlikely.
--
Andrea SCIAN
DAVE Embedded Systems
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
