* [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0.
@ 2013-05-07 8:14 Marek Belisko
2013-05-07 8:14 ` [PATCH 2/2] clk: si5351: Powerdown output when clock rate " Marek Belisko
2013-05-07 10:03 ` [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT " Sebastian Hesselbarth
0 siblings, 2 replies; 6+ messages in thread
From: Marek Belisko @ 2013-05-07 8:14 UTC (permalink / raw)
To: linux-arm-kernel
When p3 is eual to 0 then result for fOUT is 0. In that case we should
return 0 not parent_rate;
This issue was causing deadlock in si5351 chip when user set_rate for
ms0->clk0 and then set_rate for ms1->clk1 (both ms sourced from plla).
After that clk1 was 0 and it wasn't possible to enable it again (try also
manually with i2cset commands). Only power cycle helps.
Signed-off-by: Marek Belisko <marek.belisko@streamunlimited.com>
---
drivers/clk/clk-si5351.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index fc215ce..a8fc0f4 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -607,7 +607,7 @@ static unsigned long si5351_msynth_recalc_rate(struct clk_hw *hw,
si5351_read_parameters(hwdata->drvdata, reg, &hwdata->params);
if (hwdata->params.p3 == 0)
- return parent_rate;
+ return 0;
/*
* multisync0-5: fOUT = (128 * P3 * fIN) / (P1*P3 + P2 + 512*P3)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] clk: si5351: Powerdown output when clock rate is 0.
2013-05-07 8:14 [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0 Marek Belisko
@ 2013-05-07 8:14 ` Marek Belisko
2013-05-07 10:07 ` Sebastian Hesselbarth
2013-05-07 10:03 ` [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT " Sebastian Hesselbarth
1 sibling, 1 reply; 6+ messages in thread
From: Marek Belisko @ 2013-05-07 8:14 UTC (permalink / raw)
To: linux-arm-kernel
When rate is 0 powerdown clock output.
Signed-off-by: Marek Belisko <marek.belisko@streamunlimited.com>
---
drivers/clk/clk-si5351.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index a8fc0f4..e254e76 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -1042,20 +1042,22 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
struct si5351_hw_data *hwdata =
container_of(hw, struct si5351_hw_data, hw);
unsigned long new_rate, new_err, err;
- unsigned char rdiv;
+ unsigned char rdiv, pdn = 0;
/* round to closed rdiv */
rdiv = SI5351_OUTPUT_CLK_DIV_1;
- new_rate = parent_rate;
- err = abs(new_rate - rate);
- do {
- new_rate >>= 1;
- new_err = abs(new_rate - rate);
- if (new_err > err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
- break;
- rdiv++;
- err = new_err;
- } while (1);
+ if (rate) {
+ new_rate = parent_rate;
+ err = abs(new_rate - rate);
+ do {
+ new_rate >>= 1;
+ new_err = abs(new_rate - rate);
+ if (new_err > err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
+ break;
+ rdiv++;
+ err = new_err;
+ } while (1);
+ }
/* write output divider */
switch (hwdata->num) {
@@ -1075,9 +1077,13 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
rdiv << SI5351_OUTPUT_CLK_DIV_SHIFT);
}
- /* powerup clkout */
+ /* when rate is 0 powerdown output */
+ if (rate == 0)
+ pdn = SI5351_CLK_POWERDOWN;
+
+ /* powerup/down clkout */
si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
- SI5351_CLK_POWERDOWN, 0);
+ SI5351_CLK_POWERDOWN, pdn);
dev_dbg(&hwdata->drvdata->client->dev,
"%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0.
2013-05-07 8:14 [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0 Marek Belisko
2013-05-07 8:14 ` [PATCH 2/2] clk: si5351: Powerdown output when clock rate " Marek Belisko
@ 2013-05-07 10:03 ` Sebastian Hesselbarth
2013-05-07 10:17 ` Marek Belisko
1 sibling, 1 reply; 6+ messages in thread
From: Sebastian Hesselbarth @ 2013-05-07 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On 05/07/2013 10:14 AM, Marek Belisko wrote:
> When p3 is eual to 0 then result for fOUT is 0. In that case we should
> return 0 not parent_rate;
>
> This issue was causing deadlock in si5351 chip when user set_rate for
> ms0->clk0 and then set_rate for ms1->clk1 (both ms sourced from plla).
> After that clk1 was 0 and it wasn't possible to enable it again (try also
> manually with i2cset commands). Only power cycle helps.
Marek,
does the fix below also affect the behavior above, i.e. not causing
si5351 to hang?
Anyway, your fix looks very sane to me and you get my
Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Marek Belisko<marek.belisko@streamunlimited.com>
> ---
> drivers/clk/clk-si5351.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
> index fc215ce..a8fc0f4 100644
> --- a/drivers/clk/clk-si5351.c
> +++ b/drivers/clk/clk-si5351.c
> @@ -607,7 +607,7 @@ static unsigned long si5351_msynth_recalc_rate(struct clk_hw *hw,
> si5351_read_parameters(hwdata->drvdata, reg,&hwdata->params);
>
> if (hwdata->params.p3 == 0)
> - return parent_rate;
> + return 0;
>
> /*
> * multisync0-5: fOUT = (128 * P3 * fIN) / (P1*P3 + P2 + 512*P3)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] clk: si5351: Powerdown output when clock rate is 0.
2013-05-07 8:14 ` [PATCH 2/2] clk: si5351: Powerdown output when clock rate " Marek Belisko
@ 2013-05-07 10:07 ` Sebastian Hesselbarth
2013-05-07 10:40 ` Marek Belisko
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Hesselbarth @ 2013-05-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
On 05/07/2013 10:14 AM, Marek Belisko wrote:
> When rate is 0 powerdown clock output.
>
> Signed-off-by: Marek Belisko<marek.belisko@streamunlimited.com>
> ---
> drivers/clk/clk-si5351.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
> index a8fc0f4..e254e76 100644
> --- a/drivers/clk/clk-si5351.c
> +++ b/drivers/clk/clk-si5351.c
> @@ -1042,20 +1042,22 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> struct si5351_hw_data *hwdata =
> container_of(hw, struct si5351_hw_data, hw);
> unsigned long new_rate, new_err, err;
> - unsigned char rdiv;
> + unsigned char rdiv, pdn = 0;
>
> /* round to closed rdiv */
> rdiv = SI5351_OUTPUT_CLK_DIV_1;
> - new_rate = parent_rate;
> - err = abs(new_rate - rate);
> - do {
> - new_rate>>= 1;
> - new_err = abs(new_rate - rate);
> - if (new_err> err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
> - break;
> - rdiv++;
> - err = new_err;
> - } while (1);
> + if (rate) {
> + new_rate = parent_rate;
Marek,
initalizing pdn to SI5351_CLK_POWERDOWN and skip (with goto) to
powerup/down call on zero rate will save us a level of indention here.
> + err = abs(new_rate - rate);
> + do {
> + new_rate>>= 1;
> + new_err = abs(new_rate - rate);
> + if (new_err> err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
> + break;
> + rdiv++;
> + err = new_err;
> + } while (1);
of course, pdn should be set to 0 here.
> + }
>
> /* write output divider */
> switch (hwdata->num) {
> @@ -1075,9 +1077,13 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> rdiv<< SI5351_OUTPUT_CLK_DIV_SHIFT);
> }
>
> - /* powerup clkout */
> + /* when rate is 0 powerdown output */
> + if (rate == 0)
> + pdn = SI5351_CLK_POWERDOWN;
> +
> + /* powerup/down clkout */
> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
> - SI5351_CLK_POWERDOWN, 0);
> + SI5351_CLK_POWERDOWN, pdn);
>
> dev_dbg(&hwdata->drvdata->client->dev,
> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n",
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0.
2013-05-07 10:03 ` [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT " Sebastian Hesselbarth
@ 2013-05-07 10:17 ` Marek Belisko
0 siblings, 0 replies; 6+ messages in thread
From: Marek Belisko @ 2013-05-07 10:17 UTC (permalink / raw)
To: linux-arm-kernel
On 05/07/2013 12:03 PM, Sebastian Hesselbarth wrote:
> On 05/07/2013 10:14 AM, Marek Belisko wrote:
>> When p3 is eual to 0 then result for fOUT is 0. In that case we should
>> return 0 not parent_rate;
>>
>> This issue was causing deadlock in si5351 chip when user set_rate for
>> ms0->clk0 and then set_rate for ms1->clk1 (both ms sourced from plla).
>> After that clk1 was 0 and it wasn't possible to enable it again (try also
>> manually with i2cset commands). Only power cycle helps.
>
> Marek,
>
> does the fix below also affect the behavior above, i.e. not causing
> si5351 to hang?
Yes this fix problems with si5351 hang. Didn't investigate why but I
assume some wrong register access or something.
>
> Anyway, your fix looks very sane to me and you get my
>
> Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>
>> Signed-off-by: Marek Belisko<marek.belisko@streamunlimited.com>
>> ---
>> drivers/clk/clk-si5351.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
>> index fc215ce..a8fc0f4 100644
>> --- a/drivers/clk/clk-si5351.c
>> +++ b/drivers/clk/clk-si5351.c
>> @@ -607,7 +607,7 @@ static unsigned long
>> si5351_msynth_recalc_rate(struct clk_hw *hw,
>> si5351_read_parameters(hwdata->drvdata, reg,&hwdata->params);
>>
>> if (hwdata->params.p3 == 0)
>> - return parent_rate;
>> + return 0;
>>
>> /*
>> * multisync0-5: fOUT = (128 * P3 * fIN) / (P1*P3 + P2 + 512*P3)
>
>
~marek
--
Marek Belisko
Software Developer
StreamUnlimited Engineering GmbH
Gutheil Schodergasse 8-12
A-1100 Vienna, Austria
Office: +421 267200087
e-mail: marek.belisko at streamunlimited.com
http://www.streamunlimited.com
Meet us at:
High End - Munich, 09 - 12 May, Hall 4/F05
IFA - Berlin, 6-11 September
CEDIA - Denver, 25-28 September
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] clk: si5351: Powerdown output when clock rate is 0.
2013-05-07 10:07 ` Sebastian Hesselbarth
@ 2013-05-07 10:40 ` Marek Belisko
0 siblings, 0 replies; 6+ messages in thread
From: Marek Belisko @ 2013-05-07 10:40 UTC (permalink / raw)
To: linux-arm-kernel
On 05/07/2013 12:07 PM, Sebastian Hesselbarth wrote:
> On 05/07/2013 10:14 AM, Marek Belisko wrote:
>> When rate is 0 powerdown clock output.
>>
>> Signed-off-by: Marek Belisko<marek.belisko@streamunlimited.com>
>> ---
>> drivers/clk/clk-si5351.c | 32 +++++++++++++++++++-------------
>> 1 file changed, 19 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
>> index a8fc0f4..e254e76 100644
>> --- a/drivers/clk/clk-si5351.c
>> +++ b/drivers/clk/clk-si5351.c
>> @@ -1042,20 +1042,22 @@ static int si5351_clkout_set_rate(struct
>> clk_hw *hw, unsigned long rate,
>> struct si5351_hw_data *hwdata =
>> container_of(hw, struct si5351_hw_data, hw);
>> unsigned long new_rate, new_err, err;
>> - unsigned char rdiv;
>> + unsigned char rdiv, pdn = 0;
>>
>> /* round to closed rdiv */
>> rdiv = SI5351_OUTPUT_CLK_DIV_1;
>> - new_rate = parent_rate;
>> - err = abs(new_rate - rate);
>> - do {
>> - new_rate>>= 1;
>> - new_err = abs(new_rate - rate);
>> - if (new_err> err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
>> - break;
>> - rdiv++;
>> - err = new_err;
>> - } while (1);
>> + if (rate) {
>> + new_rate = parent_rate;
>
> Marek,
>
> initalizing pdn to SI5351_CLK_POWERDOWN and skip (with goto) to
> powerup/down call on zero rate will save us a level of indention here.
OK will fix and resubmit in v2. Thanks.
>
>> + err = abs(new_rate - rate);
>> + do {
>> + new_rate>>= 1;
>> + new_err = abs(new_rate - rate);
>> + if (new_err> err || rdiv == SI5351_OUTPUT_CLK_DIV_128)
>> + break;
>> + rdiv++;
>> + err = new_err;
>> + } while (1);
>
> of course, pdn should be set to 0 here.
>
>> + }
>>
>> /* write output divider */
>> switch (hwdata->num) {
>> @@ -1075,9 +1077,13 @@ static int si5351_clkout_set_rate(struct clk_hw
>> *hw, unsigned long rate,
>> rdiv<< SI5351_OUTPUT_CLK_DIV_SHIFT);
>> }
>>
>> - /* powerup clkout */
>> + /* when rate is 0 powerdown output */
>> + if (rate == 0)
>> + pdn = SI5351_CLK_POWERDOWN;
>> +
>> + /* powerup/down clkout */
>> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
>> - SI5351_CLK_POWERDOWN, 0);
>> + SI5351_CLK_POWERDOWN, pdn);
>>
>> dev_dbg(&hwdata->drvdata->client->dev,
>> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n",
>
>
marek
--
Marek Belisko
Software Developer
StreamUnlimited Engineering GmbH
Gutheil Schodergasse 8-12
A-1100 Vienna, Austria
Office: +421 267200087
e-mail: marek.belisko at streamunlimited.com
http://www.streamunlimited.com
Meet us at:
High End - Munich, 09 - 12 May, Hall 4/F05
IFA - Berlin, 6-11 September
CEDIA - Denver, 25-28 September
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-07 10:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-07 8:14 [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT is 0 Marek Belisko
2013-05-07 8:14 ` [PATCH 2/2] clk: si5351: Powerdown output when clock rate " Marek Belisko
2013-05-07 10:07 ` Sebastian Hesselbarth
2013-05-07 10:40 ` Marek Belisko
2013-05-07 10:03 ` [PATCH 1/2] clk: si5351: Fix msynth_recalc_rate return value when fOUT " Sebastian Hesselbarth
2013-05-07 10:17 ` Marek Belisko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox