linux-kernel - Re: [PATCH 5/6] clk: tegra: Add support for Tegra132 CAR clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140716074410.GF7978@ulmo>
Date:	Wed, 16 Jul 2014 09:44:10 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Peter De Schrijver <pdeschrijver@...dia.com>
Cc:	Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Russell King <linux@....linux.org.uk>,
	Prashant Gaikwad <pgaikwad@...dia.com>,
	Mike Turquette <mturquette@...aro.org>,
	Joseph Lo <josephl@...dia.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Tuomas Tynkkynen <ttynkkynen@...dia.com>,
	devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 5/6] clk: tegra: Add support for Tegra132 CAR clocks

On Tue, Jul 15, 2014 at 06:24:35PM +0300, Peter De Schrijver wrote:
> Tegra132 CAR supports almost the same clocks as Tegra124 CAR. This patch
> deals with the small differences.
> 
> --
> I'm not entirely sure why the soc_therm clock needs to be enabled on Tegra132,
> but turning it off results in a system hang. I presume this might be because
> of fastboot initializing soc_therm.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@...dia.com>
> ---
>  drivers/clk/tegra/clk-tegra124.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> index 80efe51..b857aab 100644
> --- a/drivers/clk/tegra/clk-tegra124.c
> +++ b/drivers/clk/tegra/clk-tegra124.c
> @@ -1369,6 +1369,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
>  	{TEGRA124_CLK_XUSB_HS_SRC, TEGRA124_CLK_PLL_U_60M, 60000000, 0},
>  	{TEGRA124_CLK_XUSB_FALCON_SRC, TEGRA124_CLK_PLL_RE_OUT, 224000000, 0},
>  	{TEGRA124_CLK_XUSB_HOST_SRC, TEGRA124_CLK_PLL_RE_OUT, 112000000, 0},
> +	{TEGRA124_CLK_SOC_THERM, TEGRA124_CLK_PLL_P, 51000000, 0},
>  	/* This MUST be the last entry. */
>  	{TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0},
>  };
> @@ -1378,9 +1379,25 @@ static void __init tegra124_clock_apply_init_table(void)
>  	tegra_init_from_table(init_table, clks, TEGRA124_CLK_CLK_MAX);
>  }
>  
> +enum {
> +	TEGRA124_CLK,
> +	TEGRA132_CLK,
> +};

I'd prefer this to be something like:

	struct tegra_car_soc {
		bool has_ccplex_clk;
	};

	static const struct tegra_car_soc tegra124_car_soc = {
		.has_ccplex_clk = false,
	};

	static const struct tegra_car_soc tegra132_car_soc = {
		.has_ccplex_clk = true,
	};

> +static const struct of_device_id tegra_clock_of_match[] = {
> +	{ .compatible = "nvidia,tegra124-car", .data = (void *)TEGRA124_CLK },

	                                       .data = &tegra124_car_soc,

> +	{ .compatible = "nvidia,tegra132-car", .data = (void *)TEGRA132_CLK },

	                                       .data = &tegra132_car_soc,

>  static void __init tegra124_clock_init(struct device_node *np)
>  {
>  	struct device_node *node;
> +	const struct of_device_id *match;

	const struct tegra_car_soc *soc;

> +	uintptr_t id;

> +	match = of_match_node(tegra_clock_of_match, np);
> +	id = (uintptr_t)match->data;

	soc = match->data;

>  
>  	clk_base = of_iomap(np, 0);
>  	if (!clk_base) {
> @@ -1416,6 +1433,20 @@ static void __init tegra124_clock_init(struct device_node *np)
>  	tegra_audio_clk_init(clk_base, pmc_base, tegra124_clks, &pll_a_params);
>  	tegra_pmc_clk_init(pmc_base, tegra124_clks);
>  
> +	if (id == TEGRA132_CLK) {

	if (soc->has_ccplex_clk) {

That's somewhat more explicit and avoids a lot of ugly casting.

> +		int i;
> +
> +		tegra124_clks[tegra_clk_cclk_g].present = false;
> +		tegra124_clks[tegra_clk_cclk_lp].present = false;
> +		tegra124_clks[tegra_clk_pll_x].present = false;
> +		tegra124_clks[tegra_clk_pll_x_out0].present = false;
> +
> +		/* Tegra132 requires the soc_therm clock to be always on */
> +		for (i = 0; i < ARRAY_SIZE(init_table); i++) {
> +			if (init_table[i].clk_id == TEGRA124_CLK_SOC_THERM)
> +				init_table[i].state = 1;

I wonder if we could do this someplace else. If we could, then we'd have
the opportunity to make the init_table const.

> +		}
> +	}
>  	tegra_super_clk_gen4_init(clk_base, pmc_base, tegra124_clks,

Could use a blank line after the closing } above.

Thierry

Content of type "application/pgp-signature" skipped