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

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Wed, 27 May
2020 10:57:49 +0200:

> 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.

You are right.

STM32_FMC and Marvell controllers access these timings.

I can have a "interface_cfg" and a "best_interface_cfg". Both being
just pointers. nand_get_interface_cfg would return the interface_cfg
pointer which would always be set (either to the default basic
configuration or the quick one). 

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