From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751913AbbCBGIV (ORCPT ); Mon, 2 Mar 2015 01:08:21 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:34717 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750914AbbCBGIS (ORCPT ); Mon, 2 Mar 2015 01:08:18 -0500 X-AuditID: cbfee68f-f791c6d000004834-85-54f3fe4ff259 Message-id: <54F3FE4E.5030805@samsung.com> Date: Mon, 02 Mar 2015 15:08:14 +0900 From: Jaewon Kim User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-version: 1.0 To: Dmitry Torokhov Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-input@vger.kernel.org, Inki Dae , SangBae Lee , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Lee Jones , Chanwoo Choi , Sebastian Reichel , Beomho Seo Subject: Re: [PATCH v6 4/5] Input: add haptic drvier on max77843 References: <1424741348-8728-1-git-send-email-jaewon02.kim@samsung.com> <1424741348-8728-5-git-send-email-jaewon02.kim@samsung.com> <20150226012353.GC25965@dtor-ws> <54EE89C0.7010507@samsung.com> <20150227174941.GD6679@dtor-ws> In-reply-to: <20150227174941.GD6679@dtor-ws> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrAIsWRmVeSWpSXmKPExsWyRsSkSNf/3+cQg3dX5CxOf9rGbnH9y3NW i/lHzrFaHF70gtGi/81CVotzr1YyWky6P4HF4v7Xo4wWNz99Y7W4vGsOm8Xn3iOMFkuvX2Sy mDB9LYtF694j7BbHPx1ksTi9u8RBwGPNvDWMHpf7epk8ds66y+6xcvkXNo9NqzrZPO5c28Pm 0bdlFaPH501yARxRXDYpqTmZZalF+nYJXBnNf38wFmwRqHj6ewFjA2M3bxcjB4eEgInElmO2 XYycQKaYxIV769m6GLk4hASWMkpc+NjCApEwkeh5u4IVIrGIUeLi3QPMEM5rRomTj0+yglTx CmhJbJ10lx3EZhFQlVg97z1YnE1AW+L7+sVgtqhAhMT8Y6+ZIeoFJX5Mvge2QURAX2L77F+M IEOZBaazSBw+chSsQVjAQWLGhycsENteMkpM6/vBBJLgFNCRWDC3G2wSs4CtxIL361ggbHmJ zWvegp0nIbCUQ+L7+zWMECcJSHybfIgF4mlZiU0HmCF+k5Q4uOIGywRGsVlIjpqFZOwsJGMX MDKvYhRNLUguKE5KLzLWK07MLS7NS9dLzs/dxAiM+NP/nvXvYLx7wPoQowAHoxIPr8eczyFC rIllxZW5hxhNga6YyCwlmpwPTCt5JfGGxmZGFqYmpsZG5pZmSuK8C6V+BgsJpCeWpGanphak FsUXleakFh9iZOLglGpgtLw3427zXIm0NwK1IVdCXp0SDo58K2BiU3Xo/dX0WC3u/uz8rwar ymUaptZXv8oNvK24vW6Td8Jjzl0z/777q640fUET7/+W0u23LIImLZprsH/1XJEoPsfrUScm F6ms0ft1cNUr8Ulqdx8kll1tbuZ+J/Vmy40JSSmrZ1S/9VYp7L0qts3ylhJLcUaioRZzUXEi ACDDb97zAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrNKsWRmVeSWpSXmKPExsVy+t9jAV3/f59DDHaeFrE4/Wkbu8X1L89Z LeYfOcdqcXjRC0aL/jcLWS3OvVrJaDHp/gQWi/tfjzJa3Pz0jdXi8q45bBafe48wWiy9fpHJ YsL0tSwWrXuPsFsc/3SQxeL07hIHAY8189Ywelzu62Xy2DnrLrvHyuVf2Dw2repk87hzbQ+b R9+WVYwenzfJBXBENTDaZKQmpqQWKaTmJeenZOal2yp5B8c7x5uaGRjqGlpamCsp5CXmptoq ufgE6Lpl5gC9oKRQlphTChQKSCwuVtK3wzQhNMRN1wKmMULXNyQIrsfIAA0krGHMaP77g7Fg i0DF098LGBsYu3m7GDk5JARMJHrermCFsMUkLtxbz9bFyMUhJLCIUeLi3QPMEM5rRomTj0+C VfEKaElsnXSXHcRmEVCVWD3vPVicTUBb4vv6xWC2qECExPxjr5kh6gUlfky+xwJiiwjoS2yf /YsRZCizwHQWicNHjoI1CAs4SMz48IQFYttLRolpfT+YQBKcAjoSC+Z2g01iFrCVWPB+HQuE LS+xec1b5gmMArOQLJmFpGwWkrIFjMyrGEVTC5ILipPScw31ihNzi0vz0vWS83M3MYITyjOp HYwrGywOMQpwMCrx8HrM+RwixJpYVlyZe4hRgoNZSYT31gegEG9KYmVValF+fFFpTmrxIUZT YBhMZJYSTc4HJru8knhDYxMzI0sjc0MLI2NzJXFeJfu2ECGB9MSS1OzU1ILUIpg+Jg5OqQbG 1bvn7OewklP+///ulFOBNiFO1k/PxJUFL1yy9+pN5e8z9j1x3NCU9MzcLDbSwKfiVrnRFIts n12f6iorLaelX733eEJ+3tGMc7+aVC+oJ/o4C/auPNnt8DHHuFz4Z/K/spJ5M9xWPxVUOiue vWrjnzSJnA07/H2CgjkLtz/lFJO+ITRzy94KJZbijERDLeai4kQAyRN/jj4DAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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