Hi Boris-san,
Thanks for your reviewing and advices.
> Is this really a bug fix? Doesn't look like a bug fix to me.
No as you mentioned it is not a bug fix but just a refactoring to reduce xip_enable() line.
> Also, every time you add Cc stable you should try to find the commit
> that introduced the bug. Sometime it's not possible because the bug
> existed before git was in use, but most of the time you'll find the
> offending commit using git blame.
>
> A fixes tag should be formatted like that:
>
> Fixes: <commit-id> ("commit subject")
Okay I will do that in future.
This is just FYI.
I have just confirmed that the xip_enable() line itself was implemented by the commit 02b15e343aeef.
For this patch it is not a bug fix so I will not add the Fixes line into the commit message.
But if needed it please let me know that.
Regards,
Ikegami
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@xxxxxxxxxxx]
> Sent: Wednesday, May 30, 2018 3:58 AM
> To: IKEGAMI Tokunori
> Cc: PACKHAM Chris; Brian Norris; David Woodhouse; Boris Brezillon; Marek
> Vasut; Richard Weinberger; Cyrille Pitchen;
> linux-mtd@xxxxxxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v7 5/5] mtd: cfi_cmdset_0002: Change erase one block
> to enable XIP once
>
> On Mon, 28 May 2018 08:28:01 +0900
> Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> > To enable XIP it is executed both normal and error cases.
> > This call can be moved after the for loop as same with erase chip.
> >
> > Signed-off-by: Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Joakim Tjernlund <Joakim.Tjernlund@xxxxxxxxxxxx>
> > Cc: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> > Cc: Brian Norris <computersforpeace@xxxxxxxxx>
> > Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> > Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> > Cc: Marek Vasut <marek.vasut@xxxxxxxxx>
> > Cc: Richard Weinberger <richard@xxxxxx>
> > Cc: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxxx>
> > Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx
> > Cc: stable@xxxxxxxxxxxxxxx
>
> Is this really a bug fix? Doesn't look like a bug fix to me.
>
> Also, every time you add Cc stable you should try to find the commit
> that introduced the bug. Sometime it's not possible because the bug
> existed before git was in use, but most of the time you'll find the
> offending commit using git blame.
>
> A fixes tag should be formatted like that:
>
> Fixes: <commit-id> ("commit subject")
>
> > ---
> > drivers/mtd/chips/cfi_cmdset_0002.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> > index 6adda4dc2007..6c681cbf2d37 100644
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -2389,13 +2389,10 @@ static int __xipram do_erase_oneblock(struct
> map_info *map, struct flchip *chip,
> > chip->erase_suspended = 0;
> > }
> >
> > - if (chip_good(map, adr, map_word_ff(map))) {
> > - xip_enable(map, chip, adr);
> > + if (chip_good(map, adr, map_word_ff(map)))
> > break;
> > - }
> >
> > if (time_after(jiffies, timeo)) {
> > - xip_enable(map, chip, adr);
> > printk(KERN_WARNING "MTD %s(): software
> timeout\n",
> > __func__);
> > ret = -EIO;
> > @@ -2418,6 +2415,7 @@ static int __xipram do_erase_oneblock(struct
> map_info *map, struct flchip *chip,
> > }
> >
> > chip->state = FL_READY;
> > + xip_enable(map, chip, adr);
> > DISABLE_VPP(map);
> > put_chip(map, chip, adr);
> > mutex_unlock(&chip->mutex);
