From mboxrd@z Thu Jan 1 00:00:00 1970 From: One Thousand Gnomes Subject: Re: [PATCH] pwm: add support for Intel Low Power Subsystem PWM Date: Mon, 20 Jan 2014 13:28:14 +0000 Message-ID: <20140120132814.212929bd@alan.etchedpixels.co.uk> References: <1390240808-18582-1-git-send-email-chiau.ee.chew@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from lxorguk.ukuu.org.uk ([81.2.110.251]:57652 "EHLO lxorguk.ukuu.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753168AbaATN2h (ORCPT ); Mon, 20 Jan 2014 08:28:37 -0500 In-Reply-To: <1390240808-18582-1-git-send-email-chiau.ee.chew@intel.com> Sender: linux-pwm-owner@vger.kernel.org List-Id: linux-pwm@vger.kernel.org To: Chew Chiau Ee Cc: Thierry Reding , linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org, Mika Westerberg , Chew Kean Ho , Chang Rebecca Swee Fun > +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). I think the core config methods need some kind of locking ? Alan