Message ID | 1398334403-26181-4-git-send-email-geert+renesas@glider.be (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 24 April 2014 12:13, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > When adding a device from DT, check if its clocks are suitable for Runtime > PM, and register them with the PM core. > If Runtime PM is disabled, just enable the clock. > > This allows the PM core to automatically manage gate clocks of devices for > Runtime PM. Normally I don't think it's a good idea to "automatically" manage clocks from PM core or any other place but from the driver (and possibly the subsystem). The reason is simply that we hide things that normally is supposed to be handled by the driver. Typically a cross SOC driver should work fine both with and without a pm_domain. It should also not rely on CONFIG_PM_RUNTIME. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/of/Makefile | 1 + > drivers/of/of_clk.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/of/platform.c | 3 ++ > include/linux/of_clk.h | 18 +++++++++ > 4 files changed, 125 insertions(+) > create mode 100644 drivers/of/of_clk.c > create mode 100644 include/linux/of_clk.h > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > index ed9660adad77..49bcd413906f 100644 > --- a/drivers/of/Makefile > +++ b/drivers/of/Makefile > @@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o > obj-$(CONFIG_OF_MTD) += of_mtd.o > obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o > +obj-$(CONFIG_COMMON_CLK) += of_clk.o > diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c > new file mode 100644 > index 000000000000..35f5e9f3dd42 > --- /dev/null > +++ b/drivers/of/of_clk.c > @@ -0,0 +1,103 @@ > +/* > + * Copyright (C) 2014 Glider bvba > + */ > + > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/of.h> > +#include <linux/of_clk.h> > +#include <linux/platform_device.h> > +#include <linux/pm_clock.h> > +#include <linux/pm_runtime.h> > + > + > +#ifdef CONFIG_PM_RUNTIME > + > +static int of_clk_pm_runtime_suspend(struct device *dev) > +{ > + int ret; > + > + ret = pm_generic_runtime_suspend(dev); > + if (ret) > + return ret; > + > + ret = pm_clk_suspend(dev); > + if (ret) { > + pm_generic_runtime_resume(dev); > + return ret; > + } > + > + return 0; > +} > + > +static int of_clk_pm_runtime_resume(struct device *dev) > +{ > + pm_clk_resume(dev); > + return pm_generic_runtime_resume(dev); > +} > + > +static struct dev_pm_domain of_clk_pm_domain = { > + .ops = { > + .runtime_suspend = of_clk_pm_runtime_suspend, > + .runtime_resume = of_clk_pm_runtime_resume, > + USE_PLATFORM_PM_SLEEP_OPS > + }, > +}; > + > +static int of_clk_register(struct device *dev, struct clk *clk) > +{ > + int error; > + > + if (!dev->pm_domain) { > + error = pm_clk_create(dev); > + if (error) > + return error; > + > + dev->pm_domain = &of_clk_pm_domain; I am concerned about how this will work in conjunction with the generic power domain. A device can't reside in more than one pm_domain; thus I think it would be better to always use the generic power domain and not have a specific one for clocks. Typically the genpd should invoke pm_clk_resume|suspend from it's runtime PM callbacks. > + } > + > + dev_dbg(dev, "Setting up clock for runtime PM management\n"); > + return pm_clk_add_clk(dev, clk); > +} > + > +#else /* !CONFIG_PM_RUNTIME */ > + > +static int of_clk_register(struct device *dev, struct clk *clk) > +{ > + dev_dbg(dev, "Runtime PM disabled, enabling clock\n"); > + return clk_prepare_enable(clk); > +} > + > +#endif /* !CONFIG_PM_RUNTIME */ > + > + > +/** > + * of_clk_register_runtime_pm_clocks - Register clocks suitable for Runtime PM > + * with the PM core > + * @np: pointer to device tree node > + * @pdev: pointer to corresponding device to register suitable clocks for > + * > + * Returns an error code > + */ > +int of_clk_register_runtime_pm_clocks(struct device_node *np, > + struct device *dev) > +{ > + unsigned int i; > + struct clk *clk; > + int error; > + > + for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) { > + if (!clk_may_runtime_pm(clk)) { > + clk_put(clk); > + continue; > + } > + > + error = of_clk_register(dev, clk); > + if (error) { > + clk_put(clk); > + return error; > + } > + } > + > + return 0; > +} > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 404d1daebefa..29145302b6f8 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -18,6 +18,7 @@ > #include <linux/dma-mapping.h> > #include <linux/slab.h> > #include <linux/of_address.h> > +#include <linux/of_clk.h> > #include <linux/of_device.h> > #include <linux/of_irq.h> > #include <linux/of_platform.h> > @@ -182,6 +183,8 @@ struct platform_device *of_device_alloc(struct device_node *np, > else > of_device_make_bus_id(&dev->dev); > > + of_clk_register_runtime_pm_clocks(np, &dev->dev); > + What about other device than platform devices? Could we handle the DT binding at driver core at probe instead? Kind regards Ulf Hansson > return dev; > } > EXPORT_SYMBOL(of_device_alloc); > diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h > new file mode 100644 > index 000000000000..b9b15614e39b > --- /dev/null > +++ b/include/linux/of_clk.h > @@ -0,0 +1,18 @@ > +#ifndef _LINUX_OF_CLK_H > +#define _LINUX_OF_CLK_H > + > +struct device_node; > +struct device; > + > +#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) > +int of_clk_register_runtime_pm_clocks(struct device_node *np, > + struct device *dev); > +#else > +static inline int of_clk_register_runtime_pm_clocks(struct device_node *np, > + struct device *dev) > +{ > + return 0; > +} > +#endif /* CONFIG_OF && CONFIG_COMMON_CLK */ > + > +#endif /* _LINUX_OF_CLK_H */ > -- > 1.7.9.5 > > -- > 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Thu, Apr 24, 2014 at 3:11 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> +static int of_clk_register(struct device *dev, struct clk *clk) >> +{ >> + int error; >> + >> + if (!dev->pm_domain) { >> + error = pm_clk_create(dev); >> + if (error) >> + return error; >> + >> + dev->pm_domain = &of_clk_pm_domain; > > I am concerned about how this will work in conjunction with the > generic power domain. > > A device can't reside in more than one pm_domain; thus I think it > would be better to always use the generic power domain and not have a > specific one for clocks. Typically the genpd should invoke > pm_clk_resume|suspend from it's runtime PM callbacks. On shmobile SoCs supporting power domains, the power domain is overridden later by calling rmobile_add_devices_to_domains() and friends. My patch doesn't change that: the code behaved the same in the non-multi-platform case before: dev->pm_domain as set from drivers/sh/pm_runtime.c was overridden later. I'll have a deeper look into the power domain code later anyway. Thanks! 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven <geert+renesas@glider.be> writes: > When adding a device from DT, check if its clocks are suitable for Runtime > PM, and register them with the PM core. > If Runtime PM is disabled, just enable the clock. > > This allows the PM core to automatically manage gate clocks of devices for > Runtime PM. ...unless the device is already in an existing pm_domain, right? I like this approach, and it extends nicely what we already do on platforms using drivers/base/power/clock_ops.c into DT land. My only concern is how this will interact if it's used along with devices that have existing pm_domains. I don't have any specific concerns (yet, because it's Friday, and my brain is turing off), but it just made me wonder if this will be potentially confusing. Also... [...] > +static int of_clk_register(struct device *dev, struct clk *clk) > +{ > + int error; > + > + if (!dev->pm_domain) { > + error = pm_clk_create(dev); > + if (error) > + return error; > + > + dev->pm_domain = &of_clk_pm_domain; > + } > + > + dev_dbg(dev, "Setting up clock for runtime PM management\n"); > + return pm_clk_add_clk(dev, clk); I would've expected these 2 lines to be inside the pm_domain check. What's the reason for doing the pm_clk_add() when the pm_domain isn't going to be used? I suppose it's harmless, but it's a bit confusing. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24.04.2014 15:11, Ulf Hansson wrote: > On 24 April 2014 12:13, Geert Uytterhoeven <geert+renesas@glider.be> wrote: >> When adding a device from DT, check if its clocks are suitable for Runtime >> PM, and register them with the PM core. >> If Runtime PM is disabled, just enable the clock. >> >> This allows the PM core to automatically manage gate clocks of devices for >> Runtime PM. > > Normally I don't think it's a good idea to "automatically" manage > clocks from PM core or any other place but from the driver (and > possibly the subsystem). > > The reason is simply that we hide things that normally is supposed to > be handled by the driver. Typically a cross SOC driver should work > fine both with and without a pm_domain. It should also not rely on > CONFIG_PM_RUNTIME. > >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> drivers/of/Makefile | 1 + >> drivers/of/of_clk.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/of/platform.c | 3 ++ >> include/linux/of_clk.h | 18 +++++++++ >> 4 files changed, 125 insertions(+) >> create mode 100644 drivers/of/of_clk.c >> create mode 100644 include/linux/of_clk.h >> >> diff --git a/drivers/of/Makefile b/drivers/of/Makefile >> index ed9660adad77..49bcd413906f 100644 >> --- a/drivers/of/Makefile >> +++ b/drivers/of/Makefile >> @@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o >> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o >> obj-$(CONFIG_OF_MTD) += of_mtd.o >> obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o >> +obj-$(CONFIG_COMMON_CLK) += of_clk.o >> diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c >> new file mode 100644 >> index 000000000000..35f5e9f3dd42 >> --- /dev/null >> +++ b/drivers/of/of_clk.c >> @@ -0,0 +1,103 @@ >> +/* >> + * Copyright (C) 2014 Glider bvba >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/err.h> >> +#include <linux/of.h> >> +#include <linux/of_clk.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_clock.h> >> +#include <linux/pm_runtime.h> >> + >> + >> +#ifdef CONFIG_PM_RUNTIME >> + >> +static int of_clk_pm_runtime_suspend(struct device *dev) >> +{ >> + int ret; >> + >> + ret = pm_generic_runtime_suspend(dev); >> + if (ret) >> + return ret; >> + >> + ret = pm_clk_suspend(dev); >> + if (ret) { >> + pm_generic_runtime_resume(dev); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int of_clk_pm_runtime_resume(struct device *dev) >> +{ >> + pm_clk_resume(dev); >> + return pm_generic_runtime_resume(dev); >> +} >> + >> +static struct dev_pm_domain of_clk_pm_domain = { >> + .ops = { >> + .runtime_suspend = of_clk_pm_runtime_suspend, >> + .runtime_resume = of_clk_pm_runtime_resume, >> + USE_PLATFORM_PM_SLEEP_OPS >> + }, >> +}; >> + >> +static int of_clk_register(struct device *dev, struct clk *clk) >> +{ >> + int error; >> + >> + if (!dev->pm_domain) { >> + error = pm_clk_create(dev); >> + if (error) >> + return error; >> + >> + dev->pm_domain = &of_clk_pm_domain; > > I am concerned about how this will work in conjunction with the > generic power domain. > > A device can't reside in more than one pm_domain; thus I think it > would be better to always use the generic power domain and not have a > specific one for clocks. Typically the genpd should invoke > pm_clk_resume|suspend from it's runtime PM callbacks. I'm not sure about this. A typical use case would be to gate clocks ASAP and then wait until device is idle long enough to consider turning off the power domain worthwhile. Also sometimes we may want to gate the clocks, but prevent power domain from being powered off to retain hardware state (e.g. because there is no way to read it and restore later). I believe, though, that for devices that are not inside a controllable power domain, this might be a good solution. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman <khilman@linaro.org> wrote: > Geert Uytterhoeven <geert+renesas@glider.be> writes: > > > When adding a device from DT, check if its clocks are suitable for Runtime > > PM, and register them with the PM core. > > If Runtime PM is disabled, just enable the clock. > > > > This allows the PM core to automatically manage gate clocks of devices for > > Runtime PM. > > ...unless the device is already in an existing pm_domain, right? > > I like this approach, and it extends nicely what we already do on > platforms using drivers/base/power/clock_ops.c into DT land. > > My only concern is how this will interact if it's used along with > devices that have existing pm_domains. I don't have any specific > concerns (yet, because it's Friday, and my brain is turing off), but it > just made me wonder if this will be potentially confusing. I have big concerns about this approach. First, it will only work if a clock is available at deivce creation time. The conversion of irq controllers to normal device drivers has already shown that is a bad idea. I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf and Geert, On Thursday 24 April 2014 15:11:24 Ulf Hansson wrote: > On 24 April 2014 12:13, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > When adding a device from DT, check if its clocks are suitable for Runtime > > PM, and register them with the PM core. > > If Runtime PM is disabled, just enable the clock. > > > > This allows the PM core to automatically manage gate clocks of devices for > > Runtime PM. > > Normally I don't think it's a good idea to "automatically" manage > clocks from PM core or any other place but from the driver (and > possibly the subsystem). > > The reason is simply that we hide things that normally is supposed to > be handled by the driver. Typically a cross SOC driver should work > fine both with and without a pm_domain. It should also not rely on > CONFIG_PM_RUNTIME. That's a very good point. Geert, what do you think should happen if CONFIG_PM_RUNTIME is not set ? I don't have a strong opinion (yet) on whether we could require CONFIG_PM_RUNTIME, but it would indeed be nice to support both cases. One option would be to keep the clocks enabled unconditionally in that case, as not setting CONFIG_PM_RUNTIME means that the user doesn't care (or cares less) about power consumption. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
On Tuesday 29 April 2014 14:16:10 Grant Likely wrote: > On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman <khilman@linaro.org> wrote: > > Geert Uytterhoeven <geert+renesas@glider.be> writes: > > > When adding a device from DT, check if its clocks are suitable for > > > Runtime PM, and register them with the PM core. > > > If Runtime PM is disabled, just enable the clock. > > > > > > This allows the PM core to automatically manage gate clocks of devices > > > for Runtime PM. > > > > ...unless the device is already in an existing pm_domain, right? > > > > I like this approach, and it extends nicely what we already do on > > platforms using drivers/base/power/clock_ops.c into DT land. > > > > My only concern is how this will interact if it's used along with > > devices that have existing pm_domains. I don't have any specific > > concerns (yet, because it's Friday, and my brain is turing off), but it > > just made me wonder if this will be potentially confusing. > > I have big concerns about this approach. First, it will only work if > a clock is available at deivce creation time. The conversion of irq > controllers to normal device drivers has already shown that is a bad > idea. > > I also don't like that it tries to set up every clock, but there is no > guarantee that the driver will even use it. I would rather see this > behaviour linked into the function that obtains the clock at driver > .probe() time. That way it can handle deferred probe correctly and it > only sets up clocks that are actually used by the driver. I like the idea, as it gives an opt-in approach to the problem: drivers could decide whether they want the runtime PM core to handle clocks automatically (which should cover most cases), but would have the option of handling clocks manually if needed for special purposes.
On 30/04/14 14:25, Laurent Pinchart wrote: > On Tuesday 29 April 2014 14:16:10 Grant Likely wrote: >> On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman <khilman@linaro.org> wrote: >>> Geert Uytterhoeven <geert+renesas@glider.be> writes: >>>> When adding a device from DT, check if its clocks are suitable for >>>> Runtime PM, and register them with the PM core. >>>> If Runtime PM is disabled, just enable the clock. >>>> >>>> This allows the PM core to automatically manage gate clocks of devices >>>> for Runtime PM. >>> >>> ...unless the device is already in an existing pm_domain, right? >>> >>> I like this approach, and it extends nicely what we already do on >>> platforms using drivers/base/power/clock_ops.c into DT land. >>> >>> My only concern is how this will interact if it's used along with >>> devices that have existing pm_domains. I don't have any specific >>> concerns (yet, because it's Friday, and my brain is turing off), but it >>> just made me wonder if this will be potentially confusing. >> >> I have big concerns about this approach. First, it will only work if >> a clock is available at deivce creation time. The conversion of irq >> controllers to normal device drivers has already shown that is a bad >> idea. >> >> I also don't like that it tries to set up every clock, but there is no >> guarantee that the driver will even use it. I would rather see this >> behaviour linked into the function that obtains the clock at driver >> .probe() time. That way it can handle deferred probe correctly and it >> only sets up clocks that are actually used by the driver. > > I like the idea, as it gives an opt-in approach to the problem: drivers could > decide whether they want the runtime PM core to handle clocks automatically > (which should cover most cases), but would have the option of handling clocks > manually if needed for special purposes. If drivers could have a field to say that they allow the driver-core or the pm-runtime would mean that drivers could easily be change without having to deal with what the SoC/SoC family have to care about this. It would also mean that we could change drivers without having to make any changes to the SoC to say that it has to opt-in to the support.
Hi Kevin, On Sat, Apr 26, 2014 at 1:44 AM, Kevin Hilman <khilman@linaro.org> wrote: > Geert Uytterhoeven <geert+renesas@glider.be> writes: >> When adding a device from DT, check if its clocks are suitable for Runtime >> PM, and register them with the PM core. >> If Runtime PM is disabled, just enable the clock. >> >> This allows the PM core to automatically manage gate clocks of devices for >> Runtime PM. > > ...unless the device is already in an existing pm_domain, right? At this point in the kernel boot process, the device cannot be in a pm_domain yet. > I like this approach, and it extends nicely what we already do on > platforms using drivers/base/power/clock_ops.c into DT land. > > My only concern is how this will interact if it's used along with > devices that have existing pm_domains. I don't have any specific > concerns (yet, because it's Friday, and my brain is turing off), but it > just made me wonder if this will be potentially confusing. Adding devices to pm_domains is done later, so it can be overridden. > Also... > > [...] > >> +static int of_clk_register(struct device *dev, struct clk *clk) >> +{ >> + int error; >> + >> + if (!dev->pm_domain) { >> + error = pm_clk_create(dev); >> + if (error) >> + return error; >> + >> + dev->pm_domain = &of_clk_pm_domain; >> + } >> + >> + dev_dbg(dev, "Setting up clock for runtime PM management\n"); >> + return pm_clk_add_clk(dev, clk); > > I would've expected these 2 lines to be inside the pm_domain check. > > What's the reason for doing the pm_clk_add() when the pm_domain isn't > going to be used? I suppose it's harmless, but it's a bit confusing. Sorry, the !dev->pm_domain check does deserve a comment explaining this. If there are multiple clocks suitable for pm_runtime, the pm_clk_create(dev) should be done only once. Currently e.g. davinci registers 3 clocks with pm_clk ("fck", "master", and "slave"). Omap has 2 ("fck" and "ick"). BTW, keystone doesn't seem to set pm_clk_notifier_block.con_ids. From a quick look, this will crash with a NULL-pointer dereference in pm_clk_notify()? Or am I missing something here? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Grant, On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman <khilman@linaro.org> wrote: >> Geert Uytterhoeven <geert+renesas@glider.be> writes: >> >> > When adding a device from DT, check if its clocks are suitable for Runtime >> > PM, and register them with the PM core. >> > If Runtime PM is disabled, just enable the clock. >> > >> > This allows the PM core to automatically manage gate clocks of devices for >> > Runtime PM. >> >> ...unless the device is already in an existing pm_domain, right? >> >> I like this approach, and it extends nicely what we already do on >> platforms using drivers/base/power/clock_ops.c into DT land. >> >> My only concern is how this will interact if it's used along with >> devices that have existing pm_domains. I don't have any specific >> concerns (yet, because it's Friday, and my brain is turing off), but it >> just made me wonder if this will be potentially confusing. > > I have big concerns about this approach. First, it will only work if > a clock is available at deivce creation time. The conversion of irq > controllers to normal device drivers has already shown that is a bad > idea. That's indeed a valid concern that needs to be addressed. > I also don't like that it tries to set up every clock, but there is no > guarantee that the driver will even use it. I would rather see this > behaviour linked into the function that obtains the clock at driver > .probe() time. That way it can handle deferred probe correctly and it > only sets up clocks that are actually used by the driver. Not every clock. Only the clocks that are advertised by the clock driver as being suitable for runtime_pm management. These are typically module clocks, that must be enabled for the module to work. The driver doesn't always want to handle these explicitly. In fact we have one case on shmobile where the module clock for an IP core (rcar-gpio) is enabled unconditionally in one SoC, while it became controllable through a gate clock in a later SoC. With my patch, just adding the clock to the DT node is sufficient to make it work. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Laurent, On Wed, Apr 30, 2014 at 11:23 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Thursday 24 April 2014 15:11:24 Ulf Hansson wrote: >> On 24 April 2014 12:13, Geert Uytterhoeven <geert+renesas@glider.be> wrote: >> > When adding a device from DT, check if its clocks are suitable for Runtime >> > PM, and register them with the PM core. >> > If Runtime PM is disabled, just enable the clock. >> > >> > This allows the PM core to automatically manage gate clocks of devices for >> > Runtime PM. >> >> Normally I don't think it's a good idea to "automatically" manage >> clocks from PM core or any other place but from the driver (and >> possibly the subsystem). >> >> The reason is simply that we hide things that normally is supposed to >> be handled by the driver. Typically a cross SOC driver should work >> fine both with and without a pm_domain. It should also not rely on >> CONFIG_PM_RUNTIME. > > That's a very good point. Geert, what do you think should happen if > CONFIG_PM_RUNTIME is not set ? I don't have a strong opinion (yet) on whether > we could require CONFIG_PM_RUNTIME, but it would indeed be nice to support > both cases. One option would be to keep the clocks enabled unconditionally in > that case, as not setting CONFIG_PM_RUNTIME means that the user doesn't care > (or cares less) about power consumption. This is already handled by my patch. If CONFIG_PM_RUNTIME is disabled, the clocks are enabled by calling clk_prepare_enabled(). 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Grant, > > On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman <khilman@linaro.org> wrote: > >> Geert Uytterhoeven <geert+renesas@glider.be> writes: > >> > >> > When adding a device from DT, check if its clocks are suitable for Runtime > >> > PM, and register them with the PM core. > >> > If Runtime PM is disabled, just enable the clock. > >> > > >> > This allows the PM core to automatically manage gate clocks of devices for > >> > Runtime PM. > >> > >> ...unless the device is already in an existing pm_domain, right? > >> > >> I like this approach, and it extends nicely what we already do on > >> platforms using drivers/base/power/clock_ops.c into DT land. > >> > >> My only concern is how this will interact if it's used along with > >> devices that have existing pm_domains. I don't have any specific > >> concerns (yet, because it's Friday, and my brain is turing off), but it > >> just made me wonder if this will be potentially confusing. > > > > I have big concerns about this approach. First, it will only work if > > a clock is available at deivce creation time. The conversion of irq > > controllers to normal device drivers has already shown that is a bad > > idea. > > That's indeed a valid concern that needs to be addressed. > > > I also don't like that it tries to set up every clock, but there is no > > guarantee that the driver will even use it. I would rather see this > > behaviour linked into the function that obtains the clock at driver > > .probe() time. That way it can handle deferred probe correctly and it > > only sets up clocks that are actually used by the driver. > > Not every clock. Only the clocks that are advertised by the clock driver as > being suitable for runtime_pm management. These are typically module > clocks, that must be enabled for the module to work. The driver doesn't > always want to handle these explicitly. Help me out here becasue I don't understand how that works with this patch set. From my, admittedly naive, reading it looks like the setup is being done at device creation time, but if the driver (or module) gets to declare which clocks need to be enabled in order to work, then that information is not available at device creation time. > In fact we have one case on shmobile where the module clock for an IP > core (rcar-gpio) is enabled unconditionally in one SoC, while it became > controllable through a gate clock in a later SoC. > With my patch, just adding the clock to the DT node is sufficient to make > it work. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Grant, On Thu, May 1, 2014 at 10:03 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote: >> > I also don't like that it tries to set up every clock, but there is no >> > guarantee that the driver will even use it. I would rather see this >> > behaviour linked into the function that obtains the clock at driver >> > .probe() time. That way it can handle deferred probe correctly and it >> > only sets up clocks that are actually used by the driver. >> >> Not every clock. Only the clocks that are advertised by the clock driver as >> being suitable for runtime_pm management. These are typically module >> clocks, that must be enabled for the module to work. The driver doesn't >> always want to handle these explicitly. > > Help me out here becasue I don't understand how that works with this > patch set. From my, admittedly naive, reading it looks like the setup is > being done at device creation time, but if the driver (or module) gets > to declare which clocks need to be enabled in order to work, then that > information is not available at device creation time. Setup is indeed done at registration time. Note the check calling clk_may_runtime_pm(), which is introduced in "[PATCH/RFC 1/4] clk: Add CLK_RUNTIME_PM and clk_may_runtime_pm()". Clock drivers are initialized much earlier, so they can set the CLK_RUNTIME_PM flag for suitable clocks before platform devices are created from DT, cfr. the example for shmobile MSTP clocks in "[PATCH/RFC 4/4] clk: shmobile: mstp: Set CLK_RUNTIME_PM flag". I hope this makes it clear. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 1, 2014 at 2:41 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Grant, > > On Thu, May 1, 2014 at 10:03 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote: >>> > I also don't like that it tries to set up every clock, but there is no >>> > guarantee that the driver will even use it. I would rather see this >>> > behaviour linked into the function that obtains the clock at driver >>> > .probe() time. That way it can handle deferred probe correctly and it >>> > only sets up clocks that are actually used by the driver. >>> >>> Not every clock. Only the clocks that are advertised by the clock driver as >>> being suitable for runtime_pm management. These are typically module >>> clocks, that must be enabled for the module to work. The driver doesn't >>> always want to handle these explicitly. >> >> Help me out here becasue I don't understand how that works with this >> patch set. From my, admittedly naive, reading it looks like the setup is >> being done at device creation time, but if the driver (or module) gets >> to declare which clocks need to be enabled in order to work, then that >> information is not available at device creation time. > > Setup is indeed done at registration time. Note the check calling > clk_may_runtime_pm(), which is introduced in "[PATCH/RFC 1/4] clk: Add > CLK_RUNTIME_PM and clk_may_runtime_pm()". > > Clock drivers are initialized much earlier, so they can set the CLK_RUNTIME_PM > flag for suitable clocks before platform devices are created from DT, cfr. the > example for shmobile MSTP clocks in "[PATCH/RFC 4/4] clk: shmobile: mstp: > Set CLK_RUNTIME_PM flag". This is where I have issue. You're *assuming* clock drivers are initialized much earlier. That is not guaranteed. It is perfectly valid for clocks to be set up by a normal device driver, just like for interrupt controllers or gpio controllers. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Grant, On Thu, May 1, 2014 at 3:56 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Thu, May 1, 2014 at 2:41 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Thu, May 1, 2014 at 10:03 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>> On Wed, 30 Apr 2014 23:54:37 +0200, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>>> On Tue, Apr 29, 2014 at 3:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote: >>>> > I also don't like that it tries to set up every clock, but there is no >>>> > guarantee that the driver will even use it. I would rather see this >>>> > behaviour linked into the function that obtains the clock at driver >>>> > .probe() time. That way it can handle deferred probe correctly and it >>>> > only sets up clocks that are actually used by the driver. >>>> >>>> Not every clock. Only the clocks that are advertised by the clock driver as >>>> being suitable for runtime_pm management. These are typically module >>>> clocks, that must be enabled for the module to work. The driver doesn't >>>> always want to handle these explicitly. >>> >>> Help me out here becasue I don't understand how that works with this >>> patch set. From my, admittedly naive, reading it looks like the setup is >>> being done at device creation time, but if the driver (or module) gets >>> to declare which clocks need to be enabled in order to work, then that >>> information is not available at device creation time. >> >> Setup is indeed done at registration time. Note the check calling >> clk_may_runtime_pm(), which is introduced in "[PATCH/RFC 1/4] clk: Add >> CLK_RUNTIME_PM and clk_may_runtime_pm()". >> >> Clock drivers are initialized much earlier, so they can set the CLK_RUNTIME_PM >> flag for suitable clocks before platform devices are created from DT, cfr. the >> example for shmobile MSTP clocks in "[PATCH/RFC 4/4] clk: shmobile: mstp: >> Set CLK_RUNTIME_PM flag". > > This is where I have issue. You're *assuming* clock drivers are > initialized much earlier. That is not guaranteed. It is perfectly > valid for clocks to be set up by a normal device driver, just like for > interrupt controllers or gpio controllers. OK, I didn't know that. In that case, nothing happens, and everything works like before. So the drivers still have to take care of the clocks themselves. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> >> Normally I don't think it's a good idea to "automatically" manage >> clocks from PM core or any other place but from the driver (and >> possibly the subsystem). >> >> The reason is simply that we hide things that normally is supposed to >> be handled by the driver. Typically a cross SOC driver should work >> fine both with and without a pm_domain. It should also not rely on >> CONFIG_PM_RUNTIME. [Snip] >> >>> +static int of_clk_register(struct device *dev, struct clk *clk) >>> +{ >>> + int error; >>> + >>> + if (!dev->pm_domain) { >>> + error = pm_clk_create(dev); >>> + if (error) >>> + return error; >>> + >>> + dev->pm_domain = &of_clk_pm_domain; >> >> >> I am concerned about how this will work in conjunction with the >> generic power domain. >> >> A device can't reside in more than one pm_domain; thus I think it >> would be better to always use the generic power domain and not have a >> specific one for clocks. Typically the genpd should invoke >> pm_clk_resume|suspend from it's runtime PM callbacks. > > > I'm not sure about this. A typical use case would be to gate clocks ASAP and > then wait until device is idle long enough to consider turning off the power > domain worthwhile. Also sometimes we may want to gate the clocks, but > prevent power domain from being powered off to retain hardware state (e.g. > because there is no way to read it and restore later). So, in principle you prefer to have driver's handle clock gating to save power from their runtime PM callbacks, instead of from the power domain, right? Just to clarify, that's my view as well. Kind regards Ulf Hansson > > I believe, though, that for devices that are not inside a controllable power > domain, this might be a good solution. > > Best regards, > Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, Some more review comments. > + > + > +#ifdef CONFIG_PM_RUNTIME > + > +static int of_clk_pm_runtime_suspend(struct device *dev) > +{ > + int ret; > + > + ret = pm_generic_runtime_suspend(dev); > + if (ret) > + return ret; > + > + ret = pm_clk_suspend(dev); What about slow clocks? Those aren't handled with pm_clk_suspend(). > + if (ret) { > + pm_generic_runtime_resume(dev); > + return ret; > + } > + > + return 0; > +} > + > +static int of_clk_pm_runtime_resume(struct device *dev) > +{ > + pm_clk_resume(dev); What about slow clocks? Those aren't handled with pm_clk_resume(). > + return pm_generic_runtime_resume(dev); > +} > + > +static struct dev_pm_domain of_clk_pm_domain = { > + .ops = { > + .runtime_suspend = of_clk_pm_runtime_suspend, > + .runtime_resume = of_clk_pm_runtime_resume, Drivers/subsystems may invoke pm_runtime_force_suspend|resume() from some of their system PM callbacks, which requires the runtime PM callbacks to be defined for CONFIG_PM instead of CONFIG_PM_RUNTIME, I believe that should be changed here as well. > + USE_PLATFORM_PM_SLEEP_OPS What about other buses beside the platfrom bus. Certainly we need to handle devices attached to any other subsystem type as well. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Fri, May 2, 2014 at 10:56 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> +static int of_clk_pm_runtime_suspend(struct device *dev) >> +{ >> + int ret; >> + >> + ret = pm_generic_runtime_suspend(dev); >> + if (ret) >> + return ret; >> + >> + ret = pm_clk_suspend(dev); > > What about slow clocks? Those aren't handled with pm_clk_suspend(). How are slow clocks handled? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, Tomasz, On Fri, May 2, 2014 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> +static int of_clk_register(struct device *dev, struct clk *clk) >>>> +{ >>>> + int error; >>>> + >>>> + if (!dev->pm_domain) { >>>> + error = pm_clk_create(dev); >>>> + if (error) >>>> + return error; >>>> + >>>> + dev->pm_domain = &of_clk_pm_domain; >>> >>> >>> I am concerned about how this will work in conjunction with the >>> generic power domain. >>> >>> A device can't reside in more than one pm_domain; thus I think it >>> would be better to always use the generic power domain and not have a >>> specific one for clocks. Typically the genpd should invoke >>> pm_clk_resume|suspend from it's runtime PM callbacks. >> >> I'm not sure about this. A typical use case would be to gate clocks ASAP and >> then wait until device is idle long enough to consider turning off the power >> domain worthwhile. Also sometimes we may want to gate the clocks, but >> prevent power domain from being powered off to retain hardware state (e.g. >> because there is no way to read it and restore later). > > So, in principle you prefer to have driver's handle clock gating to > save power from their runtime PM callbacks, instead of from the power > domain, right? Just to clarify, that's my view as well. If there's both a gate clock and a power domain, and the driver's Runtime PM callbacks handle clock gating, who's handling the power domain? Gr{oetje,eeting}s, Geert (still trying to fit all pieces of the puzzle together) -- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 May 2014 16:35, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, > > On Fri, May 2, 2014 at 10:56 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> +static int of_clk_pm_runtime_suspend(struct device *dev) >>> +{ >>> + int ret; >>> + >>> + ret = pm_generic_runtime_suspend(dev); >>> + if (ret) >>> + return ret; >>> + >>> + ret = pm_clk_suspend(dev); >> >> What about slow clocks? Those aren't handled with pm_clk_suspend(). > > How are slow clocks handled? clk_prepare|unprepare - these functions may sleep. Kind regards Ulf Hansson > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 May 2014 16:58, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, Tomasz, > > On Fri, May 2, 2014 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> +static int of_clk_register(struct device *dev, struct clk *clk) >>>>> +{ >>>>> + int error; >>>>> + >>>>> + if (!dev->pm_domain) { >>>>> + error = pm_clk_create(dev); >>>>> + if (error) >>>>> + return error; >>>>> + >>>>> + dev->pm_domain = &of_clk_pm_domain; >>>> >>>> >>>> I am concerned about how this will work in conjunction with the >>>> generic power domain. >>>> >>>> A device can't reside in more than one pm_domain; thus I think it >>>> would be better to always use the generic power domain and not have a >>>> specific one for clocks. Typically the genpd should invoke >>>> pm_clk_resume|suspend from it's runtime PM callbacks. >>> >>> I'm not sure about this. A typical use case would be to gate clocks ASAP and >>> then wait until device is idle long enough to consider turning off the power >>> domain worthwhile. Also sometimes we may want to gate the clocks, but >>> prevent power domain from being powered off to retain hardware state (e.g. >>> because there is no way to read it and restore later). >> >> So, in principle you prefer to have driver's handle clock gating to >> save power from their runtime PM callbacks, instead of from the power >> domain, right? Just to clarify, that's my view as well. > > If there's both a gate clock and a power domain, and the driver's Runtime PM > callbacks handle clock gating, who's handling the power domain? This is my view, not sure everybody agrees :-) 1. If you have a hardware power domain you need to implement a pm_domain (preferably use the generic power domain). 2. If you don't have a hardware power domain, but still cares about having a centralized solution for dev_pm_qos - you may use the generic power domain, since it supports this. 3. If none of the above, you don't need a pm_domain at all. Kind regards Ulf Hansson > > Gr{oetje,eeting}s, > > Geert (still trying to fit all pieces of the > puzzle together) > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/of/Makefile b/drivers/of/Makefile index ed9660adad77..49bcd413906f 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o +obj-$(CONFIG_COMMON_CLK) += of_clk.o diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c new file mode 100644 index 000000000000..35f5e9f3dd42 --- /dev/null +++ b/drivers/of/of_clk.c @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2014 Glider bvba + */ + +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/of.h> +#include <linux/of_clk.h> +#include <linux/platform_device.h> +#include <linux/pm_clock.h> +#include <linux/pm_runtime.h> + + +#ifdef CONFIG_PM_RUNTIME + +static int of_clk_pm_runtime_suspend(struct device *dev) +{ + int ret; + + ret = pm_generic_runtime_suspend(dev); + if (ret) + return ret; + + ret = pm_clk_suspend(dev); + if (ret) { + pm_generic_runtime_resume(dev); + return ret; + } + + return 0; +} + +static int of_clk_pm_runtime_resume(struct device *dev) +{ + pm_clk_resume(dev); + return pm_generic_runtime_resume(dev); +} + +static struct dev_pm_domain of_clk_pm_domain = { + .ops = { + .runtime_suspend = of_clk_pm_runtime_suspend, + .runtime_resume = of_clk_pm_runtime_resume, + USE_PLATFORM_PM_SLEEP_OPS + }, +}; + +static int of_clk_register(struct device *dev, struct clk *clk) +{ + int error; + + if (!dev->pm_domain) { + error = pm_clk_create(dev); + if (error) + return error; + + dev->pm_domain = &of_clk_pm_domain; + } + + dev_dbg(dev, "Setting up clock for runtime PM management\n"); + return pm_clk_add_clk(dev, clk); +} + +#else /* !CONFIG_PM_RUNTIME */ + +static int of_clk_register(struct device *dev, struct clk *clk) +{ + dev_dbg(dev, "Runtime PM disabled, enabling clock\n"); + return clk_prepare_enable(clk); +} + +#endif /* !CONFIG_PM_RUNTIME */ + + +/** + * of_clk_register_runtime_pm_clocks - Register clocks suitable for Runtime PM + * with the PM core + * @np: pointer to device tree node + * @pdev: pointer to corresponding device to register suitable clocks for + * + * Returns an error code + */ +int of_clk_register_runtime_pm_clocks(struct device_node *np, + struct device *dev) +{ + unsigned int i; + struct clk *clk; + int error; + + for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) { + if (!clk_may_runtime_pm(clk)) { + clk_put(clk); + continue; + } + + error = of_clk_register(dev, clk); + if (error) { + clk_put(clk); + return error; + } + } + + return 0; +} diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 404d1daebefa..29145302b6f8 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -18,6 +18,7 @@ #include <linux/dma-mapping.h> #include <linux/slab.h> #include <linux/of_address.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/of_irq.h> #include <linux/of_platform.h> @@ -182,6 +183,8 @@ struct platform_device *of_device_alloc(struct device_node *np, else of_device_make_bus_id(&dev->dev); + of_clk_register_runtime_pm_clocks(np, &dev->dev); + return dev; } EXPORT_SYMBOL(of_device_alloc); diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h new file mode 100644 index 000000000000..b9b15614e39b --- /dev/null +++ b/include/linux/of_clk.h @@ -0,0 +1,18 @@ +#ifndef _LINUX_OF_CLK_H +#define _LINUX_OF_CLK_H + +struct device_node; +struct device; + +#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) +int of_clk_register_runtime_pm_clocks(struct device_node *np, + struct device *dev); +#else +static inline int of_clk_register_runtime_pm_clocks(struct device_node *np, + struct device *dev) +{ + return 0; +} +#endif /* CONFIG_OF && CONFIG_COMMON_CLK */ + +#endif /* _LINUX_OF_CLK_H */
When adding a device from DT, check if its clocks are suitable for Runtime PM, and register them with the PM core. If Runtime PM is disabled, just enable the clock. This allows the PM core to automatically manage gate clocks of devices for Runtime PM. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/of/Makefile | 1 + drivers/of/of_clk.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/of/platform.c | 3 ++ include/linux/of_clk.h | 18 +++++++++ 4 files changed, 125 insertions(+) create mode 100644 drivers/of/of_clk.c create mode 100644 include/linux/of_clk.h