Re: [PATCH 1/3] clk: shmobile: Add DIV6 clock support

Hi Kumar,

Thank you for the review.

On Tuesday 29 October 2013 18:33:00 Kumar Gala wrote:

On Oct 29, 2013, at 9:55 AM, Laurent Pinchart wrote:

quoted

DIV6 clocks are divider gate clocks controlled through a single
register. The divider is expressed on 6 bits, hence the name, and can
take values from 1/1 to 1/64.

Those clocks are found on Renesas ARM SoCs.

Signed-off-by: Laurent Pinchart
[off-list ref]
---
.../bindings/clock/renesas,cpg-div6-clocks.txt     |  27 +++
drivers/clk/shmobile/Makefile                      |   4 +-
drivers/clk/shmobile/clk-div6.c                    | 185 +++++++++++++++++
3 files changed, 215 insertions(+), 1 deletion(-)
create mode 100644
Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
create mode 100644 drivers/clk/shmobile/clk-div6.c

diff --git
a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt new
file mode 100644
index 0000000..8036f3c

--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt

@@ -0,0 +1,27 @@
+* Renesas CPG DIV6 Clock
+
+The CPG DIV6 clocks are variable factor clocks provided by the Clock

Pulse
+Generator (CPG). They clock input is divided by a configurable factor
from 1
+to 64.
+
+Required Properties:
+
+  - compatible: Must be one of the following
+    - "renesas,r8a7790-div6-clock" for R8A7790 (R-Car H2) DIV6 clocks
+    - "renesas,cpg-div6-clock" for generic DIV6 clocks
+  - reg: Base address and length of the memory resource used by the DIV6
clock
+  - clocks: Reference to the parent clock
+  - #clock-cells: Must be 0
+  - clock-output-name: The name of the clock as a free-form string

Is this suppose to be 'clock-output-names' (missing a 's')?

Yes it is, my bad. I'll fix that.

quoted

+
+
+Example
+-------
+
+	sd2_clk: sd2_clk {

example should be sd2_clk@e6150078

You're absolutely right.

quoted

+		compatible = "renesas,r8a7790-div6-clock", "renesas,cpg-div6-

clock";

quoted

+		reg = <0 0xe6150078 0 4>;

curious, do you really have a second register at 0?

No, but the r8a7790 has #address-cells and #size-cells set to 2.

quoted

+		clocks = <&pll1_div2_clk>;
+		#clock-cells = <0>;
+		clock-output-names = "sd2";
+	};

-- 
Regards,

Laurent Pinchart