Message ID | 1361959366-27634-6-git-send-email-akshay.s@samsung.com |
---|---|
State | Changes Requested |
Delegated to: | Minkyu Kang |
Headers | show |
On Wed, Feb 27, 2013 at 2:02 AM, Akshay Saraswat <akshay.s@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@google.com> > Signed-off-by: Akshay Saraswat <akshay.s@samsung.com> Acked-by: Simon Glass <sjg@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 >
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) {