From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 08/16] clk: mvebu: add suspend/resume for gatable clocks
Date: Mon, 17 Nov 2014 14:46:04 -0800 [thread overview]
Message-ID: <20141117224604.25314.96198@quantum> (raw)
In-Reply-To: <1415978496-9334-9-git-send-email-thomas.petazzoni@free-electrons.com>
Quoting Thomas Petazzoni (2014-11-14 07:21:28)
> This commit adds suspend/resume support for the gatable clock driver
> used on Marvell EBU platforms. When getting out of suspend, the
> Marvell EBU platforms go through the bootloader, which re-enables all
> gatable clocks. However, upon resume, the clock framework will not
> disable again all gatable clocks that are not used.
>
> Therefore, if the clock driver does not save/restore the state of the
> gatable clocks, all gatable clocks that are not claimed by any device
> driver will remain enabled after a resume. This is why this driver
> saves and restores the state of those clocks.
It might be a good idea to call clk_disable_unused() from the clk core
after resuming from suspend.
>
> Since clocks aren't real devices, we don't have the normal ->suspend()
> and ->resume() of the device model, and have to use the ->suspend()
> and ->resume() hooks of the syscore_ops mechanism. This mechanism has
> the unfortunate idea of not providing a way of passing private data,
> which requires us to change the driver to make the assumption that
> there is only once instance of the gatable clock control structure.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: linux-kernel at vger.kernel.org
> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clk/mvebu/common.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> index b7fcb46..8799fb8 100644
> --- a/drivers/clk/mvebu/common.c
> +++ b/drivers/clk/mvebu/common.c
> @@ -19,6 +19,7 @@
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
>
> #include "common.h"
>
> @@ -177,14 +178,18 @@ struct clk_gating_ctrl {
> spinlock_t *lock;
> struct clk **gates;
> int num_gates;
> + struct syscore_ops syscore_ops;
You are registering suspend/resume ops per clock. Have you considered
registering a single set of ops for your clock controller driver? See
drivers/clk/samsung/clk-exynos5420.c for an example.
Combined with a table of clocks registered by your driver, centralized
suspend/resume methods might be a cleaner solution.
Regards,
Mike
> + void __iomem *base;
> + u32 saved_reg;
> };
>
> #define to_clk_gate(_hw) container_of(_hw, struct clk_gate, hw)
>
> +static struct clk_gating_ctrl *ctrl;
> +
> static struct clk *clk_gating_get_src(
> struct of_phandle_args *clkspec, void *data)
> {
> - struct clk_gating_ctrl *ctrl = (struct clk_gating_ctrl *)data;
> int n;
>
> if (clkspec->args_count < 1)
> @@ -199,15 +204,30 @@ static struct clk *clk_gating_get_src(
> return ERR_PTR(-ENODEV);
> }
>
> +static int mvebu_clk_gating_suspend(void)
> +{
> + ctrl->saved_reg = readl(ctrl->base);
> + return 0;
> +}
> +
> +static void mvebu_clk_gating_resume(void)
> +{
> + writel(ctrl->saved_reg, ctrl->base);
> +}
> +
> void __init mvebu_clk_gating_setup(struct device_node *np,
> const struct clk_gating_soc_desc *desc)
> {
> - struct clk_gating_ctrl *ctrl;
> struct clk *clk;
> void __iomem *base;
> const char *default_parent = NULL;
> int n;
>
> + if (ctrl) {
> + pr_err("mvebu-clk-gating: cannot instantiate more than one gatable clock device\n");
> + return;
> + }
> +
> base = of_iomap(np, 0);
> if (WARN_ON(!base))
> return;
> @@ -225,6 +245,10 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
> /* lock must already be initialized */
> ctrl->lock = &ctrl_gating_lock;
>
> + ctrl->base = base;
> + ctrl->syscore_ops.suspend = mvebu_clk_gating_suspend;
> + ctrl->syscore_ops.resume = mvebu_clk_gating_resume;
> +
> /* Count, allocate, and register clock gates */
> for (n = 0; desc[n].name;)
> n++;
> @@ -246,6 +270,8 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
>
> of_clk_add_provider(np, clk_gating_get_src, ctrl);
>
> + register_syscore_ops(&ctrl->syscore_ops);
> +
> return;
> gates_out:
> kfree(ctrl);
> --
> 2.1.0
>
WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>,
Sebastian Hesselbarth
<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Gregory Clement
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Tawfik Bayouk <tawfik-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
Nadav Haklai <nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
Lior Amsalem <alior-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
Ezequiel Garcia
<ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCHv2 08/16] clk: mvebu: add suspend/resume for gatable clocks
Date: Mon, 17 Nov 2014 14:46:04 -0800 [thread overview]
Message-ID: <20141117224604.25314.96198@quantum> (raw)
In-Reply-To: <1415978496-9334-9-git-send-email-thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Quoting Thomas Petazzoni (2014-11-14 07:21:28)
> This commit adds suspend/resume support for the gatable clock driver
> used on Marvell EBU platforms. When getting out of suspend, the
> Marvell EBU platforms go through the bootloader, which re-enables all
> gatable clocks. However, upon resume, the clock framework will not
> disable again all gatable clocks that are not used.
>
> Therefore, if the clock driver does not save/restore the state of the
> gatable clocks, all gatable clocks that are not claimed by any device
> driver will remain enabled after a resume. This is why this driver
> saves and restores the state of those clocks.
It might be a good idea to call clk_disable_unused() from the clk core
after resuming from suspend.
>
> Since clocks aren't real devices, we don't have the normal ->suspend()
> and ->resume() of the device model, and have to use the ->suspend()
> and ->resume() hooks of the syscore_ops mechanism. This mechanism has
> the unfortunate idea of not providing a way of passing private data,
> which requires us to change the driver to make the assumption that
> there is only once instance of the gatable clock control structure.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: Mike Turquette <mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Acked-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> drivers/clk/mvebu/common.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> index b7fcb46..8799fb8 100644
> --- a/drivers/clk/mvebu/common.c
> +++ b/drivers/clk/mvebu/common.c
> @@ -19,6 +19,7 @@
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
>
> #include "common.h"
>
> @@ -177,14 +178,18 @@ struct clk_gating_ctrl {
> spinlock_t *lock;
> struct clk **gates;
> int num_gates;
> + struct syscore_ops syscore_ops;
You are registering suspend/resume ops per clock. Have you considered
registering a single set of ops for your clock controller driver? See
drivers/clk/samsung/clk-exynos5420.c for an example.
Combined with a table of clocks registered by your driver, centralized
suspend/resume methods might be a cleaner solution.
Regards,
Mike
> + void __iomem *base;
> + u32 saved_reg;
> };
>
> #define to_clk_gate(_hw) container_of(_hw, struct clk_gate, hw)
>
> +static struct clk_gating_ctrl *ctrl;
> +
> static struct clk *clk_gating_get_src(
> struct of_phandle_args *clkspec, void *data)
> {
> - struct clk_gating_ctrl *ctrl = (struct clk_gating_ctrl *)data;
> int n;
>
> if (clkspec->args_count < 1)
> @@ -199,15 +204,30 @@ static struct clk *clk_gating_get_src(
> return ERR_PTR(-ENODEV);
> }
>
> +static int mvebu_clk_gating_suspend(void)
> +{
> + ctrl->saved_reg = readl(ctrl->base);
> + return 0;
> +}
> +
> +static void mvebu_clk_gating_resume(void)
> +{
> + writel(ctrl->saved_reg, ctrl->base);
> +}
> +
> void __init mvebu_clk_gating_setup(struct device_node *np,
> const struct clk_gating_soc_desc *desc)
> {
> - struct clk_gating_ctrl *ctrl;
> struct clk *clk;
> void __iomem *base;
> const char *default_parent = NULL;
> int n;
>
> + if (ctrl) {
> + pr_err("mvebu-clk-gating: cannot instantiate more than one gatable clock device\n");
> + return;
> + }
> +
> base = of_iomap(np, 0);
> if (WARN_ON(!base))
> return;
> @@ -225,6 +245,10 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
> /* lock must already be initialized */
> ctrl->lock = &ctrl_gating_lock;
>
> + ctrl->base = base;
> + ctrl->syscore_ops.suspend = mvebu_clk_gating_suspend;
> + ctrl->syscore_ops.resume = mvebu_clk_gating_resume;
> +
> /* Count, allocate, and register clock gates */
> for (n = 0; desc[n].name;)
> n++;
> @@ -246,6 +270,8 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
>
> of_clk_add_provider(np, clk_gating_get_src, ctrl);
>
> + register_syscore_ops(&ctrl->syscore_ops);
> +
> return;
> gates_out:
> kfree(ctrl);
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
"Jason Cooper" <jason@lakedaemon.net>,
"Andrew Lunn" <andrew@lunn.ch>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
"Gregory Clement" <gregory.clement@free-electrons.com>
Cc: linux-arm-kernel@lists.infradead.org,
"Tawfik Bayouk" <tawfik@marvell.com>,
"Nadav Haklai" <nadavh@marvell.com>,
"Lior Amsalem" <alior@marvell.com>,
"Ezequiel Garcia" <ezequiel.garcia@free-electrons.com>,
devicetree@vger.kernel.org,
"Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2 08/16] clk: mvebu: add suspend/resume for gatable clocks
Date: Mon, 17 Nov 2014 14:46:04 -0800 [thread overview]
Message-ID: <20141117224604.25314.96198@quantum> (raw)
In-Reply-To: <1415978496-9334-9-git-send-email-thomas.petazzoni@free-electrons.com>
Quoting Thomas Petazzoni (2014-11-14 07:21:28)
> This commit adds suspend/resume support for the gatable clock driver
> used on Marvell EBU platforms. When getting out of suspend, the
> Marvell EBU platforms go through the bootloader, which re-enables all
> gatable clocks. However, upon resume, the clock framework will not
> disable again all gatable clocks that are not used.
>
> Therefore, if the clock driver does not save/restore the state of the
> gatable clocks, all gatable clocks that are not claimed by any device
> driver will remain enabled after a resume. This is why this driver
> saves and restores the state of those clocks.
It might be a good idea to call clk_disable_unused() from the clk core
after resuming from suspend.
>
> Since clocks aren't real devices, we don't have the normal ->suspend()
> and ->resume() of the device model, and have to use the ->suspend()
> and ->resume() hooks of the syscore_ops mechanism. This mechanism has
> the unfortunate idea of not providing a way of passing private data,
> which requires us to change the driver to make the assumption that
> there is only once instance of the gatable clock control structure.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clk/mvebu/common.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> index b7fcb46..8799fb8 100644
> --- a/drivers/clk/mvebu/common.c
> +++ b/drivers/clk/mvebu/common.c
> @@ -19,6 +19,7 @@
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
>
> #include "common.h"
>
> @@ -177,14 +178,18 @@ struct clk_gating_ctrl {
> spinlock_t *lock;
> struct clk **gates;
> int num_gates;
> + struct syscore_ops syscore_ops;
You are registering suspend/resume ops per clock. Have you considered
registering a single set of ops for your clock controller driver? See
drivers/clk/samsung/clk-exynos5420.c for an example.
Combined with a table of clocks registered by your driver, centralized
suspend/resume methods might be a cleaner solution.
Regards,
Mike
> + void __iomem *base;
> + u32 saved_reg;
> };
>
> #define to_clk_gate(_hw) container_of(_hw, struct clk_gate, hw)
>
> +static struct clk_gating_ctrl *ctrl;
> +
> static struct clk *clk_gating_get_src(
> struct of_phandle_args *clkspec, void *data)
> {
> - struct clk_gating_ctrl *ctrl = (struct clk_gating_ctrl *)data;
> int n;
>
> if (clkspec->args_count < 1)
> @@ -199,15 +204,30 @@ static struct clk *clk_gating_get_src(
> return ERR_PTR(-ENODEV);
> }
>
> +static int mvebu_clk_gating_suspend(void)
> +{
> + ctrl->saved_reg = readl(ctrl->base);
> + return 0;
> +}
> +
> +static void mvebu_clk_gating_resume(void)
> +{
> + writel(ctrl->saved_reg, ctrl->base);
> +}
> +
> void __init mvebu_clk_gating_setup(struct device_node *np,
> const struct clk_gating_soc_desc *desc)
> {
> - struct clk_gating_ctrl *ctrl;
> struct clk *clk;
> void __iomem *base;
> const char *default_parent = NULL;
> int n;
>
> + if (ctrl) {
> + pr_err("mvebu-clk-gating: cannot instantiate more than one gatable clock device\n");
> + return;
> + }
> +
> base = of_iomap(np, 0);
> if (WARN_ON(!base))
> return;
> @@ -225,6 +245,10 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
> /* lock must already be initialized */
> ctrl->lock = &ctrl_gating_lock;
>
> + ctrl->base = base;
> + ctrl->syscore_ops.suspend = mvebu_clk_gating_suspend;
> + ctrl->syscore_ops.resume = mvebu_clk_gating_resume;
> +
> /* Count, allocate, and register clock gates */
> for (n = 0; desc[n].name;)
> n++;
> @@ -246,6 +270,8 @@ void __init mvebu_clk_gating_setup(struct device_node *np,
>
> of_clk_add_provider(np, clk_gating_get_src, ctrl);
>
> + register_syscore_ops(&ctrl->syscore_ops);
> +
> return;
> gates_out:
> kfree(ctrl);
> --
> 2.1.0
>
next prev parent reply other threads:[~2014-11-17 22:46 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-14 15:21 [PATCHv2 00/16] Suspend to RAM support for Armada XP Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 01/16] Documentation: dt-bindings: minimal documentation for MVEBU SDRAM controller Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 02/16] ARM: mvebu: enable strex backoff delay Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 03/16] irqchip: irq-armada-370-xp: suspend/resume support Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 04/16] clocksource: time-armada-370-xp: add " Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-17 21:12 ` Daniel Lezcano
2014-11-17 21:12 ` Daniel Lezcano
2014-11-17 21:12 ` Daniel Lezcano
2014-11-14 15:21 ` [PATCHv2 05/16] gpio: mvebu: " Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-27 13:29 ` Linus Walleij
2014-11-27 13:29 ` Linus Walleij
2014-11-27 17:53 ` Jason Cooper
2014-11-27 17:53 ` Jason Cooper
2014-11-27 18:44 ` Thomas Petazzoni
2014-11-27 18:44 ` Thomas Petazzoni
2014-11-28 15:42 ` Linus Walleij
2014-11-28 15:42 ` Linus Walleij
2014-11-30 16:49 ` Jason Cooper
2014-11-30 16:49 ` Jason Cooper
2014-11-30 17:01 ` Jason Cooper
2014-11-30 17:01 ` Jason Cooper
2014-11-14 15:21 ` [PATCHv2 06/16] bus: mvebu-mbus: " Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 07/16] bus: mvebu-mbus: provide a mechanism to save SDRAM window configuration Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 08/16] clk: mvebu: add suspend/resume for gatable clocks Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-17 22:46 ` Mike Turquette [this message]
2014-11-17 22:46 ` Mike Turquette
2014-11-17 22:46 ` Mike Turquette
2014-11-21 8:59 ` Thomas Petazzoni
2014-11-21 8:59 ` Thomas Petazzoni
2014-11-21 8:59 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 09/16] ARM: mvebu: implement suspend/resume support for Armada XP Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 10/16] ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 11/16] ARM: mvebu: Armada XP GP specific suspend/resume code Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 12/16] ARM: mvebu: make sure MMU is disabled in armada_370_xp_cpu_resume Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 13/16] ARM: mvebu: synchronize secondary CPU clocks on resume Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 14/16] ARM: mvebu: add suspend/resume DT information for Armada XP GP Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 15/16] ARM: mvebu: adjust mbus controller description on Armada 370/XP Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
2014-11-14 15:21 ` [PATCHv2 16/16] ARM: mvebu: add SDRAM controller description for Armada XP Thomas Petazzoni
2014-11-14 15:21 ` Thomas Petazzoni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141117224604.25314.96198@quantum \
--to=mturquette@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.