* [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 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 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
* [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 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
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