Re: [PATCH] MTD: Add nand_ecc test module

[Akinobu Mita <akinobu.mita@gmail.com>]

On Sun, Oct 11, 2009 at 04:39:41PM +0300, Artem Bityutskiy wrote:
> On Fri, 2009-10-09 at 18:54 +0900, Akinobu Mita wrote:
> > This module tests NAND ECC functions.
> > 
> > The test is simple.
> > 
> > 1. Create a 256 or 512 bytes block of data filled with random bytes (data)
> > 2. Duplicate the data block and inject single bit error (error_data)
> > 3. Try to correct error_data
> > 4. Compare data and error_data
> 
> So you want to test the built-in 256/512 software ECC calculation
> and correction functions?

Yes.

> > If the test fails, this creates create these files in debugfs:
> > 
> > /sys/kernel/debug/nand-ecc-test-crashers/
> > 
> >         <testname>
> >         <testname>-error
> > 
> > <testname> contains data
> > <testname>-error contains error_data
> 
> Why you need these files? This looks like overkill. Please, remove
> them.

OK. I'll remove them and just do hexdump() if the test fails.

> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: linux-mtd@lists.infradead.org
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> >  drivers/mtd/nand/nand_ecc.c       |   25 ++++-
> >  drivers/mtd/tests/Makefile        |    1 +
> >  drivers/mtd/tests/nand_ecc-test.c |  200 +++++++++++++++++++++++++++++++++++++
> >  include/linux/mtd/nand_ecc.h      |    6 +
> >  4 files changed, 227 insertions(+), 5 deletions(-)
> >  create mode 100644 drivers/mtd/tests/nand_ecc-test.c
> 
> Could you please split this on 2 patches - one is changing the generic
> code, one is the test itself.

OK.

...

> > +static void unregister_ecc_test_crasher(struct ecc_test_crasher *crasher)
> > +{
> > +	list_del(&crasher->list);
> > +
> > +	remove_ecc_test_blob(crasher->error_data);
> > +	remove_ecc_test_blob(crasher->data);
> > +	kfree(crasher);
> > +}
> > +
> > +#if defined(CONFIG_MTD_NAND) || defined(CONFIG_MTD_NAND_MODULE)
> 
> This should not be needed. MTD tests are always compiled as modules,
> so you should not have copilation errors. And if MTD NAND support is
> not present, modprobe will just fail.

nand_calculate_ecc and nand_correct_data needs CONFIG_MTD_NAND=y|m,
otherwise it will get link error:

ERROR: "__nand_correct_data" [drivers/mtd/tests/nand_ecc-test.ko] undefined!
ERROR: "__nand_calculate_ecc" [drivers/mtd/tests/nand_ecc-test.ko] undefined!