Re: [PATCH v13 4/7] drm/sun4i: dsi: Handle bus clock via regmap_mmio_attach_clk

[Maxime Ripard]

On Thu, Dec 19, 2019 at 12:40:14AM +0530, Jagan Teki wrote:
> regmap has special API to enable the controller bus clock while
> initializing register space, and current driver is using
> devm_regmap_init_mmio_clk which require to specify bus
> clk_id argument as "bus"
>
> But, the usage of clocks are varies between different Allwinner
> DSI controllers. Clocking in A33 would need bus and mod clocks
> where as A64 would need only bus clock.
>
> Since A64 support only single bus clock, it is optional to
> specify the clock-names on the controller device tree node.
> So using NULL on clk_id would get the attached clock.
>
> To support clk_id as "bus" and "NULL" during clock enablement
> between controllers, this patch add generic code to handle
> the bus clock using regmap_mmio_attach_clk with associated
> regmap APIs.
>
> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> ---
> Changes for v13:
> - update the changes since has_mod_clk is dropped in previous patch
>
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 45 +++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 68b88a3dc4c5..de8955fbeb00 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -1081,6 +1081,7 @@ static const struct component_ops sun6i_dsi_ops = {
>  static int sun6i_dsi_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	const char *bus_clk_name = NULL;
>  	struct sun6i_dsi *dsi;
>  	struct resource *res;
>  	void __iomem *base;
> @@ -1094,6 +1095,10 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  	dsi->host.ops = &sun6i_dsi_host_ops;
>  	dsi->host.dev = dev;
>
> +	if (of_device_is_compatible(dev->of_node,
> +				    "allwinner,sun6i-a31-mipi-dsi"))
> +		bus_clk_name = "bus";
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(base)) {
> @@ -1107,25 +1112,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 = devm_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, bus_clk_name);
> +	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 {
> +		ret = regmap_mmio_attach_clk(dsi->regs, dsi->bus_clk);
> +		if (ret)
> +			goto err_bus_clk;
> +	}
> +
>  	if (of_device_is_compatible(dev->of_node,
>  				    "allwinner,sun6i-a31-mipi-dsi")) {
>  		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;
>  		}
>  	}
>
> @@ -1164,6 +1180,14 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  	pm_runtime_disable(dev);
>  err_unprotect_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);

You still have an unbalanced clk_get / clk_put here

> +err_regmap:
> +	regmap_exit(dsi->regs);

That's not needed.

>  	return ret;
>  }
>
> @@ -1177,6 +1201,11 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
>  	pm_runtime_disable(dev);
>  	clk_rate_exclusive_put(dsi->mod_clk);
>
> +	if (!IS_ERR(dsi->bus_clk))
> +		regmap_mmio_detach_clk(dsi->regs);
> +
> +	regmap_exit(dsi->regs);

Same thing here.

Maxime

Attachment: signature.asc
Description: PGP signature