lkml.org 
[lkml]   [2014]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] pwm: add support for Intel Low Power Subsystem PWM
On Mon, Jan 20, 2014 at 01:28:14PM +0000, One Thousand Gnomes wrote:
> > +static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm,
> > + int duty_ns, int period_ns)
> > +{
> > + struct pwm_lpss_chip *lpwm = to_lpwm(chip);
> > + u8 on_time_div;
> > + unsigned long c = clk_get_rate(lpwm->clk);
> > + unsigned long long base_unit, hz = 1000000000UL;
> > + u32 ctrl;
> > +
> > + do_div(hz, period_ns);
> > +
> > + /* The equation is: base_unit = ((hz / c) * 65536) + correction */
> > + base_unit = hz * 65536;
> > + do_div(base_unit, c);
> > + base_unit += PWM_DIVISION_CORRECTION;
> > + if (base_unit > PWM_LIMIT)
> > + return -EINVAL;
> > +
> > + if (duty_ns <= 0)
> > + duty_ns = 1;
> > + on_time_div = 255 - (255 * duty_ns / period_ns);
> > +
> > + ctrl = readl(lpwm->regs + PWM);
> > + ctrl &= ~(PWM_BASE_UNIT_MASK | PWM_ON_TIME_DIV_MASK);
> > + ctrl |= (u16) base_unit << PWM_BASE_UNIT_SHIFT;
> > + ctrl |= on_time_div;
> > + /* request PWM to update on next cycle */
> > + ctrl |= PWM_SW_UPDATE;
> > + writel(ctrl, lpwm->regs + PWM);
> > +
>
> Who handles the locking on all these functions. The pwm layer doesn't but
> simnply exports stuff like pwm_config() directly to other bits of the
> kernel so you are not guaranteed to be called via sysfs.
>
> (This btw looks to be a problem with a pile of the other pwm drivers, and
> with the pwm core code which doesn't properly lock its own handling of
> pwm->duty_cycle and pwm->period in pwm_config(), nor pwm->polarity in
> pwm_set_polarity).

Good point. I checked few PWM drivers as well and none of them (including
this one) does any locking around ->config().

> I think the core config methods need some kind of locking ?

Thierry, any comments on this?


\
 
 \ /
  Last update: 2014-01-21 10:41    [W:0.205 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site