Rob,
Rob Herring wrote:
Saravana,
On Wed, Aug 18, 2010 at 12:39 AM, Saravana Kannan
<skannan@xxxxxxxxxxxxxx> wrote:
I'm mostly familiar with ARM. So I will limit the discussion to ARM
boards/machines. But I think the points raised in this email would apply
for most architectures.
I'm sending this to the ARM mailing list first to see if I can get a
consensus within the ARM community before I propose this to the wider
audience in LKML.
The meaning of clk_round_rate() is very ambiguous since the nature of the
rounding is architecture and platform specific. In the case of ARM, it's up
to each ARM mach's clock driver to determine how it wants to round the rate.
To quote Russel King from an email about a month ago:
"clk_round_rate() returns the clock rate which will be set if you ask
clk_set_rate() to set that rate. It provides a way to query from the
implementation exactly what rate you'll get if you use clk_set_rate() with
that same argument."
So when someone writes a device driver for a device that's external to the
SoC or is integrated into more than one SoC, they currently have the
following options to deal with clock rates differences:
1. Use clk_round_rate() to get clock rates and test their driver on the
one/few board(s) they have at hand and hope it works on boards using
different SoCs.
2. Add each and every needed clock rate (low power rate, high performance
rate, handshake rate, etc) as fields to their platform data and have it
populated in every board file that uses the device.
3. Do a search of the frequency space of the clock by making several
clk_round_rate() calls at various intervals between their minimum and
maximum acceptable rates and hope to find one of the supported rates. If
clk_round_rate() does a +/- N percentage rounding and the interval is
larger, even this searching might not find an existing rate that's supported
between the driver's min and max acceptable rates.
IMHO, each of these options have short comings that could be alleviated by
adding a more definitive "rounding" API. Also, considering that it's the
consumer of each clock that knows best what amount of rounding and in which
direction is acceptable, IMHO the current approach of hiding the rounding
inside the clock drivers seems counter intuitive.
I would like to propose the addition of either:
long clk_find_rate(struct clk *clk,
unsigned long min_rate,
long max_rate);
or
long clk_find_rate(struct clk *clk,
unsigned long start_rate,
long end_rate);
The advantage of the 2nd suggestion is that it allows a driver to specify
which end of a frequency range it prefers. But I'm not sure how important
of an advantage that is. So, proposing both and having the community decide
which one, if any, is acceptable.
If the clk_find_rate() API is available, the driver developer wouldn't have
to worry about figuring out a way for the clk_set_rate() to work on
different platforms/machs/SoCs. If a platform/mach/SoC can provide a clock
rate that's acceptable to their hardware and software requirements, then
they can be assured to find it without having to jump through hoops or
having a driver not work when it could have.
Does the addition of one of the suggested APIs sound reasonable? If not, can
someone explain what the right/better solution is? If the addition of a new
API is reasonable, what's the community preference between the two
suggestion? If I submit a patch that will add one of the APIs is it likely
to be accepted?
Thanks for your time.
This is still ambiguous. If there are multiple valid frequencies in
the range, is the preferred rate returned the lowest rate or highest
rate in the range? For something like an SD bus clock you would want
the maximum rate within the range. For something like a LCD pixel
clock or CMOS sensor input clock, you typically only need to be
greater than a certain minimum freq and for power reasons you want it
to be closest to the minimum.
Thanks for the feed back. This ambiguity is what my alternate proposal
handles. To repeat the 2nd option:
long clk_find_rate(struct clk *clk,
unsigned long start_rate,
long end_rate);
With the above API, the clock driver tries to find a freq between
start_rate and end_rate, but starts the search from "start_rate".
So, if you prefer lower freqs, you call it as:
clk_find_rate(clk, 100000, 150000);
and if you prefer higher freqs, you call it as:
clk_find_rate(clk, 150000, 100000);
That should take care of your concerns, right?
-Saravana
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
