[U-Boot] [PATCH 5/9] Exynos: Tidy up the pwm_config function in the exynos pwm driver

[Simon Glass]

On Wed, Feb 27, 2013 at 2:02 AM, Akshay Saraswat <akshay.s at samsung.com> wrote:
> Some small fixes in the exynos pwm driver:
>
> 1. NS_IN_HZ is non-sensical since these are not compatible units. This
> constant actually describes the number of nanoseconds in a second. Renamed it
> to NS_IN_SEC. Also dropped the unnecessary parenthesis.
> 2. The variable "period" is not used to hold a period, it's used to hold a
> frequency. Renamed it to "frequency".
> 3. tcmp is an unsigned value, so (tcmp < 0) will never be true and the if
> which checks that condition will never execute. Also, there should be no
> problem if the pwm never switches, so there's no reason to subtract one from
> tcmp and therefore no reason to compare it against zero. Removed both ifs. If
> they weren't removed, tcmp should be a signed value.
> 4. Add a check for a 0 period.
>
> TEST=sf probe 1:0; time sf read 40008000 0 1000
> Try with different numbers of bytes and see that sane values are obtained
> Build and boot U-boot with this patch, backlight works properly.
>
> Signed-off-by: Gabe Black <gabeblack at google.com>
> Signed-off-by: Akshay Saraswat <akshay.s at samsung.com>

Acked-by: Simon Glass <sjg at chromium.org>

> ---
>  arch/arm/cpu/armv7/s5p-common/pwm.c | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/s5p-common/pwm.c b/arch/arm/cpu/armv7/s5p-common/pwm.c
> index 02156d1..6f401b8 100644
> --- a/arch/arm/cpu/armv7/s5p-common/pwm.c
> +++ b/arch/arm/cpu/armv7/s5p-common/pwm.c
> @@ -70,7 +70,7 @@ static unsigned long pwm_calc_tin(int pwm_id, unsigned long freq)
>         return tin_parent_rate / 16;
>  }
>
> -#define NS_IN_HZ (1000000000UL)
> +#define NS_IN_SEC 1000000000UL
>
>  int pwm_config(int pwm_id, int duty_ns, int period_ns)
>  {
> @@ -79,7 +79,7 @@ int pwm_config(int pwm_id, int duty_ns, int period_ns)
>         unsigned int offset;
>         unsigned long tin_rate;
>         unsigned long tin_ns;
> -       unsigned long period;
> +       unsigned long frequency;
>         unsigned long tcon;
>         unsigned long tcnt;
>         unsigned long tcmp;
> @@ -89,34 +89,24 @@ int pwm_config(int pwm_id, int duty_ns, int period_ns)
>          * fact that anything faster than 1GHz is easily representable
>          * by 32bits.
>          */
> -       if (period_ns > NS_IN_HZ || duty_ns > NS_IN_HZ)
> +       if (period_ns > NS_IN_SEC || duty_ns > NS_IN_SEC || period_ns == 0)
>                 return -ERANGE;
>
>         if (duty_ns > period_ns)
>                 return -EINVAL;
>
> -       period = NS_IN_HZ / period_ns;
> +       frequency = NS_IN_SEC / period_ns;
>
>         /* Check to see if we are changing the clock rate of the PWM */
> -       tin_rate = pwm_calc_tin(pwm_id, period);
> +       tin_rate = pwm_calc_tin(pwm_id, frequency);
>
> -       tin_ns = NS_IN_HZ / tin_rate;
> +       tin_ns = NS_IN_SEC / tin_rate;
>         tcnt = period_ns / tin_ns;
>
>         /* Note, counters count down */
>         tcmp = duty_ns / tin_ns;
>         tcmp = tcnt - tcmp;
>
> -       /*
> -        * the pwm hw only checks the compare register after a decrement,
> -        * so the pin never toggles if tcmp = tcnt
> -        */
> -       if (tcmp == tcnt)
> -               tcmp--;
> -
> -       if (tcmp < 0)
> -               tcmp = 0;
> -
>         /* Update the PWM register block. */
>         offset = pwm_id * 3;
>         if (pwm_id < 4) {
> --
> 1.8.0
>