From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.bootlin.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fMxMm-000751-P1 for linux-mtd@lists.infradead.org; Sun, 27 May 2018 15:13:55 +0000 Date: Sun, 27 May 2018 17:13:37 +0200 From: Boris Brezillon To: Miquel Raynal Cc: Stefan Agner , Benjamin Lindqvist , dwmw2@infradead.org, computersforpeace@gmail.com, marek.vasut@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, thierry.reding@gmail.com, mturquette@baylibre.com, sboyd@kernel.org, Lucas Stach , richard@nod.at, marcel@ziswiler.com, krzk@kernel.org, digetx@gmail.com, jonathanh@nvidia.com, pdeschrijver@nvidia.com, pgaikwad@nvidia.com, Mirza Krak , linux-mtd@lists.infradead.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Message-ID: <20180527171337.002eeb9f@bbrezillon> In-Reply-To: <20180527161832.1152be6e@xps13> References: <86fdf19ec92b732709732fb60199f16488b4b727.1526990589.git.stefan@agner.ch> <20180524135335.6aa0b7a4@bbrezillon> <146a3abbbff4dcef30ad662a0fb85ff1@agner.ch> <20180527161832.1152be6e@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sun, 27 May 2018 16:18:32 +0200 Miquel Raynal wrote: > Hi Stefan, >=20 > On Thu, 24 May 2018 14:19:18 +0200, Stefan Agner > wrote: >=20 > > On 24.05.2018 13:53, Boris Brezillon wrote: =20 > > > Hi Benjamin, > > >=20 > > > On Thu, 24 May 2018 13:30:14 +0200 > > > Benjamin Lindqvist wrote: > > > =20 > > >> Hi Stefan, > > >> > > >> It seems to me that a probe similar to what the BootROM does shouldn= 't > > >> be awfully complicated to implement - just cycle through the switch > > >> cases in case of an ECC error. But I guess that's more of an idea for > > >> further improvements rather than a comment to the patch set under > > >> review. =20 > > >=20 > > > Nope, not really an option, because you're not guaranteed that the NA= ND > > > will be used as a boot media, and the first page or first set of pages > > > might just be erased. > > > =20 > >=20 > > Yeah I did not meant probing like the Boot ROM does. > >=20 > > What I meant was using only the ECC modes which are supported by the > > Boot ROM when the driver tries to choose a viable mode. So that would > > be: > > - RS t=3D4 > > - BCH t=3D8 > > - BCH t=3D16 > >=20 > > Maybe we could add a property to enable that behavior: > >=20 > > tegra,use-bootable-ecc-only; =20 >=20 > I'm not sure a property is needed. >=20 > As there is currently no official user of this driver, why not turning > mandatory the nand-ecc-xxx properties? Not a big fan of this solution. We already have a few cases where the NAND part was changed on a design and the new NAND had different ECC requirements, With your suggestion, that means creating a new .dts file for each possible NAND part. Note that having a solution that picks the best ECC config based on chip->ecc_xxx_ds should be the preferred approach. nand-ecc- props are mainly here to address the case where you need/want to assign a config that does not match the ECC requirements exposed by the chip. > In the documentation you can add > a note saying that using other algorithms than the three above is not > supported by the BootROM. >=20 > > =20 > > >> > > >> However, I think that allowing for an override of the oobsize > > >> inference would be a good idea before merging, no? This could just be > > >> a trivial #ifdef (at least temporarily). If you agree but don't feel > > >> like doing it yourself, I'd be happy to pitch in. Let me know. =20 > > >=20 > > > That's why we have nand-ecc-xxx properties in the DT. > > > =20 > >=20 > > Yes, nand-ecc-strength is the first thing I plan to implement, that way > > strength can be defined in dt. > >=20 > > -- > > Stefan =20 >=20 > Thanks, > Miqu=C3=A8l