2023-04-07 07:59:45

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma()

On 07/04/2023 at 08:22, Markus Elfring wrote:
> Date: Fri, 7 Apr 2023 08:08:59 +0200
>
> The label “error” was used to jump to another pointer check despite of
> the detail in the implementation of the function “atmel_spi_configure_dma”
> that it was determined already that the corresponding variable
> contained an error pointer because of a failed call of
> the function “dma_request_chan”.
>
> * Thus use more appropriate labels instead.
>
> * Delete two redundant checks.
>
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 398b6b310ec85eef9d98df5963d5ded18aa92ad8 ("spi: atmel: switch to use modern name")
> Signed-off-by: Markus Elfring <[email protected]>

It's becoming a pattern, but still:
NACK.

Regards,
Nicolas

> ---
> drivers/spi/spi-atmel.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 7f06305e16cb..ed8dc93c73e5 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -511,12 +511,12 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
> * requested tx channel.
> */
> dev_dbg(dev, "No RX DMA channel, DMA is disabled\n");
> - goto error;
> + goto release_channel_tx;
> }
>
> err = atmel_spi_dma_slave_config(as, 8);
> if (err)
> - goto error;
> + goto release_channel_rx;
>
> dev_info(&as->pdev->dev,
> "Using %s (tx) and %s (rx) for DMA transfers\n",
> @@ -524,11 +524,11 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
> dma_chan_name(host->dma_rx));
>
> return 0;
> -error:
> - if (!IS_ERR(host->dma_rx))
> - dma_release_channel(host->dma_rx);
> - if (!IS_ERR(host->dma_tx))
> - dma_release_channel(host->dma_tx);
> +
> +release_channel_rx:
> + dma_release_channel(host->dma_rx);
> +release_channel_tx:
> + dma_release_channel(host->dma_tx);
> error_clear:
> host->dma_tx = host->dma_rx = NULL;
> return err;
> --
> 2.40.0
>

--
Nicolas Ferre