Re: [PATCH 02/13] mtd: nand: fsmc: rework fsmc_nand_setup() to use ->setup_data_interface()

Boris Brezillon
2017-03-22T22:24:00Z

On Wed, 22 Mar 2017 23:05:53 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
>
> On Wed, 22 Mar 2017 22:56:17 +0100, Boris Brezillon wrote:
>
> > I'm not sure this is such a good idea to move default and DT timings
> > setting in the ->setup_data_interface() hook.
> >
> > ONFI NANDs are changing an internal parameter to switch to a specific
> > timing mode. If you let the core think that you configured the
> > controller to support this timing mode, while you actually configured
> > it with the default or DT timings it might not work as expected.
>
> So what do you suggest to keep the compatibility with the existing DT
> binding for this NAND controller?

We have 2 choices here:
1/ drop the old/static timings config and only rely on the dynamic
config. This is a bit risky, because we've only tested the new
timing conversion logic on one board.
2/ keep both solutions around until all boards have switched to the new
solution. In this case, you'll only set the ->setup_data_interface()
hook if timings are not defined in the DT

>
> We also need to take into account that the timings need to be
> reconfigured upon ->resume().

Yes, I know, and I also know how painful it is to add extra code for
backward compat, but I think abusing ->setup_data_interface() is even
worse.

To sum-up, if we go for solution #2, in your resume you'll have to
conditionally call the legacy timing setup logic (only if timings are
defined in the DT) in addition to the nand_reset() call.