Re: [PATCH v2] i2c: tegra: Fix failure during probe deferral cleanup

[Andi Shyti <andi.shyti@kernel.org>]

Hi Thierry,

On Fri, Jul 07, 2023 at 03:26:19PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> If the driver fails to obtain a DMA channel, it will initiate cleanup
> and try to release the DMA channel that couldn't be retrieved. This will
> cause a crash because the cleanup will try to dereference an ERR_PTR()-
> encoded error code.

while this is a valid solution I think the best thing here is to
clear the exit path by adding another goto label.

By setting dma_chan = NULL you would go through some extra checks
that are not needed.

I guess that by doing this we could even remove the

	if (i2c_dev->dma_buf)
	if (i2c_dev->dma_chan)

in tegra_i2c_release_dma(). However you see it cleaner. I'm not
going to be picky, though. Let me know if you are up for some
more clean up, otherwise I can give you an r-b... after a little
clarification...

> However, there's nothing to clean up at this point yet, so we can avoid
> this by simply resetting the DMA channel to NULL instead of storing the
> error code.
> 
> Fixes: fcc8a89a1c83 ("i2c: tegra: Share same DMA channel for RX and TX")

... is this the correct commit that is getting fixed? I think
it's this one:

Fixes: 86c92b9965ff ("i2c: tegra: Add DMA support")
Cc: <stable@vger.kernel.org> # v5.1+

Thanks,
Andi

> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - simplify patch by setting dma_chan = NULL on channel request failure
> 
>  drivers/i2c/busses/i2c-tegra.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index f155e9028f94..2a13f11edfd1 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -460,6 +460,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
>  	i2c_dev->dma_chan = dma_request_chan(i2c_dev->dev, "tx");
>  	if (IS_ERR(i2c_dev->dma_chan)) {
>  		err = PTR_ERR(i2c_dev->dma_chan);
> +		i2c_dev->dma_chan = NULL;
>  		goto err_out;
>  	}
>  
> -- 
> 2.41.0
>