Re: [PATCH v3 2/2] mtd: rawnand: add hooks that may be called during nand_scan()

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Miquel,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Fri, 27 Jul 2018
00:47:41 +0200:

> Hi Boris, Stefan,
> 
> Boris Brezillon <boris.brezillon@bootlin.com> wrote on Fri, 27 Jul 2018
> 00:40:32 +0200:
> 
> > On Fri, 27 Jul 2018 00:20:50 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> >   
> > > On 26.07.2018 20:12, Boris Brezillon wrote:    
> > > > On Thu, 26 Jul 2018 18:22:10 +0200
> > > > Stefan Agner <stefan@agner.ch> wrote:      
> > > >> > +/**
> > > >> > + * struct nand_controller_ops - Controller operations
> > > >> > + *
> > > >> > + * @attach_chip: Callback that will be called between nand_detect() and
> > > >> > + *		 nand_scan_tail() during nand_scan() (optional).
> > > >> > + * @detach_chip: Callback that will be called from nand_cleanup() or if
> > > >> > + *		 nand_scan_tail() fails (optional).      
> > > >>
> > > >> This documentation reads not very helpful to me.
> > > >>
> > > >> It would be useful if it is written more from the driver developers
> > > >> perspective, e.g. what those callbacks ideally are supposed to do...
> > > >>      
> > > > 
> > > > Indeed. How about:
> > > > 
> > > > 	@attach_chip: this method is called between after the NAND      
> > > 
> > > I guess just after.    
> > 
> > Yep.
> >   
> > >     
> > > > 		      detection phase to let controller driver
> > > > 		      tweak/customize the configuration based on the
> > > > 		      NAND properties (page size, OOB size, ECC
> > > > 		      requirements, ...).      
> > > 
> > > Maybe explicitly state that NAND flash parameters are available in this
> > > call, e.g.
> > > 
> > > 	@attach_chip: this method is called after the NAND detection 
> > > 		      phase after flash ID and MTD fields such as
> > > 		      erase size, page size and OOB size have been
> > > 		      set up. ECC requirements are available if
> > > 		      provided by the NAND chip or device tree.
> > > 		      Typically used to chose the appropriate ECC    
> > 
> > 					^ choose
> >   
> > > 		      config and allocate associated resources.
> > > 		      This hook is optional.
> > >     
> > 
> > Sounds good.
> >   
> > >     
> > > > 	@detach_chip: free all resources allocated/claimed in
> > > > 		      nand_controller_ops->detach_chip().      
> > > 
> > > That probably should read nand_controller_ops->attach_chip().    
> > 
> > And yes, I meant attach_chip().
> >   
> > >     
> > > > 		      This hook is optional.      
> 
> Maybe I've been a bit lazy on the documentation :)
> 
> Thanks Stefan for pointing it, I'll fix when applying with the above
> proposal.

Actually I already applied that patch, but I decided to edit it to
change the kernel doc.

Thanks,
Miquèl