From: "hyunhee.kim" <hyunhee.kim@samsung.com>
To: "'Oliver Neukum'" <oneukum@suse.de>
Cc: "'Dmitry Torokhov'" <dmitry.torokhov@gmail.com>,
        broonie@opensource.wolfsonmicro.com, peter.ujfalusi@ti.com,
        wfp5p@virginia.edu, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
        kyungmin.park@samsung.com,
        "'Aristeu Sergio Rozanski Filho'" <aris@ruivo.org>
References: <000a01cec629$3cf34de0$b6d9e9a0$%kim@samsung.com>
 <20131021155558.GA4255@core.coreip.homeip.net>
 <025101ced083$35f2a270$a1d7e750$%kim@samsung.com>
 <1382603886.1559.11.camel@linux-fkkt.site>
In-reply-to: <1382603886.1559.11.camel@linux-fkkt.site>
Subject: RE: [PATCH v2] Input: add regulator haptic driver
Date: Thu, 24 Oct 2013 18:26:30 +0900
Message-id: <028401ced09b$1f6564d0$5e302e70$%kim@samsung.com>
MIME-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8BIT
Thread-index: Ac7QlGzVSRYl8tohR/yeQB2ZNfsEyAABhYlw
Content-language: ko
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1739
Lines: 59

Hi, 

Thanks for your review.
I resent patch v3 after removing wrong wrapping.

I made one toggle function because enable/disable functions have redundant codes and another reviewer suggested it.
Is it better to separate it into two functions?

Thanks,
Hyunhee Kim.

-----Original Message-----
From: Oliver Neukum [mailto:oneukum@suse.de] 
Sent: Thursday, October 24, 2013 5:38 PM
To: hyunhee.kim
Cc: 'Dmitry Torokhov'; broonie@opensource.wolfsonmicro.com; peter.ujfalusi@ti.com; wfp5p@virginia.edu; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; kyungmin.park@samsung.com; 'Aristeu Sergio Rozanski Filho'
Subject: Re: [PATCH v2] Input: add regulator haptic driver

On Thu, 2013-10-24 at 15:35 +0900, hyunhee.kim wrote:

Hi,

first of all your mail client mangled the patch.

> +static void regulator_haptic_toggle(struct regulator_haptic *haptic, bool
> enable)
> +{
> +	int ret;
> +
> +	mutex_lock(&haptic->mutex);
> +	if (enable && !haptic->enabled) {
> +		haptic->enabled = true;
> +		ret = regulator_enable(haptic->regulator);
> +		if (ret)
> +			dev_err(haptic->dev, "failed to enable
> regulator\n");
> +	} else if (!enable && haptic->enabled) {
> +		haptic->enabled = false;
> +		ret = regulator_disable(haptic->regulator);
> +		if (ret)
> +			dev_err(haptic->dev, "failed to disable
> regulator\n");
> +	}
> +	mutex_unlock(&haptic->mutex);
> +}
> +

Is there anything gained by the toggle parameter? Just code two
functions.

	Regards
		Oliver


--
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/