* Add .determine_rate to composite clk @ 2014-01-11 9:15 Lemon Dai 2014-01-11 10:28 ` Maxime Ripard 0 siblings, 1 reply; 4+ messages in thread From: Lemon Dai @ 2014-01-11 9:15 UTC (permalink / raw) To: linux-arm-kernel Hi Mike, As .determine_rate was added to struct clk and clk_mux_ops, maybe we should also copy determine_rate operation from mux_ops to clk_composite_ops in clk_register_composite( ), to make composite clk implementation more generic. Patch: diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index a33f46f..d224d6e 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -43,6 +43,20 @@ static int clk_composite_set_parent(struct clk_hw *hw, u8 index) return mux_ops->set_parent(mux_hw, index); } +static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *best_parent_rate, + struct clk **best_parent_p) +{ + struct clk_composite *composite = to_clk_composite(hw); + const struct clk_ops *mux_ops = composite->mux_ops; + struct clk_hw *mux_hw = composite->mux_hw; + + mux_hw->clk = hw->clk; + + return mux_ops->determine_rate(mux_hw, rate, best_parent_rate, + best_parent_p); +} + static unsigned long clk_composite_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -147,6 +161,11 @@ struct clk *clk_register_composite(struct device *dev, const char *name, composite->mux_ops = mux_ops; clk_composite_ops->get_parent = clk_composite_get_parent; clk_composite_ops->set_parent = clk_composite_set_parent; + + if (mux_ops->determine_rate) { + clk_composite_ops->determine_rate = + clk_composite_determine_rate; + } } if (rate_hw && rate_ops) { -- Best regards, Lemon Dai ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Add .determine_rate to composite clk 2014-01-11 9:15 Add .determine_rate to composite clk Lemon Dai @ 2014-01-11 10:28 ` Maxime Ripard [not found] ` <CAJiWf8WJxf0S32bPiXGUu3Rwmh36xYdbd0VEpLTG-pwyvVnasg@mail.gmail.com> 0 siblings, 1 reply; 4+ messages in thread From: Maxime Ripard @ 2014-01-11 10:28 UTC (permalink / raw) To: linux-arm-kernel Hi Lemon, On Sat, Jan 11, 2014 at 05:15:58PM +0800, Lemon Dai wrote: > Hi Mike, > > As .determine_rate was added to struct clk and clk_mux_ops, maybe we > should also copy determine_rate operation from mux_ops to > clk_composite_ops in clk_register_composite( ), > to make composite clk implementation more generic. A similar patch is on its way to 3.14 already (commit 107f3198 in Mike's clk-next branch). Also, for future contributions, I'd suggest to read the Documentation/SubmittingPatches file. Your patch had several style issues that are covered in this file. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140111/9c6994f3/attachment.sig> ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <CAJiWf8WJxf0S32bPiXGUu3Rwmh36xYdbd0VEpLTG-pwyvVnasg@mail.gmail.com>]
* Add .determine_rate to composite clk [not found] ` <CAJiWf8WJxf0S32bPiXGUu3Rwmh36xYdbd0VEpLTG-pwyvVnasg@mail.gmail.com> @ 2014-01-14 14:08 ` Lemon Dai 2014-01-14 21:03 ` Mike Turquette 0 siblings, 1 reply; 4+ messages in thread From: Lemon Dai @ 2014-01-14 14:08 UTC (permalink / raw) To: linux-arm-kernel Hi Maxime, The commit 107f3198 in Mike's clk-next branch shows that: (about line 72 in drivers/clk/clk-composite.c) } else if (mux_hw && mux_ops && mux_ops->determine_rate) { mux_hw->clk = hw->clk; return mux_ops->determine_rate(rate_hw, rate, best_parent_rate, best_parent_p); It should be fixed as follows, at least for reason that NULL rate_hw may be used in the old code, } else if (mux_hw && mux_ops && mux_ops->determine_rate) { mux_hw->clk = hw->clk; - return mux_ops->determine_rate(rate_hw, rate, best_parent_rate, + return mux_ops->determine_rate(mux_hw, rate, best_parent_rate, best_parent_p); -- Best regards, Lemon Dai On Sun, Jan 12, 2014 at 11:56 AM, Lemon Dai <dailemon.gl@gmail.com> wrote: > Hi Maxime, > > Thank you for your reply and suggestion. > I am sorry for sending a mail with such a lot of style issues before > reading Documentation/SubmittingPatches file. > > Best wishes, > Lemon > > On Sat, Jan 11, 2014 at 6:28 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: >> Hi Lemon, >> >> On Sat, Jan 11, 2014 at 05:15:58PM +0800, Lemon Dai wrote: >>> Hi Mike, >>> >>> As .determine_rate was added to struct clk and clk_mux_ops, maybe we >>> should also copy determine_rate operation from mux_ops to >>> clk_composite_ops in clk_register_composite( ), >>> to make composite clk implementation more generic. >> >> A similar patch is on its way to 3.14 already (commit 107f3198 in >> Mike's clk-next branch). >> >> Also, for future contributions, I'd suggest to read the >> Documentation/SubmittingPatches file. Your patch had several style >> issues that are covered in this file. >> >> Thanks! >> Maxime >> >> -- >> Maxime Ripard, Free Electrons >> Embedded Linux, Kernel and Android engineering >> http://free-electrons.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Add .determine_rate to composite clk 2014-01-14 14:08 ` Lemon Dai @ 2014-01-14 21:03 ` Mike Turquette 0 siblings, 0 replies; 4+ messages in thread From: Mike Turquette @ 2014-01-14 21:03 UTC (permalink / raw) To: linux-arm-kernel Quoting Lemon Dai (2014-01-14 06:08:44) > Hi Maxime, > > The commit 107f3198 in Mike's clk-next branch shows that: > (about line 72 in drivers/clk/clk-composite.c) > > } else if (mux_hw && mux_ops && mux_ops->determine_rate) { > mux_hw->clk = hw->clk; > return mux_ops->determine_rate(rate_hw, rate, best_parent_rate, > best_parent_p); > > > It should be fixed as follows, at least for reason that NULL rate_hw > may be used in the old code, > > } else if (mux_hw && mux_ops && mux_ops->determine_rate) { > mux_hw->clk = hw->clk; > - return mux_ops->determine_rate(rate_hw, rate, best_parent_rate, > + return mux_ops->determine_rate(mux_hw, rate, best_parent_rate, > best_parent_p); Hi Lemon Dai, I have applied this fix to clk-next. The patch is below: >From ca01f3f60aa864d7ee8124ca597d4010378926b5 Mon Sep 17 00:00:00 2001 From: Mike Turquette <mturquette@linaro.org> Date: Tue, 14 Jan 2014 12:56:01 -0800 Subject: [PATCH] clk: composite: pass mux_hw into determine_rate The composite clock's .determine_rate implementation can call the underyling .determine_rate callback corresponding to rate_hw or the underlying .determine_rate callback corresponding to mux_hw. In both cases we pass in rate_hw, which is wrong. Fixed by passing mux_hw into the correct callback. Reported-by: Lemon Dai <dailemon.gl@gmail.com> Signed-off-by: Mike Turquette <mturquette@linaro.org> --- drivers/clk/clk-composite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index 753d0b7..57a078e 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -71,7 +71,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate, best_parent_p); } else if (mux_hw && mux_ops && mux_ops->determine_rate) { mux_hw->clk = hw->clk; - return mux_ops->determine_rate(rate_hw, rate, best_parent_rate, + return mux_ops->determine_rate(mux_hw, rate, best_parent_rate, best_parent_p); } else { pr_err("clk: clk_composite_determine_rate function called, but no mux or rate callback set!\n"); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-14 21:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-11 9:15 Add .determine_rate to composite clk Lemon Dai
2014-01-11 10:28 ` Maxime Ripard
[not found] ` <CAJiWf8WJxf0S32bPiXGUu3Rwmh36xYdbd0VEpLTG-pwyvVnasg@mail.gmail.com>
2014-01-14 14:08 ` Lemon Dai
2014-01-14 21:03 ` Mike Turquette
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).