Re: [RFC PATCH 04/13] drm/tegra: Add sor-safe clock for DPAUX on Tegra210

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2929 bytes --]

On Fri, Jun 17, 2016 at 01:03:38PM +0100, Jon Hunter wrote:
> For Tegra210 the 'sor-safe' clock needs to be enabled when using DPAUX.
> Add support to the DPAUX driver for enabling this clock on Tegra210.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dpaux.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index aa3a037fcd3b..d696a7e45935 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -37,6 +37,7 @@ struct tegra_dpaux {
>  
>  	struct reset_control *rst;
>  	struct clk *clk_parent;
> +	struct clk *clk_sor;

Can we call this "clk_safe", please? On one hand that mirrors the name
of the clock in the binding and on the other hand it avoids confusion
with the real SOR clock.

>  	struct clk *clk;
>  
>  	struct regulator *vdd;
> @@ -340,18 +341,37 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>  		return PTR_ERR(dpaux->rst);
>  	}
>  
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "nvidia,tegra210-dpaux")) {
> +		dpaux->clk_sor = devm_clk_get(&pdev->dev, "sor-safe");
> +		if (IS_ERR(dpaux->clk_sor)) {
> +			dev_err(&pdev->dev,
> +				"failed to get sor-safe clock: %ld\n",
> +				PTR_ERR(dpaux->clk_sor));
> +			return PTR_ERR(dpaux->clk_sor);
> +		}
> +
> +		err = clk_prepare_enable(dpaux->clk_sor);
> +		if (err < 0) {
> +			dev_err(&pdev->dev,
> +				"failed to enable sor-safe clock: %d\n", err);
> +			return err;
> +		}
> +	}

Please make this part of a struct tegra_dpaux_soc, so that we don't have
to check the compatible string again here. This could look like:

	struct tegra_dpaux_soc {
		bool needs_safe_clock;
	};

	static const struct tegra_dpaux_soc tegra124_dpaux_soc = {
		.needs_safe_clock = false,
	};

	static const struct tegra_dpaux_soc tegra210_dpaux_soc = {
		.needs_safe_clock = true,
	};

	...

	static const struct of_device_id tegra_dpaux_of_match[] = {
		{ .compatible = "nvidia,tegra210-dpaux", .data = &tegra210_dpaux_soc },
		{ .compatible = "nvidia,tegra124-dpaux", .data = &tegra124_dpaux_soc },
		{ },
	};

> @@ -434,6 +454,9 @@ disable_parent_clk:
>  assert_reset:
>  	reset_control_assert(dpaux->rst);
>  	clk_disable_unprepare(dpaux->clk);
> +disable_sor_clk:
> +	if (dpaux->clk_sor)
> +		clk_disable_unprepare(dpaux->clk_sor);

You can drop the extra check here, since the common clock framework
ignores NULL or ERR_PTR() pointers.

>  
>  	return err;
>  }
> @@ -456,6 +479,8 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
>  	clk_disable_unprepare(dpaux->clk_parent);
>  	reset_control_assert(dpaux->rst);
>  	clk_disable_unprepare(dpaux->clk);
> +	if (dpaux->clk_sor)
> +		clk_disable_unprepare(dpaux->clk_sor);

Same here.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel