Quallenauge / Easybox-904-XDSL

Fork of openwrt with vendor specific changes from open sourced firmware 3.10.
GNU General Public License v2.0
20 stars 8 forks source link

NAND sub-pages: using size 512 instead of 2048 bytes #13

Open arnysch opened 5 years ago

arnysch commented 5 years ago

Hi everybody, hi Qauge,

There is this commit "Sysupgrade: Deactivate subpages on nand". It contains this comment: "Currently, subpages aren't working on the easybox 904 xdsl nand flash. Allthough, The nand driver seems to report it is supported, which isn't".

I am wondering if this is really the case. Maybe UBI sub-page problems were caused by mixing old and new stuff?

For example for ages we use this kernel command line parameter "ubi.mtd=12,2048". And trying to access an existing 'old' UBI partition (which did not use sub-pages) with the current UBI driver results in errors, unless a VID header offset of 2048 is explicitly specified in the ubiattach command.

On the eb904, I played around with sub-page size of 512 bytes. And I ran the "nandsubpagetest" program from the mtd-utils software.

To me it looks like 512 byte sub-page size works ok on the eb904. I have documented my tests here.

I would like to hear opinions from OpenWrt-eb904 developers, If you think it is a good idea, I would gladly provide a pull request to change the sub-page size back to 512.

BTW: my tests with U-Boot (Arcadian's version from 2010) show that UBI as part of U-Boot does not work at all. Neither with VID header offset 512 nor with 2048. U-Boot does not recognize an existing UBI partition and always reformats the partition when trying to access it with UBI commands.

Quallenauge commented 5 years ago

At the time I tried it (4.9 kernel), it was not possible to create and use a valid ubi image. Maybe in the meantime (now we use 4.14) there where kernel patches which fixed this?!

Also I never used the UBI partition in uboot.

arnysch commented 5 years ago

Guess it is ok that I close this issue. Currently sub-pages are used on the eb904.

arnysch commented 5 years ago

In the meantime we converted to subpage usage for NAND.

This is the continuation of a problem report from Qauge who tried using sub-pages.

First the theory: this linux-mtd thread discusses that Samsung's "E-die" NAND chips don't support subpages. And yes, according to their data sheet section 2.8, they support only 1 "partial program cycle". The NAND chip in the eb904 is of type "D-die", and according to its data sheet section 2.8, they support up to four partial program cycles, and this seems to mean that they support sub-pages.

Now subpages work for me, and Kovz also did not tell of any problems. BTW, the chip id reported by Qauge is Manf ID: 0xec, Chip ID: 0xdc which is the same I see with my eb904.

Now the reality: it might well be the case that sub-pages do not work reliable or that Samsung's chips with same id are actually different.

Quallenauge, before striking the colors and reverting to the old non-sub-page handling, could you run these tests hoping we can find a better explanation?

Quallenauge commented 5 years ago

Short info: I followed your proposed tests, and indeed the tests fail when using the 512 (=default) subpages. Also the U-Boot erase does not change this behavior.

(Logs will be appended here later - I have to polish the logs. Update Logs extracted from ther serial console logfile: https://paste.ee/p/d0sJi). I tried now to apply the patch also on the 0xdc if branch, and the kernel spies out:

[    5.460068] --- Disable subpage writing
[    5.462467] nand: device found, Manufacturer ID: 0xec, Chip ID: 0xdc
[    5.468816] nand: Samsung NAND 512MiB 3,3V 8-bit
[    5.473434] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    5.481616] Bad block table found at page 262080, version 0x01
[    5.487663] Bad block table found at page 262016, version 0x01
....
[    7.402918] UBI: auto-attach mtd12
[    7.405170] ubi0: attaching mtd12
[    7.408826] ubi0 error: 0x8030f230: bad VID header offset 512, expected 2048

So I think this is the way to go. I removed the printk (--- Disable subpage writing) output and uploaded an PR to solve this: #21

arnysch commented 5 years ago

Hi Qauge, Thank you very much for the time you spent to run the tests, even the 'esoteric' one where you used U-Boot for erasing. I examined your logs. Condensed the important stuff here. My hope was not to need eb904 specific changes in common kernel or common OpenWrt files. Also this crash when using the self compiled U-Boot is disturbing. Left at a loss. So I agree to turn back this sub-page stuff, for example like you proposed in the kernel driver, or wherever else.

Quallenauge commented 5 years ago

Hi @arnysch. Thanks for your feedback! Regarding the kernel patch: I think this is the cleanest solution, because it's the driver responsibility which reports the sub-page things to the user space. So this would be my preferred one solution. But I'm curious how this change behaves on your box.

The uboot crash is really annoying - Here the DDR patch comes into the game (in another issue we can discuss how I can save and restore ddr settings).

arnysch commented 5 years ago

Hi Qauge, tried out your changes. Looks very good. Congrats for your solution, you are absolutely right; have not realized what you meant when you first told about it - too obsessed about my own stuff.

Wondering if this "chip->ecc_step_ds = 512" and "chip->ecc_strength_ds = 1" (yes I know, this is in kernel 4.19) in nand_samsung.c is really necessary. Same with "nand-ecc-strength = <3>" and "nand-ecc-step-size = <256>" in the dts file. Removed all of that, built and tested: seems it does not make a difference. But I did not try to understand the nand driver code; this would take more time.

Maybe you could give me your orig U-Boot from mtd0? Would like to know if it is different from mine.

majuss commented 5 years ago

Latest image is booting correctly and can mount the overlay! Thanks a lot for the good job everyone.