[U-Boot] [PATCH] nand: fix reading after switching ecc

[Jeroen Hofstee <jeroen@myspectrum.nl>]

On 01/14/2014 09:21 PM, Scott Wood wrote:
> On Tue, 2014-01-14 at 21:11 +0100, Jeroen Hofstee wrote:
>> Hello Scott, Pekon,
>>
>> On 01/13/2014 07:18 PM, Scott Wood wrote:
>>>>> The omap_gpmc allows switching ecc at runtime. Since
>>>>> the NAND_SUBPAGE_READ flag is only set, it is kept when
>>>>> switching to hw ecc, which is not correct. This leads to
>>>>> calling chip->ecc.read_subpage which is not a valid
>>>>> pointer. Therefore also clear the flag so reading in
>>>>> hw mode works again.
>>>>>
>>>>> Cc: Scott Wood <scottwood@freescale.com>
>>>>> Cc: Pekon Gupta <pekon@ti.com>
>>>>> Cc: Nikita Kiryanov <nikita@compulab.co.il>
>>>>> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
>>>>> ---
>>>>> drivers/mtd/nand/nand_base.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>>>> index 1ce55fd..0762a19 100644
>>>>> --- a/drivers/mtd/nand/nand_base.c
>>>>> +++ b/drivers/mtd/nand/nand_base.c
>>>>> @@ -3354,6 +3354,8 @@ int nand_scan_tail(struct mtd_info *mtd)
>>>>> 	/* Large page NAND with SOFT_ECC should support subpage reads */
>>>>> 	if ((chip->ecc.mode == NAND_ECC_SOFT) && (chip->page_shift > 9))
>>>>> 		chip->options |= NAND_SUBPAGE_READ;
>>>>> +	else
>>>>> +		chip->options &= ~NAND_SUBPAGE_READ;
>>> NACK; this breaks NAND_SUBPAGE_READ with hardware ECC for drivers that
>>> support it.
>> There is something to argue in favour of that in general, but there are
>> no such drivers in u-boot at the moment though (well at least not doing
>> it on
>> purpose). I don't mind moving it to gpmc, but for correctness, it
>> doesn't break
>> things at the moment...
> It doesn't matter if there are any such drivers at the moment.  It's a
> bad change, and a gratuitous difference from Linux with which we share
> this common code.
of course it does matter, you can chose not to support certain
features, like subpage reads, interrupts, power management etc
in a bootloader. As I said, moving it to gpmc is fine with me, so lets
not making a long discussion of it.

>
> All the other cleanup required to change ECC modes is handled by the
> OMAP driver; why shouldn't this be as well?


If there are more architectures, who think it is brilliant to switch ecc
it will become a mesh if not coped with in general. And yes omap might
be the exception for now, so lets put it there.

Regards,
Jeroen