RE: [PATCH v2 06/10] mtd: cfi_cmdset_0002: Split do_write_oneword() op_done goto statement

[IKEGAMI Tokunori <ikegami@xxxxxxxxxxxxxxxxxxxx>]

> -----Original Message-----
> From: Chris Packham [mailto:Chris.Packham@xxxxxxxxxxxxxxxxxxx]
> Sent: Thursday, October 25, 2018 6:34 AM
> To: IKEGAMI Tokunori; boris.brezillon@xxxxxxxxxxxxxxxxxx
> Cc: Fabio Bettoni; Joakim Tjernlund; linux-mtd@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 06/10] mtd: cfi_cmdset_0002: Split
> do_write_oneword() op_done goto statement
> 
> On 20/10/18 5:55 AM, Tokunori Ikegami wrote:
> > To reduce function size and remove the goto statement split the op_done
> goto
> > statement part into do_write_oneword_done() created a function.
> >
> > 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 v1:
> > - Add the patch.
> >
> >   drivers/mtd/chips/cfi_cmdset_0002.c | 21 +++++++++++++++------
> >   1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> > index ae2d8bd7154e..2340a5a7cc39 100644
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -1609,6 +1609,20 @@ static int __xipram do_write_oneword_once(struct
> map_info *map, struct flchip *c
> >   	return ret;
> >   }
> >
> > +static void __xipram do_write_oneword_done(struct map_info *map,
> > +					   struct flchip *chip,
> > +					   unsigned long adr, int mode)
> > +{
> > +	if (mode == FL_OTP_WRITE)
> > +		otp_exit(map, chip, adr, map_bankwidth(map));
> > +
> > +	chip->state = FL_READY;
> > +	DISABLE_VPP(map);
> > +	put_chip(map, chip, adr);
> > +
> > +	mutex_unlock(&chip->mutex);
> 
> Personally I find it easier if the mutex_lock()/mutex_unlock() are in
> the same function. That way it's easier to audit the exit paths from a
> single function rather than needing to know that some functions
> implicitly release the lock. The same probably applies to
> get_chip/put_chip pairs.

[Ikegami] Thank you. I could understand well. Fixed but still remain the done function.
But added the start function to find easier the pairs instead of not change to split as the done function.
Also for the exit paths to find easier added the patch 11 additionally.

> 
> > +}
> > +
> >   static int __xipram do_write_oneword(struct map_info *map, struct
> flchip *chip,
> >   				     unsigned long adr, map_word datum,
> >   				     int mode)
> > @@ -1670,12 +1684,7 @@ static int __xipram do_write_oneword(struct
> map_info *map, struct flchip *chip,
> >   	xip_enable(map, chip, adr);
> >
> >    op_done:
> > -	if (mode == FL_OTP_WRITE)
> > -		otp_exit(map, chip, adr, map_bankwidth(map));
> > -	chip->state = FL_READY;
> > -	DISABLE_VPP(map);
> > -	put_chip(map, chip, adr);
> > -	mutex_unlock(&chip->mutex);
> > +	do_write_oneword_done(map, chip, adr, mode);
> >
> >   	return ret;
> >   }
> >


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