[PATCH 0/2] PWM: atmel: few fixes

[PATCH 0/2] PWM: atmel: few fixes

[Alexandre Belloni]

Hi,

Here are a few improvement for pwm-atmel. The first patch makes the driver
actually respect the polarity.

The second patch is solving an issue that I was trying to solve back in october.
When the channel was disabled, the output level got inversed. It actually was
that we had the polarity wrong from the beginning.

Alexandre Belloni (2):
  PWM: atmel-pwm: Fix polarity handling
  PWM: atmel: correct CDTY calculation

 drivers/pwm/pwm-atmel.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] PWM: atmel: Fix polarity handling

[Alexandre Belloni]

When atmel_pwm_config() calculates and then sets the prescaler, it is
overwriting the channel's CMR register so we are losing the CPOL configuration.

As atmel_pwm_config() is always called before enabling a channel, inverting the
polarity doesn't work.

Fix that by reading CMR first and only overwriting the prescaler bits.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/pwm/pwm-atmel.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index bf4144a14661..2d69e9c431dd 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -32,6 +32,7 @@
 /* Bit field in CMR */
 #define PWM_CMR_CPOL		(1 << 9)
 #define PWM_CMR_UPD_CDTY	(1 << 10)
+#define PWM_CMR_CPRE_MSK	0xF
 
 /* The following registers for PWM v1 */
 #define PWMV1_CDTY		0x04
@@ -104,6 +105,7 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	unsigned long clk_rate, prd, dty;
 	unsigned long long div;
 	unsigned int pres = 0;
+	u32 val;
 	int ret;
 
 	if (test_bit(PWMF_ENABLED, &pwm->flags) && (period_ns != pwm->period)) {
@@ -139,7 +141,10 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return ret;
 	}
 
-	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, pres);
+	/* It is necessary to preserve CPOL, inside CMR */
+	val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
+	val = (val & ~PWM_CMR_CPRE_MSK) | (pres & PWM_CMR_CPRE_MSK);
+	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
 	atmel_pwm->config(chip, pwm, dty, prd);
 
 	clk_disable(atmel_pwm->clk);
-- 
1.8.3.2

Re: [PATCH 1/2] PWM: atmel: Fix polarity handling

[Nicolas Ferre]

On 14/03/2014 15:19, Alexandre Belloni :
> When atmel_pwm_config() calculates and then sets the prescaler, it is
> overwriting the channel's CMR register so we are losing the CPOL configuration.
> 
> As atmel_pwm_config() is always called before enabling a channel, inverting the
> polarity doesn't work.
> 
> Fix that by reading CMR first and only overwriting the prescaler bits.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Indeed:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/pwm/pwm-atmel.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index bf4144a14661..2d69e9c431dd 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -32,6 +32,7 @@
>  /* Bit field in CMR */
>  #define PWM_CMR_CPOL		(1 << 9)
>  #define PWM_CMR_UPD_CDTY	(1 << 10)
> +#define PWM_CMR_CPRE_MSK	0xF
>  
>  /* The following registers for PWM v1 */
>  #define PWMV1_CDTY		0x04
> @@ -104,6 +105,7 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	unsigned long clk_rate, prd, dty;
>  	unsigned long long div;
>  	unsigned int pres = 0;
> +	u32 val;
>  	int ret;
>  
>  	if (test_bit(PWMF_ENABLED, &pwm->flags) && (period_ns != pwm->period)) {
> @@ -139,7 +141,10 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  		return ret;
>  	}
>  
> -	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, pres);
> +	/* It is necessary to preserve CPOL, inside CMR */
> +	val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> +	val = (val & ~PWM_CMR_CPRE_MSK) | (pres & PWM_CMR_CPRE_MSK);
> +	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
>  	atmel_pwm->config(chip, pwm, dty, prd);
>  
>  	clk_disable(atmel_pwm->clk);
> 


-- 
Nicolas Ferre

[PATCH 2/2] PWM: atmel: correct CDTY calculation

[Alexandre Belloni]

From the datasheet, the actual duty cycle is:
(period - (1/clk) * CDTY)/period

This actually correct the polarity of the PWM and solves the issue that pwm-leds
exhibits: when setting a duty cycle of 0 and then disabling a channel, the level
was wrong (1 when the polarity was normal and 0 when the polarity was inversed).

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/pwm/pwm-atmel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 2d69e9c431dd..0adc952cc4ef 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -133,7 +133,7 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	prd = div;
 	div *= duty_ns;
 	do_div(div, period_ns);
-	dty = div;
+	dty = prd - div;
 
 	ret = clk_enable(atmel_pwm->clk);
 	if (ret) {
-- 
1.8.3.2

Re: [PATCH 2/2] PWM: atmel: correct CDTY calculation

[Nicolas Ferre]

On 14/03/2014 15:19, Alexandre Belloni :
>>From the datasheet, the actual duty cycle is:
> (period - (1/clk) * CDTY)/period
> 
> This actually correct the polarity of the PWM and solves the issue that pwm-leds
> exhibits: when setting a duty cycle of 0 and then disabling a channel, the level
> was wrong (1 when the polarity was normal and 0 when the polarity was inversed).
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/pwm/pwm-atmel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 2d69e9c431dd..0adc952cc4ef 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -133,7 +133,7 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	prd = div;
>  	div *= duty_ns;
>  	do_div(div, period_ns);
> -	dty = div;
> +	dty = prd - div;
>  
>  	ret = clk_enable(atmel_pwm->clk);
>  	if (ret) {
> 


-- 
Nicolas Ferre

Re: [PATCH 0/2] PWM: atmel: few fixes

[Thierry Reding]

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

On Fri, Mar 14, 2014 at 03:19:07PM +0100, Alexandre Belloni wrote:
> Hi,
> 
> Here are a few improvement for pwm-atmel. The first patch makes the driver
> actually respect the polarity.
> 
> The second patch is solving an issue that I was trying to solve back in october.
> When the channel was disabled, the output level got inversed. It actually was
> that we had the polarity wrong from the beginning.
> 
> Alexandre Belloni (2):
>   PWM: atmel-pwm: Fix polarity handling
>   PWM: atmel: correct CDTY calculation
> 
>  drivers/pwm/pwm-atmel.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Both patches applied, thanks.

Thierry

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