Re: [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Hi Adam,

On Sun, 3 Dec 2017 08:57:17 -0600
Adam Ford <aford173@gmail.com> wrote:

> On Fri, Aug 4, 2017 at 3:32 AM, Boris Brezillon <
> boris.brezillon@free-electrons.com> wrote:
> 
> > On Tue, 16 May 2017 00:17:42 +0200
> > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >  
> > > Commit 7d70f334ad2b ("mtd: nand: add lock/unlock routines") introduced
> > > support for the Micron LOCK/UNLOCK commands but no one ever used the
> > > nand_lock/unlock() functions.
> > >
> > > Remove support for these vendor-specific operations from the core. If
> > > one ever wants to add them back they should be put in nand_micron.c and
> > > mtd->_lock/_unlock should be directly assigned from there instead of
> > > exporting the functions.  
> >
> >  
> I am running into some issues with a board using Micron Flash that cannot
> boot from a UBIFS partition and it appears to be related the NAND being
> locked.  I was going to investigate how to unlock the NAND and I came
> across this.  I am attempting to do what you suggested by moving these
> functions to the nand_micron.c file, but these functions are dependant on
> several static functions inside nand_base.c
> namely: check_offs_len, nand_get_device, nand_check_wp,
> and nand_release_device.  I assume that we don't want to export all those
> functions, but it seems redundant to copy these functions to nand_micron.c.

I don't see a problem in exposing those symbols so that NAND vendor
drivers can use them (note that you don't need to use EXPORT_SYMBOL()
in this case, because nand_xxx.c files are all linked in a single
object)?

This being said, I think the check_offs_len() can be dropped since it's
already checked in mtd_[un]lock().

> 
> Do you have any thoughts or suggestions on how to go about using these
> functions without replicating code?

I have no objection to adding this feature back in the Micron driver,
though I'd like to wait for the ->exec_op() series [1] to be merged,
because I'm not sure all NAND controller drivers handle the UNLOCK/LOCK
operations properly (custom ->cmdfunc() implementations tend to only
support basic operations). The idea is to check whether the controller
supports the LOCK/UNLOCK operations and in this case assign the
mtd->_lock/_unlock() hooks to micron's implementation.

Regards,

Boris

[1]http://patchwork.ozlabs.org/project/linux-mtd/list/?series=16013&state=%2A&archive=both