Hi Miquel, Miquel Raynal <miquel.raynal at bootlin.com> wrote on Fri, 27 Jul 2018 00:47:41 +0200: > Hi Boris, Stefan, > > Boris Brezillon <boris.brezillon at bootlin.com> wrote on Fri, 27 Jul 2018 > 00:40:32 +0200: > > > On Fri, 27 Jul 2018 00:20:50 +0200 > > Stefan Agner <stefan at agner.ch> wrote: > > > > > On 26.07.2018 20:12, Boris Brezillon wrote: > > > > On Thu, 26 Jul 2018 18:22:10 +0200 > > > > Stefan Agner <stefan at 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