| Message ID | 1429778744-13352-3-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23 April 2015 at 10:45, Rajendra Nayak <rnayak@codeaurora.org> wrote: > Add runtime PM support to handle (core and iface) clocks for devices > without a controllable power domain. Once the drivers for these devices > are converted to use runtime PM apis, all clock handling (for core and > iface) from these drivers can then be removed. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index 480ebf6..92b0f6d 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -15,6 +15,7 @@ > #include <linux/err.h> > #include <linux/jiffies.h> > #include <linux/pm_clock.h> > +#include <linux/platform_device.h> > #include <linux/slab.h> > #include "gdsc.h" > > @@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev) > { > of_genpd_del_provider(dev->of_node); > } > + > +static struct dev_pm_domain default_qcom_pm_domain = { > + .ops = { > + USE_PM_CLK_RUNTIME_OPS > + USE_PLATFORM_PM_SLEEP_OPS > + }, > +}; > + > +static struct pm_clk_notifier_block qcom_pm_notifier = { > + .pm_domain = &default_qcom_pm_domain, > + .con_ids = { "core", "iface" }, > +}; > + > +static int __init qcom_pm_runtime_init(void) > +{ > + pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier); > + return 0; > +} > +core_initcall(qcom_pm_runtime_init); First, I don't follow how this code is related to GDSC. Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses. Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done. Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution. Kind regards Uffe
On 04/24/2015 03:33 PM, Ulf Hansson wrote: > On 23 April 2015 at 10:45, Rajendra Nayak <rnayak@codeaurora.org> wrote: >> Add runtime PM support to handle (core and iface) clocks for devices >> without a controllable power domain. Once the drivers for these devices >> are converted to use runtime PM apis, all clock handling (for core and >> iface) from these drivers can then be removed. >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- >> drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c >> index 480ebf6..92b0f6d 100644 >> --- a/drivers/clk/qcom/gdsc.c >> +++ b/drivers/clk/qcom/gdsc.c >> @@ -15,6 +15,7 @@ >> #include <linux/err.h> >> #include <linux/jiffies.h> >> #include <linux/pm_clock.h> >> +#include <linux/platform_device.h> >> #include <linux/slab.h> >> #include "gdsc.h" >> >> @@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev) >> { >> of_genpd_del_provider(dev->of_node); >> } >> + >> +static struct dev_pm_domain default_qcom_pm_domain = { >> + .ops = { >> + USE_PM_CLK_RUNTIME_OPS >> + USE_PLATFORM_PM_SLEEP_OPS >> + }, >> +}; >> + >> +static struct pm_clk_notifier_block qcom_pm_notifier = { >> + .pm_domain = &default_qcom_pm_domain, >> + .con_ids = { "core", "iface" }, >> +}; >> + >> +static int __init qcom_pm_runtime_init(void) >> +{ >> + pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier); >> + return 0; >> +} >> +core_initcall(qcom_pm_runtime_init); > > First, I don't follow how this code is related to GDSC. Actually its not. I should probably move it to a pm_runtime.c someplace in drivers/soc/qcom maybe. > > Second, I want to see less users of pm_clk_add_notifier() since it's > not the proper/long-term way of how to assign PM domain pointers to a > device. Instead that shall be done at device registration point. In > your case while parsing the DT nodes and adding devices onto to their > buses. but these are devices which do not really have a controllable power domain, they just have controllable clocks. > > Yes, I understand that it will requires quite some work to adopt to > this behaviour - but that how it shall be done. > > Finally, an important note, you don't need to use PM domains for these > devices at all and thus you don't need to fix what I propose. Instead > you only have to implement the runtime PM callbacks for each driver > and manage the clocks from there. That is probably also a easier > solution. But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it) And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)
On Fri, Apr 24, 2015 at 12:58 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >> Second, I want to see less users of pm_clk_add_notifier() since it's >> not the proper/long-term way of how to assign PM domain pointers to a >> device. Instead that shall be done at device registration point. In >> your case while parsing the DT nodes and adding devices onto to their >> buses. > > but these are devices which do not really have a controllable power > domain, they just have controllable clocks. > >> Yes, I understand that it will requires quite some work to adopt to >> this behaviour - but that how it shall be done. >> >> Finally, an important note, you don't need to use PM domains for these >> devices at all and thus you don't need to fix what I propose. Instead >> you only have to implement the runtime PM callbacks for each driver >> and manage the clocks from there. That is probably also a easier >> solution. > > But that would mean I repeat the same code in all drivers to do a > clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" > clocks. I thought we have clock_ops.c just to avoid that (atleast up > until we have a better way of doing it) > And there are atleast a few architecture which have used it to avoid the > duplication across all drivers (omap1/davinci/sh/keystone) At least for arm/shmobile, we're planning to move away from this, cfr. http://www.spinics.net/lists/linux-sh/msg41114.html Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 24 April 2015 at 12:58, Rajendra Nayak <rnayak@codeaurora.org> wrote: > > On 04/24/2015 03:33 PM, Ulf Hansson wrote: >> >> On 23 April 2015 at 10:45, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>> >>> Add runtime PM support to handle (core and iface) clocks for devices >>> without a controllable power domain. Once the drivers for these devices >>> are converted to use runtime PM apis, all clock handling (for core and >>> iface) from these drivers can then be removed. >>> >>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >>> --- >>> drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c >>> index 480ebf6..92b0f6d 100644 >>> --- a/drivers/clk/qcom/gdsc.c >>> +++ b/drivers/clk/qcom/gdsc.c >>> @@ -15,6 +15,7 @@ >>> #include <linux/err.h> >>> #include <linux/jiffies.h> >>> #include <linux/pm_clock.h> >>> +#include <linux/platform_device.h> >>> #include <linux/slab.h> >>> #include "gdsc.h" >>> >>> @@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev) >>> { >>> of_genpd_del_provider(dev->of_node); >>> } >>> + >>> +static struct dev_pm_domain default_qcom_pm_domain = { >>> + .ops = { >>> + USE_PM_CLK_RUNTIME_OPS >>> + USE_PLATFORM_PM_SLEEP_OPS >>> + }, >>> +}; >>> + >>> +static struct pm_clk_notifier_block qcom_pm_notifier = { >>> + .pm_domain = &default_qcom_pm_domain, >>> + .con_ids = { "core", "iface" }, >>> +}; >>> + >>> +static int __init qcom_pm_runtime_init(void) >>> +{ >>> + pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier); >>> + return 0; >>> +} >>> +core_initcall(qcom_pm_runtime_init); >> >> >> First, I don't follow how this code is related to GDSC. > > > Actually its not. I should probably move it to a pm_runtime.c someplace > in drivers/soc/qcom maybe. > >> >> Second, I want to see less users of pm_clk_add_notifier() since it's >> not the proper/long-term way of how to assign PM domain pointers to a >> device. Instead that shall be done at device registration point. In >> your case while parsing the DT nodes and adding devices onto to their >> buses. > > > but these are devices which do not really have a controllable power > domain, they just have controllable clocks. That's true. But I don't see an issue why we shouldn't allow to model this in DT through the existing bindings. If the DT guys think it's a bad idea, I will anyway think the proper way to assign PM domains pointers is at device registration point. > >> >> Yes, I understand that it will requires quite some work to adopt to >> this behaviour - but that how it shall be done. >> >> Finally, an important note, you don't need to use PM domains for these >> devices at all and thus you don't need to fix what I propose. Instead >> you only have to implement the runtime PM callbacks for each driver >> and manage the clocks from there. That is probably also a easier >> solution. > > > But that would mean I repeat the same code in all drivers to do a > clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" > clocks. I thought we have clock_ops.c just to avoid that (atleast up > until we have a better way of doing it) > And there are atleast a few architecture which have used it to avoid the > duplication across all drivers (omap1/davinci/sh/keystone) Yes, the "duplications" are unavoidable if you decide to follow my suggestion. On the other hand your drivers will be more "standalone" and not depending on that you have a PM domain attached to the device to actually work. In some cases that might even be useful, when you have a cross SOC driver used in various configurations. Kind regards Uffe
Geert Uytterhoeven <geert@linux-m68k.org> writes: > On Fri, Apr 24, 2015 at 12:58 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>> Second, I want to see less users of pm_clk_add_notifier() since it's >>> not the proper/long-term way of how to assign PM domain pointers to a >>> device. Instead that shall be done at device registration point. In >>> your case while parsing the DT nodes and adding devices onto to their >>> buses. >> >> but these are devices which do not really have a controllable power >> domain, they just have controllable clocks. >> >>> Yes, I understand that it will requires quite some work to adopt to >>> this behaviour - but that how it shall be done. >>> >>> Finally, an important note, you don't need to use PM domains for these >>> devices at all and thus you don't need to fix what I propose. Instead >>> you only have to implement the runtime PM callbacks for each driver >>> and manage the clocks from there. That is probably also a easier >>> solution. >> >> But that would mean I repeat the same code in all drivers to do a >> clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" >> clocks. I thought we have clock_ops.c just to avoid that (atleast up >> until we have a better way of doing it) >> And there are atleast a few architecture which have used it to avoid the >> duplication across all drivers (omap1/davinci/sh/keystone) > > At least for arm/shmobile, we're planning to move away from this, cfr. > http://www.spinics.net/lists/linux-sh/msg41114.html Just to clarify for Rajendra's sake... SH is moving away from the pm_clk_add_notifier(), but not duplicating the clk_get/prepare/enable/disable/unprepare across all the drivers. Rather, they're using a genpd to model the clock domain, and taking advantage of the pm_domain speciic attach callback to attach the PM clocks. This should work for qcom also assuming that these device nodes don't also need to belong to a different PM domain. Kevin
On 04/28/2015 01:32 AM, Kevin Hilman wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > >> On Fri, Apr 24, 2015 at 12:58 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>>> Second, I want to see less users of pm_clk_add_notifier() since it's >>>> not the proper/long-term way of how to assign PM domain pointers to a >>>> device. Instead that shall be done at device registration point. In >>>> your case while parsing the DT nodes and adding devices onto to their >>>> buses. >>> >>> but these are devices which do not really have a controllable power >>> domain, they just have controllable clocks. >>> >>>> Yes, I understand that it will requires quite some work to adopt to >>>> this behaviour - but that how it shall be done. >>>> >>>> Finally, an important note, you don't need to use PM domains for these >>>> devices at all and thus you don't need to fix what I propose. Instead >>>> you only have to implement the runtime PM callbacks for each driver >>>> and manage the clocks from there. That is probably also a easier >>>> solution. >>> >>> But that would mean I repeat the same code in all drivers to do a >>> clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" >>> clocks. I thought we have clock_ops.c just to avoid that (atleast up >>> until we have a better way of doing it) >>> And there are atleast a few architecture which have used it to avoid the >>> duplication across all drivers (omap1/davinci/sh/keystone) >> >> At least for arm/shmobile, we're planning to move away from this, cfr. >> http://www.spinics.net/lists/linux-sh/msg41114.html > > Just to clarify for Rajendra's sake... > > SH is moving away from the pm_clk_add_notifier(), but not duplicating > the clk_get/prepare/enable/disable/unprepare across all the drivers. > > Rather, they're using a genpd to model the clock domain, and taking > advantage of the pm_domain speciic attach callback to attach the PM > clocks. > > This should work for qcom also assuming that these device nodes don't > also need to belong to a different PM domain. Thanks Kevin, I did look up the patches that Geert pointed me to, and figured I can do something similar for qcom as well like you said. There are 2 types of devices that I will need to handle, one which have clocks and also a power switch to turn the power domain on/off (camera, graphics, display), and others which only have clocks and no power switch to control the power domain (serial, sdhc, i2c, spi). I was already using genpd attach/detach to handle clocks for the devices with a power switch and genpd on/off to turn the PD on and off. I guess I can also control the rest of the devices the same way, just that the genpd on/off for them would do nothing. That way I don't have to use pm_clk_add_notifier() and can also associate the power domain (with no on/off control) to devices through DT (and there isn;t any duplication of code in the drivers)
Hi Rajendra, On Tue, Apr 28, 2015 at 4:52 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>>> But that would mean I repeat the same code in all drivers to do a >>>> clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" >>>> clocks. I thought we have clock_ops.c just to avoid that (atleast up >>>> until we have a better way of doing it) >>>> And there are atleast a few architecture which have used it to avoid the >>>> duplication across all drivers (omap1/davinci/sh/keystone) >>> >>> >>> At least for arm/shmobile, we're planning to move away from this, cfr. >>> http://www.spinics.net/lists/linux-sh/msg41114.html The above are for the pure-clock domain case (no power areas), on R-Car Gen2. >> Just to clarify for Rajendra's sake... >> >> SH is moving away from the pm_clk_add_notifier(), but not duplicating >> the clk_get/prepare/enable/disable/unprepare across all the drivers. >> >> Rather, they're using a genpd to model the clock domain, and taking >> advantage of the pm_domain speciic attach callback to attach the PM >> clocks. >> >> This should work for qcom also assuming that these device nodes don't >> also need to belong to a different PM domain. > > Thanks Kevin, I did look up the patches that Geert pointed me to, and > figured I can do something similar for qcom as well like you said. > > There are 2 types of devices that I will need to handle, one which have > clocks and also a power switch to turn the power domain on/off (camera, > graphics, display), and others which only have clocks and no power > switch to control the power domain (serial, sdhc, i2c, spi). > > I was already using genpd attach/detach to handle clocks for the > devices with a power switch and genpd on/off to turn the PD on and off. Good. > I guess I can also control the rest of the devices the same way, just > that the genpd on/off for them would do nothing. > That way I don't have to use pm_clk_add_notifier() and can also > associate the power domain (with no on/off control) to devices > through DT (and there isn;t any duplication of code in the drivers) That looks similar to what we have on R-Mobile: some devices are in controllable power areas, other are in an "always on" power area. All (most) devices have controllable clocks, which we also control through the PM domain. "git grep sysc-rmobile" will point you to the related code and DTS. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[].. >> I guess I can also control the rest of the devices the same way, just >> that the genpd on/off for them would do nothing. >> That way I don't have to use pm_clk_add_notifier() and can also >> associate the power domain (with no on/off control) to devices >> through DT (and there isn;t any duplication of code in the drivers) > > That looks similar to what we have on R-Mobile: some devices are in > controllable power areas, other are in an "always on" power area. All (most) > devices have controllable clocks, which we also control through the PM > domain. "git grep sysc-rmobile" will point you to the related code and DTS. Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled? >
Hi Rajendra, On Wed, Apr 29, 2015 at 11:49 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>> I guess I can also control the rest of the devices the same way, just >>> that the genpd on/off for them would do nothing. >>> That way I don't have to use pm_clk_add_notifier() and can also >>> associate the power domain (with no on/off control) to devices >>> through DT (and there isn;t any duplication of code in the drivers) >> >> >> That looks similar to what we have on R-Mobile: some devices are in >> controllable power areas, other are in an "always on" power area. All >> (most) >> devices have controllable clocks, which we also control through the PM >> domain. "git grep sysc-rmobile" will point you to the related code and >> DTS. > > Geert, thanks, I was wondering how you handle the !CONFIG_PM case for > rmobile. I mean who turns the clocks on for the devices when you build > with CONFIG_PM disabled? We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of pm_clk_notify() will enable the clocks at driver binding time. Hardware power domains are assumed enabled by reset state/the boot loader. Given the amount of PM infrastructure involved when hardware power and clock domains are involved, I think at one point we'll have to start restricting our builds to CONFIG_PM=y. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 29 April 2015 at 13:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Rajendra, > > On Wed, Apr 29, 2015 at 11:49 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote: >>>> I guess I can also control the rest of the devices the same way, just >>>> that the genpd on/off for them would do nothing. >>>> That way I don't have to use pm_clk_add_notifier() and can also >>>> associate the power domain (with no on/off control) to devices >>>> through DT (and there isn;t any duplication of code in the drivers) >>> >>> >>> That looks similar to what we have on R-Mobile: some devices are in >>> controllable power areas, other are in an "always on" power area. All >>> (most) >>> devices have controllable clocks, which we also control through the PM >>> domain. "git grep sysc-rmobile" will point you to the related code and >>> DTS. >> >> Geert, thanks, I was wondering how you handle the !CONFIG_PM case for >> rmobile. I mean who turns the clocks on for the devices when you build >> with CONFIG_PM disabled? > > We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if > CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of > pm_clk_notify() will enable the clocks at driver binding time. > Hardware power domains are assumed enabled by reset state/the boot > loader. Yes, it a bit tricky - but I guess that's the current only viable solution if we have when using the pm_clk API. > > Given the amount of PM infrastructure involved when hardware power and > clock domains are involved, I think at one point we'll have to start restricting > our builds to CONFIG_PM=y. I don't think that would solve the problem, since you may still have cross SoC drivers which may be used without CONFIG_PM. So all SoC that uses a driver which expects clocks to be managed using PM clocks from a PM domain, will need the above "trick". Right? Kind regards Uffe
Hi Ulf, On Wed, Apr 29, 2015 at 2:31 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Geert, thanks, I was wondering how you handle the !CONFIG_PM case for >>> rmobile. I mean who turns the clocks on for the devices when you build >>> with CONFIG_PM disabled? >> >> We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if >> CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of >> pm_clk_notify() will enable the clocks at driver binding time. >> Hardware power domains are assumed enabled by reset state/the boot >> loader. > > Yes, it a bit tricky - but I guess that's the current only viable > solution if we have when using the pm_clk API. > >> Given the amount of PM infrastructure involved when hardware power and >> clock domains are involved, I think at one point we'll have to start restricting >> our builds to CONFIG_PM=y. > > I don't think that would solve the problem, since you may still have > cross SoC drivers which may be used without CONFIG_PM. That's not as much of a problem as it sounds: - If the driver cares (needs to know) about the clock, it should already manage it itself. - If it doesn't care about the clock, it just needs Runtime PM support. That will do the right thing on platforms with (needs PM=y) and without (doesn't care about PM=x) clock domains. So the bigger "problem" is making sure all drivers have at least minimal Runtime PM support, else they can't be reused as-is on systems with PM domains. > So all SoC that uses a driver which expects clocks to be managed using > PM clocks from a PM domain, will need the above "trick". Right? One remaining issue with pm_clk_add_notifier() is that it applies to all platform devices in the system, not just the on-SoC devices. Hence it may inadvertently manage clocks for off-SoC devices it's not supposed to touch. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 29 April 2015 at 15:08, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, > > On Wed, Apr 29, 2015 at 2:31 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> Geert, thanks, I was wondering how you handle the !CONFIG_PM case for >>>> rmobile. I mean who turns the clocks on for the devices when you build >>>> with CONFIG_PM disabled? >>> >>> We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if >>> CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of >>> pm_clk_notify() will enable the clocks at driver binding time. >>> Hardware power domains are assumed enabled by reset state/the boot >>> loader. >> >> Yes, it a bit tricky - but I guess that's the current only viable >> solution if we have when using the pm_clk API. >> >>> Given the amount of PM infrastructure involved when hardware power and >>> clock domains are involved, I think at one point we'll have to start restricting >>> our builds to CONFIG_PM=y. >> >> I don't think that would solve the problem, since you may still have >> cross SoC drivers which may be used without CONFIG_PM. > > That's not as much of a problem as it sounds: > - If the driver cares (needs to know) about the clock, it should already > manage it itself. Agree! > - If it doesn't care about the clock, it just needs Runtime PM support. > That will do the right thing on platforms with (needs PM=y) and without > (doesn't care about PM=x) clock domains. How about those drivers that cares about clocks and thus manages them, but also cares about runtime PM? I believe we will get a clock reference count issue in these cases, since both the PM domain and the driver will manage the clocks. I assume that's why we have had a few attempts to have "runtime PM clocks" specially marked, one was via DT, to have clear distinguish between who will be responsible to manage them. Those attempts did get nacked, so the problem is still there I assume. > So the bigger "problem" is making sure all drivers have at least minimal > Runtime PM support, else they can't be reused as-is on systems with PM > domains. > >> So all SoC that uses a driver which expects clocks to be managed using >> PM clocks from a PM domain, will need the above "trick". Right? > > One remaining issue with pm_clk_add_notifier() is that it applies to all > platform devices in the system, not just the on-SoC devices. Hence it may > inadvertently manage clocks for off-SoC devices it's not supposed to touch. Yes. That's really bad. :-) Additionally, it means devices that isn't part of the platform bus isn't able to use PM clk domains at all. Within this context, I find it hard to advise people to use PM clk domains (via pm_clk_add_notifier()), since there just so many open issues. What works a _little_ better is to use PM clks via genpd. Kind regards Uffe
On 30 April 2015 at 08:21, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 29 April 2015 at 15:08, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> Hi Ulf, >> >> On Wed, Apr 29, 2015 at 2:31 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> Geert, thanks, I was wondering how you handle the !CONFIG_PM case for >>>>> rmobile. I mean who turns the clocks on for the devices when you build >>>>> with CONFIG_PM disabled? >>>> >>>> We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if >>>> CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of >>>> pm_clk_notify() will enable the clocks at driver binding time. >>>> Hardware power domains are assumed enabled by reset state/the boot >>>> loader. >>> >>> Yes, it a bit tricky - but I guess that's the current only viable >>> solution if we have when using the pm_clk API. >>> >>>> Given the amount of PM infrastructure involved when hardware power and >>>> clock domains are involved, I think at one point we'll have to start restricting >>>> our builds to CONFIG_PM=y. >>> >>> I don't think that would solve the problem, since you may still have >>> cross SoC drivers which may be used without CONFIG_PM. >> >> That's not as much of a problem as it sounds: >> - If the driver cares (needs to know) about the clock, it should already >> manage it itself. > > Agree! > >> - If it doesn't care about the clock, it just needs Runtime PM support. >> That will do the right thing on platforms with (needs PM=y) and without >> (doesn't care about PM=x) clock domains. > > How about those drivers that cares about clocks and thus manages them, > but also cares about runtime PM? > > I believe we will get a clock reference count issue in these cases, > since both the PM domain and the driver will manage the clocks. > > I assume that's why we have had a few attempts to have "runtime PM > clocks" specially marked, one was via DT, to have clear distinguish > between who will be responsible to manage them. > > Those attempts did get nacked, so the problem is still there I assume. > >> So the bigger "problem" is making sure all drivers have at least minimal >> Runtime PM support, else they can't be reused as-is on systems with PM >> domains. >> >>> So all SoC that uses a driver which expects clocks to be managed using >>> PM clocks from a PM domain, will need the above "trick". Right? >> >> One remaining issue with pm_clk_add_notifier() is that it applies to all >> platform devices in the system, not just the on-SoC devices. Hence it may >> inadvertently manage clocks for off-SoC devices it's not supposed to touch. > > Yes. That's really bad. :-) > > Additionally, it means devices that isn't part of the platform bus > isn't able to use PM clk domains at all. Correction: Of course they can register one PM clk notifier per bus type. The API currently also provides this option. > > Within this context, I find it hard to advise people to use PM clk > domains (via pm_clk_add_notifier()), since there just so many open > issues. What works a _little_ better is to use PM clks via genpd. > > Kind regards > Uffe
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 480ebf6..92b0f6d 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -15,6 +15,7 @@ #include <linux/err.h> #include <linux/jiffies.h> #include <linux/pm_clock.h> +#include <linux/platform_device.h> #include <linux/slab.h> #include "gdsc.h" @@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev) { of_genpd_del_provider(dev->of_node); } + +static struct dev_pm_domain default_qcom_pm_domain = { + .ops = { + USE_PM_CLK_RUNTIME_OPS + USE_PLATFORM_PM_SLEEP_OPS + }, +}; + +static struct pm_clk_notifier_block qcom_pm_notifier = { + .pm_domain = &default_qcom_pm_domain, + .con_ids = { "core", "iface" }, +}; + +static int __init qcom_pm_runtime_init(void) +{ + pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier); + return 0; +} +core_initcall(qcom_pm_runtime_init);
Add runtime PM support to handle (core and iface) clocks for devices without a controllable power domain. Once the drivers for these devices are converted to use runtime PM apis, all clock handling (for core and iface) from these drivers can then be removed. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)