Messages in this thread | | | Date | Wed, 31 Jan 2018 22:18:17 +0100 | From | stefan@agner ... | Subject | Re: [PATCH] mtd: nand: gpmi: fall back to legacy mode if no ECC information present |
| |
I accidentally removed ML/cc before, re-adding.
On 31.01.2018 10:57, Boris Brezillon wrote: > On Wed, 31 Jan 2018 10:19:05 +0100 > stefan@agner.ch wrote: > >> On 30.01.2018 14:23, Boris Brezillon wrote: >> > Hi Stefan, >> > >> > On Mon, 29 Jan 2018 15:44:40 +0100 >> > Stefan Agner <stefan@agner.ch> wrote: >> > >> >> In case fsl,use-minimum-ecc is set, the driver tries to determine >> >> ECC layout by using the ECC information provided by the MTD stack. >> >> However, in case the NAND chip does not provide any information, >> >> the driver currently fails with: >> >> nand: device found, Manufacturer ID: 0xc2, Chip ID: 0xf1 >> >> nand: Macronix NAND 128MiB 3,3V 8-bit >> >> nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 >> >> gpmi-nand 1806000.gpmi-nand: Error setting BCH geometry : 1 >> >> gpmi-nand: probe of 1806000.gpmi-nand failed with error 1 >> >> >> >> Fall back to implementation specific default mode if no ECC >> >> information are provided by the NAND chip and fsl,use-minimum-ecc >> >> is specified. >> > >> > Hm, this sounds a bit fragile: if we ever fix the Macronix driver >> > (which should be done BTW) to set the appropriate ECC requirements, it >> > will break all platforms that were relying on this 'fall back to legacy >> > logic'. >> >> I see. It is just that downstream behaves that way, hence we sell >> modules which use minimal ECC on ONFI enabled chips and legacy (maximum >> ECC which fits into OOB) on modules with non-ONFI chips. > > And I guess you use the same DT for both variants of the board :-/ >
Actually we only have two SKUs, and they differ also otherwise so I have two DTs anyway.
>> >> Currently we operate the above Macronix chip with 8-bit ECC since quite >> a while. > > Honestly, I don't see a good solution here except adding an extra DT or > live-patching it from the bootloader, because, even if this hack works > for you know, it might not in the future.
Extra DT is fine for Linux.
The problem is more with U-Boot, where I tried to add minimal ECC support via Kconfig symbol and align with Linux behavior. For U-Boot I would really prefer to have a single binary for all SKUs...
I already sent a first patchset https://patchwork.ozlabs.org/patch/867180/
I guess it should be somehow possible to do a board specific selection of ECC. But this is a discussion for another thread.
> > In the future, if you plan to have boards with different variants of > NANDs, I recommend that you always maximize ECC, this way you won't > have this kind of issues.
Makes sense. Unfortunately, for those products we already ship, changing would be rather painful.
> >> >> > So, if what you really want is legacy_set_geometry(), don't specify >> > "fsl,use-minimum-ecc" in your DT and you should be good. Otherwise, fix >> > the Macronix driver to initialize ->ecc_{strength,step_size}_ds >> > appropriately. >> >> The datasheet says: >> • High Reliability >> - Endurance: 100K cycles (with 1-bit ECC per 528-byte) >> >> So we would set ecc_strenght to 1? > > If the datasheet says so, then yes, you should have > ->ecc_strength_ds = 1 and ->ecc_step_size_ds = 512. > >> But then there is almost no room for >> wear leveling. I remember that I dumped the fixed bits once on such a >> chip, and there were several blocks from factory which needed one bit >> fixed... > > Well, that's a different issue. You might want to maximize the ECC > strength for your specific board. In this case, you should not specify > "fsl,use-minimum-ecc" in your DT, or, if the driver supports it (but I > doubt it does), you should add "nand-ecc-maximize". Alternatively, if > you want to keep some of the OOB space, you can ask for a specific ECC > config with the "nand-ecc-strength" and "nand-ecc-step-size" properties.
Different issue, but in the end all I care about: Does wear leveling work properly.
The NAND chip documentation also mentions that typical access is per page (2K), I guess if one uses a single ECC across the complete page then 4-bits are available, which should allow a somewhat decent wear leveling.
I guess we can go with nand-ecc-strength/nand-ecc-step-size for that chip for now.
However, in Linux we should at least fix the device tree bindings documentation for "fsl,use-minimum-ecc" then.
-- Stefan
| |