| Message ID | 20220919155843.1097473-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next,v2] media: Switch to use dev_err_probe() helper | expand |
Hi Yang On Mon, 19 Sept 2022 at 18:02, Yang Yingliang <yangyingliang@huawei.com> wrote: > > In the probe path, dev_err() can be replaced with dev_err_probe() > which will check if error code is -EPROBE_DEFER. > > Reviewed-by: Sean Young <sean@mess.org> > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> FWIW: I originally reviewed only uvc > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > v2: > - Merge the patches in one patch. > - s/replace/replaced in commit message. > - Add '\n' in xilinx-csi2rxss.c and imx274.c > - Add missing return value in media-dev.c > --- > drivers/media/cec/platform/stm32/stm32-cec.c | 9 +++---- > drivers/media/i2c/ad5820.c | 18 +++++-------- > drivers/media/i2c/imx274.c | 5 ++-- > drivers/media/i2c/tc358743.c | 9 +++---- > .../platform/mediatek/mdp/mtk_mdp_comp.c | 5 ++-- > .../platform/samsung/exynos4-is/media-dev.c | 4 +-- > drivers/media/platform/st/stm32/stm32-dcmi.c | 27 +++++++------------ > drivers/media/platform/ti/omap3isp/isp.c | 3 +-- > .../media/platform/xilinx/xilinx-csi2rxss.c | 8 +++--- > drivers/media/rc/gpio-ir-recv.c | 10 +++---- > drivers/media/rc/gpio-ir-tx.c | 9 +++---- > drivers/media/rc/ir-rx51.c | 9 ++----- > drivers/media/usb/uvc/uvc_driver.c | 9 +++---- > 13 files changed, 41 insertions(+), 84 deletions(-) > > diff --git a/drivers/media/cec/platform/stm32/stm32-cec.c b/drivers/media/cec/platform/stm32/stm32-cec.c > index 40db7911b437..7b2db46a5722 100644 > --- a/drivers/media/cec/platform/stm32/stm32-cec.c > +++ b/drivers/media/cec/platform/stm32/stm32-cec.c > @@ -288,12 +288,9 @@ static int stm32_cec_probe(struct platform_device *pdev) > return ret; > > cec->clk_cec = devm_clk_get(&pdev->dev, "cec"); > - if (IS_ERR(cec->clk_cec)) { > - if (PTR_ERR(cec->clk_cec) != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Cannot get cec clock\n"); > - > - return PTR_ERR(cec->clk_cec); > - } > + if (IS_ERR(cec->clk_cec)) > + return dev_err_probe(&pdev->dev, PTR_ERR(cec->clk_cec), > + "Cannot get cec clock\n"); > > ret = clk_prepare(cec->clk_cec); > if (ret) { > diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c > index 516de278cc49..6a7f8ef9db05 100644 > --- a/drivers/media/i2c/ad5820.c > +++ b/drivers/media/i2c/ad5820.c > @@ -301,21 +301,15 @@ static int ad5820_probe(struct i2c_client *client, > return -ENOMEM; > > coil->vana = devm_regulator_get(&client->dev, "VANA"); > - if (IS_ERR(coil->vana)) { > - ret = PTR_ERR(coil->vana); > - if (ret != -EPROBE_DEFER) > - dev_err(&client->dev, "could not get regulator for vana\n"); > - return ret; > - } > + if (IS_ERR(coil->vana)) > + return dev_err_probe(&client->dev, PTR_ERR(coil->vana), > + "could not get regulator for vana\n"); > > coil->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", > GPIOD_OUT_LOW); > - if (IS_ERR(coil->enable_gpio)) { > - ret = PTR_ERR(coil->enable_gpio); > - if (ret != -EPROBE_DEFER) > - dev_err(&client->dev, "could not get enable gpio\n"); > - return ret; > - } > + if (IS_ERR(coil->enable_gpio)) > + return dev_err_probe(&client->dev, PTR_ERR(coil->enable_gpio), > + "could not get enable gpio\n"); > > mutex_init(&coil->power_lock); > > diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > index a00761b1e18c..9219f3c9594b 100644 > --- a/drivers/media/i2c/imx274.c > +++ b/drivers/media/i2c/imx274.c > @@ -2060,9 +2060,8 @@ static int imx274_probe(struct i2c_client *client) > imx274->reset_gpio = devm_gpiod_get_optional(dev, "reset", > GPIOD_OUT_HIGH); > if (IS_ERR(imx274->reset_gpio)) { > - if (PTR_ERR(imx274->reset_gpio) != -EPROBE_DEFER) > - dev_err(dev, "Reset GPIO not setup in DT"); > - ret = PTR_ERR(imx274->reset_gpio); > + ret = dev_err_probe(dev, PTR_ERR(imx274->reset_gpio), > + "Reset GPIO not setup in DT\n"); > goto err_me; Not sure I like the use of dev_err_probe here. We only save one line. Same goes for all the other cases where there is no "return dev_err_probe..." > } > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c > index 200841c1f5cf..9197fa0b1bc2 100644 > --- a/drivers/media/i2c/tc358743.c > +++ b/drivers/media/i2c/tc358743.c > @@ -1891,12 +1891,9 @@ static int tc358743_probe_of(struct tc358743_state *state) > int ret; > > refclk = devm_clk_get(dev, "refclk"); > - if (IS_ERR(refclk)) { > - if (PTR_ERR(refclk) != -EPROBE_DEFER) > - dev_err(dev, "failed to get refclk: %ld\n", > - PTR_ERR(refclk)); > - return PTR_ERR(refclk); > - } > + if (IS_ERR(refclk)) > + return dev_err_probe(dev, PTR_ERR(refclk), > + "failed to get refclk\n"); > > ep = of_graph_get_next_endpoint(dev->of_node, NULL); > if (!ep) { > diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > index 1e3833f1c9ae..ad5fab2d8bfa 100644 > --- a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > +++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > @@ -52,9 +52,8 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { > comp->clk[i] = of_clk_get(node, i); > if (IS_ERR(comp->clk[i])) { > - if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) > - dev_err(dev, "Failed to get clock\n"); > - ret = PTR_ERR(comp->clk[i]); > + ret = dev_err_probe(dev, PTR_ERR(comp->clk[i]), > + "Failed to get clock\n"); > goto put_dev; > } > > diff --git a/drivers/media/platform/samsung/exynos4-is/media-dev.c b/drivers/media/platform/samsung/exynos4-is/media-dev.c > index 52b43ea04030..7a9d51b8bb4c 100644 > --- a/drivers/media/platform/samsung/exynos4-is/media-dev.c > +++ b/drivers/media/platform/samsung/exynos4-is/media-dev.c > @@ -1473,9 +1473,7 @@ static int fimc_md_probe(struct platform_device *pdev) > > pinctrl = devm_pinctrl_get(dev); > if (IS_ERR(pinctrl)) { > - ret = PTR_ERR(pinctrl); > - if (ret != EPROBE_DEFER) > - dev_err(dev, "Failed to get pinctrl: %d\n", ret); > + ret = dev_err_probe(dev, PTR_ERR(pinctrl), "Failed to get pinctrl\n"); > goto err_clk; > } > > diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c > index 2ca95ab2b0fe..ec138843d090 100644 > --- a/drivers/media/platform/st/stm32/stm32-dcmi.c > +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c > @@ -1946,12 +1946,9 @@ static int dcmi_probe(struct platform_device *pdev) > return -ENOMEM; > > dcmi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > - if (IS_ERR(dcmi->rstc)) { > - if (PTR_ERR(dcmi->rstc) != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Could not get reset control\n"); > - > - return PTR_ERR(dcmi->rstc); > - } > + if (IS_ERR(dcmi->rstc)) > + return dev_err_probe(&pdev->dev, PTR_ERR(dcmi->rstc), > + "Could not get reset control\n"); > > /* Get bus characteristics from devicetree */ > np = of_graph_get_next_endpoint(np, NULL); > @@ -2003,20 +2000,14 @@ static int dcmi_probe(struct platform_device *pdev) > } > > mclk = devm_clk_get(&pdev->dev, "mclk"); > - if (IS_ERR(mclk)) { > - if (PTR_ERR(mclk) != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Unable to get mclk\n"); > - return PTR_ERR(mclk); > - } > + if (IS_ERR(mclk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(mclk), > + "Unable to get mclk\n"); > > chan = dma_request_chan(&pdev->dev, "tx"); > - if (IS_ERR(chan)) { > - ret = PTR_ERR(chan); > - if (ret != -EPROBE_DEFER) > - dev_err(&pdev->dev, > - "Failed to request DMA channel: %d\n", ret); > - return ret; > - } > + if (IS_ERR(chan)) > + return dev_err_probe(&pdev->dev, PTR_ERR(chan), > + "Failed to request DMA channel\n"); > > dcmi->dma_max_burst = UINT_MAX; > ret = dma_get_slave_caps(chan, &caps); > diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c > index a6052df9bb19..5d6867b8f197 100644 > --- a/drivers/media/platform/ti/omap3isp/isp.c > +++ b/drivers/media/platform/ti/omap3isp/isp.c > @@ -1886,8 +1886,7 @@ static int isp_initialize_modules(struct isp_device *isp) > > ret = omap3isp_ccp2_init(isp); > if (ret < 0) { > - if (ret != -EPROBE_DEFER) > - dev_err(isp->dev, "CCP2 initialization failed\n"); > + dev_err_probe(isp->dev, ret, "CCP2 initialization failed\n"); > goto error_ccp2; > } > > diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > index 29b53febc2e7..d8a23f18cfbc 100644 > --- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c > +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > @@ -976,11 +976,9 @@ static int xcsi2rxss_probe(struct platform_device *pdev) > /* Reset GPIO */ > xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset", > GPIOD_OUT_HIGH); > - if (IS_ERR(xcsi2rxss->rst_gpio)) { > - if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER) > - dev_err(dev, "Video Reset GPIO not setup in DT"); > - return PTR_ERR(xcsi2rxss->rst_gpio); > - } > + if (IS_ERR(xcsi2rxss->rst_gpio)) > + return dev_err_probe(dev, PTR_ERR(xcsi2rxss->rst_gpio), > + "Video Reset GPIO not setup in DT\n"); > > ret = xcsi2rxss_parse_of(xcsi2rxss); > if (ret < 0) > diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c > index 22e524b69806..8f1fff7af6c9 100644 > --- a/drivers/media/rc/gpio-ir-recv.c > +++ b/drivers/media/rc/gpio-ir-recv.c > @@ -74,13 +74,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) > return -ENOMEM; > > gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN); > - if (IS_ERR(gpio_dev->gpiod)) { > - rc = PTR_ERR(gpio_dev->gpiod); > - /* Just try again if this happens */ > - if (rc != -EPROBE_DEFER) > - dev_err(dev, "error getting gpio (%d)\n", rc); > - return rc; > - } > + if (IS_ERR(gpio_dev->gpiod)) > + return dev_err_probe(dev, PTR_ERR(gpio_dev->gpiod), > + "error getting gpio\n"); > gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod); > if (gpio_dev->irq < 0) > return gpio_dev->irq; > diff --git a/drivers/media/rc/gpio-ir-tx.c b/drivers/media/rc/gpio-ir-tx.c > index d3063ddb472e..2b829c146db1 100644 > --- a/drivers/media/rc/gpio-ir-tx.c > +++ b/drivers/media/rc/gpio-ir-tx.c > @@ -174,12 +174,9 @@ static int gpio_ir_tx_probe(struct platform_device *pdev) > return -ENOMEM; > > gpio_ir->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW); > - if (IS_ERR(gpio_ir->gpio)) { > - if (PTR_ERR(gpio_ir->gpio) != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Failed to get gpio (%ld)\n", > - PTR_ERR(gpio_ir->gpio)); > - return PTR_ERR(gpio_ir->gpio); > - } > + if (IS_ERR(gpio_ir->gpio)) > + return dev_err_probe(&pdev->dev, PTR_ERR(gpio_ir->gpio), > + "Failed to get gpio\n"); > > rcdev->priv = gpio_ir; > rcdev->driver_name = DRIVER_NAME; > diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c > index a3b145183260..85080c3d2053 100644 > --- a/drivers/media/rc/ir-rx51.c > +++ b/drivers/media/rc/ir-rx51.c > @@ -231,13 +231,8 @@ static int ir_rx51_probe(struct platform_device *dev) > struct rc_dev *rcdev; > > pwm = pwm_get(&dev->dev, NULL); > - if (IS_ERR(pwm)) { > - int err = PTR_ERR(pwm); > - > - if (err != -EPROBE_DEFER) > - dev_err(&dev->dev, "pwm_get failed: %d\n", err); > - return err; > - } > + if (IS_ERR(pwm)) > + return dev_err_probe(&dev->dev, PTR_ERR(pwm), "pwm_get failed\n"); > > /* Use default, in case userspace does not set the carrier */ > ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm), NSEC_PER_SEC); > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 744051b52e12..94f84c3c4712 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1554,12 +1554,9 @@ static int uvc_gpio_parse(struct uvc_device *dev) > return PTR_ERR_OR_ZERO(gpio_privacy); > > irq = gpiod_to_irq(gpio_privacy); > - if (irq < 0) { > - if (irq != EPROBE_DEFER) > - dev_err(&dev->udev->dev, > - "No IRQ for privacy GPIO (%d)\n", irq); > - return irq; > - } > + if (irq < 0) > + return dev_err_probe(&dev->udev->dev, irq, > + "No IRQ for privacy GPIO\n"); > > unit = uvc_alloc_entity(UVC_EXT_GPIO_UNIT, UVC_EXT_GPIO_UNIT_ID, 0, 1); > if (!unit) > -- > 2.25.1 >
On Mon, Sep 19, 2022 at 08:25:07PM +0200, Ricardo Ribalda wrote: > Hi Yang > > On Mon, 19 Sept 2022 at 18:02, Yang Yingliang <yangyingliang@huawei.com> wrote: > > > > In the probe path, dev_err() can be replaced with dev_err_probe() > > which will check if error code is -EPROBE_DEFER. > > > > Reviewed-by: Sean Young <sean@mess.org> > > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> > FWIW: I originally reviewed only uvc I liked the split-patches version better too, but that's no reason to submit a v3 just for that. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > --- > > v2: > > - Merge the patches in one patch. > > - s/replace/replaced in commit message. > > - Add '\n' in xilinx-csi2rxss.c and imx274.c > > - Add missing return value in media-dev.c > > --- > > drivers/media/cec/platform/stm32/stm32-cec.c | 9 +++---- > > drivers/media/i2c/ad5820.c | 18 +++++-------- > > drivers/media/i2c/imx274.c | 5 ++-- > > drivers/media/i2c/tc358743.c | 9 +++---- > > .../platform/mediatek/mdp/mtk_mdp_comp.c | 5 ++-- > > .../platform/samsung/exynos4-is/media-dev.c | 4 +-- > > drivers/media/platform/st/stm32/stm32-dcmi.c | 27 +++++++------------ > > drivers/media/platform/ti/omap3isp/isp.c | 3 +-- > > .../media/platform/xilinx/xilinx-csi2rxss.c | 8 +++--- > > drivers/media/rc/gpio-ir-recv.c | 10 +++---- > > drivers/media/rc/gpio-ir-tx.c | 9 +++---- > > drivers/media/rc/ir-rx51.c | 9 ++----- > > drivers/media/usb/uvc/uvc_driver.c | 9 +++---- > > 13 files changed, 41 insertions(+), 84 deletions(-) > > > > diff --git a/drivers/media/cec/platform/stm32/stm32-cec.c b/drivers/media/cec/platform/stm32/stm32-cec.c > > index 40db7911b437..7b2db46a5722 100644 > > --- a/drivers/media/cec/platform/stm32/stm32-cec.c > > +++ b/drivers/media/cec/platform/stm32/stm32-cec.c > > @@ -288,12 +288,9 @@ static int stm32_cec_probe(struct platform_device *pdev) > > return ret; > > > > cec->clk_cec = devm_clk_get(&pdev->dev, "cec"); > > - if (IS_ERR(cec->clk_cec)) { > > - if (PTR_ERR(cec->clk_cec) != -EPROBE_DEFER) > > - dev_err(&pdev->dev, "Cannot get cec clock\n"); > > - > > - return PTR_ERR(cec->clk_cec); > > - } > > + if (IS_ERR(cec->clk_cec)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(cec->clk_cec), > > + "Cannot get cec clock\n"); > > > > ret = clk_prepare(cec->clk_cec); > > if (ret) { > > diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c > > index 516de278cc49..6a7f8ef9db05 100644 > > --- a/drivers/media/i2c/ad5820.c > > +++ b/drivers/media/i2c/ad5820.c > > @@ -301,21 +301,15 @@ static int ad5820_probe(struct i2c_client *client, > > return -ENOMEM; > > > > coil->vana = devm_regulator_get(&client->dev, "VANA"); > > - if (IS_ERR(coil->vana)) { > > - ret = PTR_ERR(coil->vana); > > - if (ret != -EPROBE_DEFER) > > - dev_err(&client->dev, "could not get regulator for vana\n"); > > - return ret; > > - } > > + if (IS_ERR(coil->vana)) > > + return dev_err_probe(&client->dev, PTR_ERR(coil->vana), > > + "could not get regulator for vana\n"); > > > > coil->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", > > GPIOD_OUT_LOW); > > - if (IS_ERR(coil->enable_gpio)) { > > - ret = PTR_ERR(coil->enable_gpio); > > - if (ret != -EPROBE_DEFER) > > - dev_err(&client->dev, "could not get enable gpio\n"); > > - return ret; > > - } > > + if (IS_ERR(coil->enable_gpio)) > > + return dev_err_probe(&client->dev, PTR_ERR(coil->enable_gpio), > > + "could not get enable gpio\n"); > > > > mutex_init(&coil->power_lock); > > > > diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > > index a00761b1e18c..9219f3c9594b 100644 > > --- a/drivers/media/i2c/imx274.c > > +++ b/drivers/media/i2c/imx274.c > > @@ -2060,9 +2060,8 @@ static int imx274_probe(struct i2c_client *client) > > imx274->reset_gpio = devm_gpiod_get_optional(dev, "reset", > > GPIOD_OUT_HIGH); > > if (IS_ERR(imx274->reset_gpio)) { > > - if (PTR_ERR(imx274->reset_gpio) != -EPROBE_DEFER) > > - dev_err(dev, "Reset GPIO not setup in DT"); > > - ret = PTR_ERR(imx274->reset_gpio); > > + ret = dev_err_probe(dev, PTR_ERR(imx274->reset_gpio), > > + "Reset GPIO not setup in DT\n"); > > goto err_me; > > Not sure I like the use of dev_err_probe here. We only save one line. > Same goes for all the other cases where there is no "return dev_err_probe..." It's not just about saving a line, dev_err_probe() also records the cause of probe deferral (without printing the message) and makes it available through debugfs. > > } > > > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c > > index 200841c1f5cf..9197fa0b1bc2 100644 > > --- a/drivers/media/i2c/tc358743.c > > +++ b/drivers/media/i2c/tc358743.c > > @@ -1891,12 +1891,9 @@ static int tc358743_probe_of(struct tc358743_state *state) > > int ret; > > > > refclk = devm_clk_get(dev, "refclk"); > > - if (IS_ERR(refclk)) { > > - if (PTR_ERR(refclk) != -EPROBE_DEFER) > > - dev_err(dev, "failed to get refclk: %ld\n", > > - PTR_ERR(refclk)); > > - return PTR_ERR(refclk); > > - } > > + if (IS_ERR(refclk)) > > + return dev_err_probe(dev, PTR_ERR(refclk), > > + "failed to get refclk\n"); > > > > ep = of_graph_get_next_endpoint(dev->of_node, NULL); > > if (!ep) { > > diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > index 1e3833f1c9ae..ad5fab2d8bfa 100644 > > --- a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > +++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > @@ -52,9 +52,8 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > > for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { > > comp->clk[i] = of_clk_get(node, i); > > if (IS_ERR(comp->clk[i])) { > > - if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) > > - dev_err(dev, "Failed to get clock\n"); > > - ret = PTR_ERR(comp->clk[i]); > > + ret = dev_err_probe(dev, PTR_ERR(comp->clk[i]), > > + "Failed to get clock\n"); > > goto put_dev; > > } > > > > diff --git a/drivers/media/platform/samsung/exynos4-is/media-dev.c b/drivers/media/platform/samsung/exynos4-is/media-dev.c > > index 52b43ea04030..7a9d51b8bb4c 100644 > > --- a/drivers/media/platform/samsung/exynos4-is/media-dev.c > > +++ b/drivers/media/platform/samsung/exynos4-is/media-dev.c > > @@ -1473,9 +1473,7 @@ static int fimc_md_probe(struct platform_device *pdev) > > > > pinctrl = devm_pinctrl_get(dev); > > if (IS_ERR(pinctrl)) { > > - ret = PTR_ERR(pinctrl); > > - if (ret != EPROBE_DEFER) > > - dev_err(dev, "Failed to get pinctrl: %d\n", ret); > > + ret = dev_err_probe(dev, PTR_ERR(pinctrl), "Failed to get pinctrl\n"); > > goto err_clk; > > } > > > > diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c > > index 2ca95ab2b0fe..ec138843d090 100644 > > --- a/drivers/media/platform/st/stm32/stm32-dcmi.c > > +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c > > @@ -1946,12 +1946,9 @@ static int dcmi_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > dcmi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > > - if (IS_ERR(dcmi->rstc)) { > > - if (PTR_ERR(dcmi->rstc) != -EPROBE_DEFER) > > - dev_err(&pdev->dev, "Could not get reset control\n"); > > - > > - return PTR_ERR(dcmi->rstc); > > - } > > + if (IS_ERR(dcmi->rstc)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(dcmi->rstc), > > + "Could not get reset control\n"); > > > > /* Get bus characteristics from devicetree */ > > np = of_graph_get_next_endpoint(np, NULL); > > @@ -2003,20 +2000,14 @@ static int dcmi_probe(struct platform_device *pdev) > > } > > > > mclk = devm_clk_get(&pdev->dev, "mclk"); > > - if (IS_ERR(mclk)) { > > - if (PTR_ERR(mclk) != -EPROBE_DEFER) > > - dev_err(&pdev->dev, "Unable to get mclk\n"); > > - return PTR_ERR(mclk); > > - } > > + if (IS_ERR(mclk)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(mclk), > > + "Unable to get mclk\n"); > > > > chan = dma_request_chan(&pdev->dev, "tx"); > > - if (IS_ERR(chan)) { > > - ret = PTR_ERR(chan); > > - if (ret != -EPROBE_DEFER) > > - dev_err(&pdev->dev, > > - "Failed to request DMA channel: %d\n", ret); > > - return ret; > > - } > > + if (IS_ERR(chan)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(chan), > > + "Failed to request DMA channel\n"); > > > > dcmi->dma_max_burst = UINT_MAX; > > ret = dma_get_slave_caps(chan, &caps); > > diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c > > index a6052df9bb19..5d6867b8f197 100644 > > --- a/drivers/media/platform/ti/omap3isp/isp.c > > +++ b/drivers/media/platform/ti/omap3isp/isp.c > > @@ -1886,8 +1886,7 @@ static int isp_initialize_modules(struct isp_device *isp) > > > > ret = omap3isp_ccp2_init(isp); > > if (ret < 0) { > > - if (ret != -EPROBE_DEFER) > > - dev_err(isp->dev, "CCP2 initialization failed\n"); > > + dev_err_probe(isp->dev, ret, "CCP2 initialization failed\n"); > > goto error_ccp2; > > } > > > > diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > index 29b53febc2e7..d8a23f18cfbc 100644 > > --- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > @@ -976,11 +976,9 @@ static int xcsi2rxss_probe(struct platform_device *pdev) > > /* Reset GPIO */ > > xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset", > > GPIOD_OUT_HIGH); > > - if (IS_ERR(xcsi2rxss->rst_gpio)) { > > - if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER) > > - dev_err(dev, "Video Reset GPIO not setup in DT"); > > - return PTR_ERR(xcsi2rxss->rst_gpio); > > - } > > + if (IS_ERR(xcsi2rxss->rst_gpio)) > > + return dev_err_probe(dev, PTR_ERR(xcsi2rxss->rst_gpio), > > + "Video Reset GPIO not setup in DT\n"); > > > > ret = xcsi2rxss_parse_of(xcsi2rxss); > > if (ret < 0) > > diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c > > index 22e524b69806..8f1fff7af6c9 100644 > > --- a/drivers/media/rc/gpio-ir-recv.c > > +++ b/drivers/media/rc/gpio-ir-recv.c > > @@ -74,13 +74,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN); > > - if (IS_ERR(gpio_dev->gpiod)) { > > - rc = PTR_ERR(gpio_dev->gpiod); > > - /* Just try again if this happens */ > > - if (rc != -EPROBE_DEFER) > > - dev_err(dev, "error getting gpio (%d)\n", rc); > > - return rc; > > - } > > + if (IS_ERR(gpio_dev->gpiod)) > > + return dev_err_probe(dev, PTR_ERR(gpio_dev->gpiod), > > + "error getting gpio\n"); > > gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod); > > if (gpio_dev->irq < 0) > > return gpio_dev->irq; > > diff --git a/drivers/media/rc/gpio-ir-tx.c b/drivers/media/rc/gpio-ir-tx.c > > index d3063ddb472e..2b829c146db1 100644 > > --- a/drivers/media/rc/gpio-ir-tx.c > > +++ b/drivers/media/rc/gpio-ir-tx.c > > @@ -174,12 +174,9 @@ static int gpio_ir_tx_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > gpio_ir->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW); > > - if (IS_ERR(gpio_ir->gpio)) { > > - if (PTR_ERR(gpio_ir->gpio) != -EPROBE_DEFER) > > - dev_err(&pdev->dev, "Failed to get gpio (%ld)\n", > > - PTR_ERR(gpio_ir->gpio)); > > - return PTR_ERR(gpio_ir->gpio); > > - } > > + if (IS_ERR(gpio_ir->gpio)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(gpio_ir->gpio), > > + "Failed to get gpio\n"); > > > > rcdev->priv = gpio_ir; > > rcdev->driver_name = DRIVER_NAME; > > diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c > > index a3b145183260..85080c3d2053 100644 > > --- a/drivers/media/rc/ir-rx51.c > > +++ b/drivers/media/rc/ir-rx51.c > > @@ -231,13 +231,8 @@ static int ir_rx51_probe(struct platform_device *dev) > > struct rc_dev *rcdev; > > > > pwm = pwm_get(&dev->dev, NULL); > > - if (IS_ERR(pwm)) { > > - int err = PTR_ERR(pwm); > > - > > - if (err != -EPROBE_DEFER) > > - dev_err(&dev->dev, "pwm_get failed: %d\n", err); > > - return err; > > - } > > + if (IS_ERR(pwm)) > > + return dev_err_probe(&dev->dev, PTR_ERR(pwm), "pwm_get failed\n"); > > > > /* Use default, in case userspace does not set the carrier */ > > ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm), NSEC_PER_SEC); > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 744051b52e12..94f84c3c4712 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -1554,12 +1554,9 @@ static int uvc_gpio_parse(struct uvc_device *dev) > > return PTR_ERR_OR_ZERO(gpio_privacy); > > > > irq = gpiod_to_irq(gpio_privacy); > > - if (irq < 0) { > > - if (irq != EPROBE_DEFER) > > - dev_err(&dev->udev->dev, > > - "No IRQ for privacy GPIO (%d)\n", irq); > > - return irq; > > - } > > + if (irq < 0) > > + return dev_err_probe(&dev->udev->dev, irq, > > + "No IRQ for privacy GPIO\n"); > > > > unit = uvc_alloc_entity(UVC_EXT_GPIO_UNIT, UVC_EXT_GPIO_UNIT_ID, 0, 1); > > if (!unit)
On Mon, 19 Sept 2022 at 20:39, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Mon, Sep 19, 2022 at 08:25:07PM +0200, Ricardo Ribalda wrote: > > Hi Yang > > > > On Mon, 19 Sept 2022 at 18:02, Yang Yingliang <yangyingliang@huawei.com> wrote: > > > > > > In the probe path, dev_err() can be replaced with dev_err_probe() > > > which will check if error code is -EPROBE_DEFER. > > > > > > Reviewed-by: Sean Young <sean@mess.org> > > > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> > > FWIW: I originally reviewed only uvc > > I liked the split-patches version better too, but that's no reason to > submit a v3 just for that. > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > > --- > > > v2: > > > - Merge the patches in one patch. > > > - s/replace/replaced in commit message. > > > - Add '\n' in xilinx-csi2rxss.c and imx274.c > > > - Add missing return value in media-dev.c > > > --- > > > drivers/media/cec/platform/stm32/stm32-cec.c | 9 +++---- > > > drivers/media/i2c/ad5820.c | 18 +++++-------- > > > drivers/media/i2c/imx274.c | 5 ++-- > > > drivers/media/i2c/tc358743.c | 9 +++---- > > > .../platform/mediatek/mdp/mtk_mdp_comp.c | 5 ++-- > > > .../platform/samsung/exynos4-is/media-dev.c | 4 +-- > > > drivers/media/platform/st/stm32/stm32-dcmi.c | 27 +++++++------------ > > > drivers/media/platform/ti/omap3isp/isp.c | 3 +-- > > > .../media/platform/xilinx/xilinx-csi2rxss.c | 8 +++--- > > > drivers/media/rc/gpio-ir-recv.c | 10 +++---- > > > drivers/media/rc/gpio-ir-tx.c | 9 +++---- > > > drivers/media/rc/ir-rx51.c | 9 ++----- > > > drivers/media/usb/uvc/uvc_driver.c | 9 +++---- > > > 13 files changed, 41 insertions(+), 84 deletions(-) > > > > > > diff --git a/drivers/media/cec/platform/stm32/stm32-cec.c b/drivers/media/cec/platform/stm32/stm32-cec.c > > > index 40db7911b437..7b2db46a5722 100644 > > > --- a/drivers/media/cec/platform/stm32/stm32-cec.c > > > +++ b/drivers/media/cec/platform/stm32/stm32-cec.c > > > @@ -288,12 +288,9 @@ static int stm32_cec_probe(struct platform_device *pdev) > > > return ret; > > > > > > cec->clk_cec = devm_clk_get(&pdev->dev, "cec"); > > > - if (IS_ERR(cec->clk_cec)) { > > > - if (PTR_ERR(cec->clk_cec) != -EPROBE_DEFER) > > > - dev_err(&pdev->dev, "Cannot get cec clock\n"); > > > - > > > - return PTR_ERR(cec->clk_cec); > > > - } > > > + if (IS_ERR(cec->clk_cec)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(cec->clk_cec), > > > + "Cannot get cec clock\n"); > > > > > > ret = clk_prepare(cec->clk_cec); > > > if (ret) { > > > diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c > > > index 516de278cc49..6a7f8ef9db05 100644 > > > --- a/drivers/media/i2c/ad5820.c > > > +++ b/drivers/media/i2c/ad5820.c > > > @@ -301,21 +301,15 @@ static int ad5820_probe(struct i2c_client *client, > > > return -ENOMEM; > > > > > > coil->vana = devm_regulator_get(&client->dev, "VANA"); > > > - if (IS_ERR(coil->vana)) { > > > - ret = PTR_ERR(coil->vana); > > > - if (ret != -EPROBE_DEFER) > > > - dev_err(&client->dev, "could not get regulator for vana\n"); > > > - return ret; > > > - } > > > + if (IS_ERR(coil->vana)) > > > + return dev_err_probe(&client->dev, PTR_ERR(coil->vana), > > > + "could not get regulator for vana\n"); > > > > > > coil->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", > > > GPIOD_OUT_LOW); > > > - if (IS_ERR(coil->enable_gpio)) { > > > - ret = PTR_ERR(coil->enable_gpio); > > > - if (ret != -EPROBE_DEFER) > > > - dev_err(&client->dev, "could not get enable gpio\n"); > > > - return ret; > > > - } > > > + if (IS_ERR(coil->enable_gpio)) > > > + return dev_err_probe(&client->dev, PTR_ERR(coil->enable_gpio), > > > + "could not get enable gpio\n"); > > > > > > mutex_init(&coil->power_lock); > > > > > > diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > > > index a00761b1e18c..9219f3c9594b 100644 > > > --- a/drivers/media/i2c/imx274.c > > > +++ b/drivers/media/i2c/imx274.c > > > @@ -2060,9 +2060,8 @@ static int imx274_probe(struct i2c_client *client) > > > imx274->reset_gpio = devm_gpiod_get_optional(dev, "reset", > > > GPIOD_OUT_HIGH); > > > if (IS_ERR(imx274->reset_gpio)) { > > > - if (PTR_ERR(imx274->reset_gpio) != -EPROBE_DEFER) > > > - dev_err(dev, "Reset GPIO not setup in DT"); > > > - ret = PTR_ERR(imx274->reset_gpio); > > > + ret = dev_err_probe(dev, PTR_ERR(imx274->reset_gpio), > > > + "Reset GPIO not setup in DT\n"); > > > goto err_me; > > > > Not sure I like the use of dev_err_probe here. We only save one line. > > Same goes for all the other cases where there is no "return dev_err_probe..." > > It's not just about saving a line, dev_err_probe() also records the > cause of probe deferral (without printing the message) and makes it > available through debugfs. Ahh, that is cool :), ignore my previous message then > > > > } > > > > > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c > > > index 200841c1f5cf..9197fa0b1bc2 100644 > > > --- a/drivers/media/i2c/tc358743.c > > > +++ b/drivers/media/i2c/tc358743.c > > > @@ -1891,12 +1891,9 @@ static int tc358743_probe_of(struct tc358743_state *state) > > > int ret; > > > > > > refclk = devm_clk_get(dev, "refclk"); > > > - if (IS_ERR(refclk)) { > > > - if (PTR_ERR(refclk) != -EPROBE_DEFER) > > > - dev_err(dev, "failed to get refclk: %ld\n", > > > - PTR_ERR(refclk)); > > > - return PTR_ERR(refclk); > > > - } > > > + if (IS_ERR(refclk)) > > > + return dev_err_probe(dev, PTR_ERR(refclk), > > > + "failed to get refclk\n"); > > > > > > ep = of_graph_get_next_endpoint(dev->of_node, NULL); > > > if (!ep) { > > > diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > > index 1e3833f1c9ae..ad5fab2d8bfa 100644 > > > --- a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > > +++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c > > > @@ -52,9 +52,8 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > > > for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { > > > comp->clk[i] = of_clk_get(node, i); > > > if (IS_ERR(comp->clk[i])) { > > > - if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) > > > - dev_err(dev, "Failed to get clock\n"); > > > - ret = PTR_ERR(comp->clk[i]); > > > + ret = dev_err_probe(dev, PTR_ERR(comp->clk[i]), > > > + "Failed to get clock\n"); > > > goto put_dev; > > > } > > > > > > diff --git a/drivers/media/platform/samsung/exynos4-is/media-dev.c b/drivers/media/platform/samsung/exynos4-is/media-dev.c > > > index 52b43ea04030..7a9d51b8bb4c 100644 > > > --- a/drivers/media/platform/samsung/exynos4-is/media-dev.c > > > +++ b/drivers/media/platform/samsung/exynos4-is/media-dev.c > > > @@ -1473,9 +1473,7 @@ static int fimc_md_probe(struct platform_device *pdev) > > > > > > pinctrl = devm_pinctrl_get(dev); > > > if (IS_ERR(pinctrl)) { > > > - ret = PTR_ERR(pinctrl); > > > - if (ret != EPROBE_DEFER) > > > - dev_err(dev, "Failed to get pinctrl: %d\n", ret); > > > + ret = dev_err_probe(dev, PTR_ERR(pinctrl), "Failed to get pinctrl\n"); > > > goto err_clk; > > > } > > > > > > diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c > > > index 2ca95ab2b0fe..ec138843d090 100644 > > > --- a/drivers/media/platform/st/stm32/stm32-dcmi.c > > > +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c > > > @@ -1946,12 +1946,9 @@ static int dcmi_probe(struct platform_device *pdev) > > > return -ENOMEM; > > > > > > dcmi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > > > - if (IS_ERR(dcmi->rstc)) { > > > - if (PTR_ERR(dcmi->rstc) != -EPROBE_DEFER) > > > - dev_err(&pdev->dev, "Could not get reset control\n"); > > > - > > > - return PTR_ERR(dcmi->rstc); > > > - } > > > + if (IS_ERR(dcmi->rstc)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(dcmi->rstc), > > > + "Could not get reset control\n"); > > > > > > /* Get bus characteristics from devicetree */ > > > np = of_graph_get_next_endpoint(np, NULL); > > > @@ -2003,20 +2000,14 @@ static int dcmi_probe(struct platform_device *pdev) > > > } > > > > > > mclk = devm_clk_get(&pdev->dev, "mclk"); > > > - if (IS_ERR(mclk)) { > > > - if (PTR_ERR(mclk) != -EPROBE_DEFER) > > > - dev_err(&pdev->dev, "Unable to get mclk\n"); > > > - return PTR_ERR(mclk); > > > - } > > > + if (IS_ERR(mclk)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(mclk), > > > + "Unable to get mclk\n"); > > > > > > chan = dma_request_chan(&pdev->dev, "tx"); > > > - if (IS_ERR(chan)) { > > > - ret = PTR_ERR(chan); > > > - if (ret != -EPROBE_DEFER) > > > - dev_err(&pdev->dev, > > > - "Failed to request DMA channel: %d\n", ret); > > > - return ret; > > > - } > > > + if (IS_ERR(chan)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(chan), > > > + "Failed to request DMA channel\n"); > > > > > > dcmi->dma_max_burst = UINT_MAX; > > > ret = dma_get_slave_caps(chan, &caps); > > > diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c > > > index a6052df9bb19..5d6867b8f197 100644 > > > --- a/drivers/media/platform/ti/omap3isp/isp.c > > > +++ b/drivers/media/platform/ti/omap3isp/isp.c > > > @@ -1886,8 +1886,7 @@ static int isp_initialize_modules(struct isp_device *isp) > > > > > > ret = omap3isp_ccp2_init(isp); > > > if (ret < 0) { > > > - if (ret != -EPROBE_DEFER) > > > - dev_err(isp->dev, "CCP2 initialization failed\n"); > > > + dev_err_probe(isp->dev, ret, "CCP2 initialization failed\n"); > > > goto error_ccp2; > > > } > > > > > > diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > > index 29b53febc2e7..d8a23f18cfbc 100644 > > > --- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > > +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c > > > @@ -976,11 +976,9 @@ static int xcsi2rxss_probe(struct platform_device *pdev) > > > /* Reset GPIO */ > > > xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset", > > > GPIOD_OUT_HIGH); > > > - if (IS_ERR(xcsi2rxss->rst_gpio)) { > > > - if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER) > > > - dev_err(dev, "Video Reset GPIO not setup in DT"); > > > - return PTR_ERR(xcsi2rxss->rst_gpio); > > > - } > > > + if (IS_ERR(xcsi2rxss->rst_gpio)) > > > + return dev_err_probe(dev, PTR_ERR(xcsi2rxss->rst_gpio), > > > + "Video Reset GPIO not setup in DT\n"); > > > > > > ret = xcsi2rxss_parse_of(xcsi2rxss); > > > if (ret < 0) > > > diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c > > > index 22e524b69806..8f1fff7af6c9 100644 > > > --- a/drivers/media/rc/gpio-ir-recv.c > > > +++ b/drivers/media/rc/gpio-ir-recv.c > > > @@ -74,13 +74,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) > > > return -ENOMEM; > > > > > > gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN); > > > - if (IS_ERR(gpio_dev->gpiod)) { > > > - rc = PTR_ERR(gpio_dev->gpiod); > > > - /* Just try again if this happens */ > > > - if (rc != -EPROBE_DEFER) > > > - dev_err(dev, "error getting gpio (%d)\n", rc); > > > - return rc; > > > - } > > > + if (IS_ERR(gpio_dev->gpiod)) > > > + return dev_err_probe(dev, PTR_ERR(gpio_dev->gpiod), > > > + "error getting gpio\n"); > > > gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod); > > > if (gpio_dev->irq < 0) > > > return gpio_dev->irq; > > > diff --git a/drivers/media/rc/gpio-ir-tx.c b/drivers/media/rc/gpio-ir-tx.c > > > index d3063ddb472e..2b829c146db1 100644 > > > --- a/drivers/media/rc/gpio-ir-tx.c > > > +++ b/drivers/media/rc/gpio-ir-tx.c > > > @@ -174,12 +174,9 @@ static int gpio_ir_tx_probe(struct platform_device *pdev) > > > return -ENOMEM; > > > > > > gpio_ir->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW); > > > - if (IS_ERR(gpio_ir->gpio)) { > > > - if (PTR_ERR(gpio_ir->gpio) != -EPROBE_DEFER) > > > - dev_err(&pdev->dev, "Failed to get gpio (%ld)\n", > > > - PTR_ERR(gpio_ir->gpio)); > > > - return PTR_ERR(gpio_ir->gpio); > > > - } > > > + if (IS_ERR(gpio_ir->gpio)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(gpio_ir->gpio), > > > + "Failed to get gpio\n"); > > > > > > rcdev->priv = gpio_ir; > > > rcdev->driver_name = DRIVER_NAME; > > > diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c > > > index a3b145183260..85080c3d2053 100644 > > > --- a/drivers/media/rc/ir-rx51.c > > > +++ b/drivers/media/rc/ir-rx51.c > > > @@ -231,13 +231,8 @@ static int ir_rx51_probe(struct platform_device *dev) > > > struct rc_dev *rcdev; > > > > > > pwm = pwm_get(&dev->dev, NULL); > > > - if (IS_ERR(pwm)) { > > > - int err = PTR_ERR(pwm); > > > - > > > - if (err != -EPROBE_DEFER) > > > - dev_err(&dev->dev, "pwm_get failed: %d\n", err); > > > - return err; > > > - } > > > + if (IS_ERR(pwm)) > > > + return dev_err_probe(&dev->dev, PTR_ERR(pwm), "pwm_get failed\n"); > > > > > > /* Use default, in case userspace does not set the carrier */ > > > ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm), NSEC_PER_SEC); > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > > index 744051b52e12..94f84c3c4712 100644 > > > --- a/drivers/media/usb/uvc/uvc_driver.c > > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > > @@ -1554,12 +1554,9 @@ static int uvc_gpio_parse(struct uvc_device *dev) > > > return PTR_ERR_OR_ZERO(gpio_privacy); > > > > > > irq = gpiod_to_irq(gpio_privacy); > > > - if (irq < 0) { > > > - if (irq != EPROBE_DEFER) > > > - dev_err(&dev->udev->dev, > > > - "No IRQ for privacy GPIO (%d)\n", irq); > > > - return irq; > > > - } > > > + if (irq < 0) > > > + return dev_err_probe(&dev->udev->dev, irq, > > > + "No IRQ for privacy GPIO\n"); > > > > > > unit = uvc_alloc_entity(UVC_EXT_GPIO_UNIT, UVC_EXT_GPIO_UNIT_ID, 0, 1); > > > if (!unit) > > -- > Regards, > > Laurent Pinchart
Hi Yang, On Mon, Sep 19, 2022 at 11:58:43PM +0800, Yang Yingliang wrote: > In the probe path, dev_err() can be replaced with dev_err_probe() > which will check if error code is -EPROBE_DEFER. I don't really disagree with changing to dev_err_probe(). But I would like to ask how have you selected the drivers and calls calls in them that you do change. E.g. the imx274 driver has a number of such calls and the patch appears to change one of them. Other drivers similar to imx274 (e.g. other sensor drivers) do use dev_err() as well. I wonder how difficult it would be to do this more systematically with Coccinelle.
Hi, On 2022/9/20 16:33, Sakari Ailus wrote: > Hi Yang, > > On Mon, Sep 19, 2022 at 11:58:43PM +0800, Yang Yingliang wrote: >> In the probe path, dev_err() can be replaced with dev_err_probe() >> which will check if error code is -EPROBE_DEFER. > I don't really disagree with changing to dev_err_probe(). But I would like > to ask how have you selected the drivers and calls calls in them that you > do change. The drivers that check if error code is EPROBE_DEFER when handling error case in probe path. > > E.g. the imx274 driver has a number of such calls and the patch appears to > change one of them. Other drivers similar to imx274 (e.g. other sensor > drivers) do use dev_err() as well. dev_err_probe() will check if error code is EPROBE_DEFER, the rest of such calls in imx274 driver don't check EPROBE_DEFER, so I don't replace them. > > I wonder how difficult it would be to do this more systematically with > Coccinelle. >
Hi Yang, On Tue, Sep 20, 2022 at 05:13:11PM +0800, Yang Yingliang wrote: > Hi, > > On 2022/9/20 16:33, Sakari Ailus wrote: > > Hi Yang, > > > > On Mon, Sep 19, 2022 at 11:58:43PM +0800, Yang Yingliang wrote: > > > In the probe path, dev_err() can be replaced with dev_err_probe() > > > which will check if error code is -EPROBE_DEFER. > > I don't really disagree with changing to dev_err_probe(). But I would like > > to ask how have you selected the drivers and calls calls in them that you > > do change. > The drivers that check if error code is EPROBE_DEFER when handling error > case in probe > path. Ah, I see you're only changing this where the printing of the error was conditional on the error being -EPROBE_DEFER. Many drivers still do print an error when returning -EPROBE_DEFER but I guess they can be handled separately. Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
diff --git a/drivers/media/cec/platform/stm32/stm32-cec.c b/drivers/media/cec/platform/stm32/stm32-cec.c index 40db7911b437..7b2db46a5722 100644 --- a/drivers/media/cec/platform/stm32/stm32-cec.c +++ b/drivers/media/cec/platform/stm32/stm32-cec.c @@ -288,12 +288,9 @@ static int stm32_cec_probe(struct platform_device *pdev) return ret; cec->clk_cec = devm_clk_get(&pdev->dev, "cec"); - if (IS_ERR(cec->clk_cec)) { - if (PTR_ERR(cec->clk_cec) != -EPROBE_DEFER) - dev_err(&pdev->dev, "Cannot get cec clock\n"); - - return PTR_ERR(cec->clk_cec); - } + if (IS_ERR(cec->clk_cec)) + return dev_err_probe(&pdev->dev, PTR_ERR(cec->clk_cec), + "Cannot get cec clock\n"); ret = clk_prepare(cec->clk_cec); if (ret) { diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c index 516de278cc49..6a7f8ef9db05 100644 --- a/drivers/media/i2c/ad5820.c +++ b/drivers/media/i2c/ad5820.c @@ -301,21 +301,15 @@ static int ad5820_probe(struct i2c_client *client, return -ENOMEM; coil->vana = devm_regulator_get(&client->dev, "VANA"); - if (IS_ERR(coil->vana)) { - ret = PTR_ERR(coil->vana); - if (ret != -EPROBE_DEFER) - dev_err(&client->dev, "could not get regulator for vana\n"); - return ret; - } + if (IS_ERR(coil->vana)) + return dev_err_probe(&client->dev, PTR_ERR(coil->vana), + "could not get regulator for vana\n"); coil->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_LOW); - if (IS_ERR(coil->enable_gpio)) { - ret = PTR_ERR(coil->enable_gpio); - if (ret != -EPROBE_DEFER) - dev_err(&client->dev, "could not get enable gpio\n"); - return ret; - } + if (IS_ERR(coil->enable_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(coil->enable_gpio), + "could not get enable gpio\n"); mutex_init(&coil->power_lock); diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c index a00761b1e18c..9219f3c9594b 100644 --- a/drivers/media/i2c/imx274.c +++ b/drivers/media/i2c/imx274.c @@ -2060,9 +2060,8 @@ static int imx274_probe(struct i2c_client *client) imx274->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(imx274->reset_gpio)) { - if (PTR_ERR(imx274->reset_gpio) != -EPROBE_DEFER) - dev_err(dev, "Reset GPIO not setup in DT"); - ret = PTR_ERR(imx274->reset_gpio); + ret = dev_err_probe(dev, PTR_ERR(imx274->reset_gpio), + "Reset GPIO not setup in DT\n"); goto err_me; } diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 200841c1f5cf..9197fa0b1bc2 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1891,12 +1891,9 @@ static int tc358743_probe_of(struct tc358743_state *state) int ret; refclk = devm_clk_get(dev, "refclk"); - if (IS_ERR(refclk)) { - if (PTR_ERR(refclk) != -EPROBE_DEFER) - dev_err(dev, "failed to get refclk: %ld\n", - PTR_ERR(refclk)); - return PTR_ERR(refclk); - } + if (IS_ERR(refclk)) + return dev_err_probe(dev, PTR_ERR(refclk), + "failed to get refclk\n"); ep = of_graph_get_next_endpoint(dev->of_node, NULL); if (!ep) { diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c index 1e3833f1c9ae..ad5fab2d8bfa 100644 --- a/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c +++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_comp.c @@ -52,9 +52,8 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { comp->clk[i] = of_clk_get(node, i); if (IS_ERR(comp->clk[i])) { - if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) - dev_err(dev, "Failed to get clock\n"); - ret = PTR_ERR(comp->clk[i]); + ret = dev_err_probe(dev, PTR_ERR(comp->clk[i]), + "Failed to get clock\n"); goto put_dev; } diff --git a/drivers/media/platform/samsung/exynos4-is/media-dev.c b/drivers/media/platform/samsung/exynos4-is/media-dev.c index 52b43ea04030..7a9d51b8bb4c 100644 --- a/drivers/media/platform/samsung/exynos4-is/media-dev.c +++ b/drivers/media/platform/samsung/exynos4-is/media-dev.c @@ -1473,9 +1473,7 @@ static int fimc_md_probe(struct platform_device *pdev) pinctrl = devm_pinctrl_get(dev); if (IS_ERR(pinctrl)) { - ret = PTR_ERR(pinctrl); - if (ret != EPROBE_DEFER) - dev_err(dev, "Failed to get pinctrl: %d\n", ret); + ret = dev_err_probe(dev, PTR_ERR(pinctrl), "Failed to get pinctrl\n"); goto err_clk; } diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c index 2ca95ab2b0fe..ec138843d090 100644 --- a/drivers/media/platform/st/stm32/stm32-dcmi.c +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c @@ -1946,12 +1946,9 @@ static int dcmi_probe(struct platform_device *pdev) return -ENOMEM; dcmi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); - if (IS_ERR(dcmi->rstc)) { - if (PTR_ERR(dcmi->rstc) != -EPROBE_DEFER) - dev_err(&pdev->dev, "Could not get reset control\n"); - - return PTR_ERR(dcmi->rstc); - } + if (IS_ERR(dcmi->rstc)) + return dev_err_probe(&pdev->dev, PTR_ERR(dcmi->rstc), + "Could not get reset control\n"); /* Get bus characteristics from devicetree */ np = of_graph_get_next_endpoint(np, NULL); @@ -2003,20 +2000,14 @@ static int dcmi_probe(struct platform_device *pdev) } mclk = devm_clk_get(&pdev->dev, "mclk"); - if (IS_ERR(mclk)) { - if (PTR_ERR(mclk) != -EPROBE_DEFER) - dev_err(&pdev->dev, "Unable to get mclk\n"); - return PTR_ERR(mclk); - } + if (IS_ERR(mclk)) + return dev_err_probe(&pdev->dev, PTR_ERR(mclk), + "Unable to get mclk\n"); chan = dma_request_chan(&pdev->dev, "tx"); - if (IS_ERR(chan)) { - ret = PTR_ERR(chan); - if (ret != -EPROBE_DEFER) - dev_err(&pdev->dev, - "Failed to request DMA channel: %d\n", ret); - return ret; - } + if (IS_ERR(chan)) + return dev_err_probe(&pdev->dev, PTR_ERR(chan), + "Failed to request DMA channel\n"); dcmi->dma_max_burst = UINT_MAX; ret = dma_get_slave_caps(chan, &caps); diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c index a6052df9bb19..5d6867b8f197 100644 --- a/drivers/media/platform/ti/omap3isp/isp.c +++ b/drivers/media/platform/ti/omap3isp/isp.c @@ -1886,8 +1886,7 @@ static int isp_initialize_modules(struct isp_device *isp) ret = omap3isp_ccp2_init(isp); if (ret < 0) { - if (ret != -EPROBE_DEFER) - dev_err(isp->dev, "CCP2 initialization failed\n"); + dev_err_probe(isp->dev, ret, "CCP2 initialization failed\n"); goto error_ccp2; } diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c index 29b53febc2e7..d8a23f18cfbc 100644 --- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c @@ -976,11 +976,9 @@ static int xcsi2rxss_probe(struct platform_device *pdev) /* Reset GPIO */ xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset", GPIOD_OUT_HIGH); - if (IS_ERR(xcsi2rxss->rst_gpio)) { - if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER) - dev_err(dev, "Video Reset GPIO not setup in DT"); - return PTR_ERR(xcsi2rxss->rst_gpio); - } + if (IS_ERR(xcsi2rxss->rst_gpio)) + return dev_err_probe(dev, PTR_ERR(xcsi2rxss->rst_gpio), + "Video Reset GPIO not setup in DT\n"); ret = xcsi2rxss_parse_of(xcsi2rxss); if (ret < 0) diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 22e524b69806..8f1fff7af6c9 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -74,13 +74,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) return -ENOMEM; gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN); - if (IS_ERR(gpio_dev->gpiod)) { - rc = PTR_ERR(gpio_dev->gpiod); - /* Just try again if this happens */ - if (rc != -EPROBE_DEFER) - dev_err(dev, "error getting gpio (%d)\n", rc); - return rc; - } + if (IS_ERR(gpio_dev->gpiod)) + return dev_err_probe(dev, PTR_ERR(gpio_dev->gpiod), + "error getting gpio\n"); gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod); if (gpio_dev->irq < 0) return gpio_dev->irq; diff --git a/drivers/media/rc/gpio-ir-tx.c b/drivers/media/rc/gpio-ir-tx.c index d3063ddb472e..2b829c146db1 100644 --- a/drivers/media/rc/gpio-ir-tx.c +++ b/drivers/media/rc/gpio-ir-tx.c @@ -174,12 +174,9 @@ static int gpio_ir_tx_probe(struct platform_device *pdev) return -ENOMEM; gpio_ir->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW); - if (IS_ERR(gpio_ir->gpio)) { - if (PTR_ERR(gpio_ir->gpio) != -EPROBE_DEFER) - dev_err(&pdev->dev, "Failed to get gpio (%ld)\n", - PTR_ERR(gpio_ir->gpio)); - return PTR_ERR(gpio_ir->gpio); - } + if (IS_ERR(gpio_ir->gpio)) + return dev_err_probe(&pdev->dev, PTR_ERR(gpio_ir->gpio), + "Failed to get gpio\n"); rcdev->priv = gpio_ir; rcdev->driver_name = DRIVER_NAME; diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c index a3b145183260..85080c3d2053 100644 --- a/drivers/media/rc/ir-rx51.c +++ b/drivers/media/rc/ir-rx51.c @@ -231,13 +231,8 @@ static int ir_rx51_probe(struct platform_device *dev) struct rc_dev *rcdev; pwm = pwm_get(&dev->dev, NULL); - if (IS_ERR(pwm)) { - int err = PTR_ERR(pwm); - - if (err != -EPROBE_DEFER) - dev_err(&dev->dev, "pwm_get failed: %d\n", err); - return err; - } + if (IS_ERR(pwm)) + return dev_err_probe(&dev->dev, PTR_ERR(pwm), "pwm_get failed\n"); /* Use default, in case userspace does not set the carrier */ ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm), NSEC_PER_SEC); diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 744051b52e12..94f84c3c4712 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1554,12 +1554,9 @@ static int uvc_gpio_parse(struct uvc_device *dev) return PTR_ERR_OR_ZERO(gpio_privacy); irq = gpiod_to_irq(gpio_privacy); - if (irq < 0) { - if (irq != EPROBE_DEFER) - dev_err(&dev->udev->dev, - "No IRQ for privacy GPIO (%d)\n", irq); - return irq; - } + if (irq < 0) + return dev_err_probe(&dev->udev->dev, irq, + "No IRQ for privacy GPIO\n"); unit = uvc_alloc_entity(UVC_EXT_GPIO_UNIT, UVC_EXT_GPIO_UNIT_ID, 0, 1); if (!unit)