Mark Brown wrote:
>
> 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?
>
Oops, thanks for your pointing out.
> > + 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.
>
Uhm...you're right.
> 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.
OK, will address comments from you :)
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
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
