All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if RDIV_FORCE2 is set
@ 2021-08-19 23:04 Pat Carr
  2021-08-29  4:35 ` Stephen Boyd
       [not found] ` <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.1a5597f4-c6ce-44a5-85ce-703181c3f3e1@emailsignatures365.codetwo.com>
  0 siblings, 2 replies; 3+ messages in thread
From: Pat Carr @ 2021-08-19 23:04 UTC (permalink / raw)
  To: linux-clk; +Cc: Mike Looijmans, Michael Turquette

Hello Mike Looijmans et al,

  The attached patch fixes the output clocks in the case when R divider
is zero, but the flag SI5341_OUT_CFG_RDIV_FORCE2 is set. The flag in
this case should take precedence and return parent_rate / 2. Otherwise,
the return value would incorrectly be zero affecting the downstream
output clock rates. This table shows this failing case before the patch:

                         enable  prepare protect                    ...
  clock                   count    count   count   rate        accuracy
-----------------------------------------------------------------------
vcxo_clk                      0        0       0   124999999          0
ref48M                        1        1       0    48000000          0
   clock-generator            0        0       0    13200000          0
      clock-generator.N3      0        0       0           0          0
      clock-generator.N2      0        0       0           0          0
      clock-generator.N1      0        0       0           0          0
      clock-generator.N0      0        0       0   200000000          0
         clock-generator.N3   0        0       0           0          0
         clock-generator.N2   0        0       0           0 ?        0
         clock-generator.N1   0        0       0           0 ?        0
         clock-generator.N0   0        0       0           0 ?        0

  The '?' marks show the zeroed out output clock rates in case the
output divider should've been set to divide-by-2. After the patch, the
output clock rates are shown below:
<...>
         clock-generator.N3   0        0       0           0          0
         clock-generator.N2   0        0       0   100000000          0
         clock-generator.N1   0        0       0   100000000          0
         clock-generator.N0   0        0       0   100000000          0

Regards,
-Pat Carr




From f60f24c9ee88442b888723bae2584863c90dc5cf Mon Sep 17 00:00:00 2001
From: Pat Carr <pat@tuxengineering.com>
Date: Thu, 19 Aug 2021 14:30:21 -0700
Subject: [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if
RDIV_FORCE2 is
 set

The si5341_output_clk_recalc_rate function returns 0 too early based solely
on the value of r_divider, without checking if SI5341_OUT_CFG_RDIV_FORCE2
is set. If set, the returned value is simply the parent rate divided by 2,
regardless of the value in SI5341_OUT_R_REG.

Signed-off-by: Pat Carr <pat@tuxengineering.com>
---
 drivers/clk/clk-si5341.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index 57ae183982d8..bf29a2c9bf87 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -798,6 +798,14 @@ static unsigned long
si5341_output_clk_recalc_rate(struct clk_hw *hw,
 	u32 r_divider;
 	u8 r[3];

+	err = regmap_read(output->data->regmap,
+			SI5341_OUT_CONFIG(output), &val);
+	if (err < 0)
+		return err;
+
+	if (val & SI5341_OUT_CFG_RDIV_FORCE2)
+		return parent_rate / 2;
+
 	err = regmap_bulk_read(output->data->regmap,
 			SI5341_OUT_R_REG(output), r, 3);
 	if (err < 0)
@@ -814,14 +822,6 @@ static unsigned long
si5341_output_clk_recalc_rate(struct clk_hw *hw,
 	r_divider += 1;
 	r_divider <<= 1;

-	err = regmap_read(output->data->regmap,
-			SI5341_OUT_CONFIG(output), &val);
-	if (err < 0)
-		return err;
-
-	if (val & SI5341_OUT_CFG_RDIV_FORCE2)
-		r_divider = 2;
-
 	return parent_rate / r_divider;
 }

-- 
2.25.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if RDIV_FORCE2 is set
  2021-08-19 23:04 [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if RDIV_FORCE2 is set Pat Carr
@ 2021-08-29  4:35 ` Stephen Boyd
       [not found] ` <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.1a5597f4-c6ce-44a5-85ce-703181c3f3e1@emailsignatures365.codetwo.com>
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:35 UTC (permalink / raw)
  To: Pat Carr, linux-clk; +Cc: Mike Looijmans, Michael Turquette

Quoting Pat Carr (2021-08-19 16:04:08)
> Hello Mike Looijmans et al,
> 
>   The attached patch fixes the output clocks in the case when R divider
> is zero, but the flag SI5341_OUT_CFG_RDIV_FORCE2 is set. The flag in
> this case should take precedence and return parent_rate / 2. Otherwise,
> the return value would incorrectly be zero affecting the downstream
> output clock rates. This table shows this failing case before the patch:
> 
>                          enable  prepare protect                    ...
>   clock                   count    count   count   rate        accuracy
> -----------------------------------------------------------------------
> vcxo_clk                      0        0       0   124999999          0
> ref48M                        1        1       0    48000000          0
>    clock-generator            0        0       0    13200000          0
>       clock-generator.N3      0        0       0           0          0
>       clock-generator.N2      0        0       0           0          0
>       clock-generator.N1      0        0       0           0          0
>       clock-generator.N0      0        0       0   200000000          0
>          clock-generator.N3   0        0       0           0          0
>          clock-generator.N2   0        0       0           0 ?        0
>          clock-generator.N1   0        0       0           0 ?        0
>          clock-generator.N0   0        0       0           0 ?        0
> 
>   The '?' marks show the zeroed out output clock rates in case the
> output divider should've been set to divide-by-2. After the patch, the
> output clock rates are shown below:
> <...>
>          clock-generator.N3   0        0       0           0          0
>          clock-generator.N2   0        0       0   100000000          0
>          clock-generator.N1   0        0       0   100000000          0
>          clock-generator.N0   0        0       0   100000000          0
> 
> 

Please read Documentation/process/submitting-patches.rst to understand
how to properly send patches for review. Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if RDIV_FORCE2 is set
       [not found]   ` <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.0d2bd5fa-15cc-4b27-b94e-83614f9e5b38.eef9dd52-c56e-4969-b691-ed7a6bb1aa17@emailsignatures365.codetwo.com>
@ 2021-08-30  5:55     ` Mike Looijmans
  0 siblings, 0 replies; 3+ messages in thread
From: Mike Looijmans @ 2021-08-30  5:55 UTC (permalink / raw)
  To: Pat Carr, linux-clk; +Cc: Michael Turquette

See below


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail


Read here our special anniversary magazine! <https://topic.nl/en/25th-anniversary-topic>
On 20-08-2021 01:04, Pat Carr wrote:
> Hello Mike Looijmans et al,
> 
>    The attached patch fixes the output clocks in the case when R divider
> is zero, but the flag SI5341_OUT_CFG_RDIV_FORCE2 is set. The flag in
> this case should take precedence and return parent_rate / 2. Otherwise,
> the return value would incorrectly be zero affecting the downstream
> output clock rates. This table shows this failing case before the patch:

Yeah, this was reported before on my todo list once but somehow escaped my 
attention.

It might be helpful to add that this only occurs with pre-programmed chips, 
the driver will never set the divider to 0 as I've observed that on some 
devices a value of "0" will disable the divider completely.


> 
>                           enable  prepare protect                    ...
>    clock                   count    count   count   rate        accuracy
> -----------------------------------------------------------------------
> vcxo_clk                      0        0       0   124999999          0
> ref48M                        1        1       0    48000000          0
>     clock-generator            0        0       0    13200000          0
>        clock-generator.N3      0        0       0           0          0
>        clock-generator.N2      0        0       0           0          0
>        clock-generator.N1      0        0       0           0          0
>        clock-generator.N0      0        0       0   200000000          0
>           clock-generator.N3   0        0       0           0          0
>           clock-generator.N2   0        0       0           0 ?        0
>           clock-generator.N1   0        0       0           0 ?        0
>           clock-generator.N0   0        0       0           0 ?        0
> 
>    The '?' marks show the zeroed out output clock rates in case the
> output divider should've been set to divide-by-2. After the patch, the
> output clock rates are shown below:
> <...>
>           clock-generator.N3   0        0       0           0          0
>           clock-generator.N2   0        0       0   100000000          0
>           clock-generator.N1   0        0       0   100000000          0
>           clock-generator.N0   0        0       0   100000000          0
> 
> Regards,
> -Pat Carr
> 
> 
> 
> 
>  From f60f24c9ee88442b888723bae2584863c90dc5cf Mon Sep 17 00:00:00 2001
> From: Pat Carr <pat@tuxengineering.com>
> Date: Thu, 19 Aug 2021 14:30:21 -0700
> Subject: [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if
> RDIV_FORCE2 is
>   set
> 
> The si5341_output_clk_recalc_rate function returns 0 too early based solely
> on the value of r_divider, without checking if SI5341_OUT_CFG_RDIV_FORCE2
> is set. If set, the returned value is simply the parent rate divided by 2,
> regardless of the value in SI5341_OUT_R_REG.
> 
> Signed-off-by: Pat Carr <pat@tuxengineering.com>
> ---
>   drivers/clk/clk-si5341.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
> index 57ae183982d8..bf29a2c9bf87 100644
> --- a/drivers/clk/clk-si5341.c
> +++ b/drivers/clk/clk-si5341.c
> @@ -798,6 +798,14 @@ static unsigned long
> si5341_output_clk_recalc_rate(struct clk_hw *hw,
>   	u32 r_divider;
>   	u8 r[3];
> 
> +	err = regmap_read(output->data->regmap,
> +			SI5341_OUT_CONFIG(output), &val);
> +	if (err < 0)
> +		return err;
> +
> +	if (val & SI5341_OUT_CFG_RDIV_FORCE2)
> +		return parent_rate / 2;
> +
>   	err = regmap_bulk_read(output->data->regmap,
>   			SI5341_OUT_R_REG(output), r, 3);
>   	if (err < 0)
> @@ -814,14 +822,6 @@ static unsigned long
> si5341_output_clk_recalc_rate(struct clk_hw *hw,
>   	r_divider += 1;
>   	r_divider <<= 1;
> 
> -	err = regmap_read(output->data->regmap,
> -			SI5341_OUT_CONFIG(output), &val);
> -	if (err < 0)
> -		return err;
> -
> -	if (val & SI5341_OUT_CFG_RDIV_FORCE2)
> -		r_divider = 2;
> -
>   	return parent_rate / r_divider;
>   }
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-30  5:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 23:04 [PATCH] clk: clk-si5341: Fix output_clk_recalc_rate if RDIV_FORCE2 is set Pat Carr
2021-08-29  4:35 ` Stephen Boyd
     [not found] ` <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.1a5597f4-c6ce-44a5-85ce-703181c3f3e1@emailsignatures365.codetwo.com>
     [not found]   ` <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.0d2bd5fa-15cc-4b27-b94e-83614f9e5b38.eef9dd52-c56e-4969-b691-ed7a6bb1aa17@emailsignatures365.codetwo.com>
2021-08-30  5:55     ` Mike Looijmans

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.