RE: [PATCH] spi: aspeed: Fix an error handling path in aspeed_spi_[read|write]_user()

[Chin-Ting Kuo <chin-ting_kuo@xxxxxxxxxxxxxx>]

Hi Christophe,

Thanks for the patch.

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> Sent: Wednesday, November 20, 2024 5:30 AM
> Subject: [PATCH] spi: aspeed: Fix an error handling path in
> aspeed_spi_[read|write]_user()
> 
> A aspeed_spi_start_user() is not balanced by a corresponding
> aspeed_spi_stop_user().
> Add the missing call.
> 
> Fixes: e3228ed92893 ("spi: spi-mem: Convert Aspeed SMC driver to spi-mem")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> ---
> Compile tested only.
> 
> 
> This patch is completely speculative, review with care!
> 
> It is only based on naming where a _start() function if not followed by a
> _stop() in some paths but is in other paths.
> ---
>  drivers/spi/spi-aspeed-smc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c index
> 8eb843ddb25f..e9beae95dded 100644
> --- a/drivers/spi/spi-aspeed-smc.c
> +++ b/drivers/spi/spi-aspeed-smc.c
> @@ -239,7 +239,7 @@ static ssize_t aspeed_spi_read_user(struct
> aspeed_spi_chip *chip,
> 
>  	ret = aspeed_spi_send_cmd_addr(chip, op->addr.nbytes, offset,
> op->cmd.opcode);
>  	if (ret < 0)
> -		return ret;
> +		goto stop_user;
> 
>  	if (op->dummy.buswidth && op->dummy.nbytes) {
>  		for (i = 0; i < op->dummy.nbytes / op->dummy.buswidth; i++) @@
> -249,8 +249,9 @@ static ssize_t aspeed_spi_read_user(struct aspeed_spi_chip
> *chip,
>  	aspeed_spi_set_io_mode(chip, io_mode);
> 
>  	aspeed_spi_read_from_ahb(buf, chip->ahb_base, len);

How about adding a blank line here?

> +stop_user:
>  	aspeed_spi_stop_user(chip);
> -	return 0;
> +	return ret;
>  }
> 
>  static ssize_t aspeed_spi_write_user(struct aspeed_spi_chip *chip, @@
> -261,10 +262,11 @@ static ssize_t aspeed_spi_write_user(struct
> aspeed_spi_chip *chip,
>  	aspeed_spi_start_user(chip);
>  	ret = aspeed_spi_send_cmd_addr(chip, op->addr.nbytes, op->addr.val,
> op->cmd.opcode);
>  	if (ret < 0)
> -		return ret;
> +		goto stop_user;
>  	aspeed_spi_write_to_ahb(chip->ahb_base, op->data.buf.out,
> op->data.nbytes);

A blank line here?

> +stop_user:
>  	aspeed_spi_stop_user(chip);
> -	return 0;
> +	return ret;
>  }
> 
>  /* support for 1-1-1, 1-1-2 or 1-1-4 */
> --
> 2.47.0

Chin-Ting