Use the generic power domains support to implement support
for power domain on EXYNOS4210.
This patch is based on your "pm-domains" branch.(v7)
Signed-off-by: Chanwoo Choi <[email protected]>
Signed-off-by: Myungjoo Ham <[email protected]>
Signed-off-by: Kyungmin.park <[email protected]>
---
arch/arm/mach-exynos4/Kconfig | 1 +
arch/arm/mach-exynos4/Makefile | 1 +
arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 78 ++++++++++++
arch/arm/mach-exynos4/mach-nuri.c | 20 +++
arch/arm/mach-exynos4/pm-exynos4210.c | 131 ++++++++++++++++++++
5 files changed, 231 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index 1435fc3..266e7f6 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -183,6 +183,7 @@ config MACH_NURI
select EXYNOS4_SETUP_SDHCI
select EXYNOS4_SETUP_USB_PHY
select SAMSUNG_DEV_PWM
+ select PM_GENERIC_DOMAINS
help
Machine support for Samsung Mobile NURI Board.
diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 60fe5ec..0e9461f 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-eint.o dma.o
obj-$(CONFIG_PM) += pm.o sleep.o
obj-$(CONFIG_CPU_FREQ) += cpufreq.o
obj-$(CONFIG_CPU_IDLE) += cpuidle.o
+obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
obj-$(CONFIG_SMP) += platsmp.o headsmp.o
diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
new file mode 100644
index 0000000..fa1c7a0
--- /dev/null
+++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
@@ -0,0 +1,78 @@
+/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
+ *
+ * Exynos4 Power management support
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.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.
+ */
+
+#ifndef __PM_EXYNOS4210_H__
+#define __PM_EXYNOS4210_H__
+
+#include <linux/pm_domain.h>
+
+struct platform_device;
+
+struct exynos4210_pm_domain {
+ struct generic_pm_domain genpd;
+
+ const char *name;
+ void __iomem *base;
+ int boot_on;
+};
+
+static inline struct exynos4210_pm_domain *to_exynos4210_pd(
+ struct generic_pm_domain *pd)
+{
+ return container_of(pd, struct exynos4210_pm_domain, genpd);
+}
+
+#ifdef CONFIG_PM
+enum exynos4210_pd_data {
+ EXYNOS4210_PD_MFC,
+ EXYNOS4210_PD_G3D,
+ EXYNOS4210_PD_LCD0,
+ EXYNOS4210_PD_LCD1,
+ EXYNOS4210_PD_TV,
+ EXYNOS4210_PD_CAM,
+ EXYNOS4210_PD_GPS,
+ EXYNOS4210_PD_MAX_NUM
+};
+
+extern struct exynos4210_pm_domain exynos4210_pd_list[EXYNOS4210_PD_MAX_NUM];
+#define EXYNOS4210_MFC (&exynos4210_pd_list[EXYNOS4210_PD_MFC])
+#define EXYNOS4210_G3D (&exynos4210_pd_list[EXYNOS4210_PD_G3D])
+#define EXYNOS4210_LCD0 (&exynos4210_pd_list[EXYNOS4210_PD_LCD0])
+#define EXYNOS4210_LCD1 (&exynos4210_pd_list[EXYNOS4210_PD_LCD1])
+#define EXYNOS4210_TV (&exynos4210_pd_list[EXYNOS4210_PD_TV])
+#define EXYNOS4210_CAM (&exynos4210_pd_list[EXYNOS4210_PD_CAM])
+#define EXYNOS4210_GPS (&exynos4210_pd_list[EXYNOS4210_PD_GPS])
+
+extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain);
+extern int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
+ struct platform_device *pdev);
+#else
+#define EXYNOS4210_MFC NULL
+#define EXYNOS4210_G3D NULL
+#define EXYNOS4210_LCD0 NULL
+#define EXYNOS4210_LCD1 NULL
+#define EXYNOS4210_TV NULL
+#define EXYNOS4210_CAM NULL
+#define EXYNOS4210_GPS NULL
+
+extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
+{
+ return 0;
+}
+extern int exynos4_add_device_to_domain(struct exynos4210_pm_domain *domain,
+ struct platform_device *pdev)
+{
+ return 0;
+}
+#endif /* CONFIG_PM */
+
+#endif /* __PM_EXYNOS4210_H__ */
diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
index 642702b..3e5c42c 100644
--- a/arch/arm/mach-exynos4/mach-nuri.c
+++ b/arch/arm/mach-exynos4/mach-nuri.c
@@ -37,6 +37,7 @@
#include <plat/iic.h>
#include <mach/map.h>
+#include <mach/pm-exynos4210.h>
/* Following are default values for UCON, ULCON and UFCON UART registers */
#define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
@@ -376,6 +377,23 @@ static struct platform_device *nuri_devices[] __initdata = {
&nuri_backlight_device,
};
+static void __init nuri_power_domain_init(void)
+{
+ int i;
+
+ for (i = 0 ; i < EXYNOS4210_PD_MAX_NUM ; i++)
+ exynos4210_pd_init(&exynos4210_pd_list[i]);
+
+ /* Add device to power-domain */
+ exynos4210_add_device_to_domain(EXYNOS4210_MFC, NULL);
+ exynos4210_add_device_to_domain(EXYNOS4210_G3D, NULL);
+ exynos4210_add_device_to_domain(EXYNOS4210_LCD0, &nuri_lcd_device);
+ exynos4210_add_device_to_domain(EXYNOS4210_LCD1, NULL);
+ exynos4210_add_device_to_domain(EXYNOS4210_TV, NULL);
+ exynos4210_add_device_to_domain(EXYNOS4210_CAM, NULL);
+ exynos4210_add_device_to_domain(EXYNOS4210_GPS, NULL);
+}
+
static void __init nuri_map_io(void)
{
s5p_init_io(NULL, 0, S5P_VA_CHIPID);
@@ -398,6 +416,8 @@ static void __init nuri_machine_init(void)
/* Last */
platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
+
+ nuri_power_domain_init();
}
MACHINE_START(NURI, "NURI")
diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-exynos4/pm-exynos4210.c
new file mode 100644
index 0000000..8e95d49
--- /dev/null
+++ b/arch/arm/mach-exynos4/pm-exynos4210.c
@@ -0,0 +1,131 @@
+/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
+ *
+ * Exynos4210 Power management support
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.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/pm.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <linux/io.h>
+
+#include <mach/regs-pmu.h>
+#include <mach/regs-clock.h>
+#include <mach/pm-exynos4210.h>
+
+struct exynos4210_pm_domain exynos4210_pd_list[] = {
+ {
+ .name = "PD_MFC",
+ .base = S5P_PMU_MFC_CONF,
+ .boot_on = 0,
+ }, {
+ .name = "PD_G3D",
+ .base = S5P_PMU_G3D_CONF,
+ .boot_on = 0,
+ }, {
+ .name = "PD_LCD0",
+ .base = S5P_PMU_LCD0_CONF,
+ .boot_on = 1,
+ }, {
+ .name = "PD_LCD1",
+ .base = S5P_PMU_LCD1_CONF,
+ .boot_on = 0,
+ }, {
+ .name = "PD_TV",
+ .base = S5P_PMU_TV_CONF,
+ .boot_on = 0,
+ }, {
+ .name = "PD_CAM",
+ .base = S5P_PMU_CAM_CONF,
+ .boot_on = 0,
+ }, {
+ .name = "PD_GPS",
+ .base = S5P_PMU_GPS_CONF,
+ .boot_on = 0,
+ },
+};
+
+static int pd_power_down(struct generic_pm_domain *genpd)
+{
+ struct exynos4210_pm_domain *domain = to_exynos4210_pd(genpd);
+ u32 timeout;
+
+ __raw_writel(0, domain->base);
+
+ /* Wait max 1ms */
+ timeout = 10;
+ while (__raw_readl(domain->base + 0x4) & S5P_INT_LOCAL_PWR_EN) {
+ if (timeout == 0) {
+ printk(KERN_ERR "Power domain %s disable failed.\n",
+ domain->name);
+ return -ETIMEDOUT;
+ }
+ timeout--;
+ udelay(100);
+ }
+
+ return 0;
+}
+
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+ struct exynos4210_pm_domain *domain = to_exynos4210_pd(genpd);
+ u32 timeout;
+
+ __raw_writel(S5P_INT_LOCAL_PWR_EN, domain->base);
+
+ /* Wait max 1ms */
+ timeout = 10;
+ while ((__raw_readl(domain->base + 0x4) & S5P_INT_LOCAL_PWR_EN)
+ != S5P_INT_LOCAL_PWR_EN) {
+ if (timeout == 0) {
+ printk(KERN_ERR "Power domain %s enable failed.\n",
+ domain->name);
+ return -ETIMEDOUT;
+ }
+ timeout--;
+ udelay(100);
+ }
+
+ return 0;
+}
+
+int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
+{
+ struct generic_pm_domain *genpd = &domain->genpd;
+
+ pm_genpd_init(genpd, NULL, false);
+
+ genpd->stop_device = pm_runtime_clk_suspend;
+ genpd->start_device = pm_runtime_clk_resume;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
+
+ if (domain->boot_on)
+ pd_power_up(genpd);
+
+ return 0;
+}
+
+int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct generic_pm_domain *genpd = &domain->genpd;
+
+ if (pdev != NULL)
+ return pm_genpd_add_device(genpd, dev);
+ else
+ return -EINVAL;
+}
Hello,
On 06/24/2011 12:06 PM, Chanwoo Choi wrote:
> Use the generic power domains support to implement support
> for power domain on EXYNOS4210.
>
> This patch is based on your "pm-domains" branch.(v7)
>
> Signed-off-by: Chanwoo Choi<[email protected]>
> Signed-off-by: Myungjoo Ham<[email protected]>
> Signed-off-by: Kyungmin.park<[email protected]>
> ---
> arch/arm/mach-exynos4/Kconfig | 1 +
> arch/arm/mach-exynos4/Makefile | 1 +
> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 78 ++++++++++++
> arch/arm/mach-exynos4/mach-nuri.c | 20 +++
> arch/arm/mach-exynos4/pm-exynos4210.c | 131 ++++++++++++++++++++
> 5 files changed, 231 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>
...
> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> new file mode 100644
> index 0000000..fa1c7a0
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> @@ -0,0 +1,78 @@
> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
> + *
> + * Exynos4 Power management support
> + *
> + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.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.
> + */
> +
> +#ifndef __PM_EXYNOS4210_H__
> +#define __PM_EXYNOS4210_H__
> +
> +#include<linux/pm_domain.h>
> +
> +struct platform_device;
> +
> +struct exynos4210_pm_domain {
> + struct generic_pm_domain genpd;
> +
> + const char *name;
> + void __iomem *base;
> + int boot_on;
> +};
> +
> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
> + struct generic_pm_domain *pd)
> +{
> + return container_of(pd, struct exynos4210_pm_domain, genpd);
> +}
> +
> +#ifdef CONFIG_PM
> +enum exynos4210_pd_data {
> + EXYNOS4210_PD_MFC,
> + EXYNOS4210_PD_G3D,
> + EXYNOS4210_PD_LCD0,
> + EXYNOS4210_PD_LCD1,
> + EXYNOS4210_PD_TV,
> + EXYNOS4210_PD_CAM,
> + EXYNOS4210_PD_GPS,
> + EXYNOS4210_PD_MAX_NUM
> +};
> +
> +extern struct exynos4210_pm_domain exynos4210_pd_list[EXYNOS4210_PD_MAX_NUM];
Could this array be made static in ../pm-exynos4210.c ?
It looks like all functions using it are there. Plus you have already
enum exynos4210_pd_data to identify each power domain.
> +#define EXYNOS4210_MFC (&exynos4210_pd_list[EXYNOS4210_PD_MFC])
> +#define EXYNOS4210_G3D (&exynos4210_pd_list[EXYNOS4210_PD_G3D])
> +#define EXYNOS4210_LCD0 (&exynos4210_pd_list[EXYNOS4210_PD_LCD0])
> +#define EXYNOS4210_LCD1 (&exynos4210_pd_list[EXYNOS4210_PD_LCD1])
> +#define EXYNOS4210_TV (&exynos4210_pd_list[EXYNOS4210_PD_TV])
> +#define EXYNOS4210_CAM (&exynos4210_pd_list[EXYNOS4210_PD_CAM])
> +#define EXYNOS4210_GPS (&exynos4210_pd_list[EXYNOS4210_PD_GPS])
> +
> +extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain);
> +extern int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
> + struct platform_device *pdev);
Is there any advantage of having these functions marked as "extern" ?
> +#else
> +#define EXYNOS4210_MFC NULL
> +#define EXYNOS4210_G3D NULL
> +#define EXYNOS4210_LCD0 NULL
> +#define EXYNOS4210_LCD1 NULL
> +#define EXYNOS4210_TV NULL
> +#define EXYNOS4210_CAM NULL
> +#define EXYNOS4210_GPS NULL
> +
> +extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
Shouldn't it be "static inline" instead ?
> +{
> + return 0;
> +}
> +extern int exynos4_add_device_to_domain(struct exynos4210_pm_domain *domain,
> + struct platform_device *pdev)
Ditto.
...
> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
> index 642702b..3e5c42c 100644
> --- a/arch/arm/mach-exynos4/mach-nuri.c
> +++ b/arch/arm/mach-exynos4/mach-nuri.c
> @@ -37,6 +37,7 @@
> #include<plat/iic.h>
>
> #include<mach/map.h>
> +#include<mach/pm-exynos4210.h>
>
> /* Following are default values for UCON, ULCON and UFCON UART registers */
> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> @@ -376,6 +377,23 @@ static struct platform_device *nuri_devices[] __initdata = {
> &nuri_backlight_device,
> };
>
> +static void __init nuri_power_domain_init(void)
> +{
> + int i;
> +
> + for (i = 0 ; i< EXYNOS4210_PD_MAX_NUM ; i++)
> + exynos4210_pd_init(&exynos4210_pd_list[i]);
> +
> + /* Add device to power-domain */
> + exynos4210_add_device_to_domain(EXYNOS4210_MFC, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_G3D, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_LCD0,&nuri_lcd_device);
> + exynos4210_add_device_to_domain(EXYNOS4210_LCD1, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_TV, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_CAM, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_GPS, NULL);
> +}
> +
> static void __init nuri_map_io(void)
> {
> s5p_init_io(NULL, 0, S5P_VA_CHIPID);
> @@ -398,6 +416,8 @@ static void __init nuri_machine_init(void)
>
> /* Last */
> platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
> +
> + nuri_power_domain_init();
> }
>
> MACHINE_START(NURI, "NURI")
> diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-exynos4/pm-exynos4210.c
> new file mode 100644
> index 0000000..8e95d49
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/pm-exynos4210.c
> @@ -0,0 +1,131 @@
> +/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
> + *
...
> +
> +struct exynos4210_pm_domain exynos4210_pd_list[] = {
> + {
> + .name = "PD_MFC",
> + .base = S5P_PMU_MFC_CONF,
> + .boot_on = 0,
> + }, {
> + .name = "PD_G3D",
> + .base = S5P_PMU_G3D_CONF,
> + .boot_on = 0,
> + }, {
...
> +
> +int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
> +{
> + struct generic_pm_domain *genpd =&domain->genpd;
> +
> + pm_genpd_init(genpd, NULL, false);
> +
> + genpd->stop_device = pm_runtime_clk_suspend;
> + genpd->start_device = pm_runtime_clk_resume;
Have you considered handling the S5P_CLKGATE_BLOCK register to make
sure the clock domains are enabled ? AFAICS it is not properly handled
by current clock API on mach-exynos4. Thus if bootloader clears
S5P_CLKGATE_BLOCK register clocks remain globally masked.
This issue have been attempted to be resolved in this patch:
http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
While at here, it might be good to resolve this either on power domains
or the clock API level.
> + genpd->power_off = pd_power_down;
> + genpd->power_on = pd_power_up;
> +
> + if (domain->boot_on)
> + pd_power_up(genpd);
> +
> + return 0;
> +}
> +
> +int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
> + struct platform_device *pdev)
> +{
> + struct device *dev =&pdev->dev;
> + struct generic_pm_domain *genpd =&domain->genpd;
> +
> + if (pdev != NULL)
> + return pm_genpd_add_device(genpd, dev);
> + else
> + return -EINVAL;
You could do instead:
+ if (pdev)
+ return pm_genpd_add_device(genpd, dev);
+
+ return -EINVAL;
> +}
--
Regards,
Sylwester
Hello,
Sylwester Nawrocki wrote:
> Hello,
>
> On 06/24/2011 12:06 PM, Chanwoo Choi wrote:
>> Use the generic power domains support to implement support
>> for power domain on EXYNOS4210.
>>
>> This patch is based on your "pm-domains" branch.(v7)
>>
>> Signed-off-by: Chanwoo Choi<[email protected]>
>> Signed-off-by: Myungjoo Ham<[email protected]>
>> Signed-off-by: Kyungmin.park<[email protected]>
>> ---
>> arch/arm/mach-exynos4/Kconfig | 1 +
>> arch/arm/mach-exynos4/Makefile | 1 +
>> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 78 ++++++++++++
>> arch/arm/mach-exynos4/mach-nuri.c | 20 +++
>> arch/arm/mach-exynos4/pm-exynos4210.c | 131 ++++++++++++++++++++
>> 5 files changed, 231 insertions(+), 0 deletions(-)
>> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>>
> ...
>
>> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> new file mode 100644
>> index 0000000..fa1c7a0
>> --- /dev/null
>> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> @@ -0,0 +1,78 @@
>> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
>> + *
>> + * Exynos4 Power management support
>> + *
>> + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.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.
>> + */
>> +
>> +#ifndef __PM_EXYNOS4210_H__
>> +#define __PM_EXYNOS4210_H__
>> +
>> +#include<linux/pm_domain.h>
>> +
>> +struct platform_device;
>> +
>> +struct exynos4210_pm_domain {
>> + struct generic_pm_domain genpd;
>> +
>> + const char *name;
>> + void __iomem *base;
>> + int boot_on;
>> +};
>> +
>> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
>> + struct generic_pm_domain *pd)
>> +{
>> + return container_of(pd, struct exynos4210_pm_domain, genpd);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +enum exynos4210_pd_data {
>> + EXYNOS4210_PD_MFC,
>> + EXYNOS4210_PD_G3D,
>> + EXYNOS4210_PD_LCD0,
>> + EXYNOS4210_PD_LCD1,
>> + EXYNOS4210_PD_TV,
>> + EXYNOS4210_PD_CAM,
>> + EXYNOS4210_PD_GPS,
>> + EXYNOS4210_PD_MAX_NUM
>> +};
>> +
>> +extern struct exynos4210_pm_domain exynos4210_pd_list[EXYNOS4210_PD_MAX_NUM];
>
> Could this array be made static in ../pm-exynos4210.c ?
> It looks like all functions using it are there. Plus you have already
> enum exynos4210_pd_data to identify each power domain.
It is poosible to define static arrary in ../pm-exynos4210.c,
but it could be defined as non-static to keep the same form
with the patch using generic I/O power domain support :
http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=commit;h=9b3c17cd4555612ff852b7c1ce9583d48d9b0c31
And I will modify that use previous definition of enum exynos4210_pd_data
to identify each power domain.
>
>> +#define EXYNOS4210_MFC (&exynos4210_pd_list[EXYNOS4210_PD_MFC])
>> +#define EXYNOS4210_G3D (&exynos4210_pd_list[EXYNOS4210_PD_G3D])
>> +#define EXYNOS4210_LCD0 (&exynos4210_pd_list[EXYNOS4210_PD_LCD0])
>> +#define EXYNOS4210_LCD1 (&exynos4210_pd_list[EXYNOS4210_PD_LCD1])
>> +#define EXYNOS4210_TV (&exynos4210_pd_list[EXYNOS4210_PD_TV])
>> +#define EXYNOS4210_CAM (&exynos4210_pd_list[EXYNOS4210_PD_CAM])
>> +#define EXYNOS4210_GPS (&exynos4210_pd_list[EXYNOS4210_PD_GPS])
>> +
>> +extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain);
>> +extern int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
>> + struct platform_device *pdev);
>
> Is there any advantage of having these functions marked as "extern" ?
>
I will remove "extern" keyword.
>> +#else
>> +#define EXYNOS4210_MFC NULL
>> +#define EXYNOS4210_G3D NULL
>> +#define EXYNOS4210_LCD0 NULL
>> +#define EXYNOS4210_LCD1 NULL
>> +#define EXYNOS4210_TV NULL
>> +#define EXYNOS4210_CAM NULL
>> +#define EXYNOS4210_GPS NULL
>> +
>> +extern int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
>
> Shouldn't it be "static inline" instead ?
My mistake. I will fix it.
>> +{
>> + return 0;
>> +}
>> +extern int exynos4_add_device_to_domain(struct exynos4210_pm_domain *domain,
>> + struct platform_device *pdev)
>
> Ditto.
>
I will fix it.
> ...
>> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
>> index 642702b..3e5c42c 100644
>> --- a/arch/arm/mach-exynos4/mach-nuri.c
>> +++ b/arch/arm/mach-exynos4/mach-nuri.c
>> @@ -37,6 +37,7 @@
>> #include<plat/iic.h>
>>
>> #include<mach/map.h>
>> +#include<mach/pm-exynos4210.h>
>>
>> /* Following are default values for UCON, ULCON and UFCON UART registers */
>> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
>> @@ -376,6 +377,23 @@ static struct platform_device *nuri_devices[] __initdata = {
>> &nuri_backlight_device,
>> };
>>
>> +static void __init nuri_power_domain_init(void)
>> +{
>> + int i;
>> +
>> + for (i = 0 ; i< EXYNOS4210_PD_MAX_NUM ; i++)
>> + exynos4210_pd_init(&exynos4210_pd_list[i]);
>> +
>> + /* Add device to power-domain */
>> + exynos4210_add_device_to_domain(EXYNOS4210_MFC, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_G3D, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_LCD0,&nuri_lcd_device);
>> + exynos4210_add_device_to_domain(EXYNOS4210_LCD1, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_TV, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_CAM, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_GPS, NULL);
>> +}
>> +
>> static void __init nuri_map_io(void)
>> {
>> s5p_init_io(NULL, 0, S5P_VA_CHIPID);
>> @@ -398,6 +416,8 @@ static void __init nuri_machine_init(void)
>>
>> /* Last */
>> platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
>> +
>> + nuri_power_domain_init();
>> }
>>
>> MACHINE_START(NURI, "NURI")
>> diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-exynos4/pm-exynos4210.c
>> new file mode 100644
>> index 0000000..8e95d49
>> --- /dev/null
>> +++ b/arch/arm/mach-exynos4/pm-exynos4210.c
>> @@ -0,0 +1,131 @@
>> +/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
>> + *
> ...
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_list[] = {
>> + {
>> + .name = "PD_MFC",
>> + .base = S5P_PMU_MFC_CONF,
>> + .boot_on = 0,
>> + }, {
>> + .name = "PD_G3D",
>> + .base = S5P_PMU_G3D_CONF,
>> + .boot_on = 0,
>> + }, {
> ...
>> +
>> +int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
>> +{
>> + struct generic_pm_domain *genpd =&domain->genpd;
>> +
>> + pm_genpd_init(genpd, NULL, false);
>> +
>> + genpd->stop_device = pm_runtime_clk_suspend;
>> + genpd->start_device = pm_runtime_clk_resume;
>
> Have you considered handling the S5P_CLKGATE_BLOCK register to make
> sure the clock domains are enabled ? AFAICS it is not properly handled
> by current clock API on mach-exynos4. Thus if bootloader clears
> S5P_CLKGATE_BLOCK register clocks remain globally masked.
>
> This issue have been attempted to be resolved in this patch:
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
>
> While at here, it might be good to resolve this either on power domains
> or the clock API level.
>
I know this issue about your commnt. This patch didn't consider the clock domain
because refer to the previous function which enable/disable only the power state
of power domain. As your comment, I will adapt that handling S5P_CLKGATE_BLOCK to
control the clock domains.
>> + genpd->power_off = pd_power_down;
>> + genpd->power_on = pd_power_up;
>> +
>> + if (domain->boot_on)
>> + pd_power_up(genpd);
>> +
>> + return 0;
>> +}
>> +
>> +int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
>> + struct platform_device *pdev)
>> +{
>> + struct device *dev =&pdev->dev;
>> + struct generic_pm_domain *genpd =&domain->genpd;
>> +
>> + if (pdev != NULL)
>> + return pm_genpd_add_device(genpd, dev);
>> + else
>> + return -EINVAL;
>
> You could do instead:
>
> + if (pdev)
> + return pm_genpd_add_device(genpd, dev);
> +
> + return -EINVAL;
>
Ok, I will modify it.
Thanks for your comment.
Regards,
Chanwoo Choi