<html><body><p>
<pre>
On Thu, 2025-02-13 at 11:34 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>
>
> 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@collabora.com>
> > > ---
> > > 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@mediatek.com>
> >
>
> 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.

OK,

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

>
> Thanks,
> Angelo
>
> > > -put_device:
> > > - put_device(hdmi->cec_dev);
> > > - return ret;
> > > }
> > >
> > > /*
> > > --
> > > 2.48.1
> > >
> >
>


</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice
 ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe
 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->