Re: [RFC PATCH 0/7] mtd: nand: Abstract away the NAND interface type

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

Hi Brian,

On Sat, 8 Oct 2016 22:34:06 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Thu, Sep 22, 2016 at 12:12:53PM +0200, Boris Brezillon wrote:
> > Hi,
> > 
> > This series is aiming at providing a generic NAND layer to share code
> > between different NAND based devices.
> > 
> > We currently have 3 different interfaces to interact with NANDs:
> > - Raw NANDs
> > - OneNANDs
> > - SPI NANDs
> > 
> > Apart from the way these NAND devices are accessed they have a lot
> > in common, like the way the memory is organized, or their constraints.
> > This is usually a good sign that some work should be done to factorize
> > the code.
> > 
> > This work has been started by Peter who wanted to re-use the BBT
> > code for its SPI-NAND driver. But I think we can push it further
> > other stuff (the software ECC implementation, or the way offsets are
> > converted to block/page number).
> > 
> > Before I continue in this direction, I'd like to get some feedback
> > from Peter and those who reviewed his initial submission (Brian,
> > Ezequiel) [1], or anyone who is interested in this topic.  
> 
> My eyes are bleeding for two reasons:

Sorry for your eyes :-).

> 
> (1) You haven't used any of git's nice rename detection for your patches

Yep, as already answered to Ezequiel on IRC, I forgot to pass -M when
generating patches with git format-patch. Will be addressed in v2.

> (2) The 'rawnand' naming seems a bit much for me. If we really need to
>     reorganize everything, keeping the name shorter, like just 'raw'
>     might be fine -- at least wherever we're already namespaces as
>     "nand". e.g., 'drivers/mtd/nand/raw/' and 'mtd: nand: raw:
>     commit subject', but you might still keep 'rawnand.h' (unless you
>     want to move things into an include/linux/mtd/nand/ subdirectory).

That's also something Ezequiel complained about. I'm perfectly fine
dropping the nand suffix when it's unneeded (will be fixed in v2).

> 
> You'll also need to update MAINTAINERS for the header pattern.

Sure, I'll patch MAINTAINERS to describe the new patterns.

> 
> Haven't looked too closely at the code yet.

Thanks for this preliminary review, but I was actually looking for a
more general review:

- Is the general approach OK (the fact that we now have a generic NAND
  layer to share code between different NAND based devices, no matter
  the interface they use)?
- Is the API defined at the generic NAND level (the interface-agnostic
  API) acceptable?

Thanks,

Boris