[PATCH 4/6] davinci: DA850/OMAP-L138: avoid using separate initcall for initializing regulator

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/6] davinci: DA850/OMAP-L138: avoid using separate initcall for initializing regulator

Nori, Sekhar
On Thu, Oct 22, 2009 at 15:12:16, Nori, Sekhar wrote:

> Using a device_initcall() for initializing the voltage regulator
> on DA850 is not such a good idea because it gets called for all
> platforms - even those who do not have a regulator implemented.
> This leads to a big fat warning message during boot-up when
> regulator cannot be found.
>
> Instead, tie initialization of voltage regulator to cpufreq init.
> Define a platform specific init call which in case of DA850 gets
> used for initializing the regulator. On other future platforms it
> can be used for other purposes.
>
> Signed-off-by: Sekhar Nori <[hidden email]>
> ---

[...]

> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index a13a6c4..c2d2724 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c

[...]

> +static int __init da850_regulator_init(void)
> +{
> +     cvdd = regulator_get(NULL, "cvdd");
> +     if (WARN(IS_ERR(cvdd), "Unable to obtain voltage regulator for CVDD;"
> +                                     " voltage scaling unsupported\n")) {
> +             return PTR_ERR(cvdd);
> +     }
> +
> +     return 0;
> +}
> +#endif
> +
>  static struct davinci_cpufreq_config cpufreq_info = {
>       .freq_table = &da850_freq_table[0],
> +#ifdef CONFIG_REGULATOR
> +     .init = da850_regulator_init,

Oops, I introduced a section mismatch here. In this case, it is
pretty harmless though since cpufreq cannot be a module.

I will fix this (by getting rid of __init for da850_regulator_init)
when reposting along with any other comments on the patch.

Thanks,
Sekhar

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/6] davinci: DA850/OMAP-L138: avoid using separate initcall for initializing regulator

Nori, Sekhar
On Mon, Nov 02, 2009 at 20:19:25, Nori, Sekhar wrote:

> On Thu, Oct 22, 2009 at 15:12:16, Nori, Sekhar wrote:
> > Using a device_initcall() for initializing the voltage regulator
> > on DA850 is not such a good idea because it gets called for all
> > platforms - even those who do not have a regulator implemented.
> > This leads to a big fat warning message during boot-up when
> > regulator cannot be found.
> >
> > Instead, tie initialization of voltage regulator to cpufreq init.
> > Define a platform specific init call which in case of DA850 gets
> > used for initializing the regulator. On other future platforms it
> > can be used for other purposes.
> >
> > Signed-off-by: Sekhar Nori <[hidden email]>
> > ---
>
> [...]
>
> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> > index a13a6c4..c2d2724 100644
> > --- a/arch/arm/mach-davinci/da850.c
> > +++ b/arch/arm/mach-davinci/da850.c
>
> [...]
>
> > +static int __init da850_regulator_init(void)
> > +{
> > +   cvdd = regulator_get(NULL, "cvdd");
> > +   if (WARN(IS_ERR(cvdd), "Unable to obtain voltage regulator for CVDD;"
> > +                                   " voltage scaling unsupported\n")) {
> > +           return PTR_ERR(cvdd);
> > +   }
> > +
> > +   return 0;
> > +}
> > +#endif
> > +
> >  static struct davinci_cpufreq_config cpufreq_info = {
> >     .freq_table = &da850_freq_table[0],
> > +#ifdef CONFIG_REGULATOR
> > +   .init = da850_regulator_init,
>
> Oops, I introduced a section mismatch here. In this case, it is
> pretty harmless though since cpufreq cannot be a module.
>
> I will fix this (by getting rid of __init for da850_regulator_init)
> when reposting along with any other comments on the patch.

Kevin, I think you missed seeing this or I confused you by posting
a revision only for patch 1/6 of the series yesterday. I will post
this fix as a separate patch.

Thanks,
Sekhar

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/6] davinci: DA850/OMAP-L138: avoid using separate initcall for initializing regulator

Kevin Hilman
"Nori, Sekhar" <[hidden email]> writes:

> On Mon, Nov 02, 2009 at 20:19:25, Nori, Sekhar wrote:
>> On Thu, Oct 22, 2009 at 15:12:16, Nori, Sekhar wrote:
>> > Using a device_initcall() for initializing the voltage regulator
>> > on DA850 is not such a good idea because it gets called for all
>> > platforms - even those who do not have a regulator implemented.
>> > This leads to a big fat warning message during boot-up when
>> > regulator cannot be found.
>> >
>> > Instead, tie initialization of voltage regulator to cpufreq init.
>> > Define a platform specific init call which in case of DA850 gets
>> > used for initializing the regulator. On other future platforms it
>> > can be used for other purposes.
>> >
>> > Signed-off-by: Sekhar Nori <[hidden email]>
>> > ---
>>
>> [...]
>>
>> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> > index a13a6c4..c2d2724 100644
>> > --- a/arch/arm/mach-davinci/da850.c
>> > +++ b/arch/arm/mach-davinci/da850.c
>>
>> [...]
>>
>> > +static int __init da850_regulator_init(void)
>> > +{
>> > +   cvdd = regulator_get(NULL, "cvdd");
>> > +   if (WARN(IS_ERR(cvdd), "Unable to obtain voltage regulator for CVDD;"
>> > +                                   " voltage scaling unsupported\n")) {
>> > +           return PTR_ERR(cvdd);
>> > +   }
>> > +
>> > +   return 0;
>> > +}
>> > +#endif
>> > +
>> >  static struct davinci_cpufreq_config cpufreq_info = {
>> >     .freq_table = &da850_freq_table[0],
>> > +#ifdef CONFIG_REGULATOR
>> > +   .init = da850_regulator_init,
>>
>> Oops, I introduced a section mismatch here. In this case, it is
>> pretty harmless though since cpufreq cannot be a module.
>>
>> I will fix this (by getting rid of __init for da850_regulator_init)
>> when reposting along with any other comments on the patch.
>
> Kevin, I think you missed seeing this or I confused you by posting
> a revision only for patch 1/6 of the series yesterday. I will post
> this fix as a separate patch.

Thanks Sekhar,

A small diff will suffice and I will fold it into the original patch
before pushing upstream.

Kevin