Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210803143256.GA5209@ubuntu> <20210806185658.5b4772a7@xps13>
In-Reply-To: <20210806185658.5b4772a7@xps13>
From:   Kestrel seventyfour <kestrelseventyfour@gmail.com>
Date:   Sun, 8 Aug 2021 08:45:38 +0200
Message-ID: <CAE9cyGQwQKp83y0cBoH1akRyHo9ipgBHaeWMjtCLVP77vRir=g@mail.gmail.com>
Subject: Re: [PATCH] mtd: rawnand: xway: No hardcoded ECC engine for Micron Chips
To:     Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Boris Brezillon <boris.brezillon@collabora.com>,
        linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
        Daniel Kestrel <kestrelseventyfour@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Am Fr., 6. Aug. 2021 um 18:57 Uhr schrieb Miquel Raynal
<miquel.raynal@bootlin.com>:
>
> Hi Daniel,
>
> Daniel Kestrel <kestrelseventyfour@gmail.com> wrote on Tue, 3 Aug 2021
> 16:32:56 +0200:
>
> > Some lantiq xway devices use Micron NAND chips, which use on-die ECC.
> > The hardcoded setting of NAND_ECC_ENGINE_TYPE_SOFT makes them unusable,
> > because the software ECC on top of the hardware ECC produces errors for
> > every read and write access, not to mention that booting does not work,
> > because the boot loader uses the correct ECC when trying to load the
> > kernel and stops loading on severe ECC errors.
> > Removing the hardcoded settings would break a number of devices that
> > work with those settings.
> > Adding a DTB property was considered, but did not work, because devices
> > of the same type but from different manufacture dates have different
> > NAND chips and as such it is not possible to determine the NAND chip
> > in advance or device specific.
>
> I understand the problem and it is a very crappy situation.
>
> >
> > Signed-off-by: Daniel Kestrel <kestrelseventyfour@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/xway_nand.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/raw/xway_nand.c b/drivers/mtd/nand/raw/xw=
ay_nand.c
> > index 26751976e502..20cb5ce2f3b0 100644
> > --- a/drivers/mtd/nand/raw/xway_nand.c
> > +++ b/drivers/mtd/nand/raw/xway_nand.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/of_platform.h>
> >
> >  #include <lantiq_soc.h>
> > +#include "internals.h"
> >
> >  /* nand registers */
> >  #define EBU_ADDSEL1          0x24
> > @@ -148,7 +149,8 @@ static void xway_write_buf(struct nand_chip *chip, =
const u_char *buf, int len)
> >
> >  static int xway_attach_chip(struct nand_chip *chip)
> >  {
> > -     chip->ecc.engine_type =3D NAND_ECC_ENGINE_TYPE_SOFT;
> > +     if (chip->manufacturer.desc->id !=3D NAND_MFR_MICRON)
> > +             chip->ecc.engine_type =3D NAND_ECC_ENGINE_TYPE_SOFT;
>
> Could we make this a little bit clever with something like this:
> https://elixir.bootlin.com/linux/v5.13-rc7/source/drivers/mtd/nand/raw/na=
nd_micron.c#L434
>
> This is far from ideal, there should definitely be a change in the DT.
> But given your initial comments I guess it is not possible.
>
> Anyway I don't find a better way as, during the attach() call, we don't
> yet ran the manufacturer code, hence we don't know if on-die ECC is
> actually available or not.
>
> Thanks,
> Miqu=C3=A8l

Hi Miqu=C3=A8l,

I have worked with an other owner of lantiq hardware and he has successfull=
y
tested adding nand-ecc-mode =3D "soft" in the device tree and removing the =
one
line of code that sets the engine type to soft.
I will send another patch version that just removes this line of code.
This change
is required for kernel 5.4 as well, but the setting is in the xway_nand_pro=
be
method at line 237.

Thanks, Daniel.