Re: [PATCH 2/6] [CPUFREQ] S5PV210: Add arm/int voltage control support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 22, 2011 at 05:07:41PM +0900, Kukjin Kim wrote:

>  	if (freqs.new > freqs.old) {
>  		/* Voltage up: will be implemented */

Presumably this comment is bitrotted?

> +		if (!IS_ERR(arm_regulator) &&
> +				!IS_ERR(int_regulator)) {

You should not do this.  The regulator API stubs itself out and provides
mechanisms for boards to fill out partial regulator configurations, and
clearly these supplies must be physically present in the system.

Plus...

>  static int __init s5pv210_cpufreq_init(void)
>  {
> +	arm_regulator = regulator_get(NULL, "vddarm");
> +	if (IS_ERR(arm_regulator)) {
> +		pr_err("failed to get regulator vddarm");
> +		return PTR_ERR(arm_regulator);
> +	}
> +
> +	int_regulator = regulator_get(NULL, "vddint");
> +	if (IS_ERR(int_regulator)) {
> +		pr_err("failed to get regulator vddint");
> +		regulator_put(arm_regulator);
> +		return PTR_ERR(int_regulator);
> +	}

...the driver won't currently bind if it didn't get the regulators.

Ideally you should also have code which checks to see if the regulators
can support all the operating points.  This will ensure that governors
don't end up spending time trying to go into states that can't be
supported for some reason.  That said this is fairly unusual so may not
be worth bothering.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux