Messages in this thread | | | From | "hyunhee.kim" <> | Subject | RE: [PATCH v2] Input: add regulator haptic driver | Date | Thu, 24 Oct 2013 18:26:30 +0900 |
| |
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
| |