Re: [RESEND PATCH v2] iio: adc: bcm_iproc_adc: Convert probe() to use dev_err_probe()
Joshua Crofts <[email protected]> Fri, 31 Jul 2026 11:53:09 +0200
| Newsgroups | org.kernel.vger.linux-iio,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 30 Jul 2026 21:27:12 +0530 [email protected] wrote: > From: Mohammad Shahid <[email protected]> > > Replace open-coded probe error handling with dev_err_probe() for > resource acquisition and initialization failures. > > This simplifies the error paths, avoids repetitive error handling, > and automatically suppresses duplicate messages when -EPROBE_DEFER is > returned while preserving the existing error messages. > > Also remove the redundant error message after iproc_adc_enable(), > allowing the error to be propagated without additional logging. Separate patch for this along with the removal of dev_err() at devm_request_threaded_irq(), see below. > > Signed-off-by: Mohammad Shahid <[email protected]> > --- > v2: > - Convert remaining probe error paths to use dev_err_probe(). > - Remove redundant error message after iproc_adc_enable() > drivers/iio/adc/bcm_iproc_adc.c | 52 +++++++++++++++------------------ > 1 file changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c > index cf4738b16e62..08e056468d8e 100644 > --- a/drivers/iio/adc/bcm_iproc_adc.c > +++ b/drivers/iio/adc/bcm_iproc_adc.c > @@ -522,52 +522,48 @@ static int iproc_adc_probe(struct platform_device *pdev) > > adc_priv->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > "adc-syscon"); > - if (IS_ERR(adc_priv->regmap)) { > - dev_err(&pdev->dev, "failed to get handle for tsc syscon\n"); > - ret = PTR_ERR(adc_priv->regmap); > - return ret; > - } > + if (IS_ERR(adc_priv->regmap)) > + return dev_err_probe(&pdev->dev, Given the amount of &pdev->dev calls, it would be a good idea to introduce a local `struct device *dev = &pdev->dev;` in a separate patch which would go before the dev_err_probe() patch. > + PTR_ERR(adc_priv->regmap), > + "failed to get handle for tsc syscon\n"); > > adc_priv->adc_clk = devm_clk_get(&pdev->dev, "tsc_clk"); > - if (IS_ERR(adc_priv->adc_clk)) { > - dev_err(&pdev->dev, > - "failed getting clock tsc_clk\n"); > - ret = PTR_ERR(adc_priv->adc_clk); > - return ret; > - } > + if (IS_ERR(adc_priv->adc_clk)) > + return dev_err_probe(&pdev->dev, > + PTR_ERR(adc_priv->adc_clk), > + "failed getting clock tsc_clk\n"); > > adc_priv->irqno = platform_get_irq(pdev, 0); > if (adc_priv->irqno < 0) > - return adc_priv->irqno; > + return dev_err_probe(&pdev->dev, > + adc_priv->irqno, > + "failed to get IRQ\n"); Remove this, platform_get_irq() already prints an error message, keep the original. > > ret = regmap_clear_bits(adc_priv->regmap, IPROC_REGCTL2, > IPROC_ADC_AUXIN_SCAN_ENA); > - if (ret) { > - dev_err(&pdev->dev, "failed to write IPROC_REGCTL2 %d\n", ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "failed to write IPROC_REGCTL2\n"); > > ret = devm_request_threaded_irq(&pdev->dev, adc_priv->irqno, > iproc_adc_interrupt_handler, > iproc_adc_interrupt_thread, > IRQF_SHARED, "iproc-adc", indio_dev); > - if (ret) { > - dev_err(&pdev->dev, "request_irq error %d\n", ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "failed to request IRQ\n"); You only need to return ret here, devm_request_threaded_irq() already prints an error message on failure. Separate patch for the removal please (ideally the first patch in your series to reduce churn). > > ret = clk_prepare_enable(adc_priv->adc_clk); > - if (ret) { > - dev_err(&pdev->dev, > - "clk_prepare_enable failed %d\n", ret); > - return ret; > - } > + if (ret) > + return dev_err_probe(&pdev->dev, > + ret, > + "failed to enable clock\n"); > > ret = iproc_adc_enable(indio_dev); > - if (ret) { > - dev_err(&pdev->dev, "failed to enable adc %d\n", ret); > + if (ret) > goto err_adc_enable; > - } > > indio_dev->name = "iproc-static-adc"; > indio_dev->info = &iproc_adc_iio_info; Also, you've missed an additional dev_err() in the iio_device_register() check, you can change that one as well. -- Kind regards, Joshua Crofts