Re: [Freedreno] [PATCH v2 2/7] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
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 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(>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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 2/7] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
Hi,
On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke 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(>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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 2/7] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Signed-off-by: Matthias Kaehlcke
---
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 1920
#define VCO_MIN_RATE 13UL
#define VCO_MAX_RATE 26UL
@@ -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 = _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(>dev, "ref");
+ if (IS_ERR(pll_14nm->vco_ref_clk)) {
+ ret = PTR_ERR(pll_14nm->vco_ref_clk);
+ if (ret != EPROBE_DEFER)
+ dev_err(>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(>dev, "failed to map CMN PHY base\n");
--
2.20.0.rc0.387.gc7a69e6b6c-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
