Re: [PATCH 2/2] drm/msm/dsi: Get PHY ref clock from the DT
From: Doug Anderson
Date: Thu Nov 08 2018 - 17:25:18 EST
Hi,
On Fri, Nov 2, 2018 at 2:45 PM Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
>
> Get the PHY ref clock from the device tree instead of hardcoding
> its name and rate.
>
> Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> index 4c03f0b7343ed..1016eb50df8f5 100644
> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> @@ -91,6 +91,8 @@ struct dsi_pll_10nm {
> void __iomem *phy_cmn_mmio;
> void __iomem *mmio;
>
> + struct clk *vco_ref_clk;
> +
> u64 vco_ref_clk_rate;
> u64 vco_current_rate;
>
> @@ -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) },
> .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");
> + return (void *)pll_10nm->vco_ref_clk;
> + }
So, ummmm. Can you follow the same pattern for all the other clocks
in this file too? All parents should get their name based on
references in the device tree.
It turns out that right now we have a mismatch because
"drivers/clk/qcom/dispcc-sdm845.c" calls "dsi0pllbyte"
"dsi0_phy_pll_out_byteclk" and calls "dsi0pll"
"dsi0_phy_pll_out_dsiclk". We might want to change the names in
dispcc-sdm845.c, but it wouldn't matter if we simply didn't hardcode
them here.
-Doug