RE: [PATCH v3 04/11] mtd: cfi_cmdset_0002: Call xip_enable() once only in do_write_buffer().

[IKEGAMI Tokunori <ikegami@xxxxxxxxxxxxxxxxxxxx>]

> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@xxxxxxxxxxx]
> Sent: Monday, November 05, 2018 10:20 PM
> To: IKEGAMI Tokunori
> Cc: boris.brezillon@xxxxxxxxxxxxxxxxxx; Fabio Bettoni; PACKHAM Chris;
> Joakim Tjernlund; linux-mtd@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 04/11] mtd: cfi_cmdset_0002: Call xip_enable() once
> only in do_write_buffer().
> 
> On Fri, 26 Oct 2018 01:32:12 +0900
> Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > By the removed goto statement it can be called xip_enable() once.
> > Also for a maintainability refactor it to call the function only once.
> 
> Would have worked with the op_done label too if you place the
> xip_enable() call just after this label, right?

Yes I think so.
Since both the change does not change the actual behavior as that the place to call xip_enable() is changed but it is called as same with the original.

Regards,
Ikegami

> 
> >
> > Signed-off-by: Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Fabio Bettoni <fbettoni@xxxxxxxxx>
> > Co: Hauke Mehrtens <hauke@xxxxxxxxxx>
> > Co: Koen Vandeputte <koen.vandeputte@xxxxxxxxxxxx>
> > Cc: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> > Cc: Joakim Tjernlund <Joakim.Tjernlund@xxxxxxxxxxxx>
> > Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> > Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx
> > ---
> > Changes since v2:
> > - None.
> >
> > Changes since v1:
> > - Split from the patch v1 3/3.
> >
> >  drivers/mtd/chips/cfi_cmdset_0002.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> > index deffafab067e..a3fa2d7b1ba0 100644
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -1882,10 +1882,8 @@ static int __xipram do_write_buffer(struct
> map_info *map, struct flchip *chip,
> >  			continue;
> >  		}
> >
> > -		if (chip_good(map, adr, datum)) {
> > -			xip_enable(map, chip, adr);
> > +		if (chip_good(map, adr, datum))
> >  			break;
> > -		}
> >
> >  		if (time_after(jiffies, timeo)) {
> >  			ret = -EIO;
> > @@ -1911,13 +1909,14 @@ static int __xipram do_write_buffer(struct
> map_info *map, struct flchip *chip,
> >  				 cfi->device_type, NULL);
> >  		cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start,
> map, cfi,
> >  				 cfi->device_type, NULL);
> > -		xip_enable(map, chip, adr);
> >  		/* FIXME - should have reset delay before continuing */
> >
> >  		printk(KERN_WARNING "MTD %s(): software timeout,
> address:0x%.8lx.\n",
> >  		       __func__, adr);
> >  	}
> >
> > +	xip_enable(map, chip, adr);
> > +
> >  	chip->state = FL_READY;
> >  	DISABLE_VPP(map);
> >  	put_chip(map, chip, adr);


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