| Message ID | 1437549069-29655-5-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 07/22/2015 12:11 AM, Rajendra Nayak wrote: > With CONFIG_PM disabled, turn the devices clocks on during > driver binding to the device, and turn them off when the > driver is unbound from the device. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/clk/qcom/gdsc.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index 3125809..9ddd2f8 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -16,6 +16,7 @@ > #include <linux/jiffies.h> > #include <linux/pm_clock.h> > #include <linux/slab.h> > +#include <linux/platform_device.h> #include <linux/clk.h>? > #include "gdsc.h" > > #define PWR_ON_MASK BIT(31) > @@ -200,3 +201,61 @@ void gdsc_unregister(struct device *dev) > { > of_genpd_del_provider(dev->of_node); > } > + > +#ifndef CONFIG_PM > +static void enable_clock(struct device *dev, const char *con_id) > +{ > + struct clk *clk; > + > + clk = clk_get(dev, con_id); > + if (!IS_ERR(clk)) { > + clk_prepare_enable(clk); > + clk_put(clk); > + } > +} > + > +static void disable_clock(struct device *dev, const char *con_id) > +{ > + struct clk *clk; > + > + clk = clk_get(dev, con_id); > + if (!IS_ERR(clk)) { > + clk_disable_unprepare(clk); > + clk_put(clk); > + } > +} Is there a reason why this whole patch isn't generic code? I recall some discussion but I forgot now and there isn't any mention of why this isn't generic code in the commit text. > + > +static int clk_notify(struct notifier_block *nb, unsigned long action, > + void *data) > +{ > + int sz; > + struct device *dev = data; > + char **con_id, *con_ids[] = { "core", "iface", NULL }; This again? > + > + if (!of_find_property(dev->of_node, "power-domains", &sz)) > + return 0; > + > + switch (action) { > + case BUS_NOTIFY_BIND_DRIVER: > + for (con_id = con_ids; *con_id; con_id++) > + enable_clock(dev, *con_id); > + break; > + case BUS_NOTIFY_UNBOUND_DRIVER: > + for (con_id = con_ids; *con_id; con_id++) > + disable_clock(dev, *con_id); > + break; > + } > + return 0; > +} > + > +struct notifier_block nb = { static? > + .notifier_call = clk_notify, > +}; > + > +int qcom_pm_runtime_init(void) static? __init? > +{ > + bus_register_notifier(&platform_bus_type, &nb); > + return 0; return bus_register_notifier()? > +} > +core_initcall(qcom_pm_runtime_init); > +#endif
[].. >> + >> +#ifndef CONFIG_PM >> +static void enable_clock(struct device *dev, const char *con_id) >> +{ >> + struct clk *clk; >> + >> + clk = clk_get(dev, con_id); >> + if (!IS_ERR(clk)) { >> + clk_prepare_enable(clk); >> + clk_put(clk); >> + } >> +} >> + >> +static void disable_clock(struct device *dev, const char *con_id) >> +{ >> + struct clk *clk; >> + >> + clk = clk_get(dev, con_id); >> + if (!IS_ERR(clk)) { >> + clk_disable_unprepare(clk); >> + clk_put(clk); >> + } >> +} > > Is there a reason why this whole patch isn't generic code? I recall some > discussion but I forgot now and there isn't any mention of why this > isn't generic code in the commit text. If by generic code, you mean using PM clocks, then this thread should give some context.. http://www.spinics.net/lists/arm-kernel/msg414072.html -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/23/2015 01:35 AM, Rajendra Nayak wrote: > [].. > >>> + >>> +#ifndef CONFIG_PM >>> +static void enable_clock(struct device *dev, const char *con_id) >>> +{ >>> + struct clk *clk; >>> + >>> + clk = clk_get(dev, con_id); >>> + if (!IS_ERR(clk)) { >>> + clk_prepare_enable(clk); >>> + clk_put(clk); >>> + } >>> +} >>> + >>> +static void disable_clock(struct device *dev, const char *con_id) >>> +{ >>> + struct clk *clk; >>> + >>> + clk = clk_get(dev, con_id); >>> + if (!IS_ERR(clk)) { >>> + clk_disable_unprepare(clk); >>> + clk_put(clk); >>> + } >>> +} >> >> Is there a reason why this whole patch isn't generic code? I recall some >> discussion but I forgot now and there isn't any mention of why this >> isn't generic code in the commit text. > > If by generic code, you mean using PM clocks, then this thread should > give some context.. > http://www.spinics.net/lists/arm-kernel/msg414072.html Sorry, I read the thread and I tried to understand what was going on but I'm still lost. Can you clarify further in the commit text somehow?
On 07/29/2015 06:34 AM, Stephen Boyd wrote: > On 07/23/2015 01:35 AM, Rajendra Nayak wrote: >> [].. >> >>>> + >>>> +#ifndef CONFIG_PM >>>> +static void enable_clock(struct device *dev, const char *con_id) >>>> +{ >>>> + struct clk *clk; >>>> + >>>> + clk = clk_get(dev, con_id); >>>> + if (!IS_ERR(clk)) { >>>> + clk_prepare_enable(clk); >>>> + clk_put(clk); >>>> + } >>>> +} >>>> + >>>> +static void disable_clock(struct device *dev, const char *con_id) >>>> +{ >>>> + struct clk *clk; >>>> + >>>> + clk = clk_get(dev, con_id); >>>> + if (!IS_ERR(clk)) { >>>> + clk_disable_unprepare(clk); >>>> + clk_put(clk); >>>> + } >>>> +} >>> >>> Is there a reason why this whole patch isn't generic code? I recall some >>> discussion but I forgot now and there isn't any mention of why this >>> isn't generic code in the commit text. >> >> If by generic code, you mean using PM clocks, then this thread should >> give some context.. >> http://www.spinics.net/lists/arm-kernel/msg414072.html > > Sorry, I read the thread and I tried to understand what was going on but > I'm still lost. Can you clarify further in the commit text somehow? So I can add this in the commit text, if it seems fine "The use of pm_clk_add_notifier() isn't appropriate here since we need to only manage clocks with valid power domain associations done via DT, instead of what pm_clk_add_notifier() does, which is manage clocks for all on SoC/off SoC devices associating all of them to a dummy power domain instead"
On 07/29, Rajendra Nayak wrote: > > On 07/29/2015 06:34 AM, Stephen Boyd wrote: > > > >Sorry, I read the thread and I tried to understand what was going on but > >I'm still lost. Can you clarify further in the commit text somehow? > > So I can add this in the commit text, if it seems fine > > "The use of pm_clk_add_notifier() isn't appropriate here since we need > to only manage clocks with valid power domain associations done via > DT, instead of what pm_clk_add_notifier() does, which is manage clocks > for all on SoC/off SoC devices associating all of them to a dummy > power domain instead" > Yes that's good, thanks. But I still wonder why the code isn't generic so that we don't have lots of drivers duplicating the same logic. I guess we can consolidate another day.
> On 07/29, Rajendra Nayak wrote: >> >> On 07/29/2015 06:34 AM, Stephen Boyd wrote: >> > >> >Sorry, I read the thread and I tried to understand what was going on >> but >> >I'm still lost. Can you clarify further in the commit text somehow? >> >> So I can add this in the commit text, if it seems fine >> >> "The use of pm_clk_add_notifier() isn't appropriate here since we need >> to only manage clocks with valid power domain associations done via >> DT, instead of what pm_clk_add_notifier() does, which is manage clocks >> for all on SoC/off SoC devices associating all of them to a dummy >> power domain instead" >> > > Yes that's good, thanks. But I still wonder why the code isn't > generic so that we don't have lots of drivers duplicating the > same logic. I guess we can consolidate another day. Yes, the code is quite generic and probably needs to be some place so it can be resued across drivers. For now I don;t see anyone else needing it, maybe sh mobile might plan to use something like it at a later time which is when we should probably consolidate.
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 3125809..9ddd2f8 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -16,6 +16,7 @@ #include <linux/jiffies.h> #include <linux/pm_clock.h> #include <linux/slab.h> +#include <linux/platform_device.h> #include "gdsc.h" #define PWR_ON_MASK BIT(31) @@ -200,3 +201,61 @@ void gdsc_unregister(struct device *dev) { of_genpd_del_provider(dev->of_node); } + +#ifndef CONFIG_PM +static void enable_clock(struct device *dev, const char *con_id) +{ + struct clk *clk; + + clk = clk_get(dev, con_id); + if (!IS_ERR(clk)) { + clk_prepare_enable(clk); + clk_put(clk); + } +} + +static void disable_clock(struct device *dev, const char *con_id) +{ + struct clk *clk; + + clk = clk_get(dev, con_id); + if (!IS_ERR(clk)) { + clk_disable_unprepare(clk); + clk_put(clk); + } +} + +static int clk_notify(struct notifier_block *nb, unsigned long action, + void *data) +{ + int sz; + struct device *dev = data; + char **con_id, *con_ids[] = { "core", "iface", NULL }; + + if (!of_find_property(dev->of_node, "power-domains", &sz)) + return 0; + + switch (action) { + case BUS_NOTIFY_BIND_DRIVER: + for (con_id = con_ids; *con_id; con_id++) + enable_clock(dev, *con_id); + break; + case BUS_NOTIFY_UNBOUND_DRIVER: + for (con_id = con_ids; *con_id; con_id++) + disable_clock(dev, *con_id); + break; + } + return 0; +} + +struct notifier_block nb = { + .notifier_call = clk_notify, +}; + +int qcom_pm_runtime_init(void) +{ + bus_register_notifier(&platform_bus_type, &nb); + return 0; +} +core_initcall(qcom_pm_runtime_init); +#endif
With CONFIG_PM disabled, turn the devices clocks on during driver binding to the device, and turn them off when the driver is unbound from the device. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/clk/qcom/gdsc.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)