Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
arnysch opened this issue Nov 7, 2018 · 8 comments
Open

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

arnysch opened this issue Nov 7, 2018 · 8 comments

Comments

@arnysch
Copy link
Contributor

arnysch commented Nov 7, 2018

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
Copy link
Owner

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
Copy link
Contributor Author

arnysch commented Nov 21, 2018

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

@arnysch arnysch closed this as completed Nov 21, 2018
@arnysch
Copy link
Contributor Author

arnysch commented Nov 30, 2018

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?

@arnysch arnysch reopened this Nov 30, 2018
@arnysch arnysch changed the title NAND sub-pages: please reconsider using size 512 instead of 2048 bytes NAND sub-pages: using size 512 instead of 2048 bytes Nov 30, 2018
@Quallenauge
Copy link
Owner

Quallenauge commented Dec 2, 2018

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
Copy link
Contributor Author

arnysch commented Dec 3, 2018

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
Copy link
Owner

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
Copy link
Contributor Author

arnysch commented Dec 3, 2018

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
Copy link

majuss commented Dec 6, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants