Re: [PATCH 08/23] mtd: rawnand: Pass a nand_chip object to ecc->read_xxx() hooks

[Boris Brezillon <boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>]

Hi Stefan,

On Mon, 20 Aug 2018 09:57:47 +0200
Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org> wrote:

> On 19.08.2018 13:26, Boris Brezillon wrote:
> > Hi Stefan,
> > 
> > On Sat, 18 Aug 2018 10:30:13 +0200
> > Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org> wrote:
> >   
> >> > diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> >> > b/drivers/mtd/nand/raw/tegra_nand.c
> >> > index 5dcee20e2a8c..bcc3a2888c4f 100644
> >> > --- a/drivers/mtd/nand/raw/tegra_nand.c
> >> > +++ b/drivers/mtd/nand/raw/tegra_nand.c
> >> > @@ -615,10 +615,10 @@ static int tegra_nand_page_xfer(struct mtd_info
> >> > *mtd, struct nand_chip *chip,
> >> >  	return ret;
> >> >  }
> >> >
> >> > -static int tegra_nand_read_page_raw(struct mtd_info *mtd,
> >> > -				    struct nand_chip *chip, u8 *buf,
> >> > +static int tegra_nand_read_page_raw(struct nand_chip *chip, u8 *buf,
> >> >  				    int oob_required, int page)
> >> >  {
> >> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> >> >  	void *oob_buf = oob_required ? chip->oob_poi : NULL;
> >> >
> >> >  	return tegra_nand_page_xfer(mtd, chip, buf, oob_buf,  
> >>
> >> Since mtd is only required to pass it to tegra_nand_page_xfer, it would
> >> be better to change tegra_nand_page_xfer to only take chip.  
> > 
> > For sure, but that's the sort of cleanups I'll leave to NAND controller
> > driver maintainers (in this case you ;-)). I only take care of the NAND
> > API here and try to make things as simple as possible to ease review and
> > avoid breaking drivers.  
> 
> Understand, but that change makes your patch simpler... Or did create
> those patches automatically? In that case it makes sense to avoid manual
> changes.

I could have written a coccinelle script, but every time I tried it
took me more time than using git grep + manual patching :-).

> 
> I can send a follow up patch no problem, but if you do a v2 and did the
> chagnes manually anyway, I really think it can go into this patchset.

The thing is, if I do that for tegra, why not doing it for other
drivers, and then the patch becomes really hard to review. So yes, I'd
prefer to keep the changes as dumb as possible and let each maintainer
cleanup their driver (which basically means, stop passing mtd_info to
all internal functions, not only this one).

Regards,

Boris