On 01/02, Bryan O'Donoghue wrote:

On 02/01/18 19:01, Stephen Boyd wrote:

quoted

On 12/31, Bryan O'Donoghue wrote:

quoted

On 30/12/17 16:36, Mikko Perttunen wrote:

quoted

FWIW, we had this problem some years ago with the Tegra CPU clock
- then it was determined that a simpler solution was to have the
determine_rate callback support unsigned long rates - so clock
drivers that need to return rates higher than 2^31 can instead
implement the determine_rate callback. That is what's currently
implemented.

Mikko

Granted we could work around it but, having both zero and less than
zero indicate error means you can't support larger than LONG_MAX
which is I think worth fixing.

Ok. But can you implement the determine_rate op instead of the
round_rate op for your clk?

Don't know .

Please try.

quoted

It's not a work-around, it's the
preferred solution. That would allow rates larger than 2^31 for
the clk without pushing through a change to all the drivers to
express zero as "error" and non-zero as the rounded rate.

I'm not entirely opposed to this approach, because we probably
don't care to pass the particular error value from a clk provider
to a clk consumer about what the error is.

Which was my thought. The return value of clk_ops->round_rate()
appears not to get pushed up the stack, which is what the last patch
in this series deals with.

[PATCH 33/33] clk: change handling of round_rate() such that only
zero is an error

Hmm? clk_core_determine_round_nolock() returns 'rate' if rate < 0
from the round_rate op. clk_core_round_rate_nolock() returns that
value to clk_round_rate() which returns it to the consumer.

quoted

It's actually what we
proposed as the solution for clk_round_rate() to return values
larger than LONG_MAX to consumers. But doing that consumer API
change or this provider side change is going to require us to
evaluate all the consumers of these clks to make sure they don't
check for some error value that's less than zero. This series
does half the work,

Do you mean users of clk_rounda_rate() ? I have a set of patches for
that but wanted to separate that from clk_ops->round_rate() so as
not to send ~70 patches out to LKML at once - even if they are in
two blocks.

Ok. What have you done to the consumers of clk_round_rate()?
Made them treat 0 as an error instead of less than zero? The
documentation in clk.h needs to be updated. See this patch from
Paul Wamsley[1] for one proposed patch that went nowhere. Also
include Russell King please. It was also proposed to change the
function signature of clk_round_rate() to return unsigned long,
but that didn't go anywhere either.

If so, I can publish that set too for reference.

AFAICT on clk_ops->round_rate the last patch #33 ought to cover the
usage of the return value of clk_ops->round_rate().

Have I missed something ?

Hopefully not!

quoted

by changing the provider side, while ignoring
the consumer side and any potential fallout of the less than zero
to zero return value change.

Can you look at #33 ? I'm not sure if you saw that one.

Yeah I looked at it. From what I can tell it makes
clk_round_rate() return 0 now instead of whatever negative value
the clk_ops::round_rate function returns.

[1] https://lkml.kernel.org/r/alpine.DEB.2.02.1311251603310.23090 at tamien

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project