Re: [PATCH v13 0/9] Per-user clock constraints

[Stephen Boyd]

On 09/24/14 01:27, Tomeu Vizoso wrote:
> On 09/23/2014 10:59 PM, Stephen Boyd wrote:
>>
>> Any thoughts on my comments on patch set #10[1]? It seems like we can
>> avoid having a flag day to support this.
> I cannot say that I fully understand your proposal, but IMO the most
> valuable thing in this patchset is precisely the API split (and thus,
> the flag day is inherent to it).
>
> I see a lot of value in clk consumers to use a defined set of functions
> that all take and/or return struct clk, and for providers to use the
> functions that take and/or return struct clk_core. Makes the API clearer
> and allows it to have a more scalable growth in the future.

I see a lot of non-value. It's hugely invasive needing every driver to
change. Invariably we're going to break something. Back porting things
across this is a real pain (good luck stable trees!). Reviewing the
patches aren't feasible given their size. There's a reason why we don't
do flag days. They suck.

We already have the consumer/provider split in the struct clk_hw and
struct clk separation. Why don't we just use struct clk_hw throughout
the provider APIs? The only op that isn't doing this is determine_rate()
which might be able to accept a flag day. Otherwise we rename it to
something else and migrate everyone over to a different named function
that doesn't take a struct clk **. Then we introduce new APIs for the
providers to use that are struct clk_hw focused instead of struct clk
focused and migrate them too. The benefit being that we get proper
review of this stuff because the patches are small. We can let
coccinelle do it too.

>
> A less important feature of the patchset are per-user clocks, which (if
> I understand correctly) your proposal would address without requiring a
> flag day.

The subject of this thread seems to imply that it's the most important
part. I'm confused.

>
> And then we have clock constraints, which is probably the least
> important feature in the grand scheme of things, but it's actually what
> I personally care about.
>
> If we wanted to add a way for clk users to specify clock constraints
> without any refactoring, we could easily do so by reusing the request
> pattern that pm_qos uses:
>
> void clk_add_constraint(struct clk_request *req,
>                         int constraint_type,
>                         unsigned long value);
>
> void clk_update_constraint(struct clk_request *req,
>                            unsigned long new_value);
>
> void clk_remove_constraint(struct clk_request *req);
>
> It wouldn't be that bad IMO, but the API refactoring was something that
> was long desired and this was seen as a good opportunity to tackle it
> before it gets worst.
>

Sure. Maybe we should just do that so we don't break things.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/