| Message ID | 1323704789-23923-3-git-send-email-thomas.abraham@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Dec 12, 2011 at 09:16:29PM +0530, Thomas Abraham wrote: > + /* Wait max 1ms */ > + timeout = 10; > + while ((__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) > + != S5P_INT_LOCAL_PWR_EN) { > + if (!timeout) { > + pr_err("Power domain %s enable failed\n", domain->name); > + return -ETIMEDOUT; > + } > + timeout--; > + udelay(100); > + } Might be worth putting a cpu_relax() in there just to be a bit nicer?
On 12/26/2011 08:06 PM, Mark Brown wrote: > On Mon, Dec 12, 2011 at 09:16:29PM +0530, Thomas Abraham wrote: > >> + /* Wait max 1ms */ >> + timeout = 10; >> + while ((__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) >> + != S5P_INT_LOCAL_PWR_EN) { >> + if (!timeout) { >> + pr_err("Power domain %s enable failed\n", domain->name); >> + return -ETIMEDOUT; >> + } >> + timeout--; >> + udelay(100); >> + } > > Might be worth putting a cpu_relax() in there just to be a bit nicer? There is a quite significant delay within the loop, thus it might make more sense to replace udelay() with usleep_range() instead.
Hi Thomas, On 12/12/2011 04:46 PM, Thomas Abraham wrote: > The generic power domain infrastructure is used to control the power domains > available on Exynos4. For non-dt platforms, the power domains are statically > instantiated. For dt platforms, the power domain nodes found in the device > tree are instantiated. > > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > --- > This patch is mainly derived from Mark Brown's work on generic power domain > support for s3c64xx platforms. The existing exynos4 power domain implementation > is not removed in this patch. The devices are not yet registered with the power > domains for non-dt platforms. > > arch/arm/mach-exynos/Kconfig | 1 + > arch/arm/mach-exynos/pm.c | 179 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 180 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig > index 0afcc3b..c1f77c7 100644 > --- a/arch/arm/mach-exynos/Kconfig > +++ b/arch/arm/mach-exynos/Kconfig > @@ -29,6 +29,7 @@ config CPU_EXYNOS4210 > default y > depends on ARCH_EXYNOS4 > select SAMSUNG_DMADEV > + select PM_GENERIC_DOMAINS > select ARM_CPU_SUSPEND if PM > select S5P_PM if PM > select S5P_SLEEP if PM > diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c > index 4093fea..a471ea6 100644 > --- a/arch/arm/mach-exynos/pm.c > +++ b/arch/arm/mach-exynos/pm.c > @@ -20,6 +20,10 @@ > #include <linux/io.h> > #include <linux/err.h> > #include <linux/clk.h> > +#include <linux/slab.h> > +#include <linux/pm_domain.h> > +#include <linux/delay.h> > +#include <linux/of_address.h> > > #include <asm/cacheflush.h> > #include <asm/hardware/cache-l2x0.h> > @@ -37,6 +41,13 @@ > #include <mach/pm-core.h> > #include <mach/pmu.h> > > +struct exynos4_pm_domain { > + void __iomem *base; > + char const *name; > + bool is_off; > + struct generic_pm_domain pd; > +}; > + > static struct sleep_save exynos4_set_clksrc[] = { > { .reg = S5P_CLKSRC_MASK_TOP , .val = 0x00000001, }, > { .reg = S5P_CLKSRC_MASK_CAM , .val = 0x11111111, }, > @@ -285,12 +296,172 @@ static struct sysdev_driver exynos4_pm_driver = { > .add = exynos4_pm_add, > }; > > +static int exynos4_pd_power_on(struct generic_pm_domain *domain) > +{ > + struct exynos4_pm_domain *pd; > + void __iomem *base; > + u32 timeout; > + > + pd = container_of(domain, struct exynos4_pm_domain, pd); > + base = pd->base; > + > + __raw_writel(S5P_INT_LOCAL_PWR_EN, base); > + > + /* Wait max 1ms */ > + timeout = 10; > + while ((__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) > + != S5P_INT_LOCAL_PWR_EN) { > + if (!timeout) { > + pr_err("Power domain %s enable failed\n", domain->name); > + return -ETIMEDOUT; > + } > + timeout--; > + udelay(100); > + } > + return 0; > +} > + > +static int exynos4_pd_power_off(struct generic_pm_domain *domain) > +{ > + struct exynos4_pm_domain *pd; > + void __iomem *base; > + u32 timeout; > + > + pd = container_of(domain, struct exynos4_pm_domain, pd); > + base = pd->base; > + > + __raw_writel(0, base); > + > + /* Wait max 1ms */ > + timeout = 10; > + while (__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) { > + if (!timeout) { > + pr_err("Power domain %s disable failed\n", domain->name); > + return -ETIMEDOUT; > + } > + timeout--; > + udelay(100); > + } > + return 0; > +} > + > +static struct exynos4_pm_domain exynos4_pd_mfc = { > + .base = (void __iomem *)S5P_PMU_MFC_CONF, > + .name = "pd-mfc", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_g3d = { > + .base = (void __iomem *)S5P_PMU_G3D_CONF, > + .name = "pd-g3d", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_lcd0 = { > + .base = (void __iomem *)S5P_PMU_LCD0_CONF, > + .name = "pd-lcd0", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_lcd1 = { > + .base = (void __iomem *)S5P_PMU_LCD1_CONF, > + .name = "pd-lcd1", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_tv = { > + .base = (void __iomem *)S5P_PMU_TV_CONF, > + .name = "pd-tv", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_cam = { > + .base = (void __iomem *)S5P_PMU_CAM_CONF, > + .name = "pd-cam", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain exynos4_pd_gps = { > + .base = (void __iomem *)S5P_PMU_GPS_CONF, > + .name = "pd-gps", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; I'm not sure if arch/arm/mach-exynos/pm.c is the right place to add this. IMHO it would be much better to put PD in separate file, pm-runtime.c or something like this. Can we assume various exynos versions will have same S5P_PMU_*_CONF addresses? For old s3c64xx SoCs such assumption could be valid but I'm a bit uncertain about Exynos series. IMHO the proper way to proceed would be to convert Samsung power domains to generic power domains and then add device tree support. Rather than doing a sort of work around. Much of the conversion to genpd work have been already done [1]. We would perhaps just need to drop the clock gating as there seem to be no agreement about this. [1]. http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg07431.html -- Thanks, Sylwester
Hi Sylwester, On 28 December 2011 04:44, Sylwester Nawrocki <snjw23@gmail.com> wrote: > Hi Thomas, > > On 12/12/2011 04:46 PM, Thomas Abraham wrote: >> The generic power domain infrastructure is used to control the power domains >> available on Exynos4. For non-dt platforms, the power domains are statically >> instantiated. For dt platforms, the power domain nodes found in the device >> tree are instantiated. >> >> Cc: Kukjin Kim <kgene.kim@samsung.com> >> Cc: Rob Herring <rob.herring@calxeda.com> >> Cc: Grant Likely <grant.likely@secretlab.ca> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> >> --- >> This patch is mainly derived from Mark Brown's work on generic power domain >> support for s3c64xx platforms. The existing exynos4 power domain implementation >> is not removed in this patch. The devices are not yet registered with the power >> domains for non-dt platforms. >> >> arch/arm/mach-exynos/Kconfig | 1 + >> arch/arm/mach-exynos/pm.c | 179 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 180 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig >> index 0afcc3b..c1f77c7 100644 >> --- a/arch/arm/mach-exynos/Kconfig >> +++ b/arch/arm/mach-exynos/Kconfig >> @@ -29,6 +29,7 @@ config CPU_EXYNOS4210 >> default y >> depends on ARCH_EXYNOS4 >> select SAMSUNG_DMADEV >> + select PM_GENERIC_DOMAINS >> select ARM_CPU_SUSPEND if PM >> select S5P_PM if PM >> select S5P_SLEEP if PM >> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c >> index 4093fea..a471ea6 100644 >> --- a/arch/arm/mach-exynos/pm.c >> +++ b/arch/arm/mach-exynos/pm.c >> @@ -20,6 +20,10 @@ >> #include <linux/io.h> >> #include <linux/err.h> >> #include <linux/clk.h> >> +#include <linux/slab.h> >> +#include <linux/pm_domain.h> >> +#include <linux/delay.h> >> +#include <linux/of_address.h> >> >> #include <asm/cacheflush.h> >> #include <asm/hardware/cache-l2x0.h> >> @@ -37,6 +41,13 @@ >> #include <mach/pm-core.h> >> #include <mach/pmu.h> >> >> +struct exynos4_pm_domain { >> + void __iomem *base; >> + char const *name; >> + bool is_off; >> + struct generic_pm_domain pd; >> +}; >> + >> static struct sleep_save exynos4_set_clksrc[] = { >> { .reg = S5P_CLKSRC_MASK_TOP , .val = 0x00000001, }, >> { .reg = S5P_CLKSRC_MASK_CAM , .val = 0x11111111, }, >> @@ -285,12 +296,172 @@ static struct sysdev_driver exynos4_pm_driver = { >> .add = exynos4_pm_add, >> }; >> >> +static int exynos4_pd_power_on(struct generic_pm_domain *domain) >> +{ >> + struct exynos4_pm_domain *pd; >> + void __iomem *base; >> + u32 timeout; >> + >> + pd = container_of(domain, struct exynos4_pm_domain, pd); >> + base = pd->base; >> + >> + __raw_writel(S5P_INT_LOCAL_PWR_EN, base); >> + >> + /* Wait max 1ms */ >> + timeout = 10; >> + while ((__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) >> + != S5P_INT_LOCAL_PWR_EN) { >> + if (!timeout) { >> + pr_err("Power domain %s enable failed\n", domain->name); >> + return -ETIMEDOUT; >> + } >> + timeout--; >> + udelay(100); >> + } >> + return 0; >> +} >> + >> +static int exynos4_pd_power_off(struct generic_pm_domain *domain) >> +{ >> + struct exynos4_pm_domain *pd; >> + void __iomem *base; >> + u32 timeout; >> + >> + pd = container_of(domain, struct exynos4_pm_domain, pd); >> + base = pd->base; >> + >> + __raw_writel(0, base); >> + >> + /* Wait max 1ms */ >> + timeout = 10; >> + while (__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) { >> + if (!timeout) { >> + pr_err("Power domain %s disable failed\n", domain->name); >> + return -ETIMEDOUT; >> + } >> + timeout--; >> + udelay(100); >> + } >> + return 0; >> +} >> + >> +static struct exynos4_pm_domain exynos4_pd_mfc = { >> + .base = (void __iomem *)S5P_PMU_MFC_CONF, >> + .name = "pd-mfc", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_g3d = { >> + .base = (void __iomem *)S5P_PMU_G3D_CONF, >> + .name = "pd-g3d", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_lcd0 = { >> + .base = (void __iomem *)S5P_PMU_LCD0_CONF, >> + .name = "pd-lcd0", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_lcd1 = { >> + .base = (void __iomem *)S5P_PMU_LCD1_CONF, >> + .name = "pd-lcd1", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_tv = { >> + .base = (void __iomem *)S5P_PMU_TV_CONF, >> + .name = "pd-tv", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_cam = { >> + .base = (void __iomem *)S5P_PMU_CAM_CONF, >> + .name = "pd-cam", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; >> + >> +static struct exynos4_pm_domain exynos4_pd_gps = { >> + .base = (void __iomem *)S5P_PMU_GPS_CONF, >> + .name = "pd-gps", >> + .pd = { >> + .power_off = exynos4_pd_power_off, >> + .power_on = exynos4_pd_power_on, >> + }, >> +}; > > I'm not sure if arch/arm/mach-exynos/pm.c is the right place to add this. > IMHO it would be much better to put PD in separate file, pm-runtime.c > or something like this. Ok. I will move this to a new file pm-runtime.c. > > Can we assume various exynos versions will have same S5P_PMU_*_CONF > addresses? For old s3c64xx SoCs such assumption could be valid but I'm > a bit uncertain about Exynos series. The addresses could be different, but it can be mapped to a S5P_PMU_*_CONF. Maybe I did not understand your question here. > > IMHO the proper way to proceed would be to convert Samsung power domains > to generic power domains and then add device tree support. Rather than Yes, this patchset is intended to replace Samsung specific power domains with generic power domains. > doing a sort of work around. Much of the conversion to genpd work have > been already done [1]. We would perhaps just need to drop the clock gating > as there seem to be no agreement about this. Ok. Thanks for the link. I think I missed looking at this patch. Thanks, Thomas. > > [1]. http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg07431.html > > -- > > Thanks, > Sylwester
Hi Thomas, On 12/28/2011 06:25 AM, Thomas Abraham wrote: >>> + >>> +static struct exynos4_pm_domain exynos4_pd_gps = { >>> + .base = (void __iomem *)S5P_PMU_GPS_CONF, >>> + .name = "pd-gps", >>> + .pd = { >>> + .power_off = exynos4_pd_power_off, >>> + .power_on = exynos4_pd_power_on, >>> + }, >>> +}; >> >> I'm not sure if arch/arm/mach-exynos/pm.c is the right place to add this. >> IMHO it would be much better to put PD in separate file, pm-runtime.c >> or something like this. > > Ok. I will move this to a new file pm-runtime.c. Thanks. >> Can we assume various exynos versions will have same S5P_PMU_*_CONF >> addresses? For old s3c64xx SoCs such assumption could be valid but I'm >> a bit uncertain about Exynos series. > > The addresses could be different, but it can be mapped to a > S5P_PMU_*_CONF. Maybe I did not understand your question here. I meant that for handling the newer SoCs (e.g. exynos5) at runtime we might need to create new power domain definitions. That's why I suggested new file for runtime PM. mach-shmobile has even separate compilation unit per SoC, with register definitions put directly in C file. I'm not sure we need separate pm-exynos4.c, pm-exynos5.c... For now your patch looks good. -- Regards, Sylwester
Hi Thomas, On 12/12/2011 04:46 PM, Thomas Abraham wrote: > The generic power domain infrastructure is used to control the power domains > available on Exynos4. For non-dt platforms, the power domains are statically > instantiated. For dt platforms, the power domain nodes found in the device > tree are instantiated. > > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > --- > This patch is mainly derived from Mark Brown's work on generic power domain > support for s3c64xx platforms. The existing exynos4 power domain implementation > is not removed in this patch. The devices are not yet registered with the power > domains for non-dt platforms. > > arch/arm/mach-exynos/Kconfig | 1 + > arch/arm/mach-exynos/pm.c | 179 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 180 insertions(+), 0 deletions(-) > [...] > + > +static struct exynos4_pm_domain exynos4_pd_gps = { > + .base = (void __iomem *)S5P_PMU_GPS_CONF, > + .name = "pd-gps", > + .pd = { > + .power_off = exynos4_pd_power_off, > + .power_on = exynos4_pd_power_on, > + }, > +}; > + > +static struct exynos4_pm_domain *exynos4_pm_domains[] = { > + &exynos4_pd_mfc, > + &exynos4_pd_g3d, > + &exynos4_pd_lcd0, > + &exynos4_pd_lcd1, > + &exynos4_pd_tv, > + &exynos4_pd_cam, > + &exynos4_pd_gps, > +}; > + > +static __init void exynos4_pm_init_power_domain(void) > +{ > + int idx; > + struct device_node *np; > + > +#ifdef CONFIG_OF > + if (!of_have_populated_dt()) > + goto no_dt; > + > + for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") { > + struct exynos4_pm_domain *pd; > + > + pd = kzalloc(sizeof(*pd), GFP_KERNEL); > + if (!pd) { > + pr_err("exynos4_pm_init_power_domain: memalloc " > + "failed\n"); > + return; > + } > + > + if (of_get_property(np, "samsung,exynos4210-pd-off", NULL)) > + pd->is_off = true; > + pd->name = np->name; > + pd->base = of_iomap(np, 0); Sorry, I haven't reviewed your patch carefully enough. So for dt platforms pd->base is initialized from "reg" property, directly from each power domain's DT node. Only the static power domain instantiation for non-dt platforms would possibly need some code modifications when new SoCs are added. Would be nice to have a relevant patch for *.dts files in this series too. :) > + pd->pd.power_off = exynos4_pd_power_off; > + pd->pd.power_on = exynos4_pd_power_on; > + pd->pd.of_node = np; > + pm_genpd_init(&pd->pd, NULL, false); > + } > + return; > +#endif /* CONFIG_OF */ > + > +no_dt: > + for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++) > + pm_genpd_init(&exynos4_pm_domains[idx]->pd, NULL, > + exynos4_pm_domains[idx]->is_off); > +} -- Thanks, Sylwester
Hi Sylwester, On 29 December 2011 00:28, Sylwester Nawrocki <snjw23@gmail.com> wrote: > Hi Thomas, > > On 12/12/2011 04:46 PM, Thomas Abraham wrote: >> The generic power domain infrastructure is used to control the power domains >> available on Exynos4. For non-dt platforms, the power domains are statically >> instantiated. For dt platforms, the power domain nodes found in the device >> tree are instantiated. >> >> Cc: Kukjin Kim <kgene.kim@samsung.com> >> Cc: Rob Herring <rob.herring@calxeda.com> >> Cc: Grant Likely <grant.likely@secretlab.ca> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> >> --- >> This patch is mainly derived from Mark Brown's work on generic power domain >> support for s3c64xx platforms. The existing exynos4 power domain implementation >> is not removed in this patch. The devices are not yet registered with the power >> domains for non-dt platforms. >> >> arch/arm/mach-exynos/Kconfig | 1 + >> arch/arm/mach-exynos/pm.c | 179 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 180 insertions(+), 0 deletions(-) [...] > Sorry, I haven't reviewed your patch carefully enough. So for dt platforms > pd->base is initialized from "reg" property, directly from each power domain's > DT node. Only the static power domain instantiation for non-dt platforms would > possibly need some code modifications when new SoCs are added. > > Would be nice to have a relevant patch for *.dts files in this series too. :) The following is a snippet from the dts file used for testing. [...] lcd0:power-domain-lcd0 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C00 0x10>; }; [...] fimd0:display-controller { compatible = "samsung,exynos4-fimd"; [...] pd = <&lcd0>; }; The fimd (display controller) driver would then do the following. parp = of_get_property(pdev->dev.of_node, "pd", NULL); pd_np = of_find_node_by_phandle(be32_to_cpup(parp)); pm_genpd_of_add_device(pd_np, &pdev->dev); The lookup is based on the node pointer of the power domain. Thanks, Thomas.
Hi Thomas, thank you for clarifying. On 01/02/2012 03:14 AM, Thomas Abraham wrote: > > The following is a snippet from the dts file used for testing. > > [...] > > lcd0:power-domain-lcd0 { > compatible = "samsung,exynos4210-pd"; > reg = <0x10023C00 0x10>; > }; > > [...] > > fimd0:display-controller { > compatible = "samsung,exynos4-fimd"; > [...] > pd = <&lcd0>; > }; > > The fimd (display controller) driver would then do the following. > > parp = of_get_property(pdev->dev.of_node, "pd", NULL); > pd_np = of_find_node_by_phandle(be32_to_cpup(parp)); > pm_genpd_of_add_device(pd_np, &pdev->dev); Sounds interesting. Currently it's platform code that adds devices to a corresponding power domain. But doing it at drivers might be more convenient for avoiding device/driver/power domain registration synchronization issues, especially that knowledge about power domain existence may be contained directly in DT description, not needing drivers to carry platform specific data. BTW, I have a feeling that "samsung" is a bit longish prefix for the bindings. Didn't you initially consider "sec" for instance ? Probably it is already too late for changing that though. > The lookup is based on the node pointer of the power domain. Thanks, Sylwester
Hi Sylwester, On 3 January 2012 03:49, Sylwester Nawrocki <snjw23@gmail.com> wrote: > Hi Thomas, > > thank you for clarifying. > > On 01/02/2012 03:14 AM, Thomas Abraham wrote: >> >> The following is a snippet from the dts file used for testing. >> >> [...] >> >> lcd0:power-domain-lcd0 { >> compatible = "samsung,exynos4210-pd"; >> reg = <0x10023C00 0x10>; >> }; >> >> [...] >> >> fimd0:display-controller { >> compatible = "samsung,exynos4-fimd"; >> [...] >> pd = <&lcd0>; >> }; >> >> The fimd (display controller) driver would then do the following. >> >> parp = of_get_property(pdev->dev.of_node, "pd", NULL); >> pd_np = of_find_node_by_phandle(be32_to_cpup(parp)); >> pm_genpd_of_add_device(pd_np, &pdev->dev); > > Sounds interesting. Currently it's platform code that adds devices to > a corresponding power domain. But doing it at drivers might be more > convenient for avoiding device/driver/power domain registration > synchronization issues, especially that knowledge about power domain > existence may be contained directly in DT description, not needing > drivers to carry platform specific data. > > BTW, I have a feeling that "samsung" is a bit longish prefix for the bindings. > Didn't you initially consider "sec" for instance ? Probably it is already > too late for changing that though. I had not thought of "sec". I agree that "sec" would have been better as it is shorter and represents bindings specific to Samsung Electronics. But it is not intuitive at the same time. If there is greater consensus on using "sec", we could try and request for a change but looks difficult to get through. Thanks, Thomas. > >> The lookup is based on the node pointer of the power domain. > > Thanks, > Sylwester
On Tue, Jan 03, 2012 at 01:53:28PM +0530, Thomas Abraham wrote: > Hi Sylwester, > > On 3 January 2012 03:49, Sylwester Nawrocki <snjw23@gmail.com> wrote: > > Hi Thomas, > > > > thank you for clarifying. > > > > On 01/02/2012 03:14 AM, Thomas Abraham wrote: > >> > >> The following is a snippet from the dts file used for testing. > >> > >> [...] > >> > >> lcd0:power-domain-lcd0 { > >> compatible = "samsung,exynos4210-pd"; > >> reg = <0x10023C00 0x10>; > >> }; > >> > >> [...] > >> > >> fimd0:display-controller { > >> compatible = "samsung,exynos4-fimd"; > >> [...] > >> pd = <&lcd0>; > >> }; > >> > >> The fimd (display controller) driver would then do the following. > >> > >> parp = of_get_property(pdev->dev.of_node, "pd", NULL); > >> pd_np = of_find_node_by_phandle(be32_to_cpup(parp)); > >> pm_genpd_of_add_device(pd_np, &pdev->dev); > > > > Sounds interesting. Currently it's platform code that adds devices to > > a corresponding power domain. But doing it at drivers might be more > > convenient for avoiding device/driver/power domain registration > > synchronization issues, especially that knowledge about power domain > > existence may be contained directly in DT description, not needing > > drivers to carry platform specific data. > > > > BTW, I have a feeling that "samsung" is a bit longish prefix for the bindings. > > Didn't you initially consider "sec" for instance ? Probably it is already > > too late for changing that though. > > I had not thought of "sec". I agree that "sec" would have been better > as it is shorter and represents bindings specific to Samsung > Electronics. But it is not intuitive at the same time. If there is > greater consensus on using "sec", we could try and request for a > change but looks difficult to get through. Don't bother. 'samsung,' is fine. g.
Grant Likely wrote: > > On Tue, Jan 03, 2012 at 01:53:28PM +0530, Thomas Abraham wrote: > > Hi Sylwester, > > > > On 3 January 2012 03:49, Sylwester Nawrocki <snjw23@gmail.com> wrote: > > > Hi Thomas, > > > > > > thank you for clarifying. > > > > > > On 01/02/2012 03:14 AM, Thomas Abraham wrote: > > >> > > >> The following is a snippet from the dts file used for testing. > > >> > > >> [...] > > >> > > >> lcd0:power-domain-lcd0 { > > >> compatible = "samsung,exynos4210-pd"; > > >> reg = <0x10023C00 0x10>; > > >> }; > > >> > > >> [...] > > >> > > >> fimd0:display-controller { > > >> compatible = "samsung,exynos4-fimd"; > > >> [...] > > >> pd = <&lcd0>; > > >> }; > > >> > > >> The fimd (display controller) driver would then do the following. > > >> > > >> parp = of_get_property(pdev->dev.of_node, "pd", NULL); > > >> pd_np = of_find_node_by_phandle(be32_to_cpup(parp)); > > >> pm_genpd_of_add_device(pd_np, &pdev->dev); > > > > > > Sounds interesting. Currently it's platform code that adds devices to > > > a corresponding power domain. But doing it at drivers might be more > > > convenient for avoiding device/driver/power domain registration > > > synchronization issues, especially that knowledge about power domain > > > existence may be contained directly in DT description, not needing > > > drivers to carry platform specific data. > > > > > > BTW, I have a feeling that "samsung" is a bit longish prefix for the > bindings. > > > Didn't you initially consider "sec" for instance ? Probably it is > already > > > too late for changing that though. > > > > I had not thought of "sec". I agree that "sec" would have been better > > as it is shorter and represents bindings specific to Samsung > > Electronics. But it is not intuitive at the same time. If there is > > greater consensus on using "sec", we could try and request for a > > change but looks difficult to get through. > > Don't bother. 'samsung,' is fine. > Same here :) Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index 0afcc3b..c1f77c7 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -29,6 +29,7 @@ config CPU_EXYNOS4210 default y depends on ARCH_EXYNOS4 select SAMSUNG_DMADEV + select PM_GENERIC_DOMAINS select ARM_CPU_SUSPEND if PM select S5P_PM if PM select S5P_SLEEP if PM diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 4093fea..a471ea6 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -20,6 +20,10 @@ #include <linux/io.h> #include <linux/err.h> #include <linux/clk.h> +#include <linux/slab.h> +#include <linux/pm_domain.h> +#include <linux/delay.h> +#include <linux/of_address.h> #include <asm/cacheflush.h> #include <asm/hardware/cache-l2x0.h> @@ -37,6 +41,13 @@ #include <mach/pm-core.h> #include <mach/pmu.h> +struct exynos4_pm_domain { + void __iomem *base; + char const *name; + bool is_off; + struct generic_pm_domain pd; +}; + static struct sleep_save exynos4_set_clksrc[] = { { .reg = S5P_CLKSRC_MASK_TOP , .val = 0x00000001, }, { .reg = S5P_CLKSRC_MASK_CAM , .val = 0x11111111, }, @@ -285,12 +296,172 @@ static struct sysdev_driver exynos4_pm_driver = { .add = exynos4_pm_add, }; +static int exynos4_pd_power_on(struct generic_pm_domain *domain) +{ + struct exynos4_pm_domain *pd; + void __iomem *base; + u32 timeout; + + pd = container_of(domain, struct exynos4_pm_domain, pd); + base = pd->base; + + __raw_writel(S5P_INT_LOCAL_PWR_EN, base); + + /* Wait max 1ms */ + timeout = 10; + while ((__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) + != S5P_INT_LOCAL_PWR_EN) { + if (!timeout) { + pr_err("Power domain %s enable failed\n", domain->name); + return -ETIMEDOUT; + } + timeout--; + udelay(100); + } + return 0; +} + +static int exynos4_pd_power_off(struct generic_pm_domain *domain) +{ + struct exynos4_pm_domain *pd; + void __iomem *base; + u32 timeout; + + pd = container_of(domain, struct exynos4_pm_domain, pd); + base = pd->base; + + __raw_writel(0, base); + + /* Wait max 1ms */ + timeout = 10; + while (__raw_readl(base + 0x4) & S5P_INT_LOCAL_PWR_EN) { + if (!timeout) { + pr_err("Power domain %s disable failed\n", domain->name); + return -ETIMEDOUT; + } + timeout--; + udelay(100); + } + return 0; +} + +static struct exynos4_pm_domain exynos4_pd_mfc = { + .base = (void __iomem *)S5P_PMU_MFC_CONF, + .name = "pd-mfc", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_g3d = { + .base = (void __iomem *)S5P_PMU_G3D_CONF, + .name = "pd-g3d", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_lcd0 = { + .base = (void __iomem *)S5P_PMU_LCD0_CONF, + .name = "pd-lcd0", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_lcd1 = { + .base = (void __iomem *)S5P_PMU_LCD1_CONF, + .name = "pd-lcd1", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_tv = { + .base = (void __iomem *)S5P_PMU_TV_CONF, + .name = "pd-tv", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_cam = { + .base = (void __iomem *)S5P_PMU_CAM_CONF, + .name = "pd-cam", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain exynos4_pd_gps = { + .base = (void __iomem *)S5P_PMU_GPS_CONF, + .name = "pd-gps", + .pd = { + .power_off = exynos4_pd_power_off, + .power_on = exynos4_pd_power_on, + }, +}; + +static struct exynos4_pm_domain *exynos4_pm_domains[] = { + &exynos4_pd_mfc, + &exynos4_pd_g3d, + &exynos4_pd_lcd0, + &exynos4_pd_lcd1, + &exynos4_pd_tv, + &exynos4_pd_cam, + &exynos4_pd_gps, +}; + +static __init void exynos4_pm_init_power_domain(void) +{ + int idx; + struct device_node *np; + +#ifdef CONFIG_OF + if (!of_have_populated_dt()) + goto no_dt; + + for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") { + struct exynos4_pm_domain *pd; + + pd = kzalloc(sizeof(*pd), GFP_KERNEL); + if (!pd) { + pr_err("exynos4_pm_init_power_domain: memalloc " + "failed\n"); + return; + } + + if (of_get_property(np, "samsung,exynos4210-pd-off", NULL)) + pd->is_off = true; + pd->name = np->name; + pd->base = of_iomap(np, 0); + pd->pd.power_off = exynos4_pd_power_off; + pd->pd.power_on = exynos4_pd_power_on; + pd->pd.of_node = np; + pm_genpd_init(&pd->pd, NULL, false); + } + return; +#endif /* CONFIG_OF */ + +no_dt: + for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++) + pm_genpd_init(&exynos4_pm_domains[idx]->pd, NULL, + exynos4_pm_domains[idx]->is_off); +} + static __init int exynos4_pm_drvinit(void) { struct clk *pll_base; unsigned int tmp; s3c_pm_init(); + exynos4_pm_init_power_domain(); /* All wakeup disable */ @@ -406,3 +577,11 @@ static __init int exynos4_pm_syscore_init(void) return 0; } arch_initcall(exynos4_pm_syscore_init); + + +static __init int exynos4_pm_late_initcall(void) +{ + pm_genpd_poweroff_unused(); + return 0; +} +late_initcall(exynos4_pm_late_initcall);
The generic power domain infrastructure is used to control the power domains available on Exynos4. For non-dt platforms, the power domains are statically instantiated. For dt platforms, the power domain nodes found in the device tree are instantiated. Cc: Kukjin Kim <kgene.kim@samsung.com> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Grant Likely <grant.likely@secretlab.ca> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> --- This patch is mainly derived from Mark Brown's work on generic power domain support for s3c64xx platforms. The existing exynos4 power domain implementation is not removed in this patch. The devices are not yet registered with the power domains for non-dt platforms. arch/arm/mach-exynos/Kconfig | 1 + arch/arm/mach-exynos/pm.c | 179 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 0 deletions(-)