Re: [PATCH 2/3] PM / OPP: Initialize OPP table from device tree

["Menon, Nishanth" <nm@ti.com>]

On Fri, Jul 20, 2012 at 3:46 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>
>> > +       ret = of_property_read_u32_array(np, propname, opp, nr);
>> > +       if (ret) {
>> > +               dev_err(dev, "%s: Unable to read OPPs\n", __func__);
>> > +               goto out;
>> > +       }
>> > +
>> > +       nr /= 3;
>> > +       for (i = 0; i < nr; i++) {
>> > +               /*
>> > +                * Each OPP is a set of tuples consisting of frequency,
>> > +                * voltage and availability like <freq-kHz vol-uV enable>.
>> > +                */
>> > +               u32 *val = opp + i * 3;
>> > +
>> > +               val[0] *= 1000;
>> > +               ret = opp_add(dev, val[0], val[1]);
>> > +               if (ret) {
>> > +                       dev_warn(dev, "%s: Failed to add OPP %d: %d\n",
>> > +                                __func__, val[0], ret);
>> > +                       continue;
>> > +               }
>> > +
>> > +               if (!val[2]) {
>> > +                       ret = opp_disable(dev, val[0]);
>> Since we are updating the table out of context of the SoC users,
>> consider what might happen if someone where to operate on the OPP
>> after opp_add, but before opp_disable?
>
> I would take this as another comment reminding me to remove the
> enabling/availability tuple from the binding.  Will do it in the
> next version.
I am not completely convinced that dropping the flag would be the best approach
on a long run, but might be a good starting point while we meet current reqs and
as a need arises, could increase the scope. Thanks once again for doing this.

Regards,
Nishanth Menon