diff mbox series

[v5,21/34] drm/mediatek: mtk_hdmi: Move CEC device parsing in new function

Message ID 20250113145232.227674-22-angelogioacchino.delregno@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add support for MT8195/88 DPI, HDMIv2 and DDCv2 | expand

Commit Message

AngeloGioacchino Del Regno Jan. 13, 2025, 2:52 p.m. UTC
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 | 84 ++++++++++++++---------------
 1 file changed, 40 insertions(+), 44 deletions(-)

Comments

Alexandre Mergnat Feb. 7, 2025, 2:45 p.m. UTC | #1
On 13/01/2025 15:52, AngeloGioacchino Del Regno wrote:
> 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.

Why CEC device parsing logic isn't done mtk_cec.c driver ? Then add "mtk_cec_get_dev" function to 
mtk_cec.c too. Finally, call this new function in mtk_hdmi_probe after mtk_hdmi_dt_parse_pdata().

> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_hdmi.c | 84 ++++++++++++++---------------
>   1 file changed, 40 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 48c37294dcbb..eb285ec394a3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1367,24 +1367,16 @@ 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 it's done in mtk_cec.c, the hardcoded compatible string method will be replaced by of_match_table 
(mtk_cec_of_ids), which is scalable and consistent.

>   	if (!cec_np)
> -		return dev_err_probe(dev, -EINVAL, "Failed to find CEC node\n");
> +		return dev_err_probe(dev, -ENOTSUPP, "Failed to find CEC node\n");
>   
>   	cec_pdev = of_find_device_by_node(cec_np);
>   	if (!cec_pdev) {
> @@ -1393,65 +1385,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;
> -put_device:
> -	put_device(hdmi->cec_dev);
> -	return ret;
>   }
>   
>   /*
CK Hu (胡俊光) Feb. 10, 2025, 6:22 a.m. UTC | #2
Hi, Angelo:

On Mon, 2025-01-13 at 15:52 +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 | 84 ++++++++++++++---------------
>  1 file changed, 40 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 48c37294dcbb..eb285ec394a3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1367,24 +1367,16 @@ 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)
> -               return dev_err_probe(dev, -EINVAL, "Failed to find CEC node\n");
> +               return dev_err_probe(dev, -ENOTSUPP, "Failed to find CEC node\n");

Changing error message should be another patch.
I see another patch has also ENOTSUPP modification.
Maybe both should in the same patch.

> 
>         cec_pdev = of_find_device_by_node(cec_np);
>         if (!cec_pdev) {
> @@ -1393,65 +1385,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;

Maybe

return mtk_hdmi_get_cec_dev(hdmi, dev, np);

Regards,
CK

> -put_device:
> -       put_device(hdmi->cec_dev);
> -       return ret;
>  }
> 
>  /*
> --
> 2.47.0
>
AngeloGioacchino Del Regno Feb. 10, 2025, 11:44 a.m. UTC | #3
Il 07/02/25 15:45, Alexandre Mergnat ha scritto:
> 
> 
> On 13/01/2025 15:52, AngeloGioacchino Del Regno wrote:
>> 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.
> 
> Why CEC device parsing logic isn't done mtk_cec.c driver ? Then add 
> "mtk_cec_get_dev" function to mtk_cec.c too. Finally, call this new function in 
> mtk_hdmi_probe after mtk_hdmi_dt_parse_pdata().
> 
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_hdmi.c | 84 ++++++++++++++---------------
>>   1 file changed, 40 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/ 
>> mtk_hdmi.c
>> index 48c37294dcbb..eb285ec394a3 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -1367,24 +1367,16 @@ 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 it's done in mtk_cec.c, the hardcoded compatible string method will be replaced 
> by of_match_table (mtk_cec_of_ids), which is scalable and consistent.
> 

That's true, yes, but the CEC driver is not supposed to be in drivers/drm/mediatek
in the first place - that should be partially refactored and moved to where the CEC
drivers belong: drivers/media/cec/platform/mediatek/

This is something that is currently in the process of making here, as the CEC v2
driver will be added there, and the CECv1 driver will be moved along side of the
new one, making things finally correct.

Of course, this code will be retained to keep retrocompatibility with older device
trees, and if I move it to mtk_cec, I will have to move it back here after the
CEC driver is moved to the correct location.

For the reasons explained above, I disagree about moving this code to the CEC
driver.

Cheers,
Angelo

>>       if (!cec_np)
>> -        return dev_err_probe(dev, -EINVAL, "Failed to find CEC node\n");
>> +        return dev_err_probe(dev, -ENOTSUPP, "Failed to find CEC node\n");
>>       cec_pdev = of_find_device_by_node(cec_np);
>>       if (!cec_pdev) {
>> @@ -1393,65 +1385,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;
>> -put_device:
>> -    put_device(hdmi->cec_dev);
>> -    return ret;
>>   }
>>   /*
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 48c37294dcbb..eb285ec394a3 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1367,24 +1367,16 @@  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)
-		return dev_err_probe(dev, -EINVAL, "Failed to find CEC node\n");
+		return dev_err_probe(dev, -ENOTSUPP, "Failed to find CEC node\n");
 
 	cec_pdev = of_find_device_by_node(cec_np);
 	if (!cec_pdev) {
@@ -1393,65 +1385,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;
-put_device:
-	put_device(hdmi->cec_dev);
-	return ret;
 }
 
 /*