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@xxxxxxxxxxxxx> [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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
