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 cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
