Re: [PATCH 2/2] drm/msm/dsi: Get PHY ref clock from the DT

From: Stephen Boyd
Date: Tue Nov 06 2018 - 18:11:53 EST


Quoting Matthias Kaehlcke (2018-11-02 14:45:34)
> @@ -630,7 +632,8 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm)
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_names = (const char *[]){
> + __clk_get_name(pll_10nm->vco_ref_clk) },

I find this syntax odd, in addition to needing to check for NULL here as
Sean pointed out. Preferably just have it be the address of the
character pointer instead of making an anonymous array and then casting
that inline, i.e

.parent_names = &ref_clk_name,

> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> @@ -786,6 +789,12 @@ struct msm_dsi_pll *msm_dsi_pll_10nm_init(struct platform_device *pdev, int id)
> pll_10nm->id = id;
> pll_10nm_list[id] = pll_10nm;
>
> + pll_10nm->vco_ref_clk = devm_clk_get(&pdev->dev, "ref");
> + if (IS_ERR(pll_10nm->vco_ref_clk)) {
> + dev_err(&pdev->dev, "couldn't get 'ref' clock\n");

This might be because of probe defer, which may be annoying to see this
failure many times.

> + return (void *)pll_10nm->vco_ref_clk;
> + }
> +
> pll_10nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
> if (IS_ERR_OR_NULL(pll_10nm->phy_cmn_mmio)) {
> dev_err(&pdev->dev, "failed to map CMN PHY base\n");