Re: [PATCH 0/4] clk: dt: add support for default rate/parent

2014-03-25 Thread Peter De Schrijver
On Tue, Mar 25, 2014 at 01:38:47AM +0100, Mike Turquette wrote:
 Quoting Peter De Schrijver (2014-03-21 01:12:17)
  On Thu, Mar 20, 2014 at 10:23:08PM +0100, Mike Turquette wrote:
   Quoting Tero Kristo (2014-03-05 05:10:17)
Ping.

Mike, any feedback on this?
   
   Hi Tero,
   
   Have you seen Sylwester's approach[1]? I prefer it since it is more
   device-oriented and less centralized. The clock consumer enumerates
   the default parent or rate of a consumed clock. This can be made to work
  
  That assumes driver writers are aware of the clock tree topology. IME that's
  seldomly the case.
 
 It assumes no such thing.
 
 The point of Sylwester's patch is that if a driver consumes a clock and
 needs to do the very typical setup regarding that clock's rate or
 parent, then we now have a sensible way to express that in DT.
 
 One of the strengths of DT is that the C code does not have to know all
 of the details about topology or how things are hooked up. We can hide
 some of those cute embedded nonsense hacks in DTS and the device
 integrator can manage it there.

It would be much better to specify this as part of the clock provider binding
though, as the person writing those, generally knows what topology needs to be
setup. The driver writer writing the consumer node often doesn't.

Cheers,

Peter.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] clk: dt: add support for default rate/parent

2014-03-21 Thread Tero Kristo

On 03/20/2014 11:23 PM, Mike Turquette wrote:

Quoting Tero Kristo (2014-03-05 05:10:17)

Ping.

Mike, any feedback on this?


Hi Tero,

Have you seen Sylwester's approach[1]? I prefer it since it is more
device-oriented and less centralized. The clock consumer enumerates
the default parent or rate of a consumed clock. This can be made to work
like your approach by having the clock driver consume these clocks and
set them up with default rates or parents. What do you think?


Just saw the patches yesterday. I think that approach would work in most 
cases, however I guess there might be cases where you need to setup the 
rate/parent of a clock very early in boot (basically when you are 
registering the clock itself) to avoid any race issues with drivers (or 
the clock framework itself) coming in and using a clock that is not 
properly setup yet. But well, I guess these can be handled by some init 
time tweaks.


I just converted the OMAP4 code to the format provided by Sylwester's 
patches and it seems to work fine. The patch should be changed 
eventually to probe at the time when the CM/PRM instances are probed. 
Inlined here as reference:


From 1b05e03bbd3fb5a4f5192444e7d4365f177c1756 Mon Sep 17 00:00:00 2001
From: Tero Kristo t-kri...@ti.com
Date: Fri, 21 Mar 2014 09:52:47 +0200
Subject: [PATCH] CLK: TI: OMAP4: setup default clocks / rates using the 
clock

 consumer approach

Signed-off-by: Tero Kristo t-kri...@ti.com
---
 arch/arm/boot/dts/omap4.dtsi |8 
 drivers/clk/ti/clk-44xx.c|   44 
--
 drivers/clk/ti/clk.c |   41 
+++

 3 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index d3f8a6e..4826168 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -88,6 +88,14 @@
};
};

+   default-clocks {
+   compatible = ti,default-clocks;
+   clocks = abe_dpll_refclk_mux_ck, dpll_usb_ck,
+   dpll_abe_ck;
+   clock-parents = sys_32k_ck;
+   clock-rates = 0, 96000, 98304000;
+   };
+
/*
 * XXX: Use a flat representation of the OMAP4 interconnect.
 * The real OMAP interconnect network is quite complex.
diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
index ae00218..dfafb96 100644
--- a/drivers/clk/ti/clk-44xx.c
+++ b/drivers/clk/ti/clk-44xx.c
@@ -16,21 +16,6 @@
 #include linux/clkdev.h
 #include linux/clk/ti.h

-/*
- * OMAP4 ABE DPLL default frequency. In OMAP4460 TRM version V, section
- * 3.6.3.2.3 CM1_ABE Clock Generator states that the DPLL_ABE_X2_CLK
- * must be set to 196.608 MHz and hence, the DPLL locked frequency is
- * half of this value.
- */
-#define OMAP4_DPLL_ABE_DEFFREQ 98304000
-
-/*
- * OMAP4 USB DPLL default frequency. In OMAP4430 TRM version V, section
- * 3.6.3.9.5 DPLL_USB Preferred Settings shows that the preferred
- * locked frequency for the USB DPLL is 960MHz.
- */
-#define OMAP4_DPLL_USB_DEFFREQ 96000
-
 static struct ti_dt_clk omap44xx_clks[] = {
DT_CLK(NULL, extalt_clkin_ck, extalt_clkin_ck),
DT_CLK(NULL, pad_clks_src_ck, pad_clks_src_ck),
@@ -281,36 +266,7 @@ static struct ti_dt_clk omap44xx_clks[] = {

 int __init omap4xxx_dt_clk_init(void)
 {
-   int rc;
-   struct clk *abe_dpll_ref, *abe_dpll, *sys_32k_ck, *usb_dpll;
-
ti_dt_clocks_register(omap44xx_clks);
-
omap2_clk_disable_autoidle_all();
-
-   /*
-* Lock USB DPLL on OMAP4 devices so that the L3INIT power
-* domain can transition to retention state when not in use.
-*/
-   usb_dpll = clk_get_sys(NULL, dpll_usb_ck);
-   rc = clk_set_rate(usb_dpll, OMAP4_DPLL_USB_DEFFREQ);
-   if (rc)
-   pr_err(%s: failed to configure USB DPLL!\n, __func__);
-
-   /*
-* On OMAP4460 the ABE DPLL fails to turn on if in idle low-power
-* state when turning the ABE clock domain. Workaround this by
-* locking the ABE DPLL on boot.
-* Lock the ABE DPLL in any case to avoid issues with audio.
-*/
-   abe_dpll_ref = clk_get_sys(NULL, abe_dpll_refclk_mux_ck);
-   sys_32k_ck = clk_get_sys(NULL, sys_32k_ck);
-   rc = clk_set_parent(abe_dpll_ref, sys_32k_ck);
-   abe_dpll = clk_get_sys(NULL, dpll_abe_ck);
-   if (!rc)
-   rc = clk_set_rate(abe_dpll, OMAP4_DPLL_ABE_DEFFREQ);
-   if (rc)
-   pr_err(%s: failed to configure ABE DPLL!\n, __func__);
-
return 0;
 }
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index b1a6f71..469fd4e 100644
--- a/drivers/clk/ti/clk.c
+++ b/drivers/clk/ti/clk.c
@@ -21,6 +21,8 @@
 #include linux/of.h
 #include linux/of_address.h
 #include linux/list.h
+#include linux/of_platform.h
+#include linux/module.h

 #undef pr_fmt
 #define pr_fmt(fmt) 

Re: [PATCH 0/4] clk: dt: add support for default rate/parent

2014-03-21 Thread Peter De Schrijver
On Thu, Mar 20, 2014 at 10:23:08PM +0100, Mike Turquette wrote:
 Quoting Tero Kristo (2014-03-05 05:10:17)
  Ping.
  
  Mike, any feedback on this?
 
 Hi Tero,
 
 Have you seen Sylwester's approach[1]? I prefer it since it is more
 device-oriented and less centralized. The clock consumer enumerates
 the default parent or rate of a consumed clock. This can be made to work

That assumes driver writers are aware of the clock tree topology. IME that's
seldomly the case.

Cheers,

Peter.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] clk: dt: add support for default rate/parent

2014-03-20 Thread Mike Turquette
Quoting Tero Kristo (2014-03-05 05:10:17)
 Ping.
 
 Mike, any feedback on this?

Hi Tero,

Have you seen Sylwester's approach[1]? I prefer it since it is more
device-oriented and less centralized. The clock consumer enumerates
the default parent or rate of a consumed clock. This can be made to work
like your approach by having the clock driver consume these clocks and
set them up with default rates or parents. What do you think?

Regards,
Mike

[1] https://lkml.org/lkml/2014/3/3/324

 
 -Tero
 
 On 02/13/2014 11:00 AM, Tero Kristo wrote:
  Hi,
 
  This set is a mix-match of new DT properties for generic and TI specific
  clock drivers. Basically provided for commenting purposes. The patches
  provide a way to configure clock parents / rates during boot through DT.
 
  default-rate : sets rate of a clock during boot, supported for any DT
 clock type (through generic clock driver)
  ti,default-parent : selects a default parent for a multiplexer clock,
  only supported for TI specific mux clock for now,
  as generic mux clock does not support DT clocks
 
  Patch #4 provided as a reference how to move the default rates / parents
  from kernel code to DT.
 
  Default-rate logic in patch #2 looks somewhat complicated, as the clocks
  need to be sorted based on their parents to avoid cases where a child clock
  would set its rate first, just to be overridden by its parent changing
  rate later and resulting in incorrect rate for the child clock.
 
  If the default-rate generic property is not going to fly, it can be moved
  to TI only drivers also.
 
  -Tero
 
 
  ___
  linux-arm-kernel mailing list
  linux-arm-ker...@lists.infradead.org
  http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] clk: dt: add support for default rate/parent

2014-03-05 Thread Tero Kristo

Ping.

Mike, any feedback on this?

-Tero

On 02/13/2014 11:00 AM, Tero Kristo wrote:

Hi,

This set is a mix-match of new DT properties for generic and TI specific
clock drivers. Basically provided for commenting purposes. The patches
provide a way to configure clock parents / rates during boot through DT.

default-rate : sets rate of a clock during boot, supported for any DT
 clock type (through generic clock driver)
ti,default-parent : selects a default parent for a multiplexer clock,
  only supported for TI specific mux clock for now,
  as generic mux clock does not support DT clocks

Patch #4 provided as a reference how to move the default rates / parents
from kernel code to DT.

Default-rate logic in patch #2 looks somewhat complicated, as the clocks
need to be sorted based on their parents to avoid cases where a child clock
would set its rate first, just to be overridden by its parent changing
rate later and resulting in incorrect rate for the child clock.

If the default-rate generic property is not going to fly, it can be moved
to TI only drivers also.

-Tero


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html