[PULL v2] mtd: nand: Changes for 4.13

Boris Brezillon boris.brezillon at free-electrons.com
Wed Jul 12 00:19:19 PDT 2017


Le Tue, 11 Jul 2017 18:01:15 -0700,
Brian Norris <computersforpeace at gmail.com> a écrit :

> + Thomas, as I don't really want to dig up the original patch to review
> right now
> 
> On Mon, Jul 03, 2017 at 01:46:09PM +0200, Boris Brezillon wrote:
> > Hi,
> > 
> > Here's my PR for 4.13. This v2 includes Dan's fix ("mtd: nand: mtk:
> > release lock on error path").
> > 
> > As usual, let me know if you see any problem.
> > 
> > Thanks,
> > 
> > Boris
> > 
> > The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:
> > 
> >   Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)
> > 
> > are available in the git repository at:
> > 
> >   ssh://git.infradead.org/srv/git/l2-mtd.git tags/nand/for-4.13  
> 
> I happen to have SSH access to that repository :) but a
> publicly-readable link is usually a better idea, in the highly unlikely
> case that someone else wants to review your pull request.

My bad. I have 2 remotes for l2-mtd (l2-mtd and l2-mtd-pub), just passed
the wrong one to 'git request-pull' :-/.

> 
> > for you to fetch changes up to 81667e9c8ad827365f2d1e8b924caf062a19c593:
> > 
> >   mtd: nand: mtk: release lock on error path (2017-07-03 13:39:09 +0200)
> > 
> > ----------------------------------------------------------------
> > This pull request contains the following core changes:
> > 
> > * addition of on-ecc support to Micron driver  
> 
> This still works by overriding chip->ecc.{read,write}_page{,_raw}()
> callbacks... I never liked that, and there have been multiple
> submissions that tried this already, IIRC. It's for sure not going to
> work with at least one in-tree driver (brcmnand); I suspect others
> (qcom_nand? mtk_nand?).

Yes, I know it doesn't work with all drivers, but given that those
drivers are already not supporting ECC_NONE or ECC_SW, I don't think
it's a real program. 

> Can this be improved to either bail out when
> this clobbers a controller's existing callback, or even better, to
> utilize a controller's existing _raw() implementation, instead of
> assuming it can use the nand_base defaults?

IMO, if there's something to fix it's the NAND controller drivers that
are only partially complying with core expectations. I mean,
'->cmdfunc(READ0) + ->read_buf()' is a valid sequence and should work
with all NAND controllers, but for some (good?) reasons, some drivers
are taking liberties with this semantic.

Note that we are working on improving the situation with the
->exec_op() approach [4], which should provide NAND controller drivers
with all the information they need to execute a NAND operation
(CMD, ADDR, DATA, WAIT cycles), which, AFAIU was one blocking
aspect with '->cmdfunc()/->cmd_ctrl() + ->read/write_buf/byte/word()'. 

It will also let drivers return -ENOTSUPP to report when they simply
don't support a specific NAND operation (for example, when the
CMD+ADDR+DATA+WAIT sequence is not supported).

> 
> The latter seems difficult to do in time for this merge, but maybe the
> former can be done within the release?
> 
> I'm still tempted to pull this, since at least it does require somebody
> to opt in via the device tree binding. So they should probably make sure
> it works with their driver before using it.

There really no risk with this new feature because:

1/ the option has to be opted-in by the NAND controller driver (or
   through a DT property)
2/ even if it's enabled by mistake on one of these controllers, they
   either check ecc->mode [1][2] or simply override chip->ecc content
   unconditionally [3]

If one NAND controller driver is not doing #2, the driver should be
fixed not the on-die ECC logic.

> 
> > * addition of helpers to help drivers choose most appropriate ECC
> >   settings
> > * deletion of dead-code (cached programming and ->errstat() hook)
> > * make sure drivers that do not support the SET/GET FEATURES command
> >   return ENOTSUPP use a dummy ->set/get_features implementation
> >   returning -ENOTSUPP (required for Micron on-die ECC)
> > * change the semantic of ecc->write_page() for drivers setting the
> >   NAND_ECC_CUSTOM_PAGE_ACCESS flag
> > * support exiting 'GET STATUS' command in default ->cmdfunc()
> >   implementations
> > * change the prototype of ->setup_data_interface()
> > 
> > A bunch of driver related changes:
> > 
> > * various cleanup, fixes and improvements of the MTK driver
> > * OMAP DT bindings fixes
> > * support for ->setup_data_interface() in the fsmc driver
> > * support for imx7 in the gpmi driver
> > * finalization of the denali driver rework (thanks to Masahiro for the
> >   work he's done on this driver)
> > * fix "bitflips in erased pages" handling in the ifc driver
> > * addition of PM ops and dynamic timing configuration to the atmel
> >   driver
> > 
> > And as usual we also have a few minor cleanup/fixes/improvements
> > patches across the subsystem.
> >   
> 
> Brian

[1]http://elixir.free-electrons.com/linux/latest/source/drivers/mtd/nand/brcmnand/brcmnand.c#L2121
[2]http://elixir.free-electrons.com/linux/latest/source/drivers/mtd/nand/mtk_nand.c#L1171
[3]http://elixir.free-electrons.com/linux/latest/source/drivers/mtd/nand/qcom_nandc.c#L1840
[4]https://github.com/linux-nand/linux/commits/nand/exec_op



More information about the linux-mtd mailing list