On 05/15/2014 12:32 AM, Mike Turquette wrote:

Quoting Sebastian Hesselbarth (2014-05-11 13:24:35)

quoted

This adds mandatory device tree binding documentation for the clock related
IP found on Marvell Berlin2 (BG2, BG2CD, and BG2Q) SoCs.

Signed-off-by: Sebastian Hesselbarth <redacted>
---
Cc: Mike Turquette <redacted>
Cc: Rob Herring <redacted>
Cc: Pawel Moll <redacted>
Cc: Mark Rutland <redacted>
Cc: Ian Campbell <redacted>
Cc: Kumar Gala <redacted>
Cc: Randy Dunlap <redacted>
Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 .../devicetree/bindings/clock/berlin2-clock.txt    | 169 +++++++++++++++++++++
 1 file changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/berlin2-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/berlin2-clock.txt b/Documentation/devicetree/bindings/clock/berlin2-clock.txt
new file mode 100644
index 000000000000..3da87a488402
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/berlin2-clock.txt

@@ -0,0 +1,169 @@
+* Marvell Berlin2 clock bindings
+
+Marvell Berlin2 (BG2, BG2CD, BG2Q) share the same IP for PLLs and clocks,
+with some minor differences in features and register layout. The below
+describes the individual clock related IP:
+
+* Audio/Video PLL
+
+The Audio/Video PLL (AVPLL) is a dual-VCO PLL with 8 channels each. Each
+of the VCOs can sythesize a single VCO frequency based on a single input
+reference clock. Each of the 8 channels then, can derive an output clock
+from that VCO frequency by various dividers/multipliers.
+
+Required properties:
+- compatible: shall be "marvell,berlin2-avpll"
+- reg: address and length of the corresponding AVPLL registers
+- #clock-cells: shall be set to 2
+- clocks: single clock specifier referencing the AVPLL input clock
+
+To ease match-up with the desired AVPLL output clock, clock specifiers
+referencing AVPLL clocks shall contain two cells. The first refers to
+the VCO (0=AVPLL_A, 1=AVPLL_B) while the second refers to the corresponding
+channel starting with 1. For example, to reference AVPLL_B3 the clock
+specifier shall be: <&avpll 1 3>.
+
+Example:
+
+avpll: pll@ea0040 {
+       compatible = "marvell,berlin2-avpll";
+       #clock-cells = <2>;
+       reg = <0xea0050 0x100>;
+       clocks = <&refclk>;
+};

Thanks for submitting the series. It looks good. I do have some comments
about the DT bindings though. I'm encouraging new bindings (and
especially new platforms or existing platforms that are only now
converting over to CCF) to not put their per-clock data into DTS. This
has scalability problems, is unpopular with the DT crowd and sometimes
makes it hard to do things like set CLK_SET_RATE_PARENT flags for
individual clocks.

Ok, so you are proposing the have a single node for all the SoCs
internal plls and clocks. The individual SoCs will have to deal
with the differences in a single driver, right?

The following is a copy/paste from an email I sent earlier today[1]. Of
course per-clock data makes great sense if you have an off-SoC clock
such as a fixed-rate oscillator (e.g. the fixed-clock binding). Let me
know what you think:

[...]

Using this type of binding you only need to declare your clock generator
IP node in dts, and then define a mapping in the DT include chroot. Then
you can define your per-clock data inside of your clock driver instead
of putting all of the details inside of DT.

If you have a strong reason to do it the way that you originally posted
then let me know.

Actually, the intermediate patch set sent before this one had a single
DT clock node. The most important draw-back of a single clock node
is that Berlin's global registers are more like a register dumpster.
Vital other registers, e.g. reset, are intermixed with clock registers.

Given the lack of public datasheets (I look everything up in some
auto-generated BSP includes), I like the current approach because it
helps to get in at least some structure to the register mess ;)

Considering the postponed of_clk_create_name() helper, that would allow
us to remove at least the names from DT again. Another option would be
a syscon node for the registers, that clk, reset, pinctrl drivers can
access. But IIRC early syscon support isn't settled, yet?

So, my current idea is:
- take this as is, stabilize berlin branches for v3.16
- review of_clk_create_name() and of_clk_get_parent_name() to allow
  to remove clock-output-names properties from Berlin (and other) dtsis
- maybe switch to early syscon if it is available in v3.16

I know this would likely break DT ABI policy, but hey who else boots
mainline Linux on his Chromecast currently except me :P

Is that okay with you (and DT folks)?

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html