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]
Date:	Wed, 9 Dec 2015 12:03:27 +0000
From:	Jon Hunter <jonathanh@...dia.com>
To:	Arnd Bergmann <arnd@...db.de>, <linux-pm@...r.kernel.org>
CC:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Stephen Warren <swarren@...dotorg.org>,
	"Thierry Reding" <thierry.reding@...il.com>,
	Alexandre Courbot <gnurou@...il.com>,
	<linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>
Subject: Re: [PATCH] cpufreq: tegra: add regulator dependency for T124

Hi Arnd,

On 08/12/15 21:52, Arnd Bergmann wrote:
> This driver is the only one that calls regulator_sync_voltage(), but it
> can currently be built with CONFIG_REGULATOR disabled, producing
> this build error:
> 
> drivers/cpufreq/tegra124-cpufreq.c: In function 'tegra124_cpu_switch_to_pllx':
> drivers/cpufreq/tegra124-cpufreq.c:68:2: error: implicit declaration of function 'regulator_sync_voltage' [-Werror=implicit-function-declaration]
>   regulator_sync_voltage(priv->vdd_cpu_reg);
> 
> My first attempt was to implement a helper for this function
> for regulator_sync_voltage, but Mark Brown explained:
> 
>    We don't do this for *all* regulator API functions - there's some where
>    using them strongly suggests that there is actually a dependency on
>    the regulator API.  This does seem like it might be falling into the
>    specialist category [...]
>    Looking at the code I'm pretty unclear on what the authors think the
>    use of _sync_voltage() is doing in the first place so it may be even
>    better to just remove the call.  It seems to have been included in the
>    first commit so there's not changelog explaining things and there's
>    no comment either.  I'd *expect* it to be a noop as far as I can see.

In this sequence we are switching from the DFLL clock source (which
directly controls the voltage) back to a PLL (which does not control the
voltage directly). What we want to do is to restore the voltage back to
the voltage it was operating at before we switched to the DFLL clock
(which could have changed it).

I am not familiar with regulator_sync_voltage() but from the comment it
does say that it will re-apply the last voltage that was configured for
the regulator. So I can see what they were doing. The question I have
is, if the consumer has not explicitly called regulator_set_voltage()
then what does regulator_sync_voltage() do? I am wondering if we should
have been doing a regulator_get_voltage() during the probe and a
regulator_set_voltage() when switching back?

> This adds the dependency to make the driver always build successfully
> or not be enabled at all. Alternatively, we could investigate if the
> driver should stop calling regulator_sync_voltage instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 235a1ba73d92..b1f8a73e5a94 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -226,7 +226,7 @@ config ARM_TEGRA20_CPUFREQ
>  
>  config ARM_TEGRA124_CPUFREQ
>  	tristate "Tegra124 CPUFreq support"
> -	depends on ARCH_TEGRA && CPUFREQ_DT
> +	depends on ARCH_TEGRA && CPUFREQ_DT && REGULATOR
>  	default y
>  	help
>  	  This adds the CPUFreq driver support for Tegra124 SOCs.

Acked-by: Jon Hunter <jonathanh@...dia.com>

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ