Skip to contentSkip to conversation

Broken ecc.size when switching between sw and hw ecc (beagleboard)

U-Boot Orjan Friberg 3 participants 6 messages Feb 29, 2012 – Mar 5, 2012

lore
Share & tools
lore
#0 lore raw

For the beagleboard, ecc.size is not explicitly set when doing 'nandecc sw'. If it's not set for the NAND_ECC_SOFT case in nand_scan_tail, it's set to 256 bytes.

When doing 'nandecc hw', ecc.size is set to 512 bytes. Hence, when changing back to 'nandecc sw' ecc.size remains at 512 bytes and suddenly the format has changed.

It seems the current nandecc command needs to set this explicitly, but also needs to be augmented to be able to select the newly added 4/8-bit BCH ECC.

But it also seems like nandecc selection should be more generic than for omap3 (currently it lives in arch/arm/cpu/armv7/omap3/board.c).

I have seen that TI did some work on this previously in the PSP04.02.00.07 release of u-boot and linux (2.6.37).

Is there ongoing work in this area? I'd be happy to help out if I can.

(I noticed the nand dump.oob seems to have been fixed in the u-boot-nand-flash repo.)

Thanks,
Orjan

Signature · 2 lines
Orjan Friberg
FlatFrog Laboratories AB
#1 lore raw
Quoted 4 lines · Orjan Friberg

For the beagleboard, ecc.size is not explicitly set when doing 'nandecc sw'. If it's not set for the NAND_ECC_SOFT case in nand_scan_tail, it's set to 256 bytes.

Quoted 8 lines

When doing 'nandecc hw', ecc.size is set to 512 bytes. Hence, when changing back to 'nandecc sw' ecc.size remains at 512 bytes and suddenly the format has changed.

It seems the current nandecc command needs to set this explicitly, but also needs to be augmented to be able to select the newly added 4/8-bit BCH ECC.

Yes, that looks like a bug in omap_nand_switch_ecc().

Quoted 2 lines

But it also seems like nandecc selection should be more generic than for omap3 (currently it lives in arch/arm/cpu/armv7/omap3/board.c).

ECC mode is normally not something that you want to be runtime switchable, as changing it usually changes the on-flash format. It also requires driver cooperation -- the actual implementation (as opposed to the command-line wrapper) is in drivers/mtd/nand/omap_gpmc.c.

If we end up with multiple drivers that need to expose this sort of functionality, maybe we can consider moving the command line wrapper to common code.

-Scott

#2 lore raw
Quoted 4 lines · Scott Wood

ECC mode is normally not something that you want to be runtime switchable, as changing it usually changes the on-flash format. It also requires driver cooperation -- the actual implementation (as opposed to the command-line wrapper) is in drivers/mtd/nand/omap_gpmc.c.

Are you saying that it shouldn't be switchable through a command line option (but rather based on information in the partition table or something similar), or are you saying that we shouldn't need different ECC modes for different parts of the flash (boot ROM vs. software implementations, whether block 0 has different ECC requirement etc)?

Signature · 2 lines
Orjan Friberg
FlatFrog Laboratories AB
#3 lore raw
Quoted 11 lines · Orjan Friberg
Quoted 4 lines · Scott Wood

ECC mode is normally not something that you want to be runtime switchable, as changing it usually changes the on-flash format. It also requires driver cooperation -- the actual implementation (as opposed to the command-line wrapper) is in drivers/mtd/nand/omap_gpmc.c.

Are you saying that it shouldn't be switchable through a command line option (but rather based on information in the partition table or something similar), or are you saying that we shouldn't need different ECC modes for different parts of the flash (boot ROM vs. software implementations, whether block 0 has different ECC requirement etc)?

I'm saying that right now it's an OMAP-specific requirement and the implementation of necessity involves the OMAP driver. If another driver grows this as a requirement, we can consider refactoring so the command line frontend is common.

Different ECC modes for different parts of the flash is unfortunate, but sometimes mandated by circumstance. The NAND subsystem isn't really set up to handle this, though changing it temporally is a hackish substitute.

-Scott

#4 lore raw
Quoted 8 lines · Scott Wood

I'm saying that right now it's an OMAP-specific requirement and the implementation of necessity involves the OMAP driver. If another driver grows this as a requirement, we can consider refactoring so the command line frontend is common.

Different ECC modes for different parts of the flash is unfortunate, but sometimes mandated by circumstance. The NAND subsystem isn't really set up to handle this, though changing it temporally is a hackish substitute.

Ok, thanks for clarifying.

Signature · 2 lines
Orjan Friberg
FlatFrog Laboratories AB
#5 lore raw
In reply to Scott Wood
Quoted 10 lines · Scott Wood
Quoted 9 lines · Orjan Friberg

For the beagleboard, ecc.size is not explicitly set when doing 'nandecc sw'. If it's not set for the NAND_ECC_SOFT case in nand_scan_tail, it's set to 256 bytes.

When doing 'nandecc hw', ecc.size is set to 512 bytes. Hence, when changing back to 'nandecc sw' ecc.size remains at 512 bytes and suddenly the format has changed.

Quoted 5 lines
Quoted 3 lines

It seems the current nandecc command needs to set this explicitly, but also needs to be augmented to be able to select the newly added 4/8-bit BCH ECC.

Yes, that looks like a bug in omap_nand_switch_ecc().

Correct. And wrt using the new 4/8/16-bit BCH library, that's what I was starting on when I found the nand dump issue :)

Quoted 7 lines
Quoted 2 lines

But it also seems like nandecc selection should be more generic than for omap3 (currently it lives in arch/arm/cpu/armv7/omap3/board.c).

ECC mode is normally not something that you want to be runtime switchable, as changing it usually changes the on-flash format. It also requires driver cooperation -- the actual implementation (as opposed to the command-line wrapper) is in drivers/mtd/nand/omap_gpmc.c.

There is unfortunately a long-running saga here and what we have now is what we have for a number of existing platforms. Moving forward, things are better and there's an argument at least for not making ECC mode switching a run-time command.

Signature · 1 line
Tom