Messages in this thread | | | Date | Mon, 02 Mar 2015 15:08:14 +0900 | From | Jaewon Kim <> | Subject | Re: [PATCH v6 4/5] Input: add haptic drvier on max77843 |
| |
HI Dmitry,
On 28/02/2015 02:49, Dmitry Torokhov wrote: > On Thu, Feb 26, 2015 at 11:49:36AM +0900, Jaewon Kim wrote: >> Hi Dmitry, >> >> On 26/02/2015 10:23, Dmitry Torokhov wrote: >>> Hi Jaewon, >>> >>> On Tue, Feb 24, 2015 at 10:29:07AM +0900, Jaewon Kim wrote: >>>> +static void max77843_haptic_play_work(struct work_struct *work) >>>> +{ >>>> + struct max77843_haptic *haptic = >>>> + container_of(work, struct max77843_haptic, work); >>>> + int error; >>>> + >>>> + mutex_lock(&haptic->mutex); >>>> + >>>> + if (haptic->suspended) { >>>> + goto err_play; >>>> + } >>>> + >>> You do not need braces around single statement. Also, this is not error >>> that you are handling, I'd prefer if we called this label out_unlock. >> You are right. >> I will change label name and remove braces. >>>> + error = max77843_haptic_set_duty_cycle(haptic); >>>> + if (error) { >>>> + dev_err(haptic->dev, "failed to set duty cycle: %d\n", error); >>>> + goto err_play; >>>> + } >>> Do you need to configure duty cycle if you stopping the playback? Or >>> maybe disabling pwm is enough? >> It do not need to set duty cycle requisitely when disabling haptic. >> >> I will move this function to front of max77843_haptic_enable(). >> >>>> + >>>> + if (haptic->magnitude) { >>>> + error = max77843_haptic_enable(haptic); >>>> + if (error) >>>> + dev_err(haptic->dev, >>>> + "cannot enable haptic: %d\n", error); >>>> + } else { >>>> + max77843_haptic_disable(haptic); >>>> + if (error) >>>> + dev_err(haptic->dev, >>>> + "cannot disable haptic: %d\n", error); >>> What error? You did not assign it... >> Detailed error message printed in enable/disable() function. > What I was trying to say is that you do not assign new value to 'error' > variable in this path; it still carries the value from > max77843_haptic_set_duty_cycle() above and so this "if" statement will > never work and the message will never show up. I never image at all that i am not assign 'error' variable. I will assign it.
> > Thanks. >
Thanks, Jaewon Kim
| |