<html><body><p>
<pre>
Hi, Angelo:
On Thu, 2024-12-05 at 12:45 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>
>
> In preparation for adding a new driver for the HDMI TX v2 IP,
> split out the functions that will be common between the already
> present mtk_hdmi (v1) driver and the new one.
>
> Since the probe flow for both drivers is 90% similar, add a common
> probe function that will be called from each driver's .probe()
> callback, avoiding lots of code duplication.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
[snip]
> +static int mtk_hdmi_probe(struct platform_device *pdev)
> +{
> + struct mtk_hdmi *hdmi;
>
> - ret = mtk_hdmi_clk_enable_audio(hdmi);
> - if (ret) {
> - drm_bridge_remove(&hdmi->bridge);
> - return dev_err_probe(dev, ret,
> - "Failed to enable audio clocks\n");
> - }
> + hdmi = mtk_hdmi_common_probe(pdev);
> + if (IS_ERR(hdmi))
> + return PTR_ERR(hdmi);
In original v1 driver, update_plugged_status_lock is init in probe.
I can not find the init in this patch.
It's better that moving common code seldom change the order in a function,
so you could easily to compare that moving does not change any thing.
In this patch, you not only moving code, but also change the order in a function,
so it's not easy to find something lost.
That's why I like you to separate this patch into two patches. One is adding version config and the other one is moving common code.
When I review the moving common code part, I would like a function before moving is almost same as after moving,
so it's easy to find out something lost.
Regards,
CK
>
> - return 0;
> + return mtk_hdmi_clk_enable_audio(hdmi);
> }
>
[snip]
> +struct mtk_hdmi *mtk_hdmi_common_probe(struct platform_device *pdev)
> +{
> + const struct mtk_hdmi_ver_conf *ver_conf;
> + struct device *dev = &pdev->dev;
> + struct mtk_hdmi *hdmi;
> + int ret;
> +
> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> + if (!hdmi)
> + return ERR_PTR(-ENOMEM);
> +
> + hdmi->dev = dev;
> + hdmi->conf = of_device_get_match_data(dev);
> + ver_conf = hdmi->conf->ver_conf;
> +
> + hdmi->clk = devm_kcalloc(dev, ver_conf->num_clocks, sizeof(*hdmi->clk), GFP_KERNEL);
> + if (!hdmi->clk)
> + return ERR_PTR(-ENOMEM);
> +
> + hdmi->phy = devm_phy_get(dev, "hdmi");
> + if (IS_ERR(hdmi->phy))
> + return dev_err_cast_probe(dev, hdmi->phy, "Failed to get HDMI PHY\n");
> +
> + ret = mtk_hdmi_dt_parse_pdata(hdmi, pdev, ver_conf->mtk_hdmi_clock_names,
> + ver_conf->num_clocks);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + platform_set_drvdata(pdev, hdmi);
> +
> + ret = mtk_hdmi_register_audio_driver(dev);
> + if (ret)
> + return dev_err_ptr_probe(dev, ret, "Cannot register HDMI Audio driver\n");
> +
> + hdmi->bridge.funcs = ver_conf->bridge_funcs;
> + hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
> + hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> + hdmi->bridge.of_node = pdev->dev.of_node;
> +
> + ret = devm_drm_bridge_add(dev, &hdmi->bridge);
> + if (ret)
> + return dev_err_ptr_probe(dev, ret, "Failed to add bridge\n");
> +
> + return hdmi;
> +}
> +
</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><!--}-->