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

Scott Wood
2013-12-16T23:03:01Z

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).

-Scott