Thread (11 messages) 11 messages, 5 authors, 2014-06-13

Re: [PATCH RFC v5 2/2] clk: Add handling of clk parent and rate assigned from DT

From: Rob Herring <hidden>
Date: 2014-06-13 14:34:47

On Fri, Apr 11, 2014 at 8:08 AM, Laurent Pinchart [off-list ref] wrote:
On Friday 11 April 2014 14:25:49 Sylwester Nawrocki wrote:
quoted
On 10/04/14 18:04, Rob Herring wrote:
quoted
On Wed, Apr 9, 2014 at 6:26 AM, Sylwester Nawrocki wrote:
quoted
This patch adds a helper function to configure clock parents and
rates as specified in clock-parents, clock-rates DT properties
for a consumer device and a call to it before driver is bound to
a device.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
[...]
quoted
quoted
---

 .../devicetree/bindings/clock/clock-bindings.txt   |   44 ++++++++++
 drivers/base/platform.c                            |    5 ++
 drivers/clk/Makefile                               |    3 +
 drivers/clk/clk-conf.c                             |   85 ++++++++++++++
 drivers/clk/clk.c                                  |   12 ++-
 include/linux/clk/clk-conf.h                       |   19 +++++
 6 files changed, 167 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/clk-conf.c
 create mode 100644 include/linux/clk/clk-conf.h
diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt
b/Documentation/devicetree/bindings/clock/clock-bindings.txt index
700e7aa..93513fc 100644
--- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -132,3 +132,47 @@ clock signal, and a UART.
   ("pll" and "pll-switched").
 * The UART has its baud clock connected the external oscillator and its
   register clock connected to the PLL clock (the "pll-switched" signal)
+
+==Assigned clock parents and rates==
+
+Some platforms require static initial configuration of parts of the
clocks
+controller. Such a configuration can be specified in a clock consumer
node
+through clock-parents and clock-rates DT properties. The former should
+contain a list of parent clocks in form of phandle and clock specifier
pairs,
+the latter the list of assigned clock frequency values (one cell each).
+To skip setting parent or rate of a clock its corresponding entry should
be
+set to 0, or can be omitted if it is not followed by any non-zero entry.
+
+    uart@a000 {
+        compatible = "fsl,imx-uart";
+        reg = <0xa000 0x1000>;
+        ...
+        clocks = <&clkcon 0>, <&clkcon 3>;
+        clock-names = "baud", "mux";
+
+        clock-parents = <0>, <&pll 1>;
+        clock-rates = <460800>;
Is this the input frequency or serial baud rate? Looks like a baud
rate, but the clock framework needs input (to the uart) frequency. I
would say this should be clock-frequency and specify the max baud rate
as is being done with i2c bindings. The uart driver should know how to
convert between input clock freq and baud rate.
This UART example is not quite representative for the issues I have been
trying to address with this patch set. There is a need to set (an initial)
input clock frequency. E.g. in case of multimedia devices there may be
a need to set clock parent and frequency of an input clock to multiple IP
blocks, so they are clocked synchronously and data is carried properly
across a whole processing chain. Thus there may not be even clock output
in an IP block, but still input clock needs to be set. IIUC there is
similar issue with audio, where it is difficult to calculate the clock
frequencies/determine parent clocks in individual drivers algorithmically.
Just to be used as an example, this is how the SMIA++ sensor driver computes
the PLL parameters automatically based on the input frequency, desired output
frequency and various hardware limits.

http://lxr.free-electrons.com/source/drivers/media/i2c/smiapp-pll.c

See the code complexity and keep in mind that it only handles a single device
with a single set of constraints and a single parent. If we add several
devices to the mix, as well as selectable parents, there would indeed probably
be no sane way to configure the clocks algorithmically.
How much of this complexity is PLL/clock constraints vs. consumer constraints. From the consumer side, is it not just a problem of for a given resolution and frame rate you need a minimum frequency of X and the maximum frequency of Y that a sensor can handle? clk_round_rate has long needed to be extended to be able to specify rounding criteria.

How many clocks really need to be configured? I would think it is only a handful (audio, display, camera). Trying to solve this generically may not be worth it in terms of binding complexity. Both trying to configure things dynamically or parsing DT to get the configuration add a lot of complexity for what is going to boil down to a few clock controller register writes. You could just do all that in the bootloaders and be done with it. Perhaps it would be simpler to just define clock controller node specific properties which have enough information to do the configuration.


Rob
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help