Hi Thierry,
On Fri, Jul 07, 2023 at 03:26:19PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx> # v5.1+
Thanks,
Andi
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> 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
>
