* [PATCH v3 0/3] ARM: rk3288 : Add PM Domain support
@ 2014-10-16 11:02 jinkun.hong
2014-10-16 11:02 ` [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform jinkun.hong
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: jinkun.hong @ 2014-10-16 11:02 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel
Cc: Russell King, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely, linux-kernel, devicetree,
Randy Dunlap, linux-doc, dianders, Heiko Stuebner,
linux-rockchip, Ulf Hansson, jinkun.hong
From: "jinkun.hong" <jinkun.hong@rock-chips.com>
Add power domain drivers based on generic power domain for Rockchip platform,
and support RK3288.
Based on:
- [PATCH v1 1/4] PM / clock_ops: Add pm_clk_add_clk()
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg735599.html
Changes in v3:
- change use pm_clk_resume() and pm_clk_suspend()
- DT structure has changed
- Decomposition power-controller, changed to multiple controller
(gpu-power-controller, hevc-power-controller)
Changes in v2:
- remove the "pd->pd.of_node = np"
- move clocks to "optional"
- make pd_vio clocks all one entry per line and alphabetize.
- power: power-controller move back to pinctrl: pinctrl.
jinkun.hong (3):
power-domain: add power domain drivers for Rockchip platform
dt-bindings: add document of Rockchip power domain
ARM: dts: add rk3288 power-domain node
.../bindings/arm/rockchip/power_domain.txt | 46 +++
arch/arm/boot/dts/rk3288.dtsi | 24 ++
arch/arm/mach-rockchip/Kconfig | 1 +
arch/arm/mach-rockchip/Makefile | 1 +
arch/arm/mach-rockchip/pm_domains.c | 329 ++++++++++++++++++++
5 files changed, 401 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/rockchip/power_domain.txt
create mode 100644 arch/arm/mach-rockchip/pm_domains.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform
2014-10-16 11:02 [PATCH v3 0/3] ARM: rk3288 : Add PM Domain support jinkun.hong
@ 2014-10-16 11:02 ` jinkun.hong
2014-10-16 13:06 ` Ulf Hansson
2014-10-16 11:02 ` [PATCH v3 2/3] dt-bindings: add document of Rockchip power domain jinkun.hong
2014-10-16 11:02 ` [PATCH v3 3/3] ARM: dts: add rk3288 power-domain node jinkun.hong
2 siblings, 1 reply; 6+ messages in thread
From: jinkun.hong @ 2014-10-16 11:02 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel
Cc: Russell King, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely, linux-kernel, devicetree,
Randy Dunlap, linux-doc, dianders, Heiko Stuebner,
linux-rockchip, Ulf Hansson, jinkun.hong, Jack Dai
From: "jinkun.hong" <jinkun.hong@rock-chips.com>
Add power domain drivers based on generic power domain for Rockchip platform,
and support RK3288.
Signed-off-by: Jack Dai <jack.dai@rock-chips.com>
Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
---
Changes in v3:
- change use pm_clk_resume() and pm_clk_suspend()
Changes in v2:
- remove the "pd->pd.of_node = np"
arch/arm/mach-rockchip/Kconfig | 1 +
arch/arm/mach-rockchip/Makefile | 1 +
arch/arm/mach-rockchip/pm_domains.c | 329 +++++++++++++++++++++++++++++++++++
3 files changed, 331 insertions(+)
create mode 100644 arch/arm/mach-rockchip/pm_domains.c
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index d168669..4920a88 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -12,6 +12,7 @@ config ARCH_ROCKCHIP
select DW_APB_TIMER_OF
select ARM_GLOBAL_TIMER
select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
+ select PM_GENERIC_DOMAINS if PM
help
Support for Rockchip's Cortex-A9 Single-to-Quad-Core-SoCs
containing the RK2928, RK30xx and RK31xx series.
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index b29d8ea..805268d 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -2,3 +2,4 @@ CFLAGS_platsmp.o := -march=armv7-a
obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
diff --git a/arch/arm/mach-rockchip/pm_domains.c b/arch/arm/mach-rockchip/pm_domains.c
new file mode 100644
index 0000000..1d595f1
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm_domains.c
@@ -0,0 +1,329 @@
+/*
+ * Rockchip Generic power domain support.
+ *
+ * Copyright (c) 2014 ROCKCHIP, Co. Ltd.
+ * Author: Hong Jinkun <jinkun.hong@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/pm_domain.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/sched.h>
+#include <linux/clk.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/spinlock.h>
+#include <linux/pm_clock.h>
+
+#define PWR_OFFSET 0x08
+#define STATUS_OFFSET 0x0c
+#define REQ_OFFSET 0x10
+#define IDLE_OFFSET 0x14
+#define ACK_OFFSET 0x14
+
+struct rockchip_domain {
+ struct generic_pm_domain base;
+ struct device *dev;
+ struct regmap *regmap_pmu;
+ spinlock_t idle_lock;
+ spinlock_t pmu_lock;
+ u32 pwr_shift;
+ u32 status_shift;
+ u32 req_shift;
+ u32 idle_shift;
+ u32 ack_shift;
+};
+
+#define to_rockchip_pd(_gpd) container_of(_gpd, struct rockchip_domain, base)
+
+static int rockchip_pmu_set_idle_request(struct rockchip_domain *pd,
+ bool idle)
+{
+ u32 idle_mask = BIT(pd->idle_shift);
+ u32 idle_target = idle << (pd->idle_shift);
+ u32 ack_mask = BIT(pd->ack_shift);
+ u32 ack_target = idle << (pd->ack_shift);
+ unsigned int mask = BIT(pd->req_shift);
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pd->idle_lock, flags);
+ val = (idle) ? mask : 0;
+ regmap_update_bits(pd->regmap_pmu, REQ_OFFSET, mask, val);
+ dsb();
+
+ do {
+ regmap_read(pd->regmap_pmu, ACK_OFFSET, &val);
+ } while ((val & ack_mask) != ack_target);
+
+ do {
+ regmap_read(pd->regmap_pmu, IDLE_OFFSET, &val);
+ } while ((val & idle_mask) != idle_target);
+
+ spin_unlock_irqrestore(&pd->idle_lock, flags);
+
+ return 0;
+}
+
+static bool rockchip_pmu_power_domain_is_on(struct rockchip_domain *pd)
+{
+ unsigned int val;
+
+ regmap_read(pd->regmap_pmu, STATUS_OFFSET, &val);
+
+ /* 1'b0: power on, 1'b1: power off */
+ return !(val & BIT(pd->status_shift));
+}
+
+static void rockchip_do_pmu_set_power_domain(
+ struct rockchip_domain *pd, bool on)
+{
+ unsigned int mask = BIT(pd->pwr_shift);
+ unsigned int val;
+
+ val = (on) ? 0 : mask;
+ regmap_update_bits(pd->regmap_pmu, PWR_OFFSET, mask, val);
+ dsb();
+
+ do {
+ regmap_read(pd->regmap_pmu, STATUS_OFFSET, &val);
+ } while ((val & BIT(pd->status_shift)) == on);
+}
+
+static int rockchip_pmu_set_power_domain(struct rockchip_domain *pd,
+ bool on)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&pd->pmu_lock, flags);
+ if (rockchip_pmu_power_domain_is_on(pd) == on)
+ goto out;
+
+ if (!on) {
+ /* FIXME: add code to save AXI_QOS */
+ /* if power down, idle request to NIU first */
+ rockchip_pmu_set_idle_request(pd, true);
+ }
+
+ rockchip_do_pmu_set_power_domain(pd, on);
+
+ if (on) {
+ /* if power up, idle request release to NIU */
+ rockchip_pmu_set_idle_request(pd, false);
+ /* FIXME: add code to restore AXI_QOS */
+ }
+out:
+ spin_unlock_irqrestore(&pd->pmu_lock, flags);
+ return 0;
+}
+
+static int rockchip_pd_power_on(struct generic_pm_domain *domain)
+{
+ int ret = 0;
+ struct rockchip_domain *pd = to_rockchip_pd(domain);
+
+ if (pd->dev == NULL)
+ pr_err("pd->dev is NULL, power on failed!", __func__);
+
+ pm_clk_resume(pd->dev);
+ ret = rockchip_pmu_set_power_domain(pd, true);
+ pm_clk_suspend(pd->dev);
+
+ return ret;
+}
+
+static int rockchip_pd_power_off(struct generic_pm_domain *domain)
+{
+ int ret = 0;
+ struct rockchip_domain *pd = to_rockchip_pd(domain);
+
+ if (pd->dev == NULL)
+ pr_err("pd->dev is NULL, power off failed!", __func__);
+
+ pm_clk_resume(pd->dev);
+ ret = rockchip_pmu_set_power_domain(pd, false);
+ pm_clk_suspend(pd->dev);
+
+ return 0;
+}
+
+void rockchip_pm_domain_attach_dev(struct device *dev)
+{
+ struct clk *clk;
+ int ret;
+ int i = 0;
+ struct rockchip_domain *pd;
+
+ pd = (struct rockchip_domain *)dev->pm_domain;
+ pd->dev = dev;
+
+ ret = pm_clk_create(dev);
+ if (ret) {
+ dev_err(dev, "pm_clk_create failed %d\n", ret);
+ return;
+ };
+
+ while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
+ ret = pm_clk_add_clk(dev, clk);
+ if (ret) {
+ dev_err(dev, "pm_clk_add_clk failed %d\n", ret);
+ goto clk_err;
+ };
+ }
+
+ if (!IS_ENABLED(CONFIG_PM_RUNTIME)) {
+ ret = pm_clk_resume(dev);
+ if (ret) {
+ dev_err(dev, "pm_clk_resume failed %d\n", ret);
+ goto clk_err;
+ };
+ }
+ return;
+
+clk_err:
+ pm_clk_destroy(dev);
+
+}
+
+void rockchip_pm_domain_detach_dev(struct device *dev)
+{
+ struct rockchip_domain *pd;
+
+ pd = (struct rockchip_domain *)dev->pm_domain;
+ pd->dev = NULL;
+
+ pm_clk_destroy(dev);
+}
+
+static const struct of_device_id rockchip_pm_domain_dt_match[];
+
+static int rockchip_pm_domain_probe(struct platform_device *pdev)
+{
+ struct rockchip_domain *pd;
+ const struct of_device_id *match;
+ struct clk *clk;
+ int i = 0;
+ int ret;
+ struct regmap *regmap_pmu;
+ struct device_node *node;
+
+ match = of_match_node(rockchip_pm_domain_dt_match, pdev->dev.of_node);
+ pd = (struct rockchip_domain *)match->data;
+
+ if (!pd)
+ return -ENOMEM;
+
+ node = of_parse_phandle(pdev->dev.of_node, "rockchip,pmu", 0);
+ regmap_pmu = syscon_node_to_regmap(node);
+ of_node_put(node);
+ if (IS_ERR(regmap_pmu)) {
+ pr_err("%s: failed to get regmap_pmu", __func__);
+ return PTR_ERR(regmap_pmu);
+ }
+
+ pd->regmap_pmu = regmap_pmu;
+ pd->dev = NULL;
+
+ spin_lock_init(&pd->idle_lock);
+ spin_lock_init(&pd->pmu_lock);
+
+ pm_genpd_init(&pd->base, NULL, false);
+
+ return of_genpd_add_provider_simple(pdev->dev.of_node, &pd->base);
+}
+
+static struct rockchip_domain gpu_domain = {
+ .base = {
+ .name = "pd_gpu",
+ .attach_dev = rockchip_pm_domain_attach_dev,
+ .detach_dev = rockchip_pm_domain_detach_dev,
+ .power_off = rockchip_pd_power_off,
+ .power_on = rockchip_pd_power_on,
+ },
+ .pwr_shift = 9,
+ .status_shift = 9,
+ .req_shift = 2,
+ .idle_shift = 2,
+ .ack_shift = 18,
+};
+
+static struct rockchip_domain hevc_domain = {
+ .base = {
+ .name = "pd_hevc",
+ .attach_dev = rockchip_pm_domain_attach_dev,
+ .detach_dev = rockchip_pm_domain_detach_dev,
+ .power_off = rockchip_pd_power_off,
+ .power_on = rockchip_pd_power_on,
+ },
+ .pwr_shift = 14,
+ .status_shift = 10,
+ .req_shift = 9,
+ .idle_shift = 9,
+ .ack_shift = 25,
+
+};
+
+static struct rockchip_domain video_domain = {
+ .base = {
+ .name = "pd_video",
+ .attach_dev = rockchip_pm_domain_attach_dev,
+ .detach_dev = rockchip_pm_domain_detach_dev,
+ .power_off = rockchip_pd_power_off,
+ .power_on = rockchip_pd_power_on,
+ },
+ .pwr_shift = 8,
+ .status_shift = 8,
+ .req_shift = 3,
+ .idle_shift = 3,
+ .ack_shift = 19,
+};
+
+static struct rockchip_domain vio_domain = {
+ .base = {
+ .name = "pd_vio",
+ .attach_dev = rockchip_pm_domain_attach_dev,
+ .detach_dev = rockchip_pm_domain_detach_dev,
+ .power_off = rockchip_pd_power_off,
+ .power_on = rockchip_pd_power_on,
+ },
+ .pwr_shift = 7,
+ .status_shift = 7,
+ .req_shift = 4,
+ .idle_shift = 4,
+ .ack_shift = 20,
+};
+
+static const struct of_device_id rockchip_pm_domain_dt_match[] = {
+ { .compatible = "rockchip,rk3288-power-gpu",
+ .data = (void *)&gpu_domain},
+ { .compatible = "rockchip,rk3288-power-hevc",
+ .data = (void *)&hevc_domain},
+ { .compatible = "rockchip,rk3288-power-video",
+ .data = (void *)&video_domain},
+ { .compatible = "rockchip,rk3288-power-vio",
+ .data = (void *)&vio_domain},
+ {},
+};
+MODULE_DEVICE_TABLE(of, rockchip_pm_domain_dt_match);
+
+static struct platform_driver rockchip_pm_domain_driver = {
+ .probe = rockchip_pm_domain_probe,
+ .driver = {
+ .name = "rockchip-pm-domain",
+ .owner = THIS_MODULE,
+ .of_match_table = rockchip_pm_domain_dt_match,
+ },
+};
+
+static int __init rockchip_pm_domain_drv_register(void)
+{
+ return platform_driver_register(&rockchip_pm_domain_driver);
+}
+postcore_initcall(rockchip_pm_domain_drv_register);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] dt-bindings: add document of Rockchip power domain
2014-10-16 11:02 [PATCH v3 0/3] ARM: rk3288 : Add PM Domain support jinkun.hong
2014-10-16 11:02 ` [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform jinkun.hong
@ 2014-10-16 11:02 ` jinkun.hong
2014-10-16 11:02 ` [PATCH v3 3/3] ARM: dts: add rk3288 power-domain node jinkun.hong
2 siblings, 0 replies; 6+ messages in thread
From: jinkun.hong @ 2014-10-16 11:02 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel
Cc: Russell King, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely, linux-kernel, devicetree,
Randy Dunlap, linux-doc, dianders, Heiko Stuebner,
linux-rockchip, Ulf Hansson, jinkun.hong, Jack Dai
From: "jinkun.hong" <jinkun.hong@rock-chips.com>
Signed-off-by: Jack Dai <jack.dai@rock-chips.com>
Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
---
Changes in v3:
- DT structure has changed
Changes in v2:
- move clocks to "optional"
.../bindings/arm/rockchip/power_domain.txt | 46 ++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/rockchip/power_domain.txt
diff --git a/Documentation/devicetree/bindings/arm/rockchip/power_domain.txt b/Documentation/devicetree/bindings/arm/rockchip/power_domain.txt
new file mode 100644
index 0000000..f8357b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/rockchip/power_domain.txt
@@ -0,0 +1,46 @@
+* Rockchip Power Domains
+
+Rockchip processors include support for multiple power domains which can be
+powered up/down by software based on different application scenes to save power.
+
+Required properties for power domain controller:
+- compatible: should be one of the following.
+ * rockchip,rk3288-power-gpu - for rk3288 type gpu power domain.
+ * rockchip,rk3288-power-hevc - for rk3288 type hevc power domain.
+ * rockchip,rk3288-power-video - for rk3288 type video power domain.
+ * rockchip,rk3288-power-vio - for rk3288 type vio power domain.
+- rockchip,pmu: phandle referencing a syscon providing the pmu registers
+- #power-domain-cells: Number of cells in a power-domain specifier.
+ should be 0.
+
+Example:
+
+ gpu_power: gpu-power-controller {
+ compatible = "rockchip,rk3288-power-gpu";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ hevc_power: hevc-power-controller {
+ compatible = "rockchip,rk3288-power-hevc";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ video_power: video-power-controller {
+ compatible = "rockchip,rk3288-power-video";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ vio_power: vio-power-controller {
+ compatible = "rockchip,rk3288-power-vio";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ gpu: gpu@0xffa30000 {
+ ...
+ power-domains = <&gpu_power>;
+ ...
+ };
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] ARM: dts: add rk3288 power-domain node
2014-10-16 11:02 [PATCH v3 0/3] ARM: rk3288 : Add PM Domain support jinkun.hong
2014-10-16 11:02 ` [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform jinkun.hong
2014-10-16 11:02 ` [PATCH v3 2/3] dt-bindings: add document of Rockchip power domain jinkun.hong
@ 2014-10-16 11:02 ` jinkun.hong
2 siblings, 0 replies; 6+ messages in thread
From: jinkun.hong @ 2014-10-16 11:02 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel
Cc: Russell King, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely, linux-kernel, devicetree,
Randy Dunlap, linux-doc, dianders, Heiko Stuebner,
linux-rockchip, Ulf Hansson, jinkun.hong, Jack Dai
From: "jinkun.hong" <jinkun.hong@rock-chips.com>
Signed-off-by: Jack Dai <jack.dai@rock-chips.com>
Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
---
Changes in v3:
- Decomposition power-controller, changed to multiple controller
(gpu-power-controller, hevc-power-controller)
Changes in v2:
- make pd_vio clocks all one entry per line and alphabetize.
- power: power-controller move back to pinctrl: pinctrl.
arch/arm/boot/dts/rk3288.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 5950b0a..6a9b313 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -592,4 +592,28 @@
};
};
};
+
+ gpu_power: gpu-power-controller {
+ compatible = "rockchip,rk3288-power-gpu";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ hevc_power: hevc-power-controller {
+ compatible = "rockchip,rk3288-power-hevc";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ video_power: video-power-controller {
+ compatible = "rockchip,rk3288-power-video";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
+
+ vio_power: vio-power-controller {
+ compatible = "rockchip,rk3288-power-vio";
+ rockchip,pmu = <&pmu>;
+ #power-domain-cells = <0>;
+ };
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform
2014-10-16 11:02 ` [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform jinkun.hong
@ 2014-10-16 13:06 ` Ulf Hansson
2014-10-17 13:56 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2014-10-16 13:06 UTC (permalink / raw)
To: jinkun.hong
Cc: Linus Walleij, linux-arm-kernel, Russell King, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
linux-kernel, devicetree, Randy Dunlap, linux-doc, Doug Anderson,
Heiko Stuebner, linux-rockchip, Jack Dai
Hi Jinkun,
[...]
> +
> +static int rockchip_pd_power_on(struct generic_pm_domain *domain)
> +{
> + int ret = 0;
> + struct rockchip_domain *pd = to_rockchip_pd(domain);
> +
> + if (pd->dev == NULL)
> + pr_err("pd->dev is NULL, power on failed!", __func__);
> +
> + pm_clk_resume(pd->dev);
> + ret = rockchip_pmu_set_power_domain(pd, true);
> + pm_clk_suspend(pd->dev);
> +
> + return ret;
> +}
> +
> +static int rockchip_pd_power_off(struct generic_pm_domain *domain)
> +{
> + int ret = 0;
> + struct rockchip_domain *pd = to_rockchip_pd(domain);
> +
> + if (pd->dev == NULL)
> + pr_err("pd->dev is NULL, power off failed!", __func__);
> +
> + pm_clk_resume(pd->dev);
> + ret = rockchip_pmu_set_power_domain(pd, false);
> + pm_clk_suspend(pd->dev);
> +
> + return 0;
> +}
> +
> +void rockchip_pm_domain_attach_dev(struct device *dev)
> +{
> + struct clk *clk;
> + int ret;
> + int i = 0;
> + struct rockchip_domain *pd;
> +
> + pd = (struct rockchip_domain *)dev->pm_domain;
> + pd->dev = dev;
This looks wrong.
You will update "pd->dev" for each device that gets added to the PM
domain. The last one will be set to "pd->dev".
So, it would still work as long as you only have one device per
domain. Is that the case?
Could you elaborate on why you have chosen to do it this way?
> +
> + ret = pm_clk_create(dev);
> + if (ret) {
> + dev_err(dev, "pm_clk_create failed %d\n", ret);
> + return;
> + };
> +
> + while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
> + ret = pm_clk_add_clk(dev, clk);
> + if (ret) {
> + dev_err(dev, "pm_clk_add_clk failed %d\n", ret);
> + goto clk_err;
> + };
> + }
> +
> + if (!IS_ENABLED(CONFIG_PM_RUNTIME)) {
Could you elaborate on why you need to do this check here?
When I see a check for IS_ENABLED(CONFIG_PM_RUNTIME), that indicates
to me that we actually have an another issue, perhaps in the PM clk
API.
> + ret = pm_clk_resume(dev);
> + if (ret) {
> + dev_err(dev, "pm_clk_resume failed %d\n", ret);
> + goto clk_err;
> + };
> + }
> + return;
> +
> +clk_err:
> + pm_clk_destroy(dev);
> +
> +}
> +
> +void rockchip_pm_domain_detach_dev(struct device *dev)
> +{
> + struct rockchip_domain *pd;
> +
> + pd = (struct rockchip_domain *)dev->pm_domain;
> + pd->dev = NULL;
> +
> + pm_clk_destroy(dev);
> +}
> +
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform
2014-10-16 13:06 ` Ulf Hansson
@ 2014-10-17 13:56 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-10-17 13:56 UTC (permalink / raw)
To: Ulf Hansson
Cc: jinkun.hong, Linus Walleij, linux-arm-kernel, Russell King,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Grant Likely, linux-kernel, devicetree, Randy Dunlap, linux-doc,
Doug Anderson, Heiko Stuebner, linux-rockchip, Jack Dai
Hi Ulf,
On Thu, Oct 16, 2014 at 3:06 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> +
>> + ret = pm_clk_create(dev);
>> + if (ret) {
>> + dev_err(dev, "pm_clk_create failed %d\n", ret);
>> + return;
>> + };
>> +
>> + while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
>> + ret = pm_clk_add_clk(dev, clk);
>> + if (ret) {
>> + dev_err(dev, "pm_clk_add_clk failed %d\n", ret);
>> + goto clk_err;
>> + };
>> + }
>> +
>> + if (!IS_ENABLED(CONFIG_PM_RUNTIME)) {
>
> Could you elaborate on why you need to do this check here?
If runtime PM is enabled, the clocks are managed by runtime PM, and will
be enabled/disabled later when needed.
If runtime PM is disabled, the clocks are not managed by runtime PM,
so they should be enabled at initialization time.
Grygorii and I do the same thing for keystone resp. pm-rmobile.
> When I see a check for IS_ENABLED(CONFIG_PM_RUNTIME), that indicates
> to me that we actually have an another issue, perhaps in the PM clk
> API.
Good point. Perhaps this can be handled in pm_clk_add() instead?
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-17 13:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-16 11:02 [PATCH v3 0/3] ARM: rk3288 : Add PM Domain support jinkun.hong
2014-10-16 11:02 ` [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform jinkun.hong
2014-10-16 13:06 ` Ulf Hansson
2014-10-17 13:56 ` Geert Uytterhoeven
2014-10-16 11:02 ` [PATCH v3 2/3] dt-bindings: add document of Rockchip power domain jinkun.hong
2014-10-16 11:02 ` [PATCH v3 3/3] ARM: dts: add rk3288 power-domain node jinkun.hong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).