Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1433574 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Guodong Xu <guodong.xu@linaro.org> - 2016-06-29 10:50 +0200
    Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Michael Turquette <mturquette@baylibre.com> - 2016-07-06 23:50 +0200
      Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Jorge Ramirez <jorge.ramirez-ortiz@linaro.org> - 2016-07-07 08:40 +0200
        Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Jorge Ramirez <jorge.ramirez-ortiz@linaro.org> - 2016-07-07 11:00 +0200
          Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Michael Turquette <mturquette@baylibre.com> - 2016-07-08 03:50 +0200
            Re: [PATCH v2 2/2] clk: hi6220: initialize UART1 clock to 150MHz Jorge Ramirez <jorge.ramirez-ortiz@linaro.org> - 2016-07-08 09:00 +0200

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

From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>

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 <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 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

[toc] | [next] | [standalone]

#1437957

Quoting Guodong Xu (2016-06-29 01:45:55)
> From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> 
> 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 <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> ---
>  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
> 

[toc] | [prev] | [next] | [standalone]

#1438196

On 07/06/2016 11:43 PM, Michael Turquette wrote:
> Quoting Guodong Xu (2016-06-29 01:45:55)
>> >From: Jorge Ramirez-Ortiz<jorge.ramirez-ortiz@linaro.org>
>> >
>> >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<jorge.ramirez-ortiz@linaro.org>
>> >Signed-off-by: Guodong Xu<guodong.xu@linaro.org>
>> >---
>> >  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).

[toc] | [prev] | [next] | [standalone]

#1438410

On 07/07/2016 08:31 AM, Jorge Ramirez wrote:
> On 07/06/2016 11:43 PM, Michael Turquette wrote:
>> Quoting Guodong Xu (2016-06-29 01:45:55)
>>> >From: Jorge Ramirez-Ortiz<jorge.ramirez-ortiz@linaro.org>
>>> >
>>> >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<jorge.ramirez-ortiz@linaro.org>
>>> >Signed-off-by: Guodong Xu<guodong.xu@linaro.org>
>>> >---
>>> >  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).
>
>
I have v3 ready (changes done in amba-pl011.c and devicetree/bindings)
please let me know if I should send those instead.

[toc] | [prev] | [next] | [standalone]

#1439058

Quoting Jorge Ramirez (2016-07-07 01:55:05)
> On 07/07/2016 08:31 AM, Jorge Ramirez wrote:
> > On 07/06/2016 11:43 PM, Michael Turquette wrote:
> >> Quoting Guodong Xu (2016-06-29 01:45:55)
> >>> >From: Jorge Ramirez-Ortiz<jorge.ramirez-ortiz@linaro.org>
> >>> >
> >>> >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<jorge.ramirez-ortiz@linaro.org>
> >>> >Signed-off-by: Guodong Xu<guodong.xu@linaro.org>
> >>> >---
> >>> >  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).
> >
> >
> 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

> 
> 
> 

[toc] | [prev] | [next] | [standalone]

#1439143

On 07/08/2016 03:48 AM, Michael Turquette wrote:
> Quoting Jorge Ramirez (2016-07-07 01:55:05)
>> On 07/07/2016 08:31 AM, Jorge Ramirez wrote:
>>> On 07/06/2016 11:43 PM, Michael Turquette wrote:
>>>> Quoting Guodong Xu (2016-06-29 01:45:55)
>>>>>> From: Jorge Ramirez-Ortiz<jorge.ramirez-ortiz@linaro.org>
>>>>>>
>>>>>> 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<jorge.ramirez-ortiz@linaro.org>
>>>>>> Signed-off-by: Guodong Xu<guodong.xu@linaro.org>
>>>>>> ---
>>>>>>   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).
>>>
>>>
>> 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web