Re: [PATCH v6 25/42] drm/mediatek: mtk_hdmi: Move CEC device parsing in new function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 13/02/25 08:42, CK Hu (胡俊光) ha scritto:
On Tue, 2025-02-11 at 12:33 +0100, AngeloGioacchino Del Regno wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.


Move the CEC device parsing logic to a new function called
mtk_hdmi_get_cec_dev(), and move the parsing action to the end
of mtk_hdmi_dt_parse_pdata(), allowing to remove gotos in this
function, reducing code size and improving readability.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
---
  drivers/gpu/drm/mediatek/mtk_hdmi.c | 82 ++++++++++++++---------------
  1 file changed, 39 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 6140b55c2830..03b56588fc7d 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1367,20 +1367,12 @@ static const struct drm_bridge_funcs mtk_hdmi_bridge_funcs = {
         .edid_read = mtk_hdmi_bridge_edid_read,
  };

-static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
-                                  struct platform_device *pdev)
+static int mtk_hdmi_get_cec_dev(struct mtk_hdmi *hdmi, struct device *dev, struct device_node *np)
  {
-       struct device *dev = &pdev->dev;
-       struct device_node *np = dev->of_node;
-       struct device_node *remote, *i2c_np;
         struct platform_device *cec_pdev;
-       struct regmap *regmap;
+       struct device_node *cec_np;
         int ret;

-       ret = mtk_hdmi_get_all_clk(hdmi, np);
-       if (ret)
-               return dev_err_probe(dev, ret, "Failed to get clocks\n");
-
         /* The CEC module handles HDMI hotplug detection */
         cec_np = of_get_compatible_child(np->parent, "mediatek,mt8173-cec");
         if (!cec_np)
@@ -1394,65 +1386,69 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
                 return -EPROBE_DEFER;
         }
         of_node_put(cec_np);
-       hdmi->cec_dev = &cec_pdev->dev;

         /*
          * The mediatek,syscon-hdmi property contains a phandle link to the
          * MMSYS_CONFIG device and the register offset of the HDMI_SYS_CFG
          * registers it contains.
          */
-       regmap = syscon_regmap_lookup_by_phandle(np, "mediatek,syscon-hdmi");
-       ret = of_property_read_u32_index(np, "mediatek,syscon-hdmi", 1,
-                                        &hdmi->sys_offset);
-       if (IS_ERR(regmap))
-               ret = PTR_ERR(regmap);
-       if (ret) {
-               dev_err_probe(dev, ret,
-                             "Failed to get system configuration registers\n");
-               goto put_device;
-       }
-       hdmi->sys_regmap = regmap;
+       hdmi->sys_regmap = syscon_regmap_lookup_by_phandle(np, "mediatek,syscon-hdmi");
+       if (IS_ERR(hdmi->sys_regmap))
+               return PTR_ERR(hdmi->sys_regmap);
+
+       ret = of_property_read_u32_index(np, "mediatek,syscon-hdmi", 1, &hdmi->sys_offset);
+       if (ret)
+               return dev_err_probe(dev, ret,
+                                    "Failed to get system configuration registers\n");
+
+       hdmi->cec_dev = &cec_pdev->dev;
+       return 0;
+}
+
+static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
+                                  struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct device_node *np = dev->of_node;
+       struct device_node *remote, *i2c_np;
+       int ret;
+
+       ret = mtk_hdmi_get_all_clk(hdmi, np);
+       if (ret)
+               return dev_err_probe(dev, ret, "Failed to get clocks\n");

         hdmi->regs = device_node_to_regmap(dev->of_node);
-       if (IS_ERR(hdmi->regs)) {
-               ret = PTR_ERR(hdmi->regs);
-               goto put_device;
-       }
+       if (IS_ERR(hdmi->regs))
+               return PTR_ERR(hdmi->regs);

         remote = of_graph_get_remote_node(np, 1, 0);
-       if (!remote) {
-               ret = -EINVAL;
-               goto put_device;
-       }
+       if (!remote)
+               return -EINVAL;

         if (!of_device_is_compatible(remote, "hdmi-connector")) {
                 hdmi->next_bridge = of_drm_find_bridge(remote);
                 if (!hdmi->next_bridge) {
                         dev_err(dev, "Waiting for external bridge\n");
                         of_node_put(remote);
-                       ret = -EPROBE_DEFER;
-                       goto put_device;
+                       return -EPROBE_DEFER;
                 }
         }

         i2c_np = of_parse_phandle(remote, "ddc-i2c-bus", 0);
         of_node_put(remote);
-       if (!i2c_np) {
-               ret = dev_err_probe(dev, -EINVAL, "No ddc-i2c-bus in connector\n");
-               goto put_device;
-       }
+       if (!i2c_np)
+               return dev_err_probe(dev, -EINVAL, "No ddc-i2c-bus in connector\n");

         hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np);
         of_node_put(i2c_np);
-       if (!hdmi->ddc_adpt) {
-               ret = dev_err_probe(dev, -EINVAL, "Failed to get ddc i2c adapter by node\n");
-               goto put_device;
-       }
+       if (!hdmi->ddc_adpt)
+               return dev_err_probe(dev, -EINVAL, "Failed to get ddc i2c adapter by node\n");
+
+       ret = mtk_hdmi_get_cec_dev(hdmi, dev, np);
+       if (ret)
+               return ret;

         return 0;

return mtk_hdmi_get_cec_dev(hdmi, dev, np);

or

ret = mtk_hdmi_get_cec_dev(hdmi, dev, np);

return ret;

After this,

Reviewed-by: CK Hu <ck.hu@xxxxxxxxxxxx>


Can we please avoid this change?

That's done so that any addition to the end of the function produces a smaller diff
and for return value readability.

Thanks,
Angelo

-put_device:
-       put_device(hdmi->cec_dev);
-       return ret;
  }

  /*
--
2.48.1






[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux