| Message ID | 20181025163219.25788-3-ikegami@allied-telesis.co.jp |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project | expand |
On Fri, 26 Oct 2018 01:32:10 +0900 Tokunori Ikegami <ikegami@allied-telesis.co.jp> wrote: > It is enough to use chip_good() only so chip_ready() is not necessary. I'd like a short explanation saying why chip_good() is enough: chip_good() is doing the same check chip_ready() is doing plus an extra check to make sure we end up with the data we wrote. > For this change the order to check timeout is also needed to chagne. ^change. And I don't think changing the order is a hard requirement, it's just better to avoid the case where the data update happens just after the timeout has expired. To sum-up, I'm okay with the diff, I'd just like the commit message to be adjusted. > > Signed-off-by: Tokunori Ikegami <ikegami@allied-telesis.co.jp> > Cc: Fabio Bettoni <fbettoni@gmail.com> > Co: Hauke Mehrtens <hauke@hauke-m.de> > Co: Koen Vandeputte <koen.vandeputte@ncentric.com> > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > Cc: linux-mtd@lists.infradead.org > --- > Changes since v2: > - None. > > Changes since v1: > - None. > > drivers/mtd/chips/cfi_cmdset_0002.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index 251c9e1675bd..c2e51768a02c 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -1882,14 +1882,14 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip, > continue; > } > > - if (time_after(jiffies, timeo) && !chip_ready(map, adr)) > - break; > - > if (chip_good(map, adr, datum)) { > xip_enable(map, chip, adr); > goto op_done; > } > > + if (time_after(jiffies, timeo)) > + break; > + > /* Latency issues. Drop the lock, wait a while and retry */ > UDELAY(map, chip, adr, 1); > }
Sorry for late to reply to the mail below. (The mail was unexpectedly moved from the inbox.) > -----Original Message----- > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com] > Sent: Monday, November 05, 2018 10:03 PM > To: IKEGAMI Tokunori > Cc: boris.brezillon@free-electrons.com; Fabio Bettoni; PACKHAM Chris; > Joakim Tjernlund; linux-mtd@lists.infradead.org > Subject: Re: [PATCH v3 02/11] mtd: cfi_cmdset_0002: Remove chip_ready() > from do_write_buffer() > > On Fri, 26 Oct 2018 01:32:10 +0900 > Tokunori Ikegami <ikegami@allied-telesis.co.jp> wrote: > > > It is enough to use chip_good() only so chip_ready() is not necessary. > > I'd like a short explanation saying why chip_good() is enough: > chip_good() is doing the same check chip_ready() is doing plus an extra > check to make sure we end up with the data we wrote. I see so I will update as this. > > > For this change the order to check timeout is also needed to chagne. > > > ^change. Will fix this. > > And I don't think changing the order is a hard requirement, it's just > better to avoid the case where the data update happens just after the > timeout has expired. I will update as so. > > To sum-up, I'm okay with the diff, I'd just like the commit message > to be adjusted. I see. Regards, Ikegami > > > > > Signed-off-by: Tokunori Ikegami <ikegami@allied-telesis.co.jp> > > Cc: Fabio Bettoni <fbettoni@gmail.com> > > Co: Hauke Mehrtens <hauke@hauke-m.de> > > Co: Koen Vandeputte <koen.vandeputte@ncentric.com> > > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > > Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com> > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > > Cc: linux-mtd@lists.infradead.org > > --- > > Changes since v2: > > - None. > > > > Changes since v1: > > - None. > > > > drivers/mtd/chips/cfi_cmdset_0002.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c > b/drivers/mtd/chips/cfi_cmdset_0002.c > > index 251c9e1675bd..c2e51768a02c 100644 > > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > > @@ -1882,14 +1882,14 @@ static int __xipram do_write_buffer(struct > map_info *map, struct flchip *chip, > > continue; > > } > > > > - if (time_after(jiffies, timeo) && !chip_ready(map, adr)) > > - break; > > - > > if (chip_good(map, adr, datum)) { > > xip_enable(map, chip, adr); > > goto op_done; > > } > > > > + if (time_after(jiffies, timeo)) > > + break; > > + > > /* Latency issues. Drop the lock, wait a while and retry > */ > > UDELAY(map, chip, adr, 1); > > }
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index 251c9e1675bd..c2e51768a02c 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -1882,14 +1882,14 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip, continue; } - if (time_after(jiffies, timeo) && !chip_ready(map, adr)) - break; - if (chip_good(map, adr, datum)) { xip_enable(map, chip, adr); goto op_done; } + if (time_after(jiffies, timeo)) + break; + /* Latency issues. Drop the lock, wait a while and retry */ UDELAY(map, chip, adr, 1); }
It is enough to use chip_good() only so chip_ready() is not necessary. For this change the order to check timeout is also needed to chagne. Signed-off-by: Tokunori Ikegami <ikegami@allied-telesis.co.jp> Cc: Fabio Bettoni <fbettoni@gmail.com> Co: Hauke Mehrtens <hauke@hauke-m.de> Co: Koen Vandeputte <koen.vandeputte@ncentric.com> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> Cc: linux-mtd@lists.infradead.org --- Changes since v2: - None. Changes since v1: - None. drivers/mtd/chips/cfi_cmdset_0002.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)