linux-kernel - Re: [PATCH 2/5] mtd: rawnand: qcom: remove driver specific block

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, 4 Nov 2018 16:56:27 +0100
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Abhishek Sahu <absahu@...eaurora.org>
Cc:     linux-arm-msm@...r.kernel.org,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        linux-kernel@...r.kernel.org, Marek Vasut <marek.vasut@...il.com>,
        linux-mtd@...ts.infradead.org, Richard Weinberger <richard@....at>,
        Andy Gross <andy.gross@...aro.org>,
        Brian Norris <computersforpeace@...il.com>,
        David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 2/5] mtd: rawnand: qcom: remove driver specific
 block_markbad function

Hi Abhishek,

On Fri, 20 Jul 2018 15:03:48 +0200
Boris Brezillon <boris.brezillon@...tlin.com> wrote:

> On Fri, 20 Jul 2018 17:46:38 +0530
> Abhishek Sahu <absahu@...eaurora.org> wrote:
> 
> > Hi Boris,
> > 
> > On 2018-07-19 03:13, Boris Brezillon wrote:  
> > > On Wed, 18 Jul 2018 23:23:50 +0200
> > > Miquel Raynal <miquel.raynal@...tlin.com> wrote:
> > >     
> > >> Boris,
> > >> 
> > >> Can you please check the change in qcom_nandc_write_oob() is
> > >> valid? I think it is but as this is a bit of a hack I prefer double 
> > >> checking.    
> > > 
> > > Indeed, it's hack-ish.
> > >     
> > >> 
> > >> Thanks,
> > >> Miquèl
> > >> 
> > >> 
> > >> Abhishek Sahu <absahu@...eaurora.org> wrote on Fri,  6 Jul 2018
> > >> 13:21:56 +0530:
> > >>     
> > >> > The NAND base layer calls write_oob() by setting bytes at
> > >> > chip->badblockpos with value non 0xFF for updating bad block status.
> > >> > The QCOM NAND controller skips the bad block bytes while doing normal
> > >> > write with ECC enabled. When initial support for this driver was
> > >> > added, the driver specific function was added temporarily for
> > >> > block_markbad() with assumption to change for raw read in NAND base
> > >> > layer. Moving to raw read for block_markbad() seems to take more time
> > >> > so this patch removes driver specific block_markbad() function by
> > >> > using following HACK in write_oob() function.
> > >> >
> > >> > Check for BBM bytes in OOB and accordingly do raw write for updating
> > >> > BBM bytes in NAND flash or normal write for updating available OOB
> > >> > bytes.    
> > > 
> > > Why don't we change that instead of patching the qcom driver to guess
> > > when the core tries to mark a block bad? If you're afraid of breaking
> > > existing drivers that might rely on the "write/read BBM in non-raw
> > > mode" solution (I'm sure some drivers are), you can always add a new
> > > flag in chip->options (NAND_ACCESS_BBM_IN_RAW_MODE) and only use raw
> > > accessors when this flag is set.
> > >     
> > 
> >   We started with that Only
> > 
> >   http://patchwork.ozlabs.org/patch/508565/
> > 
> >   and since we didn't conclude, we went for driver
> >   specific bad block check and mark bad block functions.
> > 
> >   Now, we wanted to get rid of driver specific functions
> > 
> >   1. For bad block check, we found the way to get the BBM bytes
> >      with ECC read. Controller updates BBM in separate register
> >      which we can read and update the same in OOB. Patch #1 of
> >      series does the same.
> > 
> >   2. For bad block mark, there is no way to update in ECC mode
> >      that's why we went for HACK to get rid of driver specific
> >      function.
> > 
> >   If adding flag is fine now then this HACK won't be required.  
> 
> Yep. I'm fine with that. Can you rebase the patch you pointed out on top
> of nand/next and move the flag to chip->options instead of
> chip->bbt_options + prefix it with NAND_ instead of NAND_BBT_?

I'm currently trying to get rid of chip->block_bad() (now placed in
chip->legacy.block_bad()), and I wanted to know if you were still
planning to submit the changes we discussed in this thread. If you
don't have time, please let me know and I'll try to do it.

Thanks,

Boris