| Message ID | 20241212135416.244504-2-andi.shyti@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Qcom Geni exit path cleanups | expand |
On 12/12/24 15:54, Andi Shyti wrote: > Replace classical dev_err with dev_err_probe in the probe > function for better error reporting. Also, use dev_err_probe in > cases where the error number is clear (e.g., -EIO or -EINVAL) to > maintain consistency. > > Additionally, remove redundant logging to simplify the code. > > Signed-off-by: Andi Shyti <andi.shyti@kernel.org> > --- > drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++----------------- > 1 file changed, 14 insertions(+), 19 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c > index 7a22e1f46e60..01db24188e29 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev) > return gi2c->irq; > > ret = geni_i2c_clk_map_idx(gi2c); > - if (ret) { > - dev_err(dev, "Invalid clk frequency %d Hz: %d\n", > - gi2c->clk_freq_out, ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n", > + gi2c->clk_freq_out); > > gi2c->adap.algo = &geni_i2c_algo; > init_completion(&gi2c->done); > @@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev) > /* Keep interrupts disabled initially to allow for low-power modes */ > ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN, > dev_name(dev), gi2c); > - if (ret) { > - dev_err(dev, "Request_irq failed:%d: err:%d\n", > - gi2c->irq, ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(dev, ret, > + "Request_irq failed: %d\n", gi2c->irq); > + > i2c_set_adapdata(&gi2c->adap, gi2c); > gi2c->adap.dev.parent = dev; > gi2c->adap.dev.of_node = dev->of_node; > @@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev) > > ret = geni_se_resources_on(&gi2c->se); > if (ret) { > - dev_err(dev, "Error turning on resources %d\n", ret); > clk_disable_unprepare(gi2c->core_clk); > - return ret; > + return dev_err_probe(dev, ret, "Error turning on resources\n"); > } > proto = geni_se_read_proto(&gi2c->se); > if (proto != GENI_SE_I2C) { > - dev_err(dev, "Invalid proto %d\n", proto); > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return -ENXIO; > + return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto); > } > > if (desc && desc->no_dma_support) > @@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > if (ret) { > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n"); > + return ret; I believe the removal is intentional, since setup_gpi_dma() reports better messages. > } > > dev_dbg(dev, "Using GPI DMA mode for I2C\n"); > @@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev) > tx_depth = desc->tx_fifo_depth; > > if (!tx_depth) { > - dev_err(dev, "Invalid TX FIFO depth\n"); > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return -EINVAL; > + return dev_err_probe(dev, -EINVAL, > + "Invalid TX FIFO depth\n"); > } > > gi2c->tx_wm = tx_depth - 1; > @@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > clk_disable_unprepare(gi2c->core_clk); > ret = geni_se_resources_off(&gi2c->se); > if (ret) { > - dev_err(dev, "Error turning off resources %d\n", ret); > + dev_err_probe(dev, ret, "Error turning off resources\n"); > goto err_dma; > } > > @@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > > ret = i2c_add_adapter(&gi2c->adap); > if (ret) { > - dev_err(dev, "Error adding i2c adapter %d\n", ret); > + dev_err_probe(dev, ret, "Error adding i2c adapter\n"); > pm_runtime_disable(gi2c->se.dev); > goto err_dma; > } Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> -- Best wishes, Vladimir
On 12/12/2024 7:24 PM, Andi Shyti wrote: > Replace classical dev_err with dev_err_probe in the probe > function for better error reporting. Also, use dev_err_probe in > cases where the error number is clear (e.g., -EIO or -EINVAL) to > maintain consistency. > > Additionally, remove redundant logging to simplify the code. > > Signed-off-by: Andi Shyti <andi.shyti@kernel.org> > --- > drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++----------------- > 1 file changed, 14 insertions(+), 19 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c > index 7a22e1f46e60..01db24188e29 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev) > return gi2c->irq; > > ret = geni_i2c_clk_map_idx(gi2c); > - if (ret) { > - dev_err(dev, "Invalid clk frequency %d Hz: %d\n", > - gi2c->clk_freq_out, ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n", > + gi2c->clk_freq_out); > > gi2c->adap.algo = &geni_i2c_algo; > init_completion(&gi2c->done); > @@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev) > /* Keep interrupts disabled initially to allow for low-power modes */ > ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN, > dev_name(dev), gi2c); > - if (ret) { > - dev_err(dev, "Request_irq failed:%d: err:%d\n", > - gi2c->irq, ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(dev, ret, > + "Request_irq failed: %d\n", gi2c->irq); > + > i2c_set_adapdata(&gi2c->adap, gi2c); > gi2c->adap.dev.parent = dev; > gi2c->adap.dev.of_node = dev->of_node; > @@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev) > > ret = geni_se_resources_on(&gi2c->se); > if (ret) { > - dev_err(dev, "Error turning on resources %d\n", ret); > clk_disable_unprepare(gi2c->core_clk); > - return ret; > + return dev_err_probe(dev, ret, "Error turning on resources\n"); > } > proto = geni_se_read_proto(&gi2c->se); > if (proto != GENI_SE_I2C) { > - dev_err(dev, "Invalid proto %d\n", proto); > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return -ENXIO; > + return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto); > } > > if (desc && desc->no_dma_support) > @@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > if (ret) { > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n"); > + return ret; > } > > dev_dbg(dev, "Using GPI DMA mode for I2C\n"); > @@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev) > tx_depth = desc->tx_fifo_depth; > > if (!tx_depth) { > - dev_err(dev, "Invalid TX FIFO depth\n"); > geni_se_resources_off(&gi2c->se); > clk_disable_unprepare(gi2c->core_clk); > - return -EINVAL; > + return dev_err_probe(dev, -EINVAL, > + "Invalid TX FIFO depth\n"); > } > > gi2c->tx_wm = tx_depth - 1; > @@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > clk_disable_unprepare(gi2c->core_clk); > ret = geni_se_resources_off(&gi2c->se); > if (ret) { > - dev_err(dev, "Error turning off resources %d\n", ret); > + dev_err_probe(dev, ret, "Error turning off resources\n"); > goto err_dma; > } > > @@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev) > > ret = i2c_add_adapter(&gi2c->adap); > if (ret) { > - dev_err(dev, "Error adding i2c adapter %d\n", ret); > + dev_err_probe(dev, ret, "Error adding i2c adapter\n"); > pm_runtime_disable(gi2c->se.dev); > goto err_dma; > } Acked-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 7a22e1f46e60..01db24188e29 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev) return gi2c->irq; ret = geni_i2c_clk_map_idx(gi2c); - if (ret) { - dev_err(dev, "Invalid clk frequency %d Hz: %d\n", - gi2c->clk_freq_out, ret); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n", + gi2c->clk_freq_out); gi2c->adap.algo = &geni_i2c_algo; init_completion(&gi2c->done); @@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev) /* Keep interrupts disabled initially to allow for low-power modes */ ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN, dev_name(dev), gi2c); - if (ret) { - dev_err(dev, "Request_irq failed:%d: err:%d\n", - gi2c->irq, ret); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, + "Request_irq failed: %d\n", gi2c->irq); + i2c_set_adapdata(&gi2c->adap, gi2c); gi2c->adap.dev.parent = dev; gi2c->adap.dev.of_node = dev->of_node; @@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev) ret = geni_se_resources_on(&gi2c->se); if (ret) { - dev_err(dev, "Error turning on resources %d\n", ret); clk_disable_unprepare(gi2c->core_clk); - return ret; + return dev_err_probe(dev, ret, "Error turning on resources\n"); } proto = geni_se_read_proto(&gi2c->se); if (proto != GENI_SE_I2C) { - dev_err(dev, "Invalid proto %d\n", proto); geni_se_resources_off(&gi2c->se); clk_disable_unprepare(gi2c->core_clk); - return -ENXIO; + return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto); } if (desc && desc->no_dma_support) @@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev) if (ret) { geni_se_resources_off(&gi2c->se); clk_disable_unprepare(gi2c->core_clk); - return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n"); + return ret; } dev_dbg(dev, "Using GPI DMA mode for I2C\n"); @@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev) tx_depth = desc->tx_fifo_depth; if (!tx_depth) { - dev_err(dev, "Invalid TX FIFO depth\n"); geni_se_resources_off(&gi2c->se); clk_disable_unprepare(gi2c->core_clk); - return -EINVAL; + return dev_err_probe(dev, -EINVAL, + "Invalid TX FIFO depth\n"); } gi2c->tx_wm = tx_depth - 1; @@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev) clk_disable_unprepare(gi2c->core_clk); ret = geni_se_resources_off(&gi2c->se); if (ret) { - dev_err(dev, "Error turning off resources %d\n", ret); + dev_err_probe(dev, ret, "Error turning off resources\n"); goto err_dma; } @@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev) ret = i2c_add_adapter(&gi2c->adap); if (ret) { - dev_err(dev, "Error adding i2c adapter %d\n", ret); + dev_err_probe(dev, ret, "Error adding i2c adapter\n"); pm_runtime_disable(gi2c->se.dev); goto err_dma; }
Replace classical dev_err with dev_err_probe in the probe function for better error reporting. Also, use dev_err_probe in cases where the error number is clear (e.g., -EIO or -EINVAL) to maintain consistency. Additionally, remove redundant logging to simplify the code. Signed-off-by: Andi Shyti <andi.shyti@kernel.org> --- drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++----------------- 1 file changed, 14 insertions(+), 19 deletions(-)