Re: [PATCH] MTD: Add nand_ecc test module

vimal singh
2009-10-15T08:53:38Z

On Thu, Oct 15, 2009 at 12:20 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Thu, 2009-10-15 at 12:12 +0530, vimal singh wrote:
>> On Thu, Oct 15, 2009 at 12:06 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > On Thu, 2009-10-15 at 10:35 +0530, vimal singh wrote:
>> >> On Wed, Oct 14, 2009 at 9:11 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> >> > On Tue, 2009-10-13 at 18:58 +0900, Akinobu Mita wrote:
>> >> >> > 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!
>> >> >
>> >> > How about splitting tests on 2 parts - one part which does not require
>> >> > NAND, and one which does, and make the second part compile only if NAND
>> >> > is enabled ("depends" operator in Kconfig) ?
>> >>
>> >> I thought purpose of this test was to test correctness of
>> >> 'nand_calculate_ecc' and 'nand_correct_data' functions (i.e. algos),
>> >> which are only compiled and available when NAND is enabled.
>> >
>> > That is exactly what I meant as well.
>>
>> Then, what do you meant to test in first part of test. Sorry if I am
>> misinterpreting you statement above.
>
> May be this is me who is misunderstanding. We have tests which work fine
> with both NOR and NAND. But some tests work only on NOR. E.g., the
> subpage test. We did not have _compile_ dependencies, so it was not a
> problem. NAND-only tests just returned on !NANDs.
>
> This new test requeres NAND support to be compiled.
>
> So what I suggested, is to split the tests on 2 groups on the Kconfig
> level, and make the NAND-only group depend on NAND support. This will
> make them be compiled only if NAND support is there. Then this new test
> will be part of the NAND-only group.
>
> I'm not 100% sure this is nice approach, though. But #ifdef NAND things
> in the test are ugly, IMO.

To me this approach looks fine. I do have seen that oobtest test
(scenario 4) does not holds good for NAND parts. Since NAND driver
allows to read past the page.

--
Regards,
Vimal Singh