Re: [PATCH v6 6/9] drm/mediatek: dsi: Register DSI host after acquiring clocks and PHY

[CK Hu (胡俊光)]

Hi, Angelo:

On Thu, 2024-02-15 at 09:53 +0100, AngeloGioacchino Del Regno wrote:
> Registering the dsi host with its ops before getting dsi->regs is
> simply wrong: even though there's nothing (for now) asynchronously
> calling those ops before the end of the probe function, installing
> ops that are using iospace(s) and clocks before even initializing
> those is too fragile.
> 
> Register the DSI host after getting clocks, iospace and PHY.
> This wil also allow to simplify the error paths in a later commit.

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

> 
> Reviewed-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 52758cab0abf..b3dd6251d611 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1114,14 +1114,6 @@ static int mtk_dsi_probe(struct
> platform_device *pdev)
>  	if (!dsi)
>  		return -ENOMEM;
>  
> -	dsi->host.ops = &mtk_dsi_ops;
> -	dsi->host.dev = dev;
> -	ret = mipi_dsi_host_register(&dsi->host);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to register DSI host: %d\n", ret);
> -		return ret;
> -	}
> -
>  	dsi->driver_data = of_device_get_match_data(dev);
>  
>  	dsi->engine_clk = devm_clk_get(dev, "engine");
> @@ -1130,7 +1122,7 @@ static int mtk_dsi_probe(struct platform_device
> *pdev)
>  
>  		if (ret != -EPROBE_DEFER)
>  			dev_err(dev, "Failed to get engine clock:
> %d\n", ret);
> -		goto err_unregister_host;
> +		return ret;
>  	}
>  
>  	dsi->digital_clk = devm_clk_get(dev, "digital");
> @@ -1139,14 +1131,14 @@ static int mtk_dsi_probe(struct
> platform_device *pdev)
>  
>  		if (ret != -EPROBE_DEFER)
>  			dev_err(dev, "Failed to get digital clock:
> %d\n", ret);
> -		goto err_unregister_host;
> +		return ret;
>  	}
>  
>  	dsi->hs_clk = devm_clk_get(dev, "hs");
>  	if (IS_ERR(dsi->hs_clk)) {
>  		ret = PTR_ERR(dsi->hs_clk);
>  		dev_err(dev, "Failed to get hs clock: %d\n", ret);
> -		goto err_unregister_host;
> +		return ret;
>  	}
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1154,20 +1146,28 @@ static int mtk_dsi_probe(struct
> platform_device *pdev)
>  	if (IS_ERR(dsi->regs)) {
>  		ret = PTR_ERR(dsi->regs);
>  		dev_err(dev, "Failed to ioremap memory: %d\n", ret);
> -		goto err_unregister_host;
> +		return ret;
>  	}
>  
>  	dsi->phy = devm_phy_get(dev, "dphy");
>  	if (IS_ERR(dsi->phy)) {
>  		ret = PTR_ERR(dsi->phy);
>  		dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
> -		goto err_unregister_host;
> +		return ret;
>  	}
>  
>  	irq_num = platform_get_irq(pdev, 0);
>  	if (irq_num < 0) {
>  		ret = irq_num;
> -		goto err_unregister_host;
> +		return ret;
> +	}
> +
> +	dsi->host.ops = &mtk_dsi_ops;
> +	dsi->host.dev = dev;
> +	ret = mipi_dsi_host_register(&dsi->host);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register DSI host: %d\n", ret);
> +		return ret;
>  	}
>  
>  	ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,