linux-kernel - Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 27 May 2018 18:30:55 +0200
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Stefan Agner <stefan@...er.ch>,
        Benjamin Lindqvist <benjamin.lindqvist@...ian.se>,
        dwmw2@...radead.org, computersforpeace@...il.com,
        marek.vasut@...il.com, robh+dt@...nel.org, mark.rutland@....com,
        thierry.reding@...il.com, mturquette@...libre.com,
        sboyd@...nel.org, Lucas Stach <dev@...xeye.de>, richard@....at,
        marcel@...wiler.com, krzk@...nel.org, digetx@...il.com,
        jonathanh@...dia.com, pdeschrijver@...dia.com, pgaikwad@...dia.com,
        Mirza Krak <mirza.krak@...il.com>,
        linux-mtd@...ts.infradead.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash
 controller driver

On Sun, 27 May 2018 17:54:03 +0200
Miquel Raynal <miquel.raynal@...tlin.com> wrote:

> Hi Boris,
> 
> On Sun, 27 May 2018 17:13:37 +0200, Boris Brezillon
> <boris.brezillon@...tlin.com> wrote:
> 
> > On Sun, 27 May 2018 16:18:32 +0200
> > Miquel Raynal <miquel.raynal@...tlin.com> wrote:
> >   
> > > Hi Stefan,
> > > 
> > > On Thu, 24 May 2018 14:19:18 +0200, Stefan Agner <stefan@...er.ch>
> > > wrote:
> > >     
> > > > On 24.05.2018 13:53, Boris Brezillon wrote:      
> > > > > Hi Benjamin,
> > > > > 
> > > > > On Thu, 24 May 2018 13:30:14 +0200
> > > > > Benjamin Lindqvist <benjamin.lindqvist@...ian.se> wrote:
> > > > >         
> > > > >> 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.        
> > > > > 
> > > > > Nope, not really an option, because you're not guaranteed that the NAND
> > > > > will be used as a boot media, and the first page or first set of pages
> > > > > might just be erased.
> > > > >         
> > > > 
> > > > Yeah I did not meant probing like the Boot ROM does.
> > > > 
> > > > 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=4
> > > > - BCH t=8
> > > > - BCH t=16
> > > > 
> > > > Maybe we could add a property to enable that behavior:
> > > > 
> > > > tegra,use-bootable-ecc-only;      
> > > 
> > > I'm not sure a property is needed.
> > > 
> > > 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.  
> 
> Ok, that's right it's a problem.
> 
> But then the driver has to choose a default algorithm if none is given.

Yep.

> In this case, should we select the one that fits best the NAND chip
> requirements, or shall we limit to the ones supported by the BootRom?

We should limit to the one used by the BootROM only if the NAND is used
as a boot medium.

> 
> The underlying question is: will we add a tegra,use-bootable-ecc-only
> property?

I guess this one is fine, because it's only adding a constraint on the
possible ECC modes that can be used, it's not forcing a specific ECC
strength.

Note that if we want to make this property generic we could name it
nand-is-boot-medium.