Skip to contentSkip to conversation

at91sam9x5: for ROM base address (bugfix for nand ECC)

PATCH In review Alessandro Rubini 4 participants 4 messages Oct 1, 2015 – Oct 2, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 at91sam9x5: for ROM base address (bugfix for nand ECC)

From: Edoardo Scaglia <[email protected]>

The ROM base address in the 9x5 processors lives at 1M, not 4M, where SMD is, as defined later in the same file.

The ROM includes some tables that are used to build error-correction data for NAND memory. By using the wrong address, we get wrong data and the result is undetected bit flips (data at 0x408000 is all zeroes).

Thus, even though our kernel was fixing bit errors in NAND, barebox was not fixing them. With UBI (our situation) we got I/O errors because the checksum verification for data nodes failed.

Using the proper address corrects the problem: barebox reports a "bitflip" message, consistent with the kernel message for the same file, and the files are properly loaded and booted.

Note: the kernel has the same wrong define, but then the magic number 0x00108000 as spelled in arch/arm/boot/dts/at91sam9x5.dtsi is used instead of the symbolic name.

---
 arch/arm/mach-at91/include/mach/at91sam9x5.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-at91/include/mach/at91sam9x5.h +1 -1
index 7ba2e3b..e230577 100644
@@ -144,7 +144,7 @@
 #define AT91SAM9X5_SRAM_BASE	0x00300000	/* Internal SRAM base address */
 #define AT91SAM9X5_SRAM_SIZE	SZ_32K		/* Internal SRAM size (32Kb) */
 
-#define AT91SAM9X5_ROM_BASE	0x00400000	/* Internal ROM base address */
+#define AT91SAM9X5_ROM_BASE	0x00100000	/* Internal ROM base address */
 #define AT91SAM9X5_ROM_SIZE	SZ_64K		/* Internal ROM size (64Kb) */
 
 #define AT91SAM9X5_SMD_BASE	0x00400000	/* SMD Controller */
Signature · 6 lines
1.7.7.2

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#1 lore raw
Quoted 1 line

Subject: [PATCH] at91sam9x5: for ROM base address (bugfix for nand ECC)

Argh! After a lot of editing, the subject line is wrong.

fix ROM base address (bugfix for nand ECC)

Anyways, let me add this detail. The current code considers the case where dev_request_mem_region() fails.

        host->pmecc_rom_base = dev_request_mem_region(dev, 3);
        if (!host->pmecc_rom_base) {
                   [...]

What it does in "[...]" is creating the tables with proper math.. I tried to skip the request_mem_region() above and ECC works perfectly, with the software-generated table.

So I wonder whether it makes sense to just remove the declaration of the ROM are memory-region-3 for NAND, and just use the software-built tables.

This wouldn't void the previous patch: the ROM address is wrong as defined, for all CPU types that fall under "at91sam9x5" (I checked the 5 data sheets: G15, G35, X25, G25, X35).

thanks
/alessandro

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

#2 lore raw
Quoted 5 lines · Alessandro Rubini
Quoted 1 line

Subject: [PATCH] at91sam9x5: for ROM base address (bugfix for nand ECC)

Argh! After a lot of editing, the subject line is wrong.

fix ROM base address (bugfix for nand ECC)

I applied the patch with this spelling fix and your name written correctly.

Quoted 8 lines

Anyways, let me add this detail. The current code considers the case where dev_request_mem_region() fails.

        host->pmecc_rom_base = dev_request_mem_region(dev, 3);
        if (!host->pmecc_rom_base) {
                   [...]
Quoted 7 lines

What it does in "[...]" is creating the tables with proper math.. I tried to skip the request_mem_region() above and ECC works perfectly, with the software-generated table.

So I wonder whether it makes sense to just remove the declaration of the ROM are memory-region-3 for NAND, and just use the software-built tables.

Are the software defined tables on all SoCs identical to the ROM tables? If not that may be a reason to use the software tables only as a fallback.

Sascha

Signature · 9 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#3 lore raw
In reply to Alessandro Rubini
Quoted 17 lines · Alessandro Rubini

From: Edoardo Scaglia <[email protected]>

The ROM base address in the 9x5 processors lives at 1M, not 4M, where SMD is, as defined later in the same file.

The ROM includes some tables that are used to build error-correction data for NAND memory. By using the wrong address, we get wrong data and the result is undetected bit flips (data at 0x408000 is all zeroes).

Thus, even though our kernel was fixing bit errors in NAND, barebox was not fixing them. With UBI (our situation) we got I/O errors because the checksum verification for data nodes failed.

Using the proper address corrects the problem: barebox reports a "bitflip" message, consistent with the kernel message for the same file, and the files are properly loaded and booted.

Quoted 7 lines

Note: the kernel has the same wrong define, but then the magic number 0x00108000 as spelled in arch/arm/boot/dts/at91sam9x5.dtsi is used instead of the symbolic name.

                     ^

Typo in your name here

Signature · 8 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox