[PATCH 0/2] LOW power sleep support for TI PWM submodules

[PATCH 0/2] LOW power sleep support for TI PWM submodules

[Philip Avinash]

TI PWM sub modules like ECAP & EHRPWM can enter in low power sleep
state during low power transitions of the platforms (like in AM33XX).
This patch series support low power sleep transition support. This
patch series depend on [1] and [2] and tested for low sleep support
on AM335x-evm for ECAP based PWM backlight.

1. ARM: OMAP2+: AM33XX: Add suspend-resume support
   http://www.mail-archive.com/linux-omap@vger.kernel.org/msg82635.html
2. HWMOD fixes for AM33xx PWM submodules and device tree nodes.
   https://lkml.org/lkml/2013/1/2/87

Philip Avinash (2):
  pwm: pwm-tiehrpwm: Low power sleep support
  pwm: pwm-tiecap: Low power sleep support

 drivers/pwm/pwm-tiecap.c   |   45 +++++++++++++++++++++++++++++
 drivers/pwm/pwm-tiehrpwm.c |   67 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

-- 
1.7.9.5

[PATCH 1/2] pwm: pwm-tiehrpwm: Low power sleep support

[Philip Avinash]

In low power modes of AM33XX platforms, peripherals power is cut off.
This patch supports low power sleep transition support for EHRPWM
driver.

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
 drivers/pwm/pwm-tiehrpwm.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 4fcafbf..831f394 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -113,6 +113,17 @@
 
 #define NUM_PWM_CHANNEL		2	/* EHRPWM channels */
 
+struct ehrpwm_context {
+	u16 tbctl;
+	u16 tbprd;
+	u16 cmpa;
+	u16 cmpb;
+	u16 aqctla;
+	u16 aqctlb;
+	u16 aqsfrc;
+	u16 aqcsfrc;
+};
+
 struct ehrpwm_pwm_chip {
 	struct pwm_chip	chip;
 	unsigned int	clk_rate;
@@ -120,6 +131,7 @@ struct ehrpwm_pwm_chip {
 	unsigned long period_cycles[NUM_PWM_CHANNEL];
 	enum pwm_polarity polarity[NUM_PWM_CHANNEL];
 	struct	clk	*tbclk;
+	struct ehrpwm_context ctx;
 };
 
 static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
@@ -127,6 +139,11 @@ static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
 	return container_of(chip, struct ehrpwm_pwm_chip, chip);
 }
 
+static u16 ehrpwm_read(void *base, int offset)
+{
+	return readw(base + offset);
+}
+
 static void ehrpwm_write(void *base, int offset, unsigned int val)
 {
 	writew(val & 0xFFFF, base + offset);
@@ -516,11 +533,59 @@ static int ehrpwm_pwm_remove(struct platform_device *pdev)
 	return pwmchip_remove(&pc->chip);
 }
 
+void ehrpwm_pwm_context_save(struct ehrpwm_pwm_chip *pc)
+{
+	pm_runtime_get_sync(pc->chip.dev);
+	pc->ctx.tbctl = ehrpwm_read(pc->mmio_base, TBCTL);
+	pc->ctx.tbprd = ehrpwm_read(pc->mmio_base, TBPRD);
+	pc->ctx.cmpa = ehrpwm_read(pc->mmio_base, CMPA);
+	pc->ctx.cmpb = ehrpwm_read(pc->mmio_base, CMPB);
+	pc->ctx.aqctla = ehrpwm_read(pc->mmio_base, AQCTLA);
+	pc->ctx.aqctlb = ehrpwm_read(pc->mmio_base, AQCTLB);
+	pc->ctx.aqsfrc = ehrpwm_read(pc->mmio_base, AQSFRC);
+	pc->ctx.aqcsfrc = ehrpwm_read(pc->mmio_base, AQCSFRC);
+	pm_runtime_put_sync(pc->chip.dev);
+}
+
+void ehrpwm_pwm_context_restore(struct ehrpwm_pwm_chip *pc)
+{
+	ehrpwm_write(pc->mmio_base, TBPRD, pc->ctx.tbprd);
+	ehrpwm_write(pc->mmio_base, CMPA, pc->ctx.cmpa);
+	ehrpwm_write(pc->mmio_base, CMPB, pc->ctx.cmpb);
+	ehrpwm_write(pc->mmio_base, AQCTLA, pc->ctx.aqctla);
+	ehrpwm_write(pc->mmio_base, AQCTLB, pc->ctx.aqctlb);
+	ehrpwm_write(pc->mmio_base, AQSFRC, pc->ctx.aqsfrc);
+	ehrpwm_write(pc->mmio_base, AQCSFRC, pc->ctx.aqcsfrc);
+	ehrpwm_write(pc->mmio_base, TBCTL, pc->ctx.tbctl);
+}
+
+static int ehrpwm_pwm_suspend(struct device *dev)
+{
+	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
+
+	ehrpwm_pwm_context_save(pc);
+	pm_runtime_put_sync(dev);
+	return 0;
+}
+
+static int ehrpwm_pwm_resume(struct device *dev)
+{
+	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+	ehrpwm_pwm_context_restore(pc);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ehrpwm_pwm_pm_ops, ehrpwm_pwm_suspend,
+		ehrpwm_pwm_resume);
+
 static struct platform_driver ehrpwm_pwm_driver = {
 	.driver = {
 		.name	= "ehrpwm",
 		.owner	= THIS_MODULE,
 		.of_match_table = ehrpwm_of_match,
+		.pm	= &ehrpwm_pwm_pm_ops,
 	},
 	.probe = ehrpwm_pwm_probe,
 	.remove = ehrpwm_pwm_remove,
-- 
1.7.9.5

[PATCH 2/2] pwm: pwm-tiecap: Low power sleep support

[Philip Avinash]

In low power modes of AM33XX platforms, peripherals power is cut off.
This patch supports low power sleep transition support for ECAP driver.

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
 drivers/pwm/pwm-tiecap.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index 5cf016d..9dde1fe 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -41,10 +41,17 @@
 #define ECCTL2_SYNC_SEL_DISA	(BIT(7) | BIT(6))
 #define ECCTL2_TSCTR_FREERUN	BIT(4)
 
+struct ecap_regs {
+	u32	cap3;
+	u32	cap4;
+	u16	ecctl2;
+};
+
 struct ecap_pwm_chip {
 	struct pwm_chip	chip;
 	unsigned int	clk_rate;
 	void __iomem	*mmio_base;
+	struct ecap_regs ctx;
 };
 
 static inline struct ecap_pwm_chip *to_ecap_pwm_chip(struct pwm_chip *chip)
@@ -288,11 +295,48 @@ static int ecap_pwm_remove(struct platform_device *pdev)
 	return pwmchip_remove(&pc->chip);
 }
 
+void ecap_pwm_save_reg(struct ecap_pwm_chip *pc)
+{
+	pm_runtime_get_sync(pc->chip.dev);
+	pc->ctx.ecctl2 = readw(pc->mmio_base + ECCTL2);
+	pc->ctx.cap4 = readl(pc->mmio_base + CAP4);
+	pc->ctx.cap3 = readl(pc->mmio_base + CAP3);
+	pm_runtime_put_sync(pc->chip.dev);
+}
+
+void ecap_pwm_restore_reg(struct ecap_pwm_chip *pc)
+{
+	writel(pc->ctx.cap3, pc->mmio_base + CAP3);
+	writel(pc->ctx.cap4, pc->mmio_base + CAP4);
+	writew(pc->ctx.ecctl2, pc->mmio_base + ECCTL2);
+}
+
+static int ecap_pwm_suspend(struct device *dev)
+{
+	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
+
+	ecap_pwm_save_reg(pc);
+	pm_runtime_put_sync(dev);
+	return 0;
+}
+
+static int ecap_pwm_resume(struct device *dev)
+{
+	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+	ecap_pwm_restore_reg(pc);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ecap_pwm_pm_ops, ecap_pwm_suspend, ecap_pwm_resume);
+
 static struct platform_driver ecap_pwm_driver = {
 	.driver = {
 		.name	= "ecap",
 		.owner	= THIS_MODULE,
 		.of_match_table = ecap_of_match,
+		.pm	= &ecap_pwm_pm_ops,
 	},
 	.probe = ecap_pwm_probe,
 	.remove = ecap_pwm_remove,
-- 
1.7.9.5

Re: [PATCH 1/2] pwm: pwm-tiehrpwm: Low power sleep support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

On Thu, Jan 10, 2013 at 06:33:43PM +0530, Philip Avinash wrote:
[...]
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
[...]
> +static int ehrpwm_pwm_suspend(struct device *dev)
> +{
> +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> +
> +	ehrpwm_pwm_context_save(pc);
> +	pm_runtime_put_sync(dev);
> +	return 0;
> +}
> +
> +static int ehrpwm_pwm_resume(struct device *dev)
> +{
> +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> +
> +	pm_runtime_get_sync(dev);
> +	ehrpwm_pwm_context_restore(pc);
> +	return 0;
> +}

According to Documentation/power/runtime_pm.txt, the PM core runs the
pm_runtime_get_noresume() and pm_runtime_put_sync() before executing the
.suspend() and .resume() callbacks. Are you sure you need to call them
here explicitly?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] pwm: pwm-tiecap: Low power sleep support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Thu, Jan 10, 2013 at 06:33:44PM +0530, Philip Avinash wrote:
> In low power modes of AM33XX platforms, peripherals power is cut off.
> This patch supports low power sleep transition support for ECAP driver.
> 
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> ---
>  drivers/pwm/pwm-tiecap.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
> index 5cf016d..9dde1fe 100644
> --- a/drivers/pwm/pwm-tiecap.c
> +++ b/drivers/pwm/pwm-tiecap.c
> @@ -41,10 +41,17 @@
>  #define ECCTL2_SYNC_SEL_DISA	(BIT(7) | BIT(6))
>  #define ECCTL2_TSCTR_FREERUN	BIT(4)
>  
> +struct ecap_regs {
> +	u32	cap3;
> +	u32	cap4;
> +	u16	ecctl2;
> +};

Perhaps name this ecap_context for consistency with the EHRPWM driver?

> +void ecap_pwm_save_reg(struct ecap_pwm_chip *pc)
> +{
> +	pm_runtime_get_sync(pc->chip.dev);
> +	pc->ctx.ecctl2 = readw(pc->mmio_base + ECCTL2);
> +	pc->ctx.cap4 = readl(pc->mmio_base + CAP4);
> +	pc->ctx.cap3 = readl(pc->mmio_base + CAP3);
> +	pm_runtime_put_sync(pc->chip.dev);
> +}
> +
> +void ecap_pwm_restore_reg(struct ecap_pwm_chip *pc)
> +{
> +	writel(pc->ctx.cap3, pc->mmio_base + CAP3);
> +	writel(pc->ctx.cap4, pc->mmio_base + CAP4);
> +	writew(pc->ctx.ecctl2, pc->mmio_base + ECCTL2);
> +}

Then rename these ecap_pwm_{save,restore}_context()?

> +static int ecap_pwm_suspend(struct device *dev)
> +{
> +	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
> +
> +	ecap_pwm_save_reg(pc);
> +	pm_runtime_put_sync(dev);
> +	return 0;
> +}
> +
> +static int ecap_pwm_resume(struct device *dev)
> +{
> +	struct ecap_pwm_chip *pc = dev_get_drvdata(dev);
> +
> +	pm_runtime_get_sync(dev);
> +	ecap_pwm_restore_reg(pc);
> +	return 0;
> +}

Same comment as for the EHRPWM driver applies here.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH 2/2] pwm: pwm-tiecap: Low power sleep support

[Philip, Avinash]

On Mon, Jan 14, 2013 at 12:40:34, Thierry Reding wrote:
> On Thu, Jan 10, 2013 at 06:33:44PM +0530, Philip Avinash wrote:
> > In low power modes of AM33XX platforms, peripherals power is cut off.
> > This patch supports low power sleep transition support for ECAP driver.
> > 
> > Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> > ---
...
> > +struct ecap_regs {
> > +	u32	cap3;
> > +	u32	cap4;
> > +	u16	ecctl2;
> > +};
> 
> Perhaps name this ecap_context for consistency with the EHRPWM driver?

I will correct it.

> 
> > +void ecap_pwm_save_reg(struct ecap_pwm_chip *pc)
> > +{
> > +	pm_runtime_get_sync(pc->chip.dev);
> > +	pc->ctx.ecctl2 = readw(pc->mmio_base + ECCTL2);
> > +	pc->ctx.cap4 = readl(pc->mmio_base + CAP4);
> > +	pc->ctx.cap3 = readl(pc->mmio_base + CAP3);
> > +	pm_runtime_put_sync(pc->chip.dev);
> > +}
> > +
> > +void ecap_pwm_restore_reg(struct ecap_pwm_chip *pc)
> > +{
> > +	writel(pc->ctx.cap3, pc->mmio_base + CAP3);
> > +	writel(pc->ctx.cap4, pc->mmio_base + CAP4);
> > +	writew(pc->ctx.ecctl2, pc->mmio_base + ECCTL2);
> > +}
> 
> Then rename these ecap_pwm_{save,restore}_context()?

I will correct in next version.

...

> Same comment as for the EHRPWM driver applies here.

I will correct in next version

Thanks
Avinash

> 
> Thierry
> 

RE: [PATCH 1/2] pwm: pwm-tiehrpwm: Low power sleep support

[Philip, Avinash]

On Mon, Jan 14, 2013 at 12:38:56, Thierry Reding wrote:
> On Thu, Jan 10, 2013 at 06:33:43PM +0530, Philip Avinash wrote:
> [...]
> > diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> [...]
> > +static int ehrpwm_pwm_suspend(struct device *dev)
> > +{
> > +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> > +
> > +	ehrpwm_pwm_context_save(pc);
> > +	pm_runtime_put_sync(dev);
> > +	return 0;
> > +}
> > +
> > +static int ehrpwm_pwm_resume(struct device *dev)
> > +{
> > +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> > +
> > +	pm_runtime_get_sync(dev);
> > +	ehrpwm_pwm_context_restore(pc);
> > +	return 0;
> > +}
> 
> According to Documentation/power/runtime_pm.txt, the PM core runs the
> pm_runtime_get_noresume() and pm_runtime_put_sync() before executing the
> .suspend() and .resume() callbacks. Are you sure you need to call them
> here explicitly?

I understand the problem of calling pm_runtime_get_sync() from .resume().
But this has to be called if pwm was running while going to suspend so that
pwm can continues to run after resume.

So I will add check of test_bit(PWMF_ENABLED, &pwm->flags) before 
pm_runtime_get/put_sync calls.

Thanks
Avinash

> 
> Thierry
> 

Re: [PATCH 1/2] pwm: pwm-tiehrpwm: Low power sleep support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

On Wed, Jan 16, 2013 at 12:14:01PM +0000, Philip, Avinash wrote:
> On Mon, Jan 14, 2013 at 12:38:56, Thierry Reding wrote:
> > On Thu, Jan 10, 2013 at 06:33:43PM +0530, Philip Avinash wrote:
> > [...]
> > > diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> > [...]
> > > +static int ehrpwm_pwm_suspend(struct device *dev)
> > > +{
> > > +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> > > +
> > > +	ehrpwm_pwm_context_save(pc);
> > > +	pm_runtime_put_sync(dev);
> > > +	return 0;
> > > +}
> > > +
> > > +static int ehrpwm_pwm_resume(struct device *dev)
> > > +{
> > > +	struct ehrpwm_pwm_chip *pc = dev_get_drvdata(dev);
> > > +
> > > +	pm_runtime_get_sync(dev);
> > > +	ehrpwm_pwm_context_restore(pc);
> > > +	return 0;
> > > +}
> > 
> > According to Documentation/power/runtime_pm.txt, the PM core runs the
> > pm_runtime_get_noresume() and pm_runtime_put_sync() before executing the
> > .suspend() and .resume() callbacks. Are you sure you need to call them
> > here explicitly?
> 
> I understand the problem of calling pm_runtime_get_sync() from .resume().
> But this has to be called if pwm was running while going to suspend so that
> pwm can continues to run after resume.

Okay. I misread the documentation and/or your patch. The documentation
says that the core calls pm_runtime_get_noresume() before executing the
.suspend() callback so you're not in fact calling it twice. Sorry for
the confusion.

> So I will add check of test_bit(PWMF_ENABLED, &pwm->flags) before 
> pm_runtime_get/put_sync calls.

Yes, that sounds reasonable.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]