| Message ID | 1374132670-791-5-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Jul 18, 2013 at 03:31:10PM +0800, Shawn Guo wrote: > The CLKO is widely used by imx6q board designs to clock audio codec. > Since most codecs accept 24 MHz frequency, let's initially set up CLKO > with OSC24M (cko <-- cko2 <-- osc). Then those board specific CLKO > setup for audio codec can be removed. > > The board dts files also need an update on cko reference in codec node. > > + /* > + * Let's initially set up CLKO with OSC24M, since this configuration > + * is widely used by imx6q board designs to clock audio codec. > + */ > + ret = clk_set_parent(clk[cko2_sel], clk[osc]); > + if (!ret) > + ret = clk_set_parent(clk[cko], clk[cko2]); > + if (ret) > + pr_warn("failed to set up CLKO: %d\n", ret); I don't really like that the clock setup code fiddles with clocks it has no knowledge about just because it happens to be correct for several boards. I currently have no example where this bites me, so I think I can live with it for now. I just wonder what I should do when this clock is used for something vital to the system (clocking an FPGA for example?). Sascha
On Thu, Jul 18, 2013 at 10:36:03AM +0200, Sascha Hauer wrote: > On Thu, Jul 18, 2013 at 03:31:10PM +0800, Shawn Guo wrote: > > The CLKO is widely used by imx6q board designs to clock audio codec. > > Since most codecs accept 24 MHz frequency, let's initially set up CLKO > > with OSC24M (cko <-- cko2 <-- osc). Then those board specific CLKO > > setup for audio codec can be removed. > > > > The board dts files also need an update on cko reference in codec node. > > > > + /* > > + * Let's initially set up CLKO with OSC24M, since this configuration > > + * is widely used by imx6q board designs to clock audio codec. > > + */ > > + ret = clk_set_parent(clk[cko2_sel], clk[osc]); > > + if (!ret) > > + ret = clk_set_parent(clk[cko], clk[cko2]); > > + if (ret) > > + pr_warn("failed to set up CLKO: %d\n", ret); > > > I don't really like that the clock setup code fiddles with clocks it has > no knowledge about just because it happens to be correct for several > boards. I have to admit it's not the best way to handle it, and I can understand your concern. What I like about it is we can remove those board specific hooks from mach-imx6q.c at least for now. > > I currently have no example where this bites me, so I think I can live > with it for now. I just wonder what I should do when this clock is used > for something vital to the system (clocking an FPGA for example?). Go back to set up CLKO in mach-imx6q.c with board specific hook for the system? I think I chose to change the default CLKO setup for audio use case, as all the users we've seen so far is audio codec. I can still have this setup done in mach-imx6q.c with of_machine_is_compatible() check on all those board. But that also means we need to register lookups for those CLKO clocks and call clk_get_sys() to get them. Shawn
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0f4cd0..3530280 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -167,7 +167,7 @@ codec: sgtl5000@0a { compatible = "fsl,sgtl5000"; reg = <0x0a>; - clocks = <&clks 169>; + clocks = <&clks 201>; VDDA-supply = <®_2p5v>; VDDIO-supply = <®_3p3v>; }; diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi index 166b9ae..75b36b2 100644 --- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi +++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi @@ -109,7 +109,7 @@ codec: wm8962@1a { compatible = "wlf,wm8962"; reg = <0x1a>; - clocks = <&clks 169>; + clocks = <&clks 201>; DCVDD-supply = <®_audio>; DBVDD-supply = <®_audio>; AVDD-supply = <®_audio>; diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi index 7a46f64..77188cb 100644 --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi @@ -60,7 +60,7 @@ codec: sgtl5000@0a { compatible = "fsl,sgtl5000"; reg = <0x0a>; - clocks = <&clks 169>; + clocks = <&clks 201>; VDDA-supply = <®_2p5v>; VDDIO-supply = <®_3p3v>; }; diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index ed19b33..97f2702 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -288,6 +288,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) struct device_node *np; void __iomem *base; int i, irq; + int ret; clk[dummy] = imx_clk_fixed("dummy", 0); clk[ckil] = imx_obtain_fixed_clock("ckil", 0); @@ -591,6 +592,16 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk_prepare_enable(clk[usbphy2_gate]); } + /* + * Let's initially set up CLKO with OSC24M, since this configuration + * is widely used by imx6q board designs to clock audio codec. + */ + ret = clk_set_parent(clk[cko2_sel], clk[osc]); + if (!ret) + ret = clk_set_parent(clk[cko], clk[cko2]); + if (ret) + pr_warn("failed to set up CLKO: %d\n", ret); + /* Set initial power mode */ imx6q_set_lpm(WAIT_CLOCKED); diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index 9f06cc8..2460d49 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -159,30 +159,6 @@ static int ar8031_phy_fixup(struct phy_device *dev) return 0; } -static void __init imx6q_sabrelite_cko1_setup(void) -{ - struct clk *cko1_sel, *ahb, *cko1; - unsigned long rate; - - cko1_sel = clk_get_sys(NULL, "cko1_sel"); - ahb = clk_get_sys(NULL, "ahb"); - cko1 = clk_get_sys(NULL, "cko1"); - if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) { - pr_err("cko1 setup failed!\n"); - goto put_clk; - } - clk_set_parent(cko1_sel, ahb); - rate = clk_round_rate(cko1, 16000000); - clk_set_rate(cko1, rate); -put_clk: - if (!IS_ERR(cko1_sel)) - clk_put(cko1_sel); - if (!IS_ERR(ahb)) - clk_put(ahb); - if (!IS_ERR(cko1)) - clk_put(cko1); -} - #define PHY_ID_AR8031 0x004dd074 static void __init imx6q_enet_phy_init(void) @@ -197,45 +173,6 @@ static void __init imx6q_enet_phy_init(void) } } -static void __init imx6q_sabresd_cko1_setup(void) -{ - struct clk *cko1_sel, *pll4, *pll4_post, *cko1; - unsigned long rate; - - cko1_sel = clk_get_sys(NULL, "cko1_sel"); - pll4 = clk_get_sys(NULL, "pll4_audio"); - pll4_post = clk_get_sys(NULL, "pll4_post_div"); - cko1 = clk_get_sys(NULL, "cko1"); - if (IS_ERR(cko1_sel) || IS_ERR(pll4) - || IS_ERR(pll4_post) || IS_ERR(cko1)) { - pr_err("cko1 setup failed!\n"); - goto put_clk; - } - /* - * Setting pll4 at 768MHz (24MHz * 32) - * So its child clock can get 24MHz easily - */ - clk_set_rate(pll4, 768000000); - - clk_set_parent(cko1_sel, pll4_post); - rate = clk_round_rate(cko1, 24000000); - clk_set_rate(cko1, rate); -put_clk: - if (!IS_ERR(cko1_sel)) - clk_put(cko1_sel); - if (!IS_ERR(pll4_post)) - clk_put(pll4_post); - if (!IS_ERR(pll4)) - clk_put(pll4); - if (!IS_ERR(cko1)) - clk_put(cko1); -} - -static void __init imx6q_sabresd_init(void) -{ - imx6q_sabresd_cko1_setup(); -} - static void __init imx6q_1588_init(void) { struct regmap *gpr; @@ -256,12 +193,6 @@ static void __init imx6q_usb_init(void) static void __init imx6q_init_machine(void) { - if (of_machine_is_compatible("fsl,imx6q-sabrelite")) - imx6q_sabrelite_cko1_setup(); - else if (of_machine_is_compatible("fsl,imx6q-sabresd") || - of_machine_is_compatible("fsl,imx6dl-sabresd")) - imx6q_sabresd_init(); - imx6q_enet_phy_init(); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
The CLKO is widely used by imx6q board designs to clock audio codec. Since most codecs accept 24 MHz frequency, let's initially set up CLKO with OSC24M (cko <-- cko2 <-- osc). Then those board specific CLKO setup for audio codec can be removed. The board dts files also need an update on cko reference in codec node. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm/boot/dts/imx6q-sabrelite.dts | 2 +- arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 2 +- arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 2 +- arch/arm/mach-imx/clk-imx6q.c | 11 +++++ arch/arm/mach-imx/mach-imx6q.c | 69 ------------------------------ 5 files changed, 14 insertions(+), 72 deletions(-)