| Message ID | 20170130164921.20744-5-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Rob Clark <robdclark@gmail.com> writes: > Suggested by Rob Herring. We still support the old names for > compatibility with downstream android dt files. > > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Rob Clark <robdclark@gmail.com> Huh, I don't think I would have cleaned up DT bindings in exchange for adding driver code like this. But the code seems correct, so other than one optional suggestion: Reviewed-by: Eric Anholt <eric@anholt.net> > +struct clk *msm_clk_get(struct platform_device *pdev, const char *name) > +{ > + struct clk *clk; > + char name2[32]; > + > + clk = devm_clk_get(&pdev->dev, name); > + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) > + return clk; > + > + snprintf(name2, sizeof(name2), "%s_clk", name); > + > + clk = devm_clk_get(&pdev->dev, name2); > + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) > + dev_warn(&pdev->dev, "Using legacy clk name binding. Use " > + "\"%s\" instead of \"%s\"\n", name, name2); Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning printed at boot if deferring happens?
On Mon, Jan 30, 2017 at 1:15 PM, Eric Anholt <eric@anholt.net> wrote: > Rob Clark <robdclark@gmail.com> writes: > >> Suggested by Rob Herring. We still support the old names for >> compatibility with downstream android dt files. >> >> Cc: Rob Herring <robh@kernel.org> >> Signed-off-by: Rob Clark <robdclark@gmail.com> > > Huh, I don't think I would have cleaned up DT bindings in exchange for > adding driver code like this. But the code seems correct, so other than > one optional suggestion: > > Reviewed-by: Eric Anholt <eric@anholt.net> > >> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name) >> +{ >> + struct clk *clk; >> + char name2[32]; >> + >> + clk = devm_clk_get(&pdev->dev, name); >> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) >> + return clk; >> + >> + snprintf(name2, sizeof(name2), "%s_clk", name); >> + >> + clk = devm_clk_get(&pdev->dev, name2); >> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) >> + dev_warn(&pdev->dev, "Using legacy clk name binding. Use " >> + "\"%s\" instead of \"%s\"\n", name, name2); > > Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning > printed at boot if deferring happens? oh, good point.. I've fixed that up locally. I don't think we'd actually hit this case currently for gpu clks, since for gpu this codepath happens on first open() of the device file. But I should mention I have a slighly sneaky ulterior motive, which is that we could use the same cleanup for display related clks (which do currently upstream use the _clk suffix). BR, -R
On Mon, Jan 30, 2017 at 11:49:21AM -0500, Rob Clark wrote: > Suggested by Rob Herring. We still support the old names for > compatibility with downstream android dt files. > > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > Documentation/devicetree/bindings/display/msm/gpu.txt | 12 ++++++------ > drivers/gpu/drm/msm/msm_drv.c | 19 +++++++++++++++++++ > drivers/gpu/drm/msm/msm_drv.h | 1 + > drivers/gpu/drm/msm/msm_gpu.c | 7 +++---- > 4 files changed, 29 insertions(+), 10 deletions(-) Acked-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt index 7ac3052..43fac0f 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.txt +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt @@ -11,9 +11,9 @@ Required properties: - clocks: device clocks See ../clocks/clock-bindings.txt for details. - clock-names: the following clocks are required: - * "core_clk" - * "iface_clk" - * "mem_iface_clk" + * "core" + * "iface" + * "mem_iface" Example: @@ -27,9 +27,9 @@ Example: interrupts = <GIC_SPI 80 0>; interrupt-names = "kgsl_3d0_irq"; clock-names = - "core_clk", - "iface_clk", - "mem_iface_clk"; + "core", + "iface", + "mem_iface"; clocks = <&mmcc GFX3D_CLK>, <&mmcc GFX3D_AHB_CLK>, diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 6b85c41..a51d783 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -91,6 +91,25 @@ module_param(dumpstate, bool, 0600); * Util/helpers: */ +struct clk *msm_clk_get(struct platform_device *pdev, const char *name) +{ + struct clk *clk; + char name2[32]; + + clk = devm_clk_get(&pdev->dev, name); + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) + return clk; + + snprintf(name2, sizeof(name2), "%s_clk", name); + + clk = devm_clk_get(&pdev->dev, name2); + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) + dev_warn(&pdev->dev, "Using legacy clk name binding. Use " + "\"%s\" instead of \"%s\"\n", name, name2); + + return clk; +} + void __iomem *msm_ioremap(struct platform_device *pdev, const char *name, const char *dbgname) { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index ed4dad3..5f6f48f 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -318,6 +318,7 @@ static inline int msm_debugfs_late_init(struct drm_device *dev) { return 0; } static inline void msm_rd_dump_submit(struct msm_gem_submit *submit) {} #endif +struct clk *msm_clk_get(struct platform_device *pdev, const char *name); void __iomem *msm_ioremap(struct platform_device *pdev, const char *name, const char *dbgname); void msm_writel(u32 data, void __iomem *addr); diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index d8420be..7b29843 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -564,8 +564,7 @@ static irqreturn_t irq_handler(int irq, void *data) } static const char *clk_names[] = { - "core_clk", "iface_clk", "rbbmtimer_clk", "mem_clk", - "mem_iface_clk", "alt_mem_iface_clk", + "core", "iface", "rbbmtimer", "mem", "mem_iface", "alt_mem_iface", }; int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, @@ -629,13 +628,13 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, /* Acquire clocks: */ for (i = 0; i < ARRAY_SIZE(clk_names); i++) { - gpu->grp_clks[i] = devm_clk_get(&pdev->dev, clk_names[i]); + gpu->grp_clks[i] = msm_clk_get(pdev, clk_names[i]); DBG("grp_clks[%s]: %p", clk_names[i], gpu->grp_clks[i]); if (IS_ERR(gpu->grp_clks[i])) gpu->grp_clks[i] = NULL; } - gpu->ebi1_clk = devm_clk_get(&pdev->dev, "bus_clk"); + gpu->ebi1_clk = msm_clk_get(pdev, "bus"); DBG("ebi1_clk: %p", gpu->ebi1_clk); if (IS_ERR(gpu->ebi1_clk)) gpu->ebi1_clk = NULL;
Suggested by Rob Herring. We still support the old names for compatibility with downstream android dt files. Cc: Rob Herring <robh@kernel.org> Signed-off-by: Rob Clark <robdclark@gmail.com> --- Documentation/devicetree/bindings/display/msm/gpu.txt | 12 ++++++------ drivers/gpu/drm/msm/msm_drv.c | 19 +++++++++++++++++++ drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/gpu/drm/msm/msm_gpu.c | 7 +++---- 4 files changed, 29 insertions(+), 10 deletions(-)