[PATCH v2 1/2] clk: hi6220: Change syspll and media_syspll clk to 1.19GHz

Subsystems: common clk framework, the rest

STALE3712d

8 messages, 3 authors, 2016-07-08 · open the first message on its own page

[PATCH v2 1/2] clk: hi6220: Change syspll and media_syspll clk to 1.19GHz

From: Guodong Xu <hidden>
Date: 2016-06-29 08:46:12

From: Xinliang Liu <xinliang.liu@linaro.org>

In the bootloader of HiKey/96boards, syspll and media_syspll clk
was initialized to 1.19GHz. So, here changes it in kernel accordingly.

1.19GHz was chosen over 1.2GHz because at 1.19GHz we get more precise
HDMI pixel clock (1.19G/16 = 74.4MHz) for 1280x720p at 60Hz HDMI
(74.25MHz required by standards). Closer pixel clock means better
compatibility to HDMI monitors.

Signed-off-by: Guodong Xu <redacted>
Signed-off-by: Xinliang Liu <xinliang.liu@linaro.org>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index f02cb41..a36ffcb 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -34,8 +34,8 @@ static struct hisi_fixed_rate_clock hi6220_fixed_rate_clks[] __initdata = {
 	{ HI6220_PLL_BBP,	"bbppll0",	NULL, 0, 245760000, },
 	{ HI6220_PLL_GPU,	"gpupll",	NULL, 0, 1000000000,},
 	{ HI6220_PLL1_DDR,	"ddrpll1",	NULL, 0, 1066000000,},
-	{ HI6220_PLL_SYS,	"syspll",	NULL, 0, 1200000000,},
-	{ HI6220_PLL_SYS_MEDIA,	"media_syspll",	NULL, 0, 1200000000,},
+	{ HI6220_PLL_SYS,	"syspll",	NULL, 0, 1190400000,},
+	{ HI6220_PLL_SYS_MEDIA,	"media_syspll",	NULL, 0, 1190400000,},
 	{ HI6220_DDR_SRC,	"ddr_sel_src",  NULL, 0, 1200000000,},
 	{ HI6220_PLL_MEDIA,	"media_pll",    NULL, 0, 1440000000,},
 	{ HI6220_PLL_DDR,	"ddrpll0",      NULL, 0, 1600000000,},

-- 
1.9.1

[PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Guodong Xu <hidden>
Date: 2016-06-29 08:46:19

From: Jorge Ramirez-Ortiz <redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1, which
is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz <redacted>
Signed-off-by: Guodong Xu <redacted>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index a36ffcb..631c56f 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
 
 #include <linux/kernel.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/io.h>
 #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct device_node *np)
 
 	hi6220_clk_register_divider(hi6220_div_clks_sys,
 			ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+	if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC], 150000000))
+		pr_err("failed to set uart1 clock rate\n");
 }
 CLK_OF_DECLARE(hi6220_clk_sys, "hisilicon,hi6220-sysctrl", hi6220_clk_sys_init);

-- 
1.9.1

Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Michael Turquette <hidden>
Date: 2016-07-06 21:43:19

Quoting Guodong Xu (2016-06-29 01:45:55)

quoted hunk

From: Jorge Ramirez-Ortiz <redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1, which
is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz <redacted>
Signed-off-by: Guodong Xu <redacted>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index a36ffcb..631c56f 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
 
 #include <linux/kernel.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/io.h>
 #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct device_node *np)
 
        hi6220_clk_register_divider(hi6220_div_clks_sys,
                        ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+       if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC], 150000000))
+               pr_err("failed to set uart1 clock rate\n");

Why doesn't the UART driver call clk_get and then clk_set_rate on this
clock? Why do it in the clk provider driver?

Thanks,
Mike

 }
 CLK_OF_DECLARE(hi6220_clk_sys, "hisilicon,hi6220-sysctrl", hi6220_clk_sys_init);
 
-- 
1.9.1

Re: [PATCH v2 1/2] clk: hi6220: Change syspll and media_syspll clk to 1.19GHz

From: Michael Turquette <hidden>
Date: 2016-07-06 22:23:27

Quoting Guodong Xu (2016-06-29 01:45:54)

From: Xinliang Liu <xinliang.liu@linaro.org>

In the bootloader of HiKey/96boards, syspll and media_syspll clk
was initialized to 1.19GHz. So, here changes it in kernel accordingly.

1.19GHz was chosen over 1.2GHz because at 1.19GHz we get more precise
HDMI pixel clock (1.19G/16 = 74.4MHz) for 1280x720p at 60Hz HDMI
(74.25MHz required by standards). Closer pixel clock means better
compatibility to HDMI monitors.

Signed-off-by: Guodong Xu <redacted>
Signed-off-by: Xinliang Liu <xinliang.liu@linaro.org>

Applied.

Regards,
Mike

quoted hunk

---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index f02cb41..a36ffcb 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -34,8 +34,8 @@ static struct hisi_fixed_rate_clock hi6220_fixed_rate_clks[] __initdata = {
        { HI6220_PLL_BBP,       "bbppll0",      NULL, 0, 245760000, },
        { HI6220_PLL_GPU,       "gpupll",       NULL, 0, 1000000000,},
        { HI6220_PLL1_DDR,      "ddrpll1",      NULL, 0, 1066000000,},
-       { HI6220_PLL_SYS,       "syspll",       NULL, 0, 1200000000,},
-       { HI6220_PLL_SYS_MEDIA, "media_syspll", NULL, 0, 1200000000,},
+       { HI6220_PLL_SYS,       "syspll",       NULL, 0, 1190400000,},
+       { HI6220_PLL_SYS_MEDIA, "media_syspll", NULL, 0, 1190400000,},
        { HI6220_DDR_SRC,       "ddr_sel_src",  NULL, 0, 1200000000,},
        { HI6220_PLL_MEDIA,     "media_pll",    NULL, 0, 1440000000,},
        { HI6220_PLL_DDR,       "ddrpll0",      NULL, 0, 1600000000,},

-- 
1.9.1

Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Jorge Ramirez <hidden>
Date: 2016-07-07 06:31:27

On 07/06/2016 11:43 PM, Michael Turquette wrote:

Quoting Guodong Xu (2016-06-29 01:45:55)

quoted

From: Jorge Ramirez-Ortiz<redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1, which
is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz<redacted>
Signed-off-by: Guodong Xu<redacted>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index a36ffcb..631c56f 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
 
 #include <linux/kernel.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/io.h>
 #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct device_node *np)
 
        hi6220_clk_register_divider(hi6220_div_clks_sys,
                        ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+       if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC], 150000000))
+               pr_err("failed to set uart1 clock rate\n");

Why doesn't the UART driver call clk_get and then clk_set_rate on this
clock? Why do it in the clk provider driver?

yes that was my initial choice as well; in the end I opted to do it in 
the clock driver because of it being a value that will not have to ever 
change for the SoC and - maybe more importantly- because of not having a 
DT property available for the primecell pl011 uart where to  specify the 
value (so I thought this was a less intrusive implementation).

Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Jorge Ramirez <hidden>
Date: 2016-07-07 08:55:12

On 07/07/2016 08:31 AM, Jorge Ramirez wrote:

On 07/06/2016 11:43 PM, Michael Turquette wrote:

quoted

Quoting Guodong Xu (2016-06-29 01:45:55)

quoted

From: Jorge Ramirez-Ortiz<redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1,

which

quoted

is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz<redacted>
Signed-off-by: Guodong Xu<redacted>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c

b/drivers/clk/hisilicon/clk-hi6220.c

quoted

index a36ffcb..631c56f 100644

--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
 >  #include <linux/kernel.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/io.h>
 #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct

device_node *np)

quoted

 > hi6220_clk_register_divider(hi6220_div_clks_sys,
                        ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+       if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC],

150000000))

quoted

+               pr_err("failed to set uart1 clock rate\n");

Why doesn't the UART driver call clk_get and then clk_set_rate on this
clock? Why do it in the clk provider driver?

yes that was my initial choice as well; in the end I opted to do it in 
the clock driver because of it being a value that will not have to 
ever change for the SoC and - maybe more importantly- because of not 
having a DT property available for the primecell pl011 uart where to  
specify the value (so I thought this was a less intrusive 
implementation).

I have v3 ready (changes done in amba-pl011.c and devicetree/bindings)
please let me know if I should send those instead.

Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Michael Turquette <hidden>
Date: 2016-07-08 01:48:34

Quoting Jorge Ramirez (2016-07-07 01:55:05)

On 07/07/2016 08:31 AM, Jorge Ramirez wrote:

quoted

On 07/06/2016 11:43 PM, Michael Turquette wrote:

quoted

Quoting Guodong Xu (2016-06-29 01:45:55)

quoted

From: Jorge Ramirez-Ortiz<redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1,

which

quoted

is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz<redacted>
Signed-off-by: Guodong Xu<redacted>
---
 drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c

b/drivers/clk/hisilicon/clk-hi6220.c

quoted

index a36ffcb..631c56f 100644

--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
 >  #include <linux/kernel.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/io.h>
 #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct

device_node *np)

quoted

 > hi6220_clk_register_divider(hi6220_div_clks_sys,
                        ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+       if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC],

150000000))

quoted

+               pr_err("failed to set uart1 clock rate\n");

Why doesn't the UART driver call clk_get and then clk_set_rate on this
clock? Why do it in the clk provider driver?

yes that was my initial choice as well; in the end I opted to do it in 
the clock driver because of it being a value that will not have to 
ever change for the SoC and - maybe more importantly- because of not 
having a DT property available for the primecell pl011 uart where to  
specify the value (so I thought this was a less intrusive 
implementation).

I have v3 ready (changes done in amba-pl011.c and devicetree/bindings)
please let me know if I should send those instead.

Yes, please do. Are you using the clock-assigned-rates property?

Regards,
Mike

Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz

From: Jorge Ramirez <hidden>
Date: 2016-07-08 06:58:41

On 07/08/2016 03:48 AM, Michael Turquette wrote:

Quoting Jorge Ramirez (2016-07-07 01:55:05)

quoted

On 07/07/2016 08:31 AM, Jorge Ramirez wrote:

quoted

On 07/06/2016 11:43 PM, Michael Turquette wrote:

quoted

Quoting Guodong Xu (2016-06-29 01:45:55)

quoted

From: Jorge Ramirez-Ortiz<redacted>

Early at boot, during the sys_clk initialization, make sure UART1 uses
the higher frequency clock, 150MHz.

This enables support for higher baud rates (up to 3Mbps) in UART1,

which

quoted

is required by faster bluetooth transfers.

v2: use clk_set_rate() to propergate clock settings.

Signed-off-by: Jorge Ramirez-Ortiz<redacted>
Signed-off-by: Guodong Xu<redacted>
---
  drivers/clk/hisilicon/clk-hi6220.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c

b/drivers/clk/hisilicon/clk-hi6220.c

quoted

index a36ffcb..631c56f 100644

--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c

@@ -12,6 +12,7 @@
  >  #include <linux/kernel.h>
  #include <linux/clk-provider.h>
+#include <linux/clk.h>
  #include <linux/clkdev.h>
  #include <linux/io.h>
  #include <linux/of.h>

@@ -192,6 +193,9 @@ static void __init hi6220_clk_sys_init(struct

device_node *np)

quoted

  > hi6220_clk_register_divider(hi6220_div_clks_sys,
                         ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
+
+       if (clk_set_rate(clk_data->clk_data.clks[HI6220_UART1_SRC],

150000000))

quoted

+               pr_err("failed to set uart1 clock rate\n");

Why doesn't the UART driver call clk_get and then clk_set_rate on this
clock? Why do it in the clk provider driver?

yes that was my initial choice as well; in the end I opted to do it in
the clock driver because of it being a value that will not have to
ever change for the SoC and - maybe more importantly- because of not
having a DT property available for the primecell pl011 uart where to
specify the value (so I thought this was a less intrusive
implementation).

I have v3 ready (changes done in amba-pl011.c and devicetree/bindings)
please let me know if I should send those instead.

Yes, please do. Are you using the clock-assigned-rates property?

oops (was using clock-frequency), yes it is now. thanks will send it 
shortly.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help