mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Trent Piepho <trent.piepho@igorinstitute.com>
To: Andrej Picej <andrej.picej@norik.com>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH 2/2] ARM: i.MX: xload: consider ECC strength when reading page
Date: Mon, 7 Jun 2021 13:03:41 -0700	[thread overview]
Message-ID: <CAMHeXxNtHQdWBZdDc0fXJ1Q2Fj-=H2anmFGosKGrV12Ux1cncg@mail.gmail.com> (raw)
In-Reply-To: <20210607090951.107269-2-andrej.picej@norik.com>

On Mon, Jun 7, 2021 at 2:32 AM Andrej Picej <andrej.picej@norik.com> wrote:
> Some NAND update tools/flashers do not take the full advantage of NAND's
> entire page area for ECC purposes. For example, they might only use 2112
> bytes of available 2176 bytes. In this case, ECC parameters have to be
> read from the FCB table and taken into account in GPMI NAND xloader to
> properly calculate page data length so DMA chain can be executed
> correctly.
>
> Tested on PHYTEC phyCARD i.MX6Q board with following NANDs:
> - Samsung K9K8G08U0E (pagesize: 0x800, oobsize: 0x40)
> - Winbond W29N08GVSIAA (pagesize: 0x800, oobsize: 0x40) and
> - Spansion S34ML08G201FI00 (pagesize: 0x800, oobsize: 0x80).
>
> All NANDs having set ECC strength to 4 (13 bytes) despite Spansion NAND
> chip supporting ECC strength of 9 (29 bytes).

There is a bug in NXP's latest imx kernel, lf-5.10.y-1.0.0, that
results in the kernel driver incorrectly using the minimum ECC
specified in the ONFI nand specs instead of calculating a maximal ecc
value and using that, which is what prior kernels and the upstream
kernel use.  It was caused by incorrectly resolving a conflict when
they rebased one of their old patches to 5.10.

The common pagesize 0x800, oobsize 0x40 should use 8-bit ECC.  That's
what the uboot, barebox, and linux drivers would do since the first
mxs nand support years ago.  It's only the recent kernel bug in nxp's
kernel that will choose 4.

So rather than switch to 4-bit, it would be better to fix these boards
to use 8-bit like they should.  More reliable ECC, and it will work
correctly on barebox, u-boot, old imx kernels, current upstream
kernels, and hopefully future imx kernels.

Using the FCB data here might not be such a good idea.  While it seems
like the right thing, there are some issues:
The barebox main gpmi nand driver doesn't use the FCB
U-boot doesn't use the FCB
No Linux kernel uses the FCB

If you try to read/write nand from any of those places, it won't work.
The only way to make it work, is to have the FCB match what those
drivers do.

I think it would have been better if the original design had been for
the bootloader to read the FCB, use that to load the kernel, and then
fixup the ECC config into the device tree for the kernel to use too.
One source, the FCB, which is propagated to all users.  Everyone will
agree on the ECC and there are no independent settings to keep in
sync.

But they didn't do that.  Each driver figures it out on it's own and
hopefully they use matching algorithms that arrive at the same answer.
But of course this fails, like with nxp's lf-5.10.y-1.0.0 kernel.
This isn't the first time, this same type of bug appeared back in 2013
in 2febcdf84b and was fixed in 031e2777e.

So while your commit will allow these boards using poorly chosen FCB
values to work with the xloader, they will be corrupted if nand is
written to from barebox non-xload or from linux.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  reply	other threads:[~2021-06-07 20:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07  9:09 [PATCH 1/2] ARM: i.MX: xload-gpmi-nand: fix bad block mark swapping Andrej Picej
2021-06-07  9:09 ` [PATCH 2/2] ARM: i.MX: xload: consider ECC strength when reading page Andrej Picej
2021-06-07 20:03   ` Trent Piepho [this message]
2021-06-08  6:28     ` Sascha Hauer
2021-06-08  7:23     ` Andrej Picej
2021-06-08 12:38       ` Trent Piepho
2021-06-09  6:34         ` Andrej Picej
2021-06-14 22:14           ` Trent Piepho
2021-06-15 14:35             ` Sascha Hauer
2021-06-15 20:25               ` Trent Piepho
2021-06-16  7:48                 ` Andrej Picej

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMHeXxNtHQdWBZdDc0fXJ1Q2Fj-=H2anmFGosKGrV12Ux1cncg@mail.gmail.com' \
    --to=trent.piepho@igorinstitute.com \
    --cc=andrej.picej@norik.com \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox