On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
> From: Cristian Ciocaltea <[email protected]>
>
> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
> i.e. in some cases the error code is not included in the message, while
> in some other cases there is no check for -EPROBE_DEFER.
>
> Since this is part of the probe path, address the aforementioned issues
> by switching to dev_err_probe(), which also reduces the code a bit.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 62 +++++++------------
> 1 file changed, 24 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 7d531b6f4c09..4e7794aa2dde 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev,
> struct device *master,
> return -ENODEV;
>
> if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
> - !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
> - dev_err(dev, "Missing platform ctrl ops\n");
> - return -ENODEV;
> - }
> + !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
> + return dev_err_probe(dev, -ENODEV, "Missing platform ctrl
> ops\n");
This only makes sense for the purpose of unification.
>
> hdmi->ctrl_ops = cfg->ctrl_ops;
> hdmi->dev = &pdev->dev;
> @@ -473,10 +471,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev,
> struct device *master,
> break;
> }
> }
> - if (hdmi->port_id < 0) {
> - dev_err(hdmi->dev, "Failed to match HDMI port ID\n");
> - return hdmi->port_id;
> - }
> + if (hdmi->port_id < 0)
> + return dev_err_probe(hdmi->dev, hdmi->port_id,
> + "Failed to match HDMI port ID\n");
The port_id can't become -EPROBE_DEFER, so it also unnecessary.
>
> plat_data.phy_ops = cfg->phy_ops;
> plat_data.phy_data = hdmi;
> @@ -497,39 +494,30 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev,
> struct device *master,
>
> hdmi->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> - if (IS_ERR(hdmi->regmap)) {
> - dev_err(hdmi->dev, "Unable to get rockchip,grf\n");
> - return PTR_ERR(hdmi->regmap);
> - }
> + if (IS_ERR(hdmi->regmap))
> + return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->regmap),
> + "Unable to get rockchip,grf\n");
>
> hdmi->vo_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,vo-grf");
> - if (IS_ERR(hdmi->vo_regmap)) {
> - dev_err(hdmi->dev, "Unable to get rockchip,vo-grf\n");
> - return PTR_ERR(hdmi->vo_regmap);
> - }
> + if (IS_ERR(hdmi->vo_regmap))
> + return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->vo_regmap),
> + "Unable to get rockchip,vo-grf\n");
>
> ret = devm_clk_bulk_get_all_enabled(hdmi->dev, &clks);
> - if (ret < 0) {
> - dev_err(hdmi->dev, "Failed to get clocks: %d\n", ret);
> - return ret;
> - }
> + if (ret < 0)
> + return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
>
> hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
> GPIOD_OUT_HIGH);
> - if (IS_ERR(hdmi->enable_gpio)) {
> - ret = PTR_ERR(hdmi->enable_gpio);
> - dev_err(hdmi->dev, "Failed to request enable GPIO: %d\n", ret);
> - return ret;
> - }
> + if (IS_ERR(hdmi->enable_gpio))
> + return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->enable_gpio),
> + "Failed to request enable GPIO\n");
>
> hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
> - if (IS_ERR(hdmi->phy)) {
> - ret = PTR_ERR(hdmi->phy);
> - if (ret != -EPROBE_DEFER)
> - dev_err(hdmi->dev, "failed to get phy: %d\n", ret);
> - return ret;
> - }
> + if (IS_ERR(hdmi->phy))
> + return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->phy),
> + "Failed to get phy\n");
>
> cfg->ctrl_ops->io_init(hdmi);
>
> @@ -558,17 +546,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev,
> struct device *master,
>
> hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
> if (IS_ERR(hdmi->hdmi)) {
> - ret = PTR_ERR(hdmi->hdmi);
> drm_encoder_cleanup(encoder);
> - return ret;
> + return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
> + "Failed to bind dw-hdmi-qp");
> }
>
> connector = drm_bridge_connector_init(drm, encoder);
This one is also not supposed to return -EPROBE_DEFER.
> - if (IS_ERR(connector)) {
> - ret = PTR_ERR(connector);
> - dev_err(hdmi->dev, "failed to init bridge connector: %d\n",
> ret);
> - return ret;
> - }
> + if (IS_ERR(connector))
> + return dev_err_probe(hdmi->dev, PTR_ERR(connector),
> + "Failed to init bridge connector\n");
>
> return drm_connector_attach_encoder(connector, encoder);
> }
> --
> Regards,
>
> Laurent Pinchart
>
--
With best wishes
Dmitry
