* [PATCH -next v2] media: Switch to use dev_err_probe() helper
@ 2022-09-19 15:58 Yang Yingliang
2022-09-19 18:25 ` Ricardo Ribalda
2022-09-20 8:33 ` Sakari Ailus
0 siblings, 2 replies; 7+ messages in thread
From: Yang Yingliang @ 2022-09-19 15:58 UTC (permalink / raw)
To: linux-media
Cc: mchehab, hverkuil-cisco, pavel, sakari.ailus, sean, laurent.pinchart
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>
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;
}
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-19 15:58 [PATCH -next v2] media: Switch to use dev_err_probe() helper Yang Yingliang
@ 2022-09-19 18:25 ` Ricardo Ribalda
2022-09-19 18:39 ` Laurent Pinchart
2022-09-20 8:33 ` Sakari Ailus
1 sibling, 1 reply; 7+ messages in thread
From: Ricardo Ribalda @ 2022-09-19 18:25 UTC (permalink / raw)
To: Yang Yingliang
Cc: linux-media, mchehab, hverkuil-cisco, pavel, sakari.ailus, sean,
laurent.pinchart
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
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-19 18:25 ` Ricardo Ribalda
@ 2022-09-19 18:39 ` Laurent Pinchart
2022-09-19 20:12 ` Ricardo Ribalda
0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2022-09-19 18:39 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Yang Yingliang, linux-media, mchehab, hverkuil-cisco, pavel,
sakari.ailus, sean
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)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-19 18:39 ` Laurent Pinchart
@ 2022-09-19 20:12 ` Ricardo Ribalda
0 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda @ 2022-09-19 20:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Yang Yingliang, linux-media, mchehab, hverkuil-cisco, pavel,
sakari.ailus, sean
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
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-19 15:58 [PATCH -next v2] media: Switch to use dev_err_probe() helper Yang Yingliang
2022-09-19 18:25 ` Ricardo Ribalda
@ 2022-09-20 8:33 ` Sakari Ailus
2022-09-20 9:13 ` Yang Yingliang
1 sibling, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2022-09-20 8:33 UTC (permalink / raw)
To: Yang Yingliang
Cc: linux-media, mchehab, hverkuil-cisco, pavel, sakari.ailus, sean,
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.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-20 8:33 ` Sakari Ailus
@ 2022-09-20 9:13 ` Yang Yingliang
2022-09-20 10:41 ` Sakari Ailus
0 siblings, 1 reply; 7+ messages in thread
From: Yang Yingliang @ 2022-09-20 9:13 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, mchehab, hverkuil-cisco, pavel, sakari.ailus, sean,
laurent.pinchart
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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] media: Switch to use dev_err_probe() helper
2022-09-20 9:13 ` Yang Yingliang
@ 2022-09-20 10:41 ` Sakari Ailus
0 siblings, 0 replies; 7+ messages in thread
From: Sakari Ailus @ 2022-09-20 10:41 UTC (permalink / raw)
To: Yang Yingliang
Cc: linux-media, mchehab, hverkuil-cisco, pavel, sakari.ailus, sean,
laurent.pinchart
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>
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-20 10:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 15:58 [PATCH -next v2] media: Switch to use dev_err_probe() helper Yang Yingliang
2022-09-19 18:25 ` Ricardo Ribalda
2022-09-19 18:39 ` Laurent Pinchart
2022-09-19 20:12 ` Ricardo Ribalda
2022-09-20 8:33 ` Sakari Ailus
2022-09-20 9:13 ` Yang Yingliang
2022-09-20 10:41 ` Sakari Ailus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.