On Sunday, August 14, 2011, Jean Pihet wrote: > Hi Rafael, Mark, > > On Sat, Aug 13, 2011 at 10:34 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > On Saturday, August 13, 2011, mark gross wrote: > >> On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pihet@xxxxxxxxxxxxxx wrote: > >> > From: Jean Pihet <j-pihet@xxxxxx> > >> > > >> > In preparation for the per-device constratins support: > >> > - rename update_target to pm_qos_update_target > >> > - generalize and export pm_qos_update_target for usage by the upcoming > >> > per-device latency constraints framework: > >> > . operate on struct pm_qos_constraints for constraints management, > >> > . introduce an 'action' parameter for constraints add/update/remove, > >> > . the return value indicates if the aggregated constraint value has > >> > changed, > >> > - update the internal code to operate on struct pm_qos_constraints > >> > - add a NULL pointer check in the API functions > >> > > >> > Signed-off-by: Jean Pihet <j-pihet@xxxxxx> > ... > >> > +/* Action requested to pm_qos_update_target */ > >> > +enum pm_qos_req_action { > >> > + PM_QOS_ADD_REQ, /* Add a new request */ > >> > + PM_QOS_UPDATE_REQ, /* Update an existing request */ > >> > + PM_QOS_REMOVE_REQ /* Remove an existing request */ > >> > +}; > >> > + > >> > >> What do you need this enum for? The function names *_update_*, *_add_*, > >> and *_remove_* seem to be pretty redundant if you have to pass an enum > >> that could possibly conflict with the function name. > >> > >> > #ifdef CONFIG_PM > >> > +int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node, > >> > + enum pm_qos_req_action action, int value); > >> The action for update_target better damn well be "PM_QOS_UPDATE_REQ" or > >> there is something strange going on.... BTW what shold this function do > >> if the pm_qos_req_action was *not* the UPDATE one? > > The meaning of pm_qos_update_target is 'update the PM QoS target > constraints lists'. As described in the changelog the intention of > this patch is to implement the constraints lists management logic in > update_target and simplify the API functions (add/update/remove). It > is also exported for the upcoming (patch 06/15]) to use it as well. The enums are fine by me and they allow us to simplify the code quite a bit. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html