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.
Thanks.
Thanks to review my patch. Thanks, Jaewon Kim -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
