> > + ret = tegra_i2c_init_dma(i2c_dev);
> > + if (ret < 0)
> > + goto disable_div_clk;
> > +
> > ret = tegra_i2c_init(i2c_dev);
> > if (ret) {
> > dev_err(&pdev->dev, "Failed to initialize i2c controller\n");
> > @@
> > -1160,6 +1500,7 @@ static int tegra_i2c_probe(struct platform_device
> > *pdev)
> > disable_div_clk:
> > if (i2c_dev->is_multimaster_mode)
> > clk_disable(i2c_dev->div_clk);
> > + tegra_i2c_release_dma(i2c_dev);
>
> Please use "release_dma:" variant that I suggested in the comment to v14
> because:
>
> 1) It's just a good (and common-style in kernel) tone to unwind errors
> handling in the opposite order, it makes code more straight forward and helps
> to avoid silly mistakes.
>
> 2) It keeps the current code consistent in regards to probe error-handling.
>
Can move tegra_i2c_release_dma under the same disable_div_clk label before
clk_disable
Order in probe is
clk_prepare
i2c_runtime_resume
clk_enable in case of multimaster mode
i2c_init_dma
unwinding should be order is
release_dma
disable_div_clk
disable_rpm
unprepared
Probe failure from tegra_i2c_init_dma, tegra_i2c_init, devm_request_irq,
i2c_add_numbered_adapter all of them perform goto disable_div_clk
I can add tegra_i2c_release_dma before performing clock disable under same
disable_div_clk so all existing disable_div_clk labels remain same.
Otherwise adding new label release_dma causes all goto disable_div_clk
statement also to be changed but its same as adding release_dma under
disable_div_clk above the clk_disable.
> >
> > disable_rpm:
> > pm_runtime_disable(&pdev->dev);
>
>
> [snip]
>
> Please also address my comment to the "fix maximum transfer size" patch. And
> please add Wolfram Sang to the CC list.
>
> Looks like v16 should be the final, so with everything being addressed:
>
> Reviewed-by: Dmitry Osipenko <[email protected]>
> Tested-by: Dmitry Osipenko <[email protected]>
