Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758413AbbBFS2s (ORCPT ); Fri, 6 Feb 2015 13:28:48 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:47731 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758312AbbBFS2p (ORCPT ); Fri, 6 Feb 2015 13:28:45 -0500 Date: Fri, 6 Feb 2015 10:27:25 -0800 From: Guenter Roeck To: Lukasz Majewski Cc: Eduardo Valentin , Kamil Debski , Jean Delvare , Kukjin Kim , lm-sensors@lm-sensors.org, Linux PM list , "linux-samsung-soc@vger.kernel.org" , devicetree@vger.kernel.org, Lukasz Majewski , Kukjin Kim , linux-kernel@vger.kernel.org, Sjoerd Simons , Abhilash Kesavan , Abhilash Kesavan Subject: Re: [PATCH v3 6/8] hwmon: pwm-fan: Extract __set_pwm() function to only modify PWM duty cycle Message-ID: <20150206182725.GA25410@roeck-us.net> References: <1418897591-18332-1-git-send-email-l.majewski@samsung.com> <1423241948-31981-1-git-send-email-l.majewski@samsung.com> <1423241948-31981-7-git-send-email-l.majewski@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423241948-31981-7-git-send-email-l.majewski@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-CTCH-PVer: 0000001 X-CTCH-Spam: Unknown X-CTCH-VOD: Unknown X-CTCH-Flags: 0 X-CTCH-RefID: str=0001.0A020202.54D507DC.025F,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 X-CTCH-Score: 0.001 X-CTCH-ScoreCust: 0.000 X-CTCH-Rules: C_4847, X-CTCH-SenderID: linux@roeck-us.net X-CTCH-SenderID-Flags: 0 X-CTCH-SenderID-TotalMessages: 12 X-CTCH-SenderID-TotalSpam: 0 X-CTCH-SenderID-TotalSuspected: 0 X-CTCH-SenderID-TotalConfirmed: 0 X-CTCH-SenderID-TotalBulk: 0 X-CTCH-SenderID-TotalVirus: 0 X-CTCH-SenderID-TotalRecipients: 0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: mailgid no entry from get_relayhosts_entry X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2616 Lines: 91 On Fri, Feb 06, 2015 at 05:59:06PM +0100, Lukasz Majewski wrote: > It was necessary to decouple code handling writing to sysfs from the one > responsible for setting PWM of the fan. > Due to that, new __set_pwm() method was extracted, which is responsible for > only setting new PWM duty cycle. > > Signed-off-by: Lukasz Majewski > --- > Changes for v2: > - None > Changes for v3: > - The commit headline has been reedited. > --- > drivers/hwmon/pwm-fan.c | 35 ++++++++++++++++++++++------------- > 1 file changed, 22 insertions(+), 13 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index 1991d903..870e100 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -33,21 +33,15 @@ struct pwm_fan_ctx { > unsigned char pwm_value; > }; > > -static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, > - const char *buf, size_t count) > +static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm) > { > - struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > - unsigned long pwm, duty; > - ssize_t ret; > - > - if (kstrtoul(buf, 10, &pwm) || pwm > MAX_PWM) > - return -EINVAL; > - > - mutex_lock(&ctx->lock); > + unsigned long duty; > + int ret; > > if (ctx->pwm_value == pwm) > - goto exit_set_pwm_no_change; > + return 0; > Why did you move this check outside the lock ? With this change there is no guarantee that pwm_value wasn't changed while waiting for the lock. Guenter > + mutex_lock(&ctx->lock); > if (pwm == 0) { > pwm_disable(ctx->pwm); > goto exit_set_pwm; > @@ -66,13 +60,28 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, > > exit_set_pwm: > ctx->pwm_value = pwm; > -exit_set_pwm_no_change: > - ret = count; > exit_set_pwm_err: > mutex_unlock(&ctx->lock); > return ret; > } > > +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > + unsigned long pwm; > + int ret; > + > + if (kstrtoul(buf, 10, &pwm) || pwm > MAX_PWM) > + return -EINVAL; > + > + ret = __set_pwm(ctx, pwm); > + if (ret) > + return ret; > + > + return count; > +} > + > static ssize_t show_pwm(struct device *dev, > struct device_attribute *attr, char *buf) > { > -- > 2.0.0.rc2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/