2013-04-23 06:01:38

by Axel Lin

[permalink] [raw]
Subject: [RESEND][PATCH RFT 1/2] pwm: lpc32xx: Properly set PWM_ENABLE bit in lpc32xx_pwm_[enable|disable]

According to the LPC32x0 User Manual [1]:

For both PWM1 and PWM2 Control Registers:
BIT 31:
This bit gates the PWM_CLK signal and enables the external output pin
to the PWM_PIN_STATE logical level.

0 = PWM disabled. (Default)
1 = PWM enabled

So in lpc32xx_pwm_enable(), we should set PWM_ENABLE bit.
In lpc32xx_pwm_disable(), we should just clear PWM_ENABLE bit rather than
write 0 to the register which will also clear PWMx_RELOADV and PWMx_DUTY bits.

[1] http://www.nxp.com/documents/user_manual/UM10326.pdf

Signed-off-by: Axel Lin <[email protected]>
---
Hi,
I don't have this hardware handy so I'd appreciate if someone can test this
patch serial.

This patch serial was sent on https://lkml.org/lkml/2013/3/30/104
Seems no feedback so far.
So I just try again, maybe someone can help testing it.
Thanks,
Axel

drivers/pwm/pwm-lpc32xx.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index b3f0d0d..1a5075e 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -77,15 +77,29 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
static int lpc32xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
+ u32 val;
+ int ret;
+
+ ret = clk_enable(lpc32xx->clk);
+ if (ret)
+ return ret;

- return clk_enable(lpc32xx->clk);
+ val = readl(lpc32xx->base + (pwm->hwpwm << 2));
+ val |= PWM_ENABLE;
+ writel(val, lpc32xx->base + (pwm->hwpwm << 2));
+
+ return 0;
}

static void lpc32xx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
+ u32 val;
+
+ val = readl(lpc32xx->base + (pwm->hwpwm << 2));
+ val &= ~PWM_ENABLE;
+ writel(val, lpc32xx->base + (pwm->hwpwm << 2));

- writel(0, lpc32xx->base + (pwm->hwpwm << 2));
clk_disable(lpc32xx->clk);
}

--
1.7.10.4



2013-04-23 06:02:55

by Axel Lin

[permalink] [raw]
Subject: [RESEND][PATCH RFT 2/2] pwm: lpc32xx: Don't change PWM_ENABLE bit in lpc32xx_pwm_config

lpc32xx_pwm_config() is supposed to set duty_ns and period_ns,
it should not change PWM_ENABLE bit.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pwm/pwm-lpc32xx.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 1a5075e..e936202 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -37,6 +37,7 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
unsigned long long c;
int period_cycles, duty_cycles;
+ u32 val;

c = clk_get_rate(lpc32xx->clk) / 256;
c = c * period_ns;
@@ -68,8 +69,10 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
c = 255;
duty_cycles = 256 - c;

- writel(PWM_ENABLE | PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles),
- lpc32xx->base + (pwm->hwpwm << 2));
+ val = readl(lpc32xx->base + (pwm->hwpwm << 2));
+ val &= ~0xFFFF;
+ val |= PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles);
+ writel(val, lpc32xx->base + (pwm->hwpwm << 2));

return 0;
}
--
1.7.10.4


2013-04-23 08:51:19

by Roland Stigge

[permalink] [raw]
Subject: Re: [RESEND][PATCH RFT 1/2] pwm: lpc32xx: Properly set PWM_ENABLE bit in lpc32xx_pwm_[enable|disable]

On 04/23/2013 08:01 AM, Axel Lin wrote:
> According to the LPC32x0 User Manual [1]:
>
> For both PWM1 and PWM2 Control Registers:
> BIT 31:
> This bit gates the PWM_CLK signal and enables the external output pin
> to the PWM_PIN_STATE logical level.
>
> 0 = PWM disabled. (Default)
> 1 = PWM enabled
>
> So in lpc32xx_pwm_enable(), we should set PWM_ENABLE bit.
> In lpc32xx_pwm_disable(), we should just clear PWM_ENABLE bit rather than
> write 0 to the register which will also clear PWMx_RELOADV and PWMx_DUTY bits.
>
> [1] http://www.nxp.com/documents/user_manual/UM10326.pdf
>
> Signed-off-by: Axel Lin <[email protected]>

Tested-by: Roland Stigge <[email protected]>

> ---
> Hi,
> I don't have this hardware handy so I'd appreciate if someone can test this
> patch serial.
>
> This patch serial was sent on https://lkml.org/lkml/2013/3/30/104
> Seems no feedback so far.
> So I just try again, maybe someone can help testing it.
> Thanks,
> Axel
>
> drivers/pwm/pwm-lpc32xx.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
> index b3f0d0d..1a5075e 100644
> --- a/drivers/pwm/pwm-lpc32xx.c
> +++ b/drivers/pwm/pwm-lpc32xx.c
> @@ -77,15 +77,29 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> static int lpc32xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
> struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
> + u32 val;
> + int ret;
> +
> + ret = clk_enable(lpc32xx->clk);
> + if (ret)
> + return ret;
>
> - return clk_enable(lpc32xx->clk);
> + val = readl(lpc32xx->base + (pwm->hwpwm << 2));
> + val |= PWM_ENABLE;
> + writel(val, lpc32xx->base + (pwm->hwpwm << 2));
> +
> + return 0;
> }
>
> static void lpc32xx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
> struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
> + u32 val;
> +
> + val = readl(lpc32xx->base + (pwm->hwpwm << 2));
> + val &= ~PWM_ENABLE;
> + writel(val, lpc32xx->base + (pwm->hwpwm << 2));
>
> - writel(0, lpc32xx->base + (pwm->hwpwm << 2));
> clk_disable(lpc32xx->clk);
> }
>

2013-04-23 08:51:35

by Roland Stigge

[permalink] [raw]
Subject: Re: [RESEND][PATCH RFT 2/2] pwm: lpc32xx: Don't change PWM_ENABLE bit in lpc32xx_pwm_config

On 04/23/2013 08:02 AM, Axel Lin wrote:
> lpc32xx_pwm_config() is supposed to set duty_ns and period_ns,
> it should not change PWM_ENABLE bit.
>
> Signed-off-by: Axel Lin <[email protected]>

Tested-by: Roland Stigge <[email protected]>

> ---
> drivers/pwm/pwm-lpc32xx.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
> index 1a5075e..e936202 100644
> --- a/drivers/pwm/pwm-lpc32xx.c
> +++ b/drivers/pwm/pwm-lpc32xx.c
> @@ -37,6 +37,7 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> struct lpc32xx_pwm_chip *lpc32xx = to_lpc32xx_pwm_chip(chip);
> unsigned long long c;
> int period_cycles, duty_cycles;
> + u32 val;
>
> c = clk_get_rate(lpc32xx->clk) / 256;
> c = c * period_ns;
> @@ -68,8 +69,10 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> c = 255;
> duty_cycles = 256 - c;
>
> - writel(PWM_ENABLE | PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles),
> - lpc32xx->base + (pwm->hwpwm << 2));
> + val = readl(lpc32xx->base + (pwm->hwpwm << 2));
> + val &= ~0xFFFF;
> + val |= PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles);
> + writel(val, lpc32xx->base + (pwm->hwpwm << 2));
>
> return 0;
> }

2013-04-23 09:02:16

by Thierry Reding

[permalink] [raw]
Subject: Re: [RESEND][PATCH RFT 1/2] pwm: lpc32xx: Properly set PWM_ENABLE bit in lpc32xx_pwm_[enable|disable]

On Tue, Apr 23, 2013 at 02:01:31PM +0800, Axel Lin wrote:
> According to the LPC32x0 User Manual [1]:
>
> For both PWM1 and PWM2 Control Registers:
> BIT 31:
> This bit gates the PWM_CLK signal and enables the external output pin
> to the PWM_PIN_STATE logical level.
>
> 0 = PWM disabled. (Default)
> 1 = PWM enabled
>
> So in lpc32xx_pwm_enable(), we should set PWM_ENABLE bit.
> In lpc32xx_pwm_disable(), we should just clear PWM_ENABLE bit rather than
> write 0 to the register which will also clear PWMx_RELOADV and PWMx_DUTY bits.
>
> [1] http://www.nxp.com/documents/user_manual/UM10326.pdf
>
> Signed-off-by: Axel Lin <[email protected]>
> ---
> Hi,
> I don't have this hardware handy so I'd appreciate if someone can test this
> patch serial.
>
> This patch serial was sent on https://lkml.org/lkml/2013/3/30/104
> Seems no feedback so far.
> So I just try again, maybe someone can help testing it.
> Thanks,
> Axel

Both patches applied with Roland's Tested-by. Thanks.

Thierry


Attachments:
(No filename) (997.00 B)
(No filename) (836.00 B)
Download all attachments