On Fri, Jul 20, 2012 at 3:46 AM, Shawn Guo <shawn.guo@xxxxxxxxxx> 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
--
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
