Hi Maxime,
On Sun, Nov 3, 2019 at 11:02 PM Maxime Ripard mripard@kernel.org wrote:
On Fri, Nov 01, 2019 at 07:42:55PM +0530, Jagan Teki wrote:
Hi Maxime,
On Tue, Oct 29, 2019 at 2:24 PM Maxime Ripard mripard@kernel.org wrote:
On Tue, Oct 29, 2019 at 04:03:56AM +0530, Jagan Teki wrote:
explicit handling of common clock would require since the A64 doesn't need to mention the clock-names explicitly in dts since it support only one bus clock.
Also pass clk_id NULL instead "bus" to regmap clock init function since the single clock variants no need to mention clock-names explicitly.
You don't need explicit clock handling. Passing NULL as the argument in regmap_init_mmio_clk will make it use the first clock, which is the bus clock.
Indeed I tried that, since NULL clk_id wouldn't enable the bus clock during regmap_mmio_gen_context code, passing NULL triggering vblank timeout.
There's a bunch of users of NULL in tree, so finding out why NULL doesn't work is the way forward.
I'd have looked the some of the users before checking the code as well. As I said passing NULL clk_id to devm_regmap_init_mmio_clk => __devm_regmap_init_mmio_clk would return before processing the clock.
Here is the code snippet on the tree just to make sure I'm on the same page or not.
static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev, const char *clk_id, void __iomem *regs, const struct regmap_config *config) { ----------------------- -------------- if (clk_id == NULL) return ctx;
ctx->clk = clk_get(dev, clk_id); if (IS_ERR(ctx->clk)) { ret = PTR_ERR(ctx->clk); goto err_free; } ret = clk_prepare(ctx->clk); if (ret < 0) { clk_put(ctx->clk); goto err_free; } ------------- ---------------}
Yes, I did check on the driver in the tree before committing explicit clock handle, which make similar requirements like us in [1]. this imx2 wdt driver is handling the explicit clock as well. I'm sure this driver is updated as I have seen few changes related to this driver in ML.
I guess we have two ways to go at this then.
Either we remove the return, but it might have a few side-effects, or we call clk_get with NULL or bus depending on the case, and then call regmap_mmio_attach_clk.
Thanks for the inputs.
Please have a look at this snippet, I have used your second suggestions. let me know if you have any comments?
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 8fa90cfc2ac8..91c95e56d870 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -1109,24 +1109,36 @@ static int sun6i_dsi_probe(struct platform_device *pdev) return PTR_ERR(dsi->regulator); }
- dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base, - &sun6i_dsi_regmap_config); - if (IS_ERR(dsi->regs)) { - dev_err(dev, "Couldn't create the DSI encoder regmap\n"); - return PTR_ERR(dsi->regs); - } - dsi->reset = devm_reset_control_get_shared(dev, NULL); if (IS_ERR(dsi->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(dsi->reset); }
+ dsi->regs = regmap_init_mmio(dev, base, &sun6i_dsi_regmap_config); + if (IS_ERR(dsi->regs)) { + dev_err(dev, "Couldn't init regmap\n"); + return PTR_ERR(dsi->regs); + } + + dsi->bus_clk = devm_clk_get(dev, NULL); + if (IS_ERR(dsi->bus_clk)) { + dev_err(dev, "Couldn't get the DSI bus clock\n"); + ret = PTR_ERR(dsi->bus_clk); + goto err_regmap; + } else { + printk("Jagan.. Got the BUS clock\n"); + ret = regmap_mmio_attach_clk(dsi->regs, dsi->bus_clk); + if (ret) + goto err_bus_clk; + } + if (dsi->variant->has_mod_clk) { dsi->mod_clk = devm_clk_get(dev, "mod"); if (IS_ERR(dsi->mod_clk)) { dev_err(dev, "Couldn't get the DSI mod clock\n"); - return PTR_ERR(dsi->mod_clk); + ret = PTR_ERR(dsi->mod_clk); + goto err_attach_clk; } }
@@ -1167,6 +1179,14 @@ static int sun6i_dsi_probe(struct platform_device *pdev) err_unprotect_clk: if (dsi->variant->has_mod_clk) clk_rate_exclusive_put(dsi->mod_clk); +err_attach_clk: + if (!IS_ERR(dsi->bus_clk)) + regmap_mmio_detach_clk(dsi->regs); +err_bus_clk: + if (!IS_ERR(dsi->bus_clk)) + clk_put(dsi->bus_clk); +err_regmap: + regmap_exit(dsi->regs); return ret; }
@@ -1181,6 +1201,13 @@ static int sun6i_dsi_remove(struct platform_device *pdev) if (dsi->variant->has_mod_clk) clk_rate_exclusive_put(dsi->mod_clk);
+ if (!IS_ERR(dsi->bus_clk)) { + regmap_mmio_detach_clk(dsi->regs); + clk_put(dsi->bus_clk); + } + + regmap_exit(dsi->regs); + return 0; }
Jagan.