Re: [PATCH v5 28/28] mtd: rawnand: Allocate the best data interface structure dynamically

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Wed, 27 May 2020 10:49:11 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Wed, 27 May
> 2020 10:35:19 +0200:
> 
> > On Wed, 27 May 2020 09:57:32 +0200
> > Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> >   
> > > Maybe I misunderstood your request, you were saying that allocating a
> > > "best data interface object" would be good, so I interpreted it as:
> > > rename it, and allocated it dynamically. I'm fine keeping
> > > data_interface and just declaring it as a pointer.    
> > 
> > Correct, renaming it into best_iface_cfg is probably good, but then,
> > maybe we should have a current_iface_cfg, so the core/drivers always
> > have a pointer to the currently applied config (which after a reset
> > can be the reset config for a short period of time).  
> 
> That's why I created an indirection on chip->data_interface.
> nand_get_interface_cfg() is here for that -> the drivers do not care
> about which one is applied. I don't think we need more than I already
> proposed:
> -> there is one default reset configuration object that can be used by  
>    anyone
> -> there is a best configuration  
> 
> If the best configuration has been derived, then it will be used.
> Otherwise, the helper will fallback to the default slower one, and this
> covers all the cases :)

My point is, during a reset, you want to use the reset config, but
nand_get_interface_cfg() will always return the best config, which is
not the one currently applied until you actually return back to the
best interface config. That means that any driver using
nand_get_interface_cfg() in the exec_op() path will get a wrong config.
So I'd suggest either not exposing nand_get_interface_cfg() to
controller drivers (which would be a valid approach) or making sure it
always return the currently applied timings.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/