| Message ID | 20230808012954.1643834-10-liaochang1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use dev_err_probe in i2c probe function | expand |
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
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
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
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);
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(-)