| Message ID | 8EC4D15B-4A89-43FA-953E-95AF81417067@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/25, Alexander Kochetkov wrote: > > > 21 дек. 2017 г., в 23:07, Stephen Boyd <sboyd@codeaurora.org> написал(а): > > > > Can you convert to the determine_rate op instead of round_rate? > > That function should tell you the min/max limits so that you > > don't need to query that information from the core. > > I converted rockchip_fractional_approximation() to rockchip_determine_rate() (see the patch attached). > If it increase parent’s clock for out of limits value, than clock request will fail with -EINVAL, like > with round_rate() approach. > > The problem is that min/max limits provided to determine_rate() is for clock for which the determine_rate() > was called. While rockchip_determine_rate() (rockchip_fractional_approximation()) requires information > about parent clock limits. Are these limits the min/max limits that the parent clk can output at? Or the min/max limits that software has constrained on the clk? > > How can I know parents clock limits for current clock? Implement determine_rate() for each parent clocks > the same way I did for this one clock? If the parent can change rate, then the idea is that the child will calculate the limits that it can handle based on what it can do with the incoming min/max constraints, and then call __clk_determine_rate() on its parent with a request structure that has limits for whatever the child clk is able to handle. The parent can then determine a rate it can output that's within that range and tell the child clk if it will satisfy the constraints or not along with the resulting rate it will output when the __clk_determine_rate() function returns. I would expect the constraints to get closer together the higher in the tree we go. I haven't looked in detail at this rockchip_fractional_approximation() code, but it shouldn't be doing the work of both the child rate determination and the parent rate determination in one place. It should work with the parent to figure out the rate the parent can provide and then figure out how to achieve the desired rate from there.
Initial thread here: https://www.spinics.net/lists/linux-clk/msg21682.html > 27 дек. 2017 г., в 4:06, Stephen Boyd <sboyd@codeaurora.org> написал(а): > > Are these limits the min/max limits that the parent clk can > output at? Or the min/max limits that software has constrained on > the clk? > Don’t know how to answer. For example, parent can output 768MHz, but some IP work unstable with that parent rate. This issues was observed by me and I didn’t get official confirmation from rockchip. So, I limit such clock to 192MHz using clk_set_max_rate(). May be I have to limit clk rate using another approach. Anybody from rockchip can confirm that? Or may be contact me clarifying details? > I haven't looked in detail at this > rockchip_fractional_approximation() code, but it shouldn't be > doing the work of both the child rate determination and the > parent rate determination in one place. It should work with the > parent to figure out the rate the parent can provide and then > figure out how to achieve the desired rate from there. Here is clock tree: clock rate ----- ---- xin24m 24000000 pll_gpll 768000000 gpll 768000000 i2s_src 768000000 i2s0_pre 192000000 i2s0_frac 16384000 sclk_i2s0 16384000 I limit i2s0_pre rate to 192MHz in order to I2S IP work properly. rockchip_fractional_approximation() get called for i2s0_frac. if i2s0_pre rate is 20x times less than i2s0_frac, than rockchip_fractional_approximation() tries to set i2s0_pre rate to i2s_src rate. It tries to increase it’s parent rate in order to maximise relation between nominator and denominator. If I convert rockchip_fractional_approximation() to rockchip_determine_rate(), than I get min=0, max=192MHz limits inside rockchip_determine_rate(). May be there should be new logic inside clk framework based on some new clk flags, that will provide max=768MHz for rockchip_determine_rate(). Also found, that rockchip_fractional_approximation() increase parents rate unconditionally without taking into account CLK_SET_RATE_PARENT flag. Stephen, thanks a lot for deep description of determine_rate() background. I’ll taking some time thinking about possible solutions. Regards, Alexander.
On 12/28, Alexander Kochetkov wrote: > Initial thread here: > https://www.spinics.net/lists/linux-clk/msg21682.html > > > > 27 дек. 2017 г., в 4:06, Stephen Boyd <sboyd@codeaurora.org> написал(а): > > > > Are these limits the min/max limits that the parent clk can > > output at? Or the min/max limits that software has constrained on > > the clk? > > > > Don’t know how to answer. For example, parent can output 768MHz, > but some IP work unstable with that parent rate. This issues was observed by > me and I didn’t get official confirmation from rockchip. So, I limit > such clock to 192MHz using clk_set_max_rate(). May be I have to limit clk rate > using another approach. I'm asking if the rate is capped on the consumer side with clk_set_max_rate() or if it's capped on the clk provider side to express a hardware constraint. > > Anybody from rockchip can confirm that? Or may be contact me clarifying details? > > > I haven't looked in detail at this > > rockchip_fractional_approximation() code, but it shouldn't be > > doing the work of both the child rate determination and the > > parent rate determination in one place. It should work with the > > parent to figure out the rate the parent can provide and then > > figure out how to achieve the desired rate from there. > > Here is clock tree: > > clock rate > ----- ---- > xin24m 24000000 > pll_gpll 768000000 > gpll 768000000 > i2s_src 768000000 > i2s0_pre 192000000 > i2s0_frac 16384000 > sclk_i2s0 16384000 > > I limit i2s0_pre rate to 192MHz in order to I2S IP work properly. > rockchip_fractional_approximation() get called for i2s0_frac. > if i2s0_pre rate is 20x times less than i2s0_frac, than rockchip_fractional_approximation() > tries to set i2s0_pre rate to i2s_src rate. It tries to increase it’s parent rate in order > to maximise relation between nominator and denominator. > > If I convert rockchip_fractional_approximation() to rockchip_determine_rate(), than I get > min=0, max=192MHz limits inside rockchip_determine_rate(). May be there should be > new logic inside clk framework based on some new clk flags, that will provide max=768MHz > for rockchip_determine_rate(). > > Also found, that rockchip_fractional_approximation() increase parents rate unconditionally > without taking into account CLK_SET_RATE_PARENT flag. > > Stephen, thanks a lot for deep description of determine_rate() background. I’ll taking some > time thinking about possible solutions. > Sounds like there are some things to be figured out here still. I can take a closer look next week. Maybe Heiko will respond before then.
> 29 дек. 2017 г., в 3:14, Stephen Boyd <sboyd@codeaurora.org> написал(а): > > I'm asking if the rate is capped on the consumer side with > clk_set_max_rate() or if it's capped on the clk provider side to > express a hardware constraint. I do that using clk_set_max_rate() at provider size inside clk-rk3188.c. > > Sounds like there are some things to be figured out here still. I > can take a closer look next week. Maybe Heiko will respond before > then. I will be very grateful for the ideas. I can continue to work on this next week too. Happy New Year and Merry Christmas! Regards, Alexander.
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c index 3c1fb0d..1e0c701 100644 --- a/drivers/clk/rockchip/clk.c +++ b/drivers/clk/rockchip/clk.c @@ -174,23 +174,9 @@ static void rockchip_fractional_approximation(struct clk_hw *hw, unsigned long *m, unsigned long *n) { struct clk_fractional_divider *fd = to_clk_fd(hw); - unsigned long p_rate, p_parent_rate; - unsigned long min_rate = 0, max_rate = 0; - struct clk_hw *p_parent; unsigned long scale; - - p_rate = clk_hw_get_rate(clk_hw_get_parent(hw)); - if ((rate * 20 > p_rate) && (p_rate % rate != 0)) { - p_parent = clk_hw_get_parent(clk_hw_get_parent(hw)); - p_parent_rate = clk_hw_get_rate(p_parent); - clk_hw_get_boundaries(clk_hw_get_parent(hw), - &min_rate, &max_rate); - if (p_parent_rate < min_rate) - p_parent_rate = min_rate; - if (p_parent_rate > max_rate) - p_parent_rate = max_rate; - *parent_rate = p_parent_rate; - } + unsigned long rate_orig = rate; + unsigned long parent_rate_orig = *parent_rate; /* * Get rate closer to *parent_rate to guarantee there is no overflow @@ -204,8 +190,36 @@ static void rockchip_fractional_approximation(struct clk_hw *hw, rational_best_approximation(rate, *parent_rate, GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0), m, n); + + pr_info("%s: %s: rate:%lu -> %lu parent_rate:%lu -> %lu m:%lu n:%lu\n", + __func__, clk_hw_get_name(hw), rate_orig, rate, + parent_rate_orig, *parent_rate, + *m, *n); } +static int rockchip_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + unsigned long p_rate, p_parent_rate; + struct clk_hw *p_parent; + unsigned long best_parent_rate = req->best_parent_rate; + + p_rate = clk_hw_get_rate(clk_hw_get_parent(hw)); + if ((req->rate * 20 > p_rate) && (p_rate % req->rate != 0)) { + p_parent = clk_hw_get_parent(clk_hw_get_parent(hw)); + p_parent_rate = clk_hw_get_rate(p_parent); + req->best_parent_rate = p_parent_rate; + } + + pr_info("%s: %s: rate:%lu min_rate:%lu max_rate:%lu best_parent_rate:%lu -> %lu best_parent_hw:%s\n", + __func__, clk_hw_get_name(hw), req->rate, req->min_rate, req->max_rate, best_parent_rate, req->best_parent_rate, + req->best_parent_hw ? clk_hw_get_name(req->best_parent_hw) : "<null>"); + + return 0; +} + +static struct clk_ops rockchip_clk_fractional_divider_ops; + static struct clk *rockchip_clk_register_frac_branch( struct rockchip_clk_provider *ctx, const char *name, const char *const *parent_names, u8 num_parents, @@ -253,7 +267,8 @@ static struct clk *rockchip_clk_register_frac_branch( div->nmask = GENMASK(div->nwidth - 1, 0) << div->nshift; div->lock = lock; div->approximation = rockchip_fractional_approximation; - div_ops = &clk_fractional_divider_ops; + div_ops = &rockchip_clk_fractional_divider_ops; + clk = clk_register_composite(NULL, name, parent_names, num_parents, NULL, NULL, @@ -392,6 +407,9 @@ struct rockchip_clk_provider * __init rockchip_clk_init(struct device_node *np, ctx->grf = syscon_regmap_lookup_by_phandle(ctx->cru_node, "rockchip,grf"); + rockchip_clk_fractional_divider_ops = clk_fractional_divider_ops; + rockchip_clk_fractional_divider_ops.determine_rate = rockchip_determine_rate; + return ctx; err_free: