Hi Lee,

On 02/27/2014 05:36 PM, Lee Jones wrote:

quoted

Patch adds DT entries for clockgen B/C/D/E/F

Signed-off-by: Pankaj Dev <redacted>

You need to add your Signed-off-by too.

ok

quoted

---
  arch/arm/boot/dts/stih416-clock.dtsi | 170 +++++++++++++++++++++++++++++++++++
  1 file changed, 170 insertions(+)

diff --git a/arch/arm/boot/dts/stih416-clock.dtsi b/arch/arm/boot/dts/stih416-clock.dtsi
index f63b0a1..6b2e387 100644
--- a/arch/arm/boot/dts/stih416-clock.dtsi
+++ b/arch/arm/boot/dts/stih416-clock.dtsi

@@ -503,5 +503,175 @@
  						/* Remaining outputs unused */
  			};
  		};

This doesn't look right. Have you indented one tab too far?

ok i have reworked tabulation on this patch

quoted

+                /*
+                 * Frequency synthesizers on the SASG2.
+                 *
+                 */

Too many *'s

ok

<snip>

quoted

+                CLK_S_VCC_HD: CLK_S_VCC_HD {
+                        #clock-cells = <0>;
+                        compatible = "st,stih416-clkgenc-vcc-hd", "st,clkgen-mux";
+                        reg = <0xfe8308b8 4>; /* SYSCFG2558 */

0x4

ok

quoted

+                /*
+                 * Add a dummy clock for the HDMI PHY for the VCC input mux
+                 */
+                CLK_S_TMDS_FROMPHY: CLK_S_TMDS_FROMPHY {
+                        #clock-cells = <0>;
+                        compatible = "fixed-clock";
+                        clock-frequency = <0>;

What happens when the clock frequency is 0?

Nothing, this poses no problem.
This input clock is not managed by the driver.

quoted

+                };
+
+                CLOCKGEN_C_VCC: CLOCKGEN_C_VCC {
+                        #clock-cells = <1>;
+                        compatible = "st,stih416-clkgenc", "st,clkgen-vcc";
+                        reg = <0xfe8308ac 12>; /* SYSCFG2555,2556,2557 */

0x12, or 0x0C, whichever is appropriate.

ok

quoted

+                        clocks = <&CLK_S_VCC_HD>, <&CLOCKGEN_C 1>,
+                                 <&CLK_S_TMDS_FROMPHY>, <&CLOCKGEN_C 2>;

One per line would probably be better, save confusing them for pairs.

ok

<snip>

quoted

+		/*
+		 * Frequency synthesizers on the MPE42
+		 */

Alignment.

ok i transform into 1 line : /* Frequency synthesizers on the MPE42*/

<snip>

quoted

+                CLOCKGEN_F: CLOCKGEN_F {
+                        #clock-cells = <1>;
+                        compatible = "st,stih416-quadfs660-F", "st,quadfs";
+                        reg = <0xfd320878 0xF0>;
+
+                        clocks = <&CLK_SYSIN>;
+                        clock-output-names = "CLK_M_MAIN_VIDFS",
+                                             "CLK_M_HVA_FS",
+                                             "CLK_M_FVDP_VCPU",
+					     "CLK_M_FVDP_PROC_FS";

Tabbing. Ensure you're using tabs (and not spaces) everywhere.

ok

<snip>

quoted

+                        reg = <0xfd320910 4>; /* SYSCFG8580 */

0x...

Do this for all of the below too.

ok

<snip>

quoted

+                        clock-output-names  =
+                                "CLK_M_PIX_MAIN_PIPE",  "CLK_M_PIX_AUX_PIPE",
+                                "CLK_M_PIX_MAIN_CRU",   "CLK_M_PIX_AUX_CRU",
+                                "CLK_M_XFER_BE_COMPO",  "CLK_M_XFER_PIP_COMPO",
+                                "CLK_M_XFER_AUX_COMPO", "CLK_M_VSENS",
+                                "CLK_M_PIX_HDMIRX_0",   "CLK_M_PIX_HDMIRX_1";
+                };
  	};
  };

ok

Something strange going on with these.