Hi Minkyu,

Thanks for your comments.

Dear Akshay,

On 28/02/13 19:59, Akshay Saraswat wrote:

...

First, the "div" value was being used incorrectly to compute the frequency of the PWM timer. The value passed in is a constant which reflects the value that would be found in a configuration register, 0 to 4. That should correspond to a scaling factor of 1, 2, 4, 8, or 16, 1 << div, but div + 1 was being used instead.

@@ -167,20 +167,24 @@ int pwm_init(int pwm_id, int div, int invert) val |= (div & 0xf) << MUX_DIV_SHIFT(pwm_id); writel(val, &pwm->tcfg1);

• timer_rate_hz = get_pwm_clk() / ((prescaler + 1) *

• 	(div + 1));
  

• if (pwm_id == 4) {

• /*
  

•  * TODO(sjg): Use this as a countdown timer for now. We count
  

•  * down from the maximum value to 0, then reset.
  

•  */
  

• ticks_per_period = -1UL;
  

• } else {

• const unsigned long pwm_hz = 1000;
  

• unsigned long timer_rate_hz = get_pwm_clk() /
  

• 	((prescaler + 1) * (1 << div));
  

good catch. thanks.

...

• timer_rate_hz = timer_rate_hz / CONFIG_SYS_HZ;

• ticks_per_period = timer_rate_hz / pwm_hz;
  

why don't you use CONFIG_SYS_HZ? pwm_hz seems to constant.

CONFIG_SYS_HZ may change with boards and as per needs. But requirement here is for a constant value 1000 for all calculations which should remain 1000 only. That's why using pwm_hz and not CONFIG_SYS_HZ.

...

• }

  /* set count value */ offset = pwm_id * 3;

• /*

• * TODO(sjg): Use this as a countdown timer for now. We count down
  

• * from the maximum value to 0, then reset.
  

• */
  

• timer_rate_hz = -1;
• writel(timer_rate_hz, &pwm->tcntb0 + offset);

• writel(ticks_per_period, &pwm->tcntb0 + offset);

  val = readl(&pwm->tcon) & ~(0xf << TCON_OFFSET(pwm_id)); if (invert && (pwm_id < 4))

Thanks, Minkyu Kang.

Regards, Akshay Saraswat