Re: [PATCH 08/13] OMAP2+: powerdomain: control power domains next state

[Jean Pihet <jean.pihet@newoldbits.com>]

Hi Todd,

On Fri, Jul 29, 2011 at 8:00 PM, Todd Poynor <toddpoynor@google.com> wrote:
> On Fri, Jul 29, 2011 at 10:47:43AM +0200, Jean Pihet wrote:
>> On Fri, Jul 29, 2011 at 9:59 AM, Todd Poynor <toddpoynor@google.com> wrote:
> ...
>> > All min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE paths need
>> > free_new_user = 1.
>> free_new_user = 1 is only needed if no existing constraint has been
>> found, i.e. user stays at NULL. This is implemented in the check for
>> an existing constraint (plist_for_each_entry(...)).
>
> Oops, I meant to say it applies in all cases where min_latency ==
> PM_QOS_DEV_LAT_DEFAULT_VALUE, since the new node is only used for
> adding a constraint (and no existing constraint found).  I'd suggest
> something like:
>
>        if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
>                new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
>                        GFP_KERNEL);
>                <check NULL return>
>                free_new_user = 1;
>        }
>
> and then set free_new_user = 0 only if no existing constraint is found
> for the add case.  Because it's easy to miss cases where the
> allocated memory needs to be freed when that's not the default, and you
> might as well skip the allocate on a constraint removal.  Pretty minor
> point, though.

This has been addressed in the latest patch set (v4).

Regards,
Jean

>
>
> Todd
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html