diff mbox

[RESEND] pwm: mxs: fix period divider computation

Message ID 1426075692-19340-1-git-send-email-ghug@induct.be
State Accepted
Headers show

Commit Message

Gaetan Hug March 11, 2015, 12:08 p.m. UTC
The driver computes which clock divider it sould be using from the
requested period. This computation assumes that the link between the
register value and the actual divider value is raising 2 to the power of
the registry value.

    div = 1 << regvalue

This is true only for the first 5 values out of 8. Next values are 64,
256 and, 1024 - instead of 32, 64, 128.
This affects only the users requesting a period > 0.04369s.

Replace the computation with a look-up table.

Signed-off-by: Gaetan Hug <ghug@induct.be>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/pwm/pwm-mxs.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Thierry Reding March 11, 2015, 12:13 p.m. UTC | #1
On Wed, Mar 11, 2015 at 01:08:12PM +0100, Gaetan Hug wrote:
> The driver computes which clock divider it sould be using from the
> requested period. This computation assumes that the link between the
> register value and the actual divider value is raising 2 to the power of
> the registry value.
> 
>     div = 1 << regvalue
> 
> This is true only for the first 5 values out of 8. Next values are 64,
> 256 and, 1024 - instead of 32, 64, 128.
> This affects only the users requesting a period > 0.04369s.
> 
> Replace the computation with a look-up table.
> 
> Signed-off-by: Gaetan Hug <ghug@induct.be>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/pwm/pwm-mxs.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied, though I made some last-minute changes, see below.

> diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
> index f75ecb0..c65e183 100644
> --- a/drivers/pwm/pwm-mxs.c
> +++ b/drivers/pwm/pwm-mxs.c
> @@ -35,6 +35,8 @@
>  #define  PERIOD_CDIV(div)	(((div) & 0x7) << 20)
>  #define  PERIOD_CDIV_MAX	8
>  
> +static unsigned const int cdiv[PERIOD_CDIV_MAX] = {1, 2, 4, 8, 16, 64, 256, 1024};

I turned this into static const unsigned int, which is the more
canonical form and wrapped this over multiple lines so as not to exceed
the 80-character limit.

Thanks,
Thierry
diff mbox

Patch

diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
index f75ecb0..c65e183 100644
--- a/drivers/pwm/pwm-mxs.c
+++ b/drivers/pwm/pwm-mxs.c
@@ -35,6 +35,8 @@ 
 #define  PERIOD_CDIV(div)	(((div) & 0x7) << 20)
 #define  PERIOD_CDIV_MAX	8
 
+static unsigned const int cdiv[PERIOD_CDIV_MAX] = {1, 2, 4, 8, 16, 64, 256, 1024};
+
 struct mxs_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
@@ -54,13 +56,13 @@  static int mxs_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	rate = clk_get_rate(mxs->clk);
 	while (1) {
-		c = rate / (1 << div);
+		c = rate / cdiv[div];
 		c = c * period_ns;
 		do_div(c, 1000000000);
 		if (c < PERIOD_PERIOD_MAX)
 			break;
 		div++;
-		if (div > PERIOD_CDIV_MAX)
+		if (div >= PERIOD_CDIV_MAX)
 			return -EINVAL;
 	}