[PATCH/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core
Ulf Hansson
ulf.hansson at linaro.org
Thu Apr 24 06:11:24 PDT 2014
On 24 April 2014 12:13, Geert Uytterhoeven <geert+renesas at 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 at 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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
More information about the linux-arm-kernel
mailing list