| Message ID | 20160322012215.GA6963@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/22/2016 03:22 AM, Ladislav Michl wrote: > On Thu, Mar 17, 2016 at 07:42:38AM -0700, Tony Lindgren wrote: >> * Richard Watts <rrw@kynesim.co.uk> [160316 10:14]: >>> However, there are several possible options for a workaround if you are >>> using a 13MHz or 26MHz xtal - see >>> <http://www.ti.com/lit/er/sprz319f/sprz319f.pdf> PDF p111. It might perhaps >>> be civilised to give the user the option, since which is appropriate to your >>> board is a matter of board characterisation. >> >> Seems like we should just configure dpll5 based on the SYS_CLK rate >> automatically. > > Something like patch v0 bellow. However note, that rate is determined using > determine_rate, therefore omap3630_noncore_dpll_determine_rate which is just > a copy of omap2_noncore_dpll_determine_rate is calling > omap3630_dpll_round_rate - shame, shall we use function pointer > determine_rate? Neither is nice and elegant. Also we somehow need to know > soc_is_omap3630() and bringing is to drivers/clk does not seem as an option. > Also note, that there's no choice between options for 26MHz. Hints? What do you mean, no choice between options for 26MHz? > >>> I see no shame in simply exposing something like: >>> >>> ti,omap3-dpll5-clock = < 443 5 8 > >> >> If we want a binding like that it should be Linux generic and disucced >> on the linux-clk list. It's usually best to stick to standard >> bindings and hide the quirks in the driver(s). It seem at most we just >> need to specify the 36xx related compatible flag for dpll5. >> >>> I have a Beagle xM or two here I can send out to anyone in need - throw me an >>> address. >> >> Cool, I think also Tomi Valkeinen was looking for a 36xx test board >> for occasional DSS testing. I have a beagle xm on loan here so I'm >> all set for now. > > I'm considering this errata runtime tested enough, for now it would be nice > to have patch ready and verify that dd->mult_div1_reg contains the right > value ;-) > >>> OMAP3530 should also suffer from this problem, but it is not listed in the >>> errata sheet. >>> >>> Other than that, the only devices I know of that have this combination of >>> DPLL and HSUSB are OMAP3630 and DM3730 - if it's important, I can try and >>> find the appropriate people in TI? >> >> AFAIK 3630 and dm3730 are exactly the same. The dm3730 is just the >> catalog part GP version. > > Ok, let's prefix errata functions omap3630_ then. > > Regards, > ladis > > --- drivers/clk/ti/dpll.c.orig 2016-03-21 22:53:16.379746383 +0100 > +++ drivers/clk/ti/dpll.c 2016-03-22 01:48:36.988896607 +0100 > @@ -114,6 +114,18 @@ > .round_rate = &omap2_dpll_round_rate, > }; > > +static const struct clk_ops omap3630_dpll_ck_ops = { > + .enable = &omap3_noncore_dpll_enable, > + .disable = &omap3_noncore_dpll_disable, > + .get_parent = &omap2_init_dpll_parent, > + .recalc_rate = &omap3_dpll_recalc, > + .set_rate = &omap3_noncore_dpll_set_rate, > + .set_parent = &omap3_noncore_dpll_set_parent, > + .set_rate_and_parent = &omap3_noncore_dpll_set_rate_and_parent, > + .determine_rate = &omap3630_noncore_dpll_determine_rate, > + .round_rate = &omap3630_dpll_round_rate, > +}; > + > static const struct clk_ops omap3_dpll_per_ck_ops = { > .enable = &omap3_noncore_dpll_enable, > .disable = &omap3_noncore_dpll_disable, > @@ -448,6 +460,7 @@ > #ifdef CONFIG_ARCH_OMAP3 > static void __init of_ti_omap3_dpll_setup(struct device_node *node) > { > + const struct clk_ops *ops = &omap3_dpll_ck_ops; > const struct dpll_data dd = { > .idlest_mask = 0x1, > .enable_mask = 0x7, > @@ -461,7 +474,10 @@ > .modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED), > }; > > - of_ti_dpll_setup(node, &omap3_dpll_ck_ops, &dd); > + if (/* soc_is_omap3630() && */ strcmp(node->name, "dpll5_ck") == 0) > + ops = &omap3630_dpll_ck_ops; > + > + of_ti_dpll_setup(node, ops, &dd); > } > CLK_OF_DECLARE(ti_omap3_dpll_clock, "ti,omap3-dpll-clock", > of_ti_omap3_dpll_setup); Just add a new compatible: > CLK_OF_DECLARE(ti_omap3630_dpll5_clock, "ti,omap3630-dpll5-clock", > of_ti_omap3630_dpll5_setup); Makes life simpler for everyone. > --- drivers/clk/ti/clock.h.orig 2016-03-21 22:55:09.011746383 +0100 > +++ drivers/clk/ti/clock.h 2016-03-22 01:42:45.840896607 +0100 > @@ -252,8 +252,12 @@ > u8 index); > int omap3_noncore_dpll_determine_rate(struct clk_hw *hw, > struct clk_rate_request *req); > +int omap3630_noncore_dpll_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req); > long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, > unsigned long *parent_rate); > +long omap3630_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, > + unsigned long *parent_rate); > unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw, > unsigned long parent_rate); > > --- drivers/clk/ti/clkt_dpll.c.orig 2016-03-22 01:59:21.724896607 +0100 > +++ drivers/clk/ti/clkt_dpll.c 2016-03-22 01:53:41.072896607 +0100 > @@ -368,3 +368,46 @@ > > return dd->last_rounded_rate; > } > + > +/** > + * omap3630_dpll_round_rate - round a target rate for an OMAP DPLL > + * @clk: struct clk * for a DPLL > + * @target_rate: desired DPLL clock rate > + * > + * DM3730 errata (sprz319e), advisory 2.1, table 36 implementation > + */ > +long omap3630_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, > + unsigned long *parent_rate) > +{ > + unsigned long rate; > + struct dpll_data *dd; > + struct clk_hw_omap *clk = to_clk_hw_omap(hw); > + > + if (!clk || !clk->dpll_data) > + return ~0; I don't think you need these checks here, this condition can never happen. > + > + dd = clk->dpll_data; > + rate = target_rate == 120000000 ? *parent_rate : 0; > + > + switch (rate) { > + case 13000000: > + dd->last_rounded_m = 443; > + dd->last_rounded_n = 5; > + break; > + case 26000000: > + dd->last_rounded_m = 443; > + dd->last_rounded_n = 11; > + break; Add all the other frequencies listed in the errata here also. > + default: > + return omap2_dpll_round_rate(hw, target_rate, parent_rate); > + } > + /* actual divide value, value of the register is n - 1 */ > + dd->last_rounded_n++; This +1 you can just squash above within the switch statement. > + dd->last_rounded_rate = rate * dd->last_rounded_m / dd->last_rounded_n; > + > + pr_debug("clock: %s: fixed m = %d, n = %d, new_rate = %lu\n", > + clk_hw_get_name(hw), dd->last_rounded_m, dd->last_rounded_n, > + dd->last_rounded_rate); > + > + return dd->last_rounded_rate; > +} > --- drivers/clk/ti/dpll3xxx.c.orig 2016-03-21 22:55:29.515746383 +0100 > +++ drivers/clk/ti/dpll3xxx.c 2016-03-22 01:43:54.004896607 +0100 > @@ -534,6 +534,33 @@ > return 0; > } > > +int omap3630_noncore_dpll_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + struct clk_hw_omap *clk = to_clk_hw_omap(hw); > + struct dpll_data *dd; > + > + if (!req->rate) > + return -EINVAL; > + > + dd = clk->dpll_data; > + if (!dd) > + return -EINVAL; > + > + if (clk_get_rate(dd->clk_bypass) == req->rate && > + (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) { > + req->best_parent_hw = __clk_get_hw(dd->clk_bypass); > + } else { > + req->rate = omap3630_dpll_round_rate(hw, req->rate, > + &req->best_parent_rate); > + req->best_parent_hw = __clk_get_hw(dd->clk_ref); > + } > + > + req->best_parent_rate = req->rate; > + > + return 0; How about something like: int omap3630_noncore_dpll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { struct clk_hw_omap *clk = to_clk_hw_omap(hw); struct dpll_data *dd; dd = clk->dpll_data; if (!dd) return -EINVAL; if (req->rate == 120000000) { req->rate = omap3630_dpll_round_rate(hw, req->rate, &req->best_parent_rate); req->best_parent_hw = __clk_get_hw(dd->clk_ref); return 0; } return omap3_noncore_dpll_determine_rate(hw, req); } You can drop the checks against 120MHz inside round_rate then. > +} > + > /** > * omap3_noncore_dpll_set_parent - set parent for a DPLL clock > * @hw: pointer to the clock to set parent for > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- drivers/clk/ti/dpll.c.orig 2016-03-21 22:53:16.379746383 +0100 +++ drivers/clk/ti/dpll.c 2016-03-22 01:48:36.988896607 +0100 @@ -114,6 +114,18 @@ .round_rate = &omap2_dpll_round_rate, }; +static const struct clk_ops omap3630_dpll_ck_ops = { + .enable = &omap3_noncore_dpll_enable, + .disable = &omap3_noncore_dpll_disable, + .get_parent = &omap2_init_dpll_parent, + .recalc_rate = &omap3_dpll_recalc, + .set_rate = &omap3_noncore_dpll_set_rate, + .set_parent = &omap3_noncore_dpll_set_parent, + .set_rate_and_parent = &omap3_noncore_dpll_set_rate_and_parent, + .determine_rate = &omap3630_noncore_dpll_determine_rate, + .round_rate = &omap3630_dpll_round_rate, +}; + static const struct clk_ops omap3_dpll_per_ck_ops = { .enable = &omap3_noncore_dpll_enable, .disable = &omap3_noncore_dpll_disable, @@ -448,6 +460,7 @@ #ifdef CONFIG_ARCH_OMAP3 static void __init of_ti_omap3_dpll_setup(struct device_node *node) { + const struct clk_ops *ops = &omap3_dpll_ck_ops; const struct dpll_data dd = { .idlest_mask = 0x1, .enable_mask = 0x7, @@ -461,7 +474,10 @@ .modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED), }; - of_ti_dpll_setup(node, &omap3_dpll_ck_ops, &dd); + if (/* soc_is_omap3630() && */ strcmp(node->name, "dpll5_ck") == 0) + ops = &omap3630_dpll_ck_ops; + + of_ti_dpll_setup(node, ops, &dd); } CLK_OF_DECLARE(ti_omap3_dpll_clock, "ti,omap3-dpll-clock", of_ti_omap3_dpll_setup); --- drivers/clk/ti/clock.h.orig 2016-03-21 22:55:09.011746383 +0100 +++ drivers/clk/ti/clock.h 2016-03-22 01:42:45.840896607 +0100 @@ -252,8 +252,12 @@ u8 index); int omap3_noncore_dpll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req); +int omap3630_noncore_dpll_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req); long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, unsigned long *parent_rate); +long omap3630_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, + unsigned long *parent_rate); unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw, unsigned long parent_rate); --- drivers/clk/ti/clkt_dpll.c.orig 2016-03-22 01:59:21.724896607 +0100 +++ drivers/clk/ti/clkt_dpll.c 2016-03-22 01:53:41.072896607 +0100 @@ -368,3 +368,46 @@ return dd->last_rounded_rate; } + +/** + * omap3630_dpll_round_rate - round a target rate for an OMAP DPLL + * @clk: struct clk * for a DPLL + * @target_rate: desired DPLL clock rate + * + * DM3730 errata (sprz319e), advisory 2.1, table 36 implementation + */ +long omap3630_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate, + unsigned long *parent_rate) +{ + unsigned long rate; + struct dpll_data *dd; + struct clk_hw_omap *clk = to_clk_hw_omap(hw); + + if (!clk || !clk->dpll_data) + return ~0; + + dd = clk->dpll_data; + rate = target_rate == 120000000 ? *parent_rate : 0; + + switch (rate) { + case 13000000: + dd->last_rounded_m = 443; + dd->last_rounded_n = 5; + break; + case 26000000: + dd->last_rounded_m = 443; + dd->last_rounded_n = 11; + break; + default: + return omap2_dpll_round_rate(hw, target_rate, parent_rate); + } + /* actual divide value, value of the register is n - 1 */ + dd->last_rounded_n++; + dd->last_rounded_rate = rate * dd->last_rounded_m / dd->last_rounded_n; + + pr_debug("clock: %s: fixed m = %d, n = %d, new_rate = %lu\n", + clk_hw_get_name(hw), dd->last_rounded_m, dd->last_rounded_n, + dd->last_rounded_rate); + + return dd->last_rounded_rate; +} --- drivers/clk/ti/dpll3xxx.c.orig 2016-03-21 22:55:29.515746383 +0100 +++ drivers/clk/ti/dpll3xxx.c 2016-03-22 01:43:54.004896607 +0100 @@ -534,6 +534,33 @@ return 0; } +int omap3630_noncore_dpll_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + struct clk_hw_omap *clk = to_clk_hw_omap(hw); + struct dpll_data *dd; + + if (!req->rate) + return -EINVAL; + + dd = clk->dpll_data; + if (!dd) + return -EINVAL; + + if (clk_get_rate(dd->clk_bypass) == req->rate && + (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) { + req->best_parent_hw = __clk_get_hw(dd->clk_bypass); + } else { + req->rate = omap3630_dpll_round_rate(hw, req->rate, + &req->best_parent_rate); + req->best_parent_hw = __clk_get_hw(dd->clk_ref); + } + + req->best_parent_rate = req->rate; + + return 0; +} + /** * omap3_noncore_dpll_set_parent - set parent for a DPLL clock * @hw: pointer to the clock to set parent for