[PATCH v2 0/7] drm/msm/dsi: Get PHY ref clocks from the DT

[PATCH v2 0/7] drm/msm/dsi: Get PHY ref clocks from the DT

[Matthias Kaehlcke]

The MSM DSI PHY drivers currently hardcode the name and the rate of
the PHY ref clock. Get the ref clock from the device tree instead.

Note: testing of this series was limited to SDM845 and the 10nm PHY

Major changes in v2:
- apply to all MSM DSI PHY drivers, not only 10nm

Matthias Kaehlcke (7):
  dt-bindings: msm/dsi: Add ref clock for PHYs
  drm/msm/dsi: 14nm PHY: Get ref clock from the DT
  drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
  arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY
  drm/msm/dsi: 28nm PHY: Get ref clock from the DT
  arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs
  drm/msm/dsi: 10nm PHY: Get ref clock from the DT

 .../devicetree/bindings/display/msm/dsi.txt   |  1 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi         |  5 ++--
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 10 ++++---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c    | 13 ++++++++-
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c    | 16 ++++++++--
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c    | 29 +++++++++++++------
 .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c   | 17 +++++++++--
 7 files changed, 69 insertions(+), 22 deletions(-)

-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

[PATCH v2 1/7] dt-bindings: msm/dsi: Add ref clock for PHYs

[Matthias Kaehlcke]

Allow the PHY drivers to get the ref clock from the DT.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- add the ref clock for all PHYs, not only the 10nm one
- updated commit message
---
 Documentation/devicetree/bindings/display/msm/dsi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
index dfc743219bd88..b0485559a719c 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -106,6 +106,7 @@ Required properties:
 - clocks: Phandles to device clocks. See [1] for details on clock bindings.
 - clock-names: the following clocks are required:
   * "iface"
+  * "ref"
   For 28nm HPM/LP, 28nm 8960 PHYs:
 - vddio-supply: phandle to vdd-io regulator device node
   For 20nm PHY:
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 1/7] dt-bindings: msm/dsi: Add ref clock for PHYs

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Allow the PHY drivers to get the ref clock from the DT.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v2:
> - add the ref clock for all PHYs, not only the 10nm one
> - updated commit message
> ---
>  Documentation/devicetree/bindings/display/msm/dsi.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index dfc743219bd88..b0485559a719c 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -106,6 +106,7 @@ Required properties:
>  - clocks: Phandles to device clocks. See [1] for details on clock bindings.
>  - clock-names: the following clocks are required:
>    * "iface"
> +  * "ref"

We can't quite make ref "required" because there are some old device
tree files dating back to 2016 that would be broken.  It could be
listed as optional, but I _think_ Rob is OK with it being listed as
"required" for all new device tree files with the footnote that if
it's not present then the code will still work.

Re: [PATCH v2 1/7] dt-bindings: msm/dsi: Add ref clock for PHYs

[Matthias Kaehlcke]

On Tue, Nov 27, 2018 at 09:41:39PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Allow the PHY drivers to get the ref clock from the DT.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > Changes in v2:
> > - add the ref clock for all PHYs, not only the 10nm one
> > - updated commit message
> > ---
> >  Documentation/devicetree/bindings/display/msm/dsi.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > index dfc743219bd88..b0485559a719c 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > @@ -106,6 +106,7 @@ Required properties:
> >  - clocks: Phandles to device clocks. See [1] for details on clock bindings.
> >  - clock-names: the following clocks are required:
> >    * "iface"
> > +  * "ref"
> 
> We can't quite make ref "required" because there are some old device
> tree files dating back to 2016 that would be broken.  It could be
> listed as optional, but I _think_ Rob is OK with it being listed as
> "required" for all new device tree files with the footnote that if
> it's not present then the code will still work.

Ok, will update the code to keep supporting old DT files and update
the description.

[PATCH v2 2/7] drm/msm/dsi: 14nm PHY: Get ref clock from the DT

[Matthias Kaehlcke]

Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
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(-)

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..f58298bd6c423 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");
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

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

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Get the ref clock of the PHY from the device tree instead of
> hardcoding its name and rate.

In the case of the 14nm PHY I think it's OK that you break
compatibility with old device tree files (as this patch does) since
the 14nm support was added sorta recently and "git grep" shows no
users in linuxnext.  You should note that you're breaking
compatibility with old DTS files in the commit message here so that if
someone crawls out of the woodwork it will be easy for them to
understand what happened.


> +       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)

Shouldn't this check against -EPROBE_DEFER, not against EPROBE_DEFER?
It's negative.  Presumably this same feedback needs to be applied to
the whole patch series.

Other than that this looks good to me and you can feel free to add my
Reviewed-by tag FWIW.

-Doug

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

[Matthias Kaehlcke]

On Tue, Nov 27, 2018 at 09:56:46PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Get the ref clock of the PHY from the device tree instead of
> > hardcoding its name and rate.
> 
> In the case of the 14nm PHY I think it's OK that you break
> compatibility with old device tree files (as this patch does) since
> the 14nm support was added sorta recently and "git grep" shows no
> users in linuxnext.  You should note that you're breaking
> compatibility with old DTS files in the commit message here so that if
> someone crawls out of the woodwork it will be easy for them to
> understand what happened.

ok, I'll add the note

> > +       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)
> 
> Shouldn't this check against -EPROBE_DEFER, not against EPROBE_DEFER?
> It's negative.  Presumably this same feedback needs to be applied to
> the whole patch series.

You are right, will fix it throughout the series, thanks!

> Other than that this looks good to me and you can feel free to add my
> Reviewed-by tag FWIW.

Great, thanks for the review!

Matthias

[PATCH v2 3/7] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT

[Matthias Kaehlcke]

Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- patch added to the series
---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
index 49008451085b8..461975c410fc4 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
@@ -47,7 +47,6 @@
 
 #define NUM_PROVIDED_CLKS	2
 
-#define VCO_REF_CLK_RATE	27000000
 #define VCO_MIN_RATE		600000000
 #define VCO_MAX_RATE		1200000000
 
@@ -75,6 +74,8 @@ struct dsi_pll_28nm {
 	struct platform_device *pdev;
 	void __iomem *mmio;
 
+	struct clk *vco_ref_clk;
+
 	/* custom byte clock divider */
 	struct clk_bytediv *bytediv;
 
@@ -125,7 +126,7 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	DBG("rate=%lu, parent's=%lu", rate, parent_rate);
 
 	temp = rate / 10;
-	val = VCO_REF_CLK_RATE / 10;
+	val = parent_rate / 10;
 	fb_divider = (temp * VCO_PREF_DIV_RATIO) / val;
 	fb_divider = fb_divider / 2 - 1;
 	pll_write(base + REG_DSI_28nm_8960_PHY_PLL_CTRL_1,
@@ -409,8 +410,9 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
 {
 	char *clk_name, *parent_name, *vco_name;
+	const char *ref_clk_name = __clk_get_name(pll_28nm->vco_ref_clk);
 	struct clk_init_data vco_init = {
-		.parent_names = (const char *[]){ "pxo" },
+		.parent_names = &ref_clk_name,
 		.num_parents = 1,
 		.flags = CLK_IGNORE_UNUSED,
 		.ops = &clk_ops_dsi_pll_28nm_vco,
@@ -506,6 +508,15 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct platform_device *pdev,
 	pll_28nm->pdev = pdev;
 	pll_28nm->id = id + 1;
 
+	pll_28nm->vco_ref_clk = devm_clk_get(&pdev->dev, "ref");
+	if (IS_ERR(pll_28nm->vco_ref_clk)) {
+		ret = PTR_ERR(pll_28nm->vco_ref_clk);
+		if (ret != EPROBE_DEFER)
+			dev_err(&pdev->dev, "couldn't get 'ref' clock: %d\n",
+				ret);
+		return ERR_PTR(ret);
+	}
+
 	pll_28nm->mmio = msm_ioremap(pdev, "dsi_pll", "DSI_PLL");
 	if (IS_ERR_OR_NULL(pll_28nm->mmio)) {
 		dev_err(&pdev->dev, "%s: failed to map pll base\n", __func__);
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 3/7] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> @@ -409,8 +410,9 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
>  {
>         char *clk_name, *parent_name, *vco_name;
> +       const char *ref_clk_name = __clk_get_name(pll_28nm->vco_ref_clk);

IMO for the 28nm PHY driver you should probably make things work OK
even if the "ref" clock wasn't supplied.  In the spirit of the stable
device tree it would be nice (even if nobody actually ships device
trees separate from kernels).  ...and also it makes the whole thing
easier to land.  If you add compatibility here then the code and
device tree patch can go in separately.

Depending on what others think you could potentially spit some type of
warning in the logs if the ref clock wasn't specified though.

-Doug

Re: [PATCH v2 3/7] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT

[Matthias Kaehlcke]

On Tue, Nov 27, 2018 at 10:00:50PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> > @@ -409,8 +410,9 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
> >  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
> >  {
> >         char *clk_name, *parent_name, *vco_name;
> > +       const char *ref_clk_name = __clk_get_name(pll_28nm->vco_ref_clk);
> 
> IMO for the 28nm PHY driver you should probably make things work OK
> even if the "ref" clock wasn't supplied.  In the spirit of the stable
> device tree it would be nice (even if nobody actually ships device
> trees separate from kernels).  ...and also it makes the whole thing
> easier to land.  If you add compatibility here then the code and
> device tree patch can go in separately.

Ok, I'll make it fall back to the 'default' values if the ref clock is
not specified.

[PATCH v2 4/7] arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY

[Matthias Kaehlcke]

Add 'xo_board' as ref clock for the DSI PHYs, it was previously
hardcoded in the PLL 'driver' for the 28nm PHY.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- patch added to the series
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index d302d8d639a12..89f30f34ff896 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -959,8 +959,9 @@
 				#clock-cells = <1>;
 				#phy-cells = <0>;
 
-				clocks = <&gcc GCC_MDSS_AHB_CLK>;
-				clock-names = "iface";
+				clocks = <&gcc GCC_MDSS_AHB_CLK>,
+					 <&xo_board>;
+				clock-names = "iface", "ref";
 			};
 		};
 
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 4/7] arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Add 'xo_board' as ref clock for the DSI PHYs, it was previously
> hardcoded in the PLL 'driver' for the 28nm PHY.

Note: presumably your series should have one more patch to fix
"arch/arm/boot/dts/qcom-apq8064.dtsi" too?


> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v2:
> - patch added to the series
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

This looks right to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v2 5/7] drm/msm/dsi: 28nm PHY: Get ref clock from the DT

[Matthias Kaehlcke]

Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- patch added to the series
---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c | 29 +++++++++++++++-------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
index 26e3a01a99c2b..a1ab5ecbf7c7d 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
@@ -40,7 +40,6 @@
 
 #define NUM_PROVIDED_CLKS		2
 
-#define VCO_REF_CLK_RATE		19200000
 #define VCO_MIN_RATE			350000000
 #define VCO_MAX_RATE			750000000
 
@@ -81,6 +80,7 @@ struct dsi_pll_28nm {
 	struct platform_device *pdev;
 	void __iomem *mmio;
 
+	struct clk *vco_ref_clk;
 	int vco_delay;
 
 	/* private clocks: */
@@ -139,6 +139,7 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct dsi_pll_28nm *pll_28nm = to_pll_28nm(pll);
 	struct device *dev = &pll_28nm->pdev->dev;
 	void __iomem *base = pll_28nm->mmio;
+	u64 ref_clk_rate = parent_rate;
 	unsigned long div_fbx1000, gen_vco_clk;
 	u32 refclk_cfg, frac_n_mode, frac_n_value;
 	u32 sdm_cfg0, sdm_cfg1, sdm_cfg2, sdm_cfg3;
@@ -166,17 +167,17 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC1_CFG, 0x70);
 	pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC2_CFG, 0x15);
 
-	rem = rate % VCO_REF_CLK_RATE;
+	rem = rate % ref_clk_rate;
 	if (rem) {
 		refclk_cfg = DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
 		frac_n_mode = 1;
-		div_fbx1000 = rate / (VCO_REF_CLK_RATE / 500);
-		gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 500);
+		div_fbx1000 = rate / (ref_clk_rate / 500);
+		gen_vco_clk = div_fbx1000 * (ref_clk_rate / 500);
 	} else {
 		refclk_cfg = 0x0;
 		frac_n_mode = 0;
-		div_fbx1000 = rate / (VCO_REF_CLK_RATE / 1000);
-		gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 1000);
+		div_fbx1000 = rate / (ref_clk_rate / 1000);
+		gen_vco_clk = div_fbx1000 * (ref_clk_rate / 1000);
 	}
 
 	DBG("refclk_cfg = %d", refclk_cfg);
@@ -265,7 +266,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct clk_hw *hw,
 	void __iomem *base = pll_28nm->mmio;
 	u32 sdm0, doubler, sdm_byp_div;
 	u32 sdm_dc_off, sdm_freq_seed, sdm2, sdm3;
-	u32 ref_clk = VCO_REF_CLK_RATE;
+	u32 ref_clk = parent_rate;
 	unsigned long vco_rate;
 
 	VERB("parent_rate=%lu", parent_rate);
@@ -273,7 +274,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct clk_hw *hw,
 	/* Check to see if the ref clk doubler is enabled */
 	doubler = pll_read(base + REG_DSI_28nm_PHY_PLL_REFCLK_CFG) &
 			DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
-	ref_clk += (doubler * VCO_REF_CLK_RATE);
+	ref_clk += (doubler * ref_clk);
 
 	/* see if it is integer mode or sdm mode */
 	sdm0 = pll_read(base + REG_DSI_28nm_PHY_PLL_SDM_CFG0);
@@ -517,8 +518,9 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
 {
 	char clk_name[32], parent1[32], parent2[32], vco_name[32];
+	const char *ref_clk_name = __clk_get_name(pll_28nm->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,
@@ -605,6 +607,15 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct platform_device *pdev,
 	pll_28nm->pdev = pdev;
 	pll_28nm->id = id;
 
+	pll_28nm->vco_ref_clk = devm_clk_get(&pdev->dev, "ref");
+	if (IS_ERR(pll_28nm->vco_ref_clk)) {
+		ret = PTR_ERR(pll_28nm->vco_ref_clk);
+		if (ret != EPROBE_DEFER)
+			dev_err(&pdev->dev, "couldn't get 'ref' clock: %d\n",
+				ret);
+		return ERR_PTR(ret);
+	}
+
 	pll_28nm->mmio = msm_ioremap(pdev, "dsi_pll", "DSI_PLL");
 	if (IS_ERR_OR_NULL(pll_28nm->mmio)) {
 		dev_err(&pdev->dev, "%s: failed to map pll base\n", __func__);
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 5/7] drm/msm/dsi: 28nm PHY: Get ref clock from the DT

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Get the ref clock of the PHY from the device tree instead of
> hardcoding its name and rate.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v2:
> - patch added to the series
> ---
>  drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c | 29 +++++++++++++++-------
>  1 file changed, 20 insertions(+), 9 deletions(-)

Generally same feedback as with "28nm 8960 PHY" patch, plus...


> @@ -81,6 +80,7 @@ struct dsi_pll_28nm {
>         struct platform_device *pdev;
>         void __iomem *mmio;
>
> +       struct clk *vco_ref_clk;
>         int vco_delay;
>
>         /* private clocks: */
> @@ -139,6 +139,7 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>         struct dsi_pll_28nm *pll_28nm = to_pll_28nm(pll);
>         struct device *dev = &pll_28nm->pdev->dev;
>         void __iomem *base = pll_28nm->mmio;
> +       u64 ref_clk_rate = parent_rate;

Are you sure it's good to stash this in a u64 instead of just using
the parent_rate directly?

[PATCH v2 6/7] arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs

[Matthias Kaehlcke]

Add 'bi_tcxo' as ref clock for the DSI PHYs, it was previously
hardcoded in the PLL 'driver' for the 10nm PHY.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
based on "[v4,1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file"
  (https://patchwork.kernel.org/patch/10666253/)

Changes in v2:
- patch added to the series
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5728b4cfae269..cdb5a9bb23e69 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1372,8 +1372,9 @@
 				#clock-cells = <1>;
 				#phy-cells = <0>;
 
-				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
-				clock-names = "iface";
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
 			};
 
 			dsi1: dsi@ae96000 {
@@ -1434,8 +1435,9 @@
 				#clock-cells = <1>;
 				#phy-cells = <0>;
 
-				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
-				clock-names = "iface";
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
 			};
 		};
 
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 6/7] arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Add 'bi_tcxo' as ref clock for the DSI PHYs, it was previously
> hardcoded in the PLL 'driver' for the 10nm PHY.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> based on "[v4,1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file"
>   (https://patchwork.kernel.org/patch/10666253/)
>
> Changes in v2:
> - patch added to the series
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v2 7/7] drm/msm/dsi: 10nm PHY: Get ref clock from the DT

[Matthias Kaehlcke]

Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- remove anonymous array in clk_init_data assignment
- log error code if devm_clk_get() fails
- don't log devm_clk_get() failures for -EPROBE_DEFER
- updated commit message
---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 13 ++++++++++++-
 1 file changed, 12 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..32e31574e1432 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,7 @@ 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;
 
@@ -629,8 +630,9 @@ 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];
+	const char *ref_clk_name = __clk_get_name(pll_10nm->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,
@@ -786,6 +788,15 @@ 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)) {
+		ret = PTR_ERR(pll_10nm->vco_ref_clk);
+		if (ret != EPROBE_DEFER)
+			dev_err(&pdev->dev, "couldn't get 'ref' clock: %d\n",
+				ret);
+		return ERR_PTR(ret);
+	}
+
 	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");
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [PATCH v2 7/7] drm/msm/dsi: 10nm PHY: Get ref clock from the DT

[Doug Anderson]

Hi,

On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Get the ref clock of the PHY from the device tree instead of
> hardcoding its name and rate.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v2:
> - remove anonymous array in clk_init_data assignment
> - log error code if devm_clk_get() fails
> - don't log devm_clk_get() failures for -EPROBE_DEFER
> - updated commit message
> ---
>  drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Same feedback as with 14nm PHY patch; if you address that feedback
feel free to add my Reviewed-by here too.

-Doug