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

[Chris Packham <Chris.Packham@xxxxxxxxxxxxxxxxxxx>]

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.

> +}
> +
>   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/