[v2,1/2] i2c: qcom-geni: Use dev_err_probe in the probe function

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(-)

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