diff mbox series

clk: fractional-divider: check parent rate only for general approximation

Message ID 20190107132124.27788-1-katsuhiro@katsuster.net (mailing list archive)
State New, archived
Headers show
Series clk: fractional-divider: check parent rate only for general approximation | expand

Commit Message

Katsuhiro Suzuki Jan. 7, 2019, 1:21 p.m. UTC
Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate evne if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
---
 drivers/clk/clk-fractional-divider.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Katsuhiro Suzuki Jan. 26, 2019, 11:13 a.m. UTC | #1
Ping...

On 2019/01/07 22:21, Katsuhiro Suzuki wrote:
> Custom approximation of fractional-divider may not need parent clock
> rate checking. For example Rockchip SoCs work fine using grand parent
> clock rate evne if target rate is greater than parent.
> 
> This patch removes parent clock rate check from custom approximation.
> 
> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> ---
>   drivers/clk/clk-fractional-divider.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
> index 545dceec0bbf..b0fc5509e0ff 100644
> --- a/drivers/clk/clk-fractional-divider.c
> +++ b/drivers/clk/clk-fractional-divider.c
> @@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
>   	unsigned long m, n;
>   	u64 ret;
>   
> -	if (!rate || rate >= *parent_rate)
> +	if (!rate)
>   		return *parent_rate;
>   
> -	if (fd->approximation)
> +	if (fd->approximation) {
>   		fd->approximation(hw, rate, parent_rate, &m, &n);
> -	else
> +	} else {
> +		if (rate >= *parent_rate)
> +			return *parent_rate;
> +
>   		clk_fd_general_approximation(hw, rate, parent_rate, &m, &n);
> +	}
>   
>   	ret = (u64)*parent_rate * m;
>   	do_div(ret, n);
>
Stephen Boyd Jan. 29, 2019, 9:55 p.m. UTC | #2
Quoting Katsuhiro Suzuki (2019-01-07 05:21:24)
> Custom approximation of fractional-divider may not need parent clock
> rate checking. For example Rockchip SoCs work fine using grand parent
> clock rate evne if target rate is greater than parent.
> 
> This patch removes parent clock rate check from custom approximation.
> 
> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> ---
>  drivers/clk/clk-fractional-divider.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
> index 545dceec0bbf..b0fc5509e0ff 100644
> --- a/drivers/clk/clk-fractional-divider.c
> +++ b/drivers/clk/clk-fractional-divider.c
> @@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
>         unsigned long m, n;
>         u64 ret;
>  
> -       if (!rate || rate >= *parent_rate)
> +       if (!rate)

Is your fraction multiplying the rate up? If it's a divider it shouldn't
be increasing the rate more than whatever the parent is supplying so
this looks odd. How does the grandparent matter here? Maybe you can show
the example in the commit text so we can all understand what's going on
here.

>                 return *parent_rate;
>
Katsuhiro Suzuki Jan. 30, 2019, 11:44 p.m. UTC | #3
Hello Stephen,

Thank you for your comment.
I'll add detail description and send it.

Best Regards,
Katsuhiro Suzuki

On 2019/01/30 6:55, Stephen Boyd wrote:
> Quoting Katsuhiro Suzuki (2019-01-07 05:21:24)
>> Custom approximation of fractional-divider may not need parent clock
>> rate checking. For example Rockchip SoCs work fine using grand parent
>> clock rate evne if target rate is greater than parent.
>>
>> This patch removes parent clock rate check from custom approximation.
>>
>> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
>> ---
>>   drivers/clk/clk-fractional-divider.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
>> index 545dceec0bbf..b0fc5509e0ff 100644
>> --- a/drivers/clk/clk-fractional-divider.c
>> +++ b/drivers/clk/clk-fractional-divider.c
>> @@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
>>          unsigned long m, n;
>>          u64 ret;
>>   
>> -       if (!rate || rate >= *parent_rate)
>> +       if (!rate)
> 
> Is your fraction multiplying the rate up? If it's a divider it shouldn't
> be increasing the rate more than whatever the parent is supplying so
> this looks odd. How does the grandparent matter here? Maybe you can show
> the example in the commit text so we can all understand what's going on
> here.
> 
>>                  return *parent_rate;
>>   
> 
>
diff mbox series

Patch

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@  static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long m, n;
 	u64 ret;
 
-	if (!rate || rate >= *parent_rate)
+	if (!rate)
 		return *parent_rate;
 
-	if (fd->approximation)
+	if (fd->approximation) {
 		fd->approximation(hw, rate, parent_rate, &m, &n);
-	else
+	} else {
+		if (rate >= *parent_rate)
+			return *parent_rate;
+
 		clk_fd_general_approximation(hw, rate, parent_rate, &m, &n);
+	}
 
 	ret = (u64)*parent_rate * m;
 	do_div(ret, n);