Patchwork [v4,4/8] drm/msm/dsi: 14nm PHY: Get ref clock from the DT

login
register
mail settings
Submitter Matthias Kaehlcke
Date Dec. 4, 2018, 10:42 p.m.
Message ID <20181204224234.62619-5-mka@chromium.org>
Download mbox | patch
Permalink /patch/672451/
State New
Headers show

Comments

Matthias Kaehlcke - Dec. 4, 2018, 10:42 p.m.
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.

Note: This change could break old out-of-tree DTS files that
use the 14nm PHY.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- none

Changes in v3:
- fixed check for EPROBE_DEFER
- added note to commit message about breaking old DTS files
- added 'Reviewed-by: Douglas Anderson <dianders@chromium.org>' tag

Changes in v2:
- patch added to the series
---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Stephen Boyd - Dec. 10, 2018, 3:51 p.m.
Quoting Matthias Kaehlcke (2018-12-04 14:42:30)
> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> index 71fe60e5f01f1..032bf3e8614bd 100644
> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> @@ -40,7 +40,6 @@
>  
>  #define NUM_PROVIDED_CLKS              2
>  
> -#define VCO_REF_CLK_RATE               19200000
>  #define VCO_MIN_RATE                   1300000000UL
>  #define VCO_MAX_RATE                   2600000000UL
>  
> @@ -139,6 +138,7 @@ struct dsi_pll_14nm {
>         /* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
>         spinlock_t postdiv_lock;
>  
> +       struct clk *vco_ref_clk;

Is there any need to keep it in the struct? Or just get the clk, find
the rate, and then put the clk and call pll_14nm_postdiv_register()?

>         u64 vco_current_rate;
>         u64 vco_ref_clk_rate;
>
Matthias Kaehlcke - Dec. 19, 2018, 10:22 p.m.
On Mon, Dec 10, 2018 at 07:51:19AM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2018-12-04 14:42:30)
> > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > index 71fe60e5f01f1..032bf3e8614bd 100644
> > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > @@ -40,7 +40,6 @@
> >  
> >  #define NUM_PROVIDED_CLKS              2
> >  
> > -#define VCO_REF_CLK_RATE               19200000
> >  #define VCO_MIN_RATE                   1300000000UL
> >  #define VCO_MAX_RATE                   2600000000UL
> >  
> > @@ -139,6 +138,7 @@ struct dsi_pll_14nm {
> >         /* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
> >         spinlock_t postdiv_lock;
> >  
> > +       struct clk *vco_ref_clk;
> 
> Is there any need to keep it in the struct? Or just get the clk, find
> the rate, and then put the clk and call pll_14nm_postdiv_register()?

I suppose you mean passing the clock name to pll_14nm_register()?

Is putting the clock really needed or preferable, or is it just fine
to auto-put it when the device is deleted?
Stephen Boyd - Dec. 19, 2018, 10:32 p.m.
Quoting Matthias Kaehlcke (2018-12-19 14:22:22)
> On Mon, Dec 10, 2018 at 07:51:19AM -0800, Stephen Boyd wrote:
> > Quoting Matthias Kaehlcke (2018-12-04 14:42:30)
> > > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > > index 71fe60e5f01f1..032bf3e8614bd 100644
> > > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
> > > @@ -40,7 +40,6 @@
> > >  
> > >  #define NUM_PROVIDED_CLKS              2
> > >  
> > > -#define VCO_REF_CLK_RATE               19200000
> > >  #define VCO_MIN_RATE                   1300000000UL
> > >  #define VCO_MAX_RATE                   2600000000UL
> > >  
> > > @@ -139,6 +138,7 @@ struct dsi_pll_14nm {
> > >         /* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
> > >         spinlock_t postdiv_lock;
> > >  
> > > +       struct clk *vco_ref_clk;
> > 
> > Is there any need to keep it in the struct? Or just get the clk, find
> > the rate, and then put the clk and call pll_14nm_postdiv_register()?
> 
> I suppose you mean passing the clock name to pll_14nm_register()?

Yes, whatever makes it possible to avoid storing the pointer in the
struct.

> 
> Is putting the clock really needed or preferable, or is it just fine
> to auto-put it when the device is deleted?

Up to you. If you don't have a need for the clk anymore it seems fine to
just put the clk and be done.

Patch

diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
index 71fe60e5f01f1..032bf3e8614bd 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
@@ -40,7 +40,6 @@ 
 
 #define NUM_PROVIDED_CLKS		2
 
-#define VCO_REF_CLK_RATE		19200000
 #define VCO_MIN_RATE			1300000000UL
 #define VCO_MAX_RATE			2600000000UL
 
@@ -139,6 +138,7 @@  struct dsi_pll_14nm {
 	/* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
 	spinlock_t postdiv_lock;
 
+	struct clk *vco_ref_clk;
 	u64 vco_current_rate;
 	u64 vco_ref_clk_rate;
 
@@ -591,7 +591,7 @@  static int dsi_pll_14nm_vco_set_rate(struct clk_hw *hw, unsigned long rate,
 	    parent_rate);
 
 	pll_14nm->vco_current_rate = rate;
-	pll_14nm->vco_ref_clk_rate = VCO_REF_CLK_RATE;
+	pll_14nm->vco_ref_clk_rate = parent_rate;
 
 	dsi_pll_14nm_input_init(pll_14nm);
 
@@ -950,8 +950,9 @@  static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
 static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm)
 {
 	char clk_name[32], parent[32], vco_name[32];
+	const char *ref_clk_name = __clk_get_name(pll_14nm->vco_ref_clk);
 	struct clk_init_data vco_init = {
-		.parent_names = (const char *[]){ "xo" },
+		.parent_names = &ref_clk_name,
 		.num_parents = 1,
 		.name = vco_name,
 		.flags = CLK_IGNORE_UNUSED,
@@ -1065,6 +1066,15 @@  struct msm_dsi_pll *msm_dsi_pll_14nm_init(struct platform_device *pdev, int id)
 	pll_14nm->id = id;
 	pll_14nm_list[id] = pll_14nm;
 
+	pll_14nm->vco_ref_clk = devm_clk_get(&pdev->dev, "ref");
+	if (IS_ERR(pll_14nm->vco_ref_clk)) {
+		ret = PTR_ERR(pll_14nm->vco_ref_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "couldn't get 'ref' clock: %d\n",
+				ret);
+		return ERR_PTR(ret);
+	}
+
 	pll_14nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
 	if (IS_ERR_OR_NULL(pll_14nm->phy_cmn_mmio)) {
 		dev_err(&pdev->dev, "failed to map CMN PHY base\n");