Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates

[Tomeu Vizoso <tomeu.vizoso@collabora.com>]

On 07/22/2014 07:50 PM, Stephen Warren wrote:
> On 07/17/2014 08:13 AM, Tomeu Vizoso wrote:
>> Adds a way for clock consumers to set maximum and minimum rates. This can be
>> used for thermal drivers to set ceiling rates, or by misc. drivers to set
>> floor rates to assure a minimum performance level.
>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>
>> +struct rate_constraint {
>> +	struct hlist_node	node;
>> +	const char		*dev_id;
>> +	const char		*con_id;
>> +	enum constraint_type	type;
>> +	unsigned long		rate;
>> +};
>
> I would personally still prefer either:
>
> a) The struct rate_constraints be stored in struct clk rather than
> struct clk_core, so they're stored in the container that "set" the
> constraints. This would mean iterating over a struct clk_core's
> associated struct clks, then iterating over the struct rate_constraints
> within each struct clk.
>
> or:
>
> b) struct rate_constraint to contain a pointer to the struct clk that
> created the constraint, rather than copying the dev_id/con_id from that
> struct clk into the struct rate_constraint.
>
> With either of those changes, the constraints are directly associated
> with the clock client object that created the constraint much better
> than right now, where the matching is only because the struct clk and
> struct rate_constraint "happen to" have the same dev_id/con_id values.
>
> Still, this is a pretty minor issue, and overall this series looks
> reasonable to me at a quick look.

Yeah, I agree and personally prefer a), but given the little feedback 
that I have gotten so far on the refactoring, I'm starting to consider 
forgetting about the per-user clk struct and go instead with a 
request-based API similar to that of pm_qos, for setting floor and 
ceiling frequencies.

Regards,

Tomeu