From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Tue, 17 Dec 2013 15:16:27 +0200 Subject: [U-Boot] [PATCH 2/2] mtd: nand: omap: fix ecc ops assignment when changing ecc In-Reply-To: <1387234981.10013.412.camel@snotra.buserror.net> References: <1387214342-7411-1-git-send-email-nikita@compulab.co.il> <1387214342-7411-3-git-send-email-nikita@compulab.co.il> <1387234981.10013.412.camel@snotra.buserror.net> Message-ID: <52B04EAB.5040307@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> Cc: Pekon Gupta >> Signed-off-by: Nikita Kiryanov >> --- >> 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.