| Message ID | 1353403339-11679-3-git-send-email-dmitry.torokhov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 20, 2012 at 01:22:18AM -0800, Dmitry Torokhov wrote: > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* > conversion so let's uninline the pair. Again, what about stuff not using drivers/clk/clk.c ?
On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* > conversion so let's uninline the pair. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> This one matches its expectations :) Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* > conversion so let's uninline the pair. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Mike, are you taking these patches?
[Adding Linaro id of Mike] On 16 December 2012 17:10, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: >> We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* >> conversion so let's uninline the pair. >> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Mike, are you taking these patches?
On Sun, Dec 16, 2012 at 05:10:36PM +0530, Viresh Kumar wrote: > On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* > > conversion so let's uninline the pair. > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Mike, are you taking these patches? And what about my comments, some of which you've failed to reply to?
On 16 December 2012 17:27, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sun, Dec 16, 2012 at 05:10:36PM +0530, Viresh Kumar wrote: >> On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: >> > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* >> > conversion so let's uninline the pair. >> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >> >> Mike, are you taking these patches? > > And what about my comments, some of which you've failed to reply to? Surprised!! I thought there was nothing missing after the last discussion i remember. Dmitry agreed to drop the first patch and all other looked fine, as nobody objected to them again. Yes, you did in the beginning but there were valid replies to them, on which you never objected. So, i thought its all good now. Can you point again to the issues still left ? -- viresh
On Sun, Dec 16, 2012 at 05:50:44PM +0530, Viresh Kumar wrote: > On 16 December 2012 17:27, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Sun, Dec 16, 2012 at 05:10:36PM +0530, Viresh Kumar wrote: > >> On 20 November 2012 14:52, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > >> > We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* > >> > conversion so let's uninline the pair. > >> > > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > >> > >> Mike, are you taking these patches? > > > > And what about my comments, some of which you've failed to reply to? > > Surprised!! I thought there was nothing missing after the last > discussion i remember. > Dmitry agreed to drop the first patch and all other looked fine, as > nobody objected > to them again. Yes, you did in the beginning but there were valid > replies to them, on > which you never objected. > > So, i thought its all good now. Can you point again to the issues still left ? Well, there's my comment against patch 2 which never got a reply: "Again, what about stuff not using drivers/clk/clk.c ?" Has this been addressed?
On 16 December 2012 18:10, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > Well, there's my comment against patch 2 which never got a reply: > > "Again, what about stuff not using drivers/clk/clk.c ?" > > Has this been addressed? Hmm.. I misread it and thought it is same as breaking other platforms because there are no dummy routines. But i was wrong :( So, the problem is, platform not using common-clock framework uses this routine, and they don't want it to be dummy but call prepare & enable.. Because Dmirty requires this one to be non-inline, either he can move these routines to drivers/clk/clk-devres.c (which would be wrong) or can add wrappers over them in clk-devres file. -- viresh
On Sun, Dec 16, 2012 at 06:35:24PM +0530, Viresh Kumar wrote: > On 16 December 2012 18:10, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > Well, there's my comment against patch 2 which never got a reply: > > > > "Again, what about stuff not using drivers/clk/clk.c ?" > > > > Has this been addressed? > > Hmm.. I misread it and thought it is same as breaking other platforms > because there are > no dummy routines. But i was wrong :( > > So, the problem is, platform not using common-clock framework uses > this routine, and they > don't want it to be dummy but call prepare & enable.. > > Because Dmirty requires this one to be non-inline, either he can move > these routines to > drivers/clk/clk-devres.c (which would be wrong) or can add wrappers > over them in clk-devres > file. The point of the inlines in linux/clk.h is so that people using the clk API have a way to transition to the new prepare+enable solution without having their drivers break. This patch series totally wrecks that by making clk_prepare() private to the common clock framework. All the time that it does that, it's totally and utterly unsuitable for going into mainline. Is that strong enough language that my point is properly heard?
Hi Viresh, On Sun, Dec 16, 2012 at 06:35:24PM +0530, Viresh Kumar wrote: > On 16 December 2012 18:10, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > Well, there's my comment against patch 2 which never got a reply: > > > > "Again, what about stuff not using drivers/clk/clk.c ?" > > > > Has this been addressed? > > Hmm.. I misread it and thought it is same as breaking other platforms > because there are > no dummy routines. But i was wrong :( > > So, the problem is, platform not using common-clock framework uses > this routine, and they > don't want it to be dummy but call prepare & enable.. > > Because Dmirty requires this one to be non-inline, either he can move > these routines to > drivers/clk/clk-devres.c (which would be wrong) or can add wrappers > over them in clk-devres > file. They do not _have_ to be non-inline, I think we should simply drop the first 2 patches and I will refresh and ressend the 3rd one. Thanks.
On 17 December 2012 11:12, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > They do not _have_ to be non-inline, I think we should simply drop the > first 2 patches and I will refresh and ressend the 3rd one. I haven't seen this patch since a long time? Any updates?
Dmitry, what is the status of this patchseries? Are you continue to make it upstream? On Mon, Apr 8, 2013 at 1:19 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 17 December 2012 11:12, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: >> They do not _have_ to be non-inline, I think we should simply drop the >> first 2 patches and I will refresh and ressend the 3rd one. > > I haven't seen this patch since a long time? Any updates? > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 1b642f2..69dc7ba 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -561,6 +561,29 @@ int clk_enable(struct clk *clk) } EXPORT_SYMBOL_GPL(clk_enable); +int clk_prepare_enable(struct clk *clk) +{ + int ret; + + ret = clk_prepare(clk); + if (ret) + return ret; + + ret = clk_enable(clk); + if (ret) + clk_unprepare(clk); + + return ret; +} +EXPORT_SYMBOL_GPL(clk_prepare_enable); + +void clk_disable_unprepare(struct clk *clk) +{ + clk_disable(clk); + clk_unprepare(clk); +} +EXPORT_SYMBOL_GPL(clk_disable_unprepare); + /** * __clk_round_rate - round the given rate for a clk * @clk: round the rate of this clock diff --git a/include/linux/clk.h b/include/linux/clk.h index f8204c3..8bf149e 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -172,6 +172,26 @@ int clk_enable(struct clk *clk); void clk_disable(struct clk *clk); /** + * clk_prepare_enable - prepare and enable a clock source + * @clk: clock source + * + * This prepares the clock source for use and enables it. + * + * Must not be called from within atomic context. + */ +int clk_prepare_enable(struct clk *clk); + +/** + * clk_disable_unprepare - disable and undo preparation of a clock source + * @clk: clock source + * + * This disables and undoes a previously prepared clock. + * + * Must not be called from within atomic context. + */ +void clk_disable_unprepare(struct clk *clk); + +/** * clk_get_rate - obtain the current clock rate (in Hz) for a clock source. * This is only valid once the clock source has been enabled. * @clk: clock source @@ -295,6 +315,17 @@ static inline int clk_enable(struct clk *clk) static inline void clk_disable(struct clk *clk) {} +static inline int clk_prepare_enable(struct clk *clk) +{ + might_sleep(); + return 0; +} + +static inline void clk_disable_unprepare(struct clk *clk) +{ + might_sleep(); +} + static inline unsigned long clk_get_rate(struct clk *clk) { return 0; @@ -322,28 +353,6 @@ static inline struct clk *clk_get_parent(struct clk *clk) #endif -/* clk_prepare_enable helps cases using clk_enable in non-atomic context. */ -static inline int clk_prepare_enable(struct clk *clk) -{ - int ret; - - ret = clk_prepare(clk); - if (ret) - return ret; - ret = clk_enable(clk); - if (ret) - clk_unprepare(clk); - - return ret; -} - -/* clk_disable_unprepare helps cases using clk_disable in non-atomic context. */ -static inline void clk_disable_unprepare(struct clk *clk) -{ - clk_disable(clk); - clk_unprepare(clk); -} - /** * clk_add_alias - add a new clock alias * @alias: name for clock alias
We'll need to invoke clk_disable_unprepare() via a pointer in our devm_* conversion so let's uninline the pair. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/clk/clk.c | 23 +++++++++++++++++++++++ include/linux/clk.h | 53 +++++++++++++++++++++++++++++++---------------------- 2 files changed, 54 insertions(+), 22 deletions(-)