Use the dev_err_probe function instead of dev_err in the probe function
so that the printed messge includes the return value and also handles
-EPROBE_DEFER nicely.

Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
 drivers/i2c/busses/i2c-synquacer.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c
index 4cc196ca8f6d..bbea521b05dd 100644
--- a/drivers/i2c/busses/i2c-synquacer.c
+++ b/drivers/i2c/busses/i2c-synquacer.c
@@ -557,20 +557,16 @@ static int synquacer_i2c_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "clock source %p\n", i2c->pclk);
 
 		ret = clk_prepare_enable(i2c->pclk);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable clock (%d)\n",
-				ret);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret, "failed to enable clock\n");
 		i2c->pclkrate = clk_get_rate(i2c->pclk);
 	}
 
 	if (i2c->pclkrate < SYNQUACER_I2C_MIN_CLK_RATE ||
-	    i2c->pclkrate > SYNQUACER_I2C_MAX_CLK_RATE) {
-		dev_err(&pdev->dev, "PCLK missing or out of range (%d)\n",
-			i2c->pclkrate);
-		return -EINVAL;
-	}
+	    i2c->pclkrate > SYNQUACER_I2C_MAX_CLK_RATE)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "PCLK missing or out of range (%d)\n",
+				     i2c->pclkrate);
 
 	i2c->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(i2c->base))
@@ -582,10 +578,8 @@ static int synquacer_i2c_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(&pdev->dev, i2c->irq, synquacer_i2c_isr,
 			       0, dev_name(&pdev->dev), i2c);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "cannot claim IRQ %d\n", i2c->irq);
 
 	i2c->state = STATE_IDLE;
 	i2c->dev = &pdev->dev;
@@ -605,10 +599,8 @@ static int synquacer_i2c_probe(struct platform_device *pdev)
 	synquacer_i2c_hw_init(i2c);
 
 	ret = i2c_add_numbered_adapter(&i2c->adapter);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to add bus to i2c core\n");
 
 	platform_set_drvdata(pdev, i2c);
 
-- 
2.25.1

Hi Liao,

On Tue, Aug 08, 2023 at 09:29:54AM +0800, Liao Chang wrote:
> Use the dev_err_probe function instead of dev_err in the probe function
> so that the printed messge includes the return value and also handles
> -EPROBE_DEFER nicely.
> 
> Signed-off-by: Liao Chang <liaochang1@huawei.com>

After some discussions and time, I think it's right to r-b this
patch. If you agree more with Krzysztof, feel free to follow his
recommendation and send another version otherwise I will go ahead
and take this series in my branch. I do not really mind, both
arguments are valid.

Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 

Thanks,
Andi

Hi Andi and Krzysztof,

在 2023/8/10 3:21, Andi Shyti 写道:
> Hi Liao,
> 
> On Tue, Aug 08, 2023 at 09:29:54AM +0800, Liao Chang wrote:
>> Use the dev_err_probe function instead of dev_err in the probe function
>> so that the printed messge includes the return value and also handles
>> -EPROBE_DEFER nicely.
>>
>> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> 
> After some discussions and time, I think it's right to r-b this
> patch. If you agree more with Krzysztof, feel free to follow his
> recommendation and send another version otherwise I will go ahead
> and take this series in my branch. I do not really mind, both
> arguments are valid.

I saw that Frank Li developed some new APIs that look like they would be very
helpful for aligning the error messages of devm_request_irq in device probes.
However, the patches have been pending for weeks and the author hasn't sent a
new version. So I'm not planning to switch to the new APIs in this patch series,
if there are no objections.

Do I need to resend a new revision to add your R-B at this patch?

Thanks.

> 
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 
> 
> Thanks,
> Andi

-- 
BR
Liao, Chang

Hi Liao,

On Thu, Aug 10, 2023 at 10:33:58AM +0800, Liao, Chang wrote:
> Hi Andi and Krzysztof,
> 
> 在 2023/8/10 3:21, Andi Shyti 写道:
> > Hi Liao,
> > 
> > On Tue, Aug 08, 2023 at 09:29:54AM +0800, Liao Chang wrote:
> >> Use the dev_err_probe function instead of dev_err in the probe function
> >> so that the printed messge includes the return value and also handles
> >> -EPROBE_DEFER nicely.
> >>
> >> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> > 
> > After some discussions and time, I think it's right to r-b this
> > patch. If you agree more with Krzysztof, feel free to follow his
> > recommendation and send another version otherwise I will go ahead
> > and take this series in my branch. I do not really mind, both
> > arguments are valid.
> 
> I saw that Frank Li developed some new APIs that look like they would be very
> helpful for aligning the error messages of devm_request_irq in device probes.
> However, the patches have been pending for weeks and the author hasn't sent a
> new version. So I'm not planning to switch to the new APIs in this patch series,
> if there are no objections.
> 
> Do I need to resend a new revision to add your R-B at this patch?

no, no need. I'm also going to fix a typo in the commit message
(messge/message) that Markus has pointed out.

Thanks for this series of cleanups,
Andi

> Thanks.
> 
> > 
> > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 
> > 
> > Thanks,
> > Andi
> 
> -- 
> BR
> Liao, Chang

On 08/08/2023 03:29, Liao Chang wrote:
> Use the dev_err_probe function instead of dev_err in the probe function
> so that the printed messge includes the return value and also handles
> -EPROBE_DEFER nicely.
> 
> Signed-off-by: Liao Chang <liaochang1@huawei.com>


>  	i2c->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(i2c->base))
> @@ -582,10 +578,8 @@ static int synquacer_i2c_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, i2c->irq, synquacer_i2c_isr,
>  			       0, dev_name(&pdev->dev), i2c);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "cannot claim IRQ %d\n", i2c->irq);
>  

I don't think this is needed:
https://lore.kernel.org/all/20230721094641.77189-1-frank.li@vivo.com/

Best regards,
Krzysztof