Hi Mukesh,
> > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> > index 01db24188e29..3fc85595a4aa 100644
> > --- a/drivers/i2c/busses/i2c-qcom-geni.c
> > +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> > @@ -867,14 +867,13 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > ret = geni_se_resources_on(&gi2c->se);
> > if (ret) {
> > - clk_disable_unprepare(gi2c->core_clk);
> > - return dev_err_probe(dev, ret, "Error turning on resources\n");
> > + dev_err_probe(dev, ret, "Error turning on resources\n");
> > + goto err_clk;
> > }
> > proto = geni_se_read_proto(&gi2c->se);
> > if (proto != GENI_SE_I2C) {
> > - geni_se_resources_off(&gi2c->se);
> > - clk_disable_unprepare(gi2c->core_clk);
> > - return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
> > + dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
> Suggestive comment, can we make this second patch as first patch ? So that
> we can have both above lines reduced in this patch.
I'm sorry, I missed this comment. I tried to swap it and there is
not much reduction in line changes[*].
The reason I chose this order is to avoid changing the same line
on both the patches, like here[**].
If it's not binding for you, I would keep the ordering.
Thanks again a lot for looking into this,
Andi
[*] https://paste.debian.net/1339486/
[**] https://paste.debian.net/1339488/
> > + goto err_off;
> > }
> > if (desc && desc->no_dma_support)
> > @@ -886,11 +885,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > /* FIFO is disabled, so we can only use GPI DMA */
> > gi2c->gpi_mode = true;
> > ret = setup_gpi_dma(gi2c);
> > - if (ret) {
> > - geni_se_resources_off(&gi2c->se);
> > - clk_disable_unprepare(gi2c->core_clk);
> > - return ret;
> > - }
> > + if (ret)
> > + goto err_off;
> > dev_dbg(dev, "Using GPI DMA mode for I2C\n");
> > } else {
> > @@ -902,10 +898,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > tx_depth = desc->tx_fifo_depth;
> > if (!tx_depth) {
> > - geni_se_resources_off(&gi2c->se);
> > - clk_disable_unprepare(gi2c->core_clk);
> > - return dev_err_probe(dev, -EINVAL,
> > - "Invalid TX FIFO depth\n");
> > + dev_err_probe(dev, -EINVAL, "Invalid TX FIFO depth\n");
> > + goto err_off;
> > }
> > gi2c->tx_wm = tx_depth - 1;
> > @@ -944,8 +938,16 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > return 0;
> return ret here ? yes, we need to initialize ret = 0.
> > +err_off:
> can we rename as err_resources ?
> > + geni_se_resources_off(&gi2c->se);
> > +err_clk:
> > + clk_disable_unprepare(gi2c->core_clk);
> > +
> > + return ret;
> > +
> > err_dma:
> > release_gpi_dma(gi2c);
> > +
> > return ret;
> > }
>
