[U-Boot] [PATCH 2/2] mtd: nand: omap: fix ecc ops assignment when changing ecc

[Nikita Kiryanov <nikita@compulab.co.il>]

On 12/17/2013 01:03 AM, Scott Wood wrote:
> On Mon, 2013-12-16 at 19:19 +0200, Nikita Kiryanov wrote:
>> If we change to software ecc and then back to hardware ecc, the nand ecc ops
>> pointers are populated with incorrect function pointers. This is related to the
>> way nand_scan_tail() handles assigning functions to ecc ops:
>>
>> If we are switching to software ecc/no ecc, it assigns default functions to the
>> ecc ops pointers unconditionally, but if we are switching to hardware ecc,
>> the default hardware ecc functions are assigned to ops pointers only if these
>> pointers are NULL (so that drivers could set their own functions). In the case
>> of omap_gpmc.c driver, when we switch to sw ecc, sw ecc functions are
>> assigned to ecc ops by nand_scan_tail(), and when we later switch to hw ecc,
>> the ecc ops pointers are not NULL, so nand_scan_tail() does not overwrite
>> them with hw ecc functions.
>> The result: sw ecc functions used to write hw ecc data.
>>
>> Clear the ecc ops pointers in omap_gpmc.c when switching ecc types, so that
>> ops which were not assigned by the driver will get the correct default values
>> from nand_scan_tail().
>>
>> Cc: Scott Wood <scottwood@freescale.com>
>> Cc: Pekon Gupta <pekon@ti.com>
>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>> ---
>>   drivers/mtd/nand/omap_gpmc.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
>> index fda1df2..19dcd45 100644
>> --- a/drivers/mtd/nand/omap_gpmc.c
>> +++ b/drivers/mtd/nand/omap_gpmc.c
>> @@ -765,6 +765,19 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
>>   	int eccsteps = pagesize / SECTOR_BYTES;
>>   	int i;
>>
>> +	nand->ecc.calculate = NULL;
>> +	nand->ecc.correct = NULL;
>> +	nand->ecc.hwctl = NULL;
>> +	nand->ecc.read_oob = NULL;
>> +	nand->ecc.read_oob_raw = NULL;
>> +	nand->ecc.read_page = NULL;
>> +	nand->ecc.read_page_raw = NULL;
>> +	nand->ecc.read_subpage = NULL;
>> +	nand->ecc.write_oob = NULL;
>> +	nand->ecc.write_oob_raw = NULL;
>> +	nand->ecc.write_page = NULL;
>> +	nand->ecc.write_page_raw = NULL;
>> +
>>   	switch (ecc_scheme) {
>>   	case OMAP_ECC_HAM1_CODE_SW:
>>   		debug("nand: selected OMAP_ECC_HAM1_CODE_SW\n");
>
> This will leave you with a broken nand->ecc if the function returns an
> error.  Instead, each case in the switch should NULL out whichever
> members it is not initializing (or perhaps just memset it, once past the
> error checks).

Yes you're right. Guess I should've resisted doing a last minue
refactor. V2 coming up.

>
> -Scott
>
>


-- 
Regards,
Nikita.