* [PATCH 1/3] clk: si5351: Apply PLL soft reset before enabling the outputs
2017-08-12 14:23 [PATCH 0/3] clk: si5351: PLL reset fixes Sergej Sawazki
@ 2017-08-12 14:23 ` Sergej Sawazki
2017-08-12 14:23 ` [PATCH 2/3] clk: si5351: Add DT property to enable PLL reset Sergej Sawazki
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sergej Sawazki @ 2017-08-12 14:23 UTC (permalink / raw)
To: sboyd, mturquette
Cc: sebastian.hesselbarth, rabeeh, linux, linux-clk, Sergej Sawazki
The "Si5351A/B/C Data Sheet" states to apply a PLL soft reset before
enabling the output clocks [1]. This is required to get a deterministic
phase relationship between the output clocks.
Without resetting the PLL, the phase relationship between the clocks is
unpredictable. Fix this by resetting the PLL in si5351_clkout_prepare().
It also fixes a regression introduced in commit 6dc669a22c77 ("clk: si5351:
Add PLL soft reset") that causes a disruption on platforms where clocks
derived from different PLLs do different things by resetting only the PLL
which the output clock is derived from.
References:
[1] https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf
Figure 12 ("I2C Programming Procedure")
Fixes: 6dc669a22c77 ("clk: si5351: Add PLL soft reset")
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Sergej Sawazki <sergej@taudac.com>
---
drivers/clk/clk-si5351.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index 2492442..46bbc95 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -898,6 +898,21 @@ static int _si5351_clkout_set_disable_state(
return 0;
}
+void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
+{
+ u8 val = si5351_reg_read(drvdata, SI5351_CLK0_CTRL + num);
+
+ switch (val & SI5351_CLK_INPUT_MASK) {
+ case SI5351_CLK_INPUT_XTAL:
+ case SI5351_CLK_INPUT_CLKIN:
+ return; /* PLL not used, no need to reset */
+ }
+
+ si5351_reg_write(drvdata, SI5351_PLL_RESET,
+ (val & SI5351_CLK_PLL_SELECT) ? SI5351_PLL_RESET_B :
+ SI5351_PLL_RESET_A);
+}
+
static int si5351_clkout_prepare(struct clk_hw *hw)
{
struct si5351_hw_data *hwdata =
@@ -905,6 +920,14 @@ static int si5351_clkout_prepare(struct clk_hw *hw)
si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
SI5351_CLK_POWERDOWN, 0);
+
+ /*
+ * Reset the PLLs before enabling the outputs to get a deterministic
+ * phase relationship between the output clocks. Otherwise, the phase
+ * offset beween the clocks is unpredictable.
+ */
+ _si5351_clkout_reset_pll(hwdata->drvdata, hwdata->num);
+
si5351_set_bits(hwdata->drvdata, SI5351_OUTPUT_ENABLE_CTRL,
(1 << hwdata->num), 0);
return 0;
@@ -1095,8 +1118,7 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
* Do a pll soft reset on both plls, needed in some cases to get
* all outputs running.
*/
- si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET,
- SI5351_PLL_RESET_A | SI5351_PLL_RESET_B);
+ _si5351_clkout_reset_pll(hwdata->drvdata, hwdata->num);
dev_dbg(&hwdata->drvdata->client->dev,
"%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n",
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] clk: si5351: Add DT property to enable PLL reset
2017-08-12 14:23 [PATCH 0/3] clk: si5351: PLL reset fixes Sergej Sawazki
2017-08-12 14:23 ` [PATCH 1/3] clk: si5351: Apply PLL soft reset before enabling the outputs Sergej Sawazki
@ 2017-08-12 14:23 ` Sergej Sawazki
2017-08-12 14:23 ` [PATCH 3/3] clk: si5351: Add a debug message for " Sergej Sawazki
2017-09-01 22:59 ` [PATCH 0/3] clk: si5351: PLL reset fixes Stephen Boyd
3 siblings, 0 replies; 6+ messages in thread
From: Sergej Sawazki @ 2017-08-12 14:23 UTC (permalink / raw)
To: sboyd, mturquette
Cc: sebastian.hesselbarth, rabeeh, linux, linux-clk, Sergej Sawazki
Add optional output clock DT property to enable PLL reset when a clock
output is enabled or the clock rate is changed.
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Signed-off-by: Sergej Sawazki <sergej@taudac.com>
---
Documentation/devicetree/bindings/clock/silabs,si5351.txt | 1 +
drivers/clk/clk-si5351.c | 11 ++++++++++-
include/linux/platform_data/si5351.h | 2 ++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
index 28b2830..157ee02 100644
--- a/Documentation/devicetree/bindings/clock/silabs,si5351.txt
+++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
@@ -45,6 +45,7 @@ Optional child node properties:
- silabs,multisynth-source: source pll A(0) or B(1) of corresponding multisynth
divider.
- silabs,pll-master: boolean, multisynth can change pll frequency.
+- silabs,pll-reset: boolean, clock output can reset its pll.
- silabs,disable-state : clock output disable state, shall be
0 = clock output is driven LOW when disabled
1 = clock output is driven HIGH when disabled
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index 46bbc95..aa8d8d9 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -900,7 +900,13 @@ static int _si5351_clkout_set_disable_state(
void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
{
- u8 val = si5351_reg_read(drvdata, SI5351_CLK0_CTRL + num);
+ struct si5351_platform_data *pdata = drvdata->client->dev.platform_data;
+ u8 val;
+
+ if (!pdata->clkout[num].pll_reset)
+ return; /* Resetting PLL disabled */
+
+ val = si5351_reg_read(drvdata, SI5351_CLK0_CTRL + num);
switch (val & SI5351_CLK_INPUT_MASK) {
case SI5351_CLK_INPUT_XTAL:
@@ -1321,6 +1327,9 @@ static int si5351_dt_parse(struct i2c_client *client,
pdata->clkout[num].pll_master =
of_property_read_bool(child, "silabs,pll-master");
+
+ pdata->clkout[num].pll_reset =
+ of_property_read_bool(child, "silabs,pll-reset");
}
client->dev.platform_data = pdata;
diff --git a/include/linux/platform_data/si5351.h b/include/linux/platform_data/si5351.h
index 533d980..da346f22 100644
--- a/include/linux/platform_data/si5351.h
+++ b/include/linux/platform_data/si5351.h
@@ -85,6 +85,7 @@ enum si5351_disable_state {
* @multisynth_src: multisynth source clock
* @clkout_src: clkout source clock
* @pll_master: if true, clkout can also change pll rate
+ * @pll_reset: if true, clkout can reset its pll
* @drive: output drive strength
* @rate: initial clkout rate, or default if 0
*/
@@ -94,6 +95,7 @@ struct si5351_clkout_config {
enum si5351_drive_strength drive;
enum si5351_disable_state disable_state;
bool pll_master;
+ bool pll_reset;
unsigned long rate;
};
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] clk: si5351: Add a debug message for PLL reset
2017-08-12 14:23 [PATCH 0/3] clk: si5351: PLL reset fixes Sergej Sawazki
2017-08-12 14:23 ` [PATCH 1/3] clk: si5351: Apply PLL soft reset before enabling the outputs Sergej Sawazki
2017-08-12 14:23 ` [PATCH 2/3] clk: si5351: Add DT property to enable PLL reset Sergej Sawazki
@ 2017-08-12 14:23 ` Sergej Sawazki
2017-09-01 22:59 ` [PATCH 0/3] clk: si5351: PLL reset fixes Stephen Boyd
3 siblings, 0 replies; 6+ messages in thread
From: Sergej Sawazki @ 2017-08-12 14:23 UTC (permalink / raw)
To: sboyd, mturquette
Cc: sebastian.hesselbarth, rabeeh, linux, linux-clk, Sergej Sawazki
Print a debug message when resetting a PLL. It is useful to figure out
what PLL is resetted and when.
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Signed-off-by: Sergej Sawazki <sergej@taudac.com>
---
drivers/clk/clk-si5351.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index aa8d8d9..7a122c7 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -917,6 +917,10 @@ void _si5351_clkout_reset_pll(struct si5351_driver_data *drvdata, int num)
si5351_reg_write(drvdata, SI5351_PLL_RESET,
(val & SI5351_CLK_PLL_SELECT) ? SI5351_PLL_RESET_B :
SI5351_PLL_RESET_A);
+
+ dev_dbg(&drvdata->client->dev, "%s - %s: pll = %d\n",
+ __func__, clk_hw_get_name(&drvdata->clkout[num].hw),
+ (val & SI5351_CLK_PLL_SELECT) ? 1 : 0);
}
static int si5351_clkout_prepare(struct clk_hw *hw)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] clk: si5351: PLL reset fixes
2017-08-12 14:23 [PATCH 0/3] clk: si5351: PLL reset fixes Sergej Sawazki
` (2 preceding siblings ...)
2017-08-12 14:23 ` [PATCH 3/3] clk: si5351: Add a debug message for " Sergej Sawazki
@ 2017-09-01 22:59 ` Stephen Boyd
2017-09-02 10:21 ` Sergej Sawazki
3 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2017-09-01 22:59 UTC (permalink / raw)
To: Sergej Sawazki
Cc: mturquette, sebastian.hesselbarth, rabeeh, linux, linux-clk
On 08/12, Sergej Sawazki wrote:
> The Si5351 clock generator has up to 8 output clocks and 2 PLLs. In order
> to get a deterministic phase offset relationship between the output clocks,
> it is necessary to reset the PLLs is certain scenarios.
>
> This patch-set:
> * fixes a regression and adds resetting the PLL before enabling the outputs
> * adds a dt-property for enabling/disabling the PLL reset
> * adds a debug message for PLL reset (it is helpful during debugging,
> probably no longer required?)
>
> Based on clk-next.
>
>
Please include Russell King on the patches. I'd like Sebastian or
Russell to review these before merging. For now, I'm going to
apply the other change from Russell.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] clk: si5351: PLL reset fixes
2017-09-01 22:59 ` [PATCH 0/3] clk: si5351: PLL reset fixes Stephen Boyd
@ 2017-09-02 10:21 ` Sergej Sawazki
0 siblings, 0 replies; 6+ messages in thread
From: Sergej Sawazki @ 2017-09-02 10:21 UTC (permalink / raw)
To: Stephen Boyd, sebastian.hesselbarth, linux, jacob
Cc: mturquette, rabeeh, linux-clk
Sebastian, Russell,
I have resent the patches. Could you please share your opinion on these?
Commit 6dc669a22c77ad "clk: si5351: Add PLL soft reset" fixed the phase
offset in case the clock rate is changed. But, if the clocks are
disabled and re-enabled again, the phase offset is still unpredictable.
These patches fix this issue.
Many thanks for you time and effort.
Best regards
Sergej
Am 02.09.2017 um 00:59 schrieb Stephen Boyd:
> On 08/12, Sergej Sawazki wrote:
>> The Si5351 clock generator has up to 8 output clocks and 2 PLLs. In order
>> to get a deterministic phase offset relationship between the output clocks,
>> it is necessary to reset the PLLs is certain scenarios.
>>
>> This patch-set:
>> * fixes a regression and adds resetting the PLL before enabling the outputs
>> * adds a dt-property for enabling/disabling the PLL reset
>> * adds a debug message for PLL reset (it is helpful during debugging,
>> probably no longer required?)
>>
>> Based on clk-next.
>>
>>
>
> Please include Russell King on the patches. I'd like Sebastian or
> Russell to review these before merging. For now, I'm going to
> apply the other change from Russell.
>
^ permalink raw reply [flat|nested] 6+ messages in thread