From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751135AbdAYBAp (ORCPT ); Tue, 24 Jan 2017 20:00:45 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:40161 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751010AbdAYBAn (ORCPT ); Tue, 24 Jan 2017 20:00:43 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: b6c32a36-f79dd6d000001a4f-a5-5887f8b81a8e Content-transfer-encoding: 8BIT Message-id: <5887F8B8.60409@samsung.com> Date: Wed, 25 Jan 2017 10:00:40 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Charles Keepax Cc: myungjoo.ham@samsung.com, linux-kernel@vger.kernel.org, patches@opensource.wolfsonmicro.com Subject: Re: [PATCH] extcon: arizona: Wait for any running HPDETs to complete on jack removal In-reply-to: <1485253044-22795-1-git-send-email-ckeepax@opensource.wolfsonmicro.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFKsWRmVeSWpSXmKPExsWy7bCmge6OH+0RBucbtSz+TbnBbnF51xw2 i9uNK9gslr/9z+bA4vFy4m82j74tqxg9Pm+SC2COSrXJSE1MSS1SSM1Lzk/JzEu3VfIOjneO NzUzMNQ1tLQwV1LIS8xNtVVy8QnQdcvMAdqmpFCWmFMKFApILC5W0rezKcovLUlVyMgvLrFV ijY0NNIzNDDXMzIy0jMxjrUyMgUqSUjNOL9ateCgaMXl5jfMDYzvBLoYOTkkBEwk7j7oYYWw xSQu3FvP1sXIxSEksINR4m7DGSYIp51JYta1uywwHdufzmOFSCxnlHj/Yj4bSIJXQFDix+R7 QEUcHMwC8hJHLmWDhJkFNCW27l7PDlF/j1Hi4d3HUPUaEiu+H2EHsVkEVCX+tVwBs9kEtCT2 v7gBVsMvoChx9cdjRhBbVCBCYuf8b2A1IgIWElOW3GKGWJAocWXeDyYQW1ggXuLU09dgvZwC YRIdC5+BvSMhMJldYtOKy2wgx0kIyEpsOsAM8YyLxM29W9khbGGJV8e3QNnSEqv+3WKC6O1m lFjzsokVwulhlGhcc5QNospY4v6De1BX8Em8+woKSJAFvBIdbUIQJR4Sd//NgIavo8Ti5Y1M 8JCbsX4J4wRGhVlIgTcLEXizkAJvASPzKkax1ILi3PTUYsMCI73ixNzi0rx0veT83E2M4HSn ZbaDcdE5n0OMAhyMSjy8L1LaI4RYE8uKK3MPMUpwMCuJ8D66ABTiTUmsrEotyo8vKs1JLT7E aAoM+4nMUqLJ+cBUnFcSb2hiZmhiZGJoaG5kYKQkzru40TpCSCA9sSQ1OzW1ILUIpo+Jg1Oq gVEnaZnvyo/H1rB6tj6b77HwqMrVNPfjbyYvFeKNbNmt+opp9e996+pYwh5Gva/RZvtxY27j 1mMLwl72OzxVW97m0Mu1aWu1Df8Ob8lQFilGY6cNwUrxy+4F/bcpULgez+S2/0D1lW98R8r+ 6LR99xBsLeYr7MkQmpMpvjn9WN+mmfx1R3dwHVRiKc5INNRiLipOBAC1nlkojQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrCIsWRmVeSWpSXmKPExsVy+t9jAd0dP9ojDLYtlLL4N+UGu8XlXXPY LG43rmCzWP72P5sDi8fLib/ZPPq2rGL0+LxJLoA5ys0mIzUxJbVIITUvOT8lMy/dVik0xE3X QkkhLzE31VYpQtc3JEhJoSwxpxTIMzJAAw7OAe7BSvp2CW4Z51erFhwUrbjc/Ia5gfGdQBcj J4eEgInE9qfzWCFsMYkL99azdTFycQgJLGWUWHjrJTNIgldAUOLH5HssXYwcHMwC8hJHLmWD hJkF1CUmzVvEDFH/gFFidct1Joh6DYkV34+wg9gsAqoS/1qugNlsAloS+1/cYAOx+QUUJa7+ eMwIMlNUIEKi+0QlSFhEwEJiypJbzBDzEyUuXmoDKxEWiJfYdsgGYtVKRom2xbMZQWo4BcIk rjRdYprAKDgLyaWzEC6dheTSBYzMqxglUguSC4qT0nMN81LL9YoTc4tL89L1kvNzNzGCY+iZ 1A7Gg7vcDzEKcDAq8fC+SGmPEGJNLCuuzD3EKMHBrCTC++gCUIg3JbGyKrUoP76oNCe1+BCj KdCrE5mlRJPzgfGdVxJvaGJuYm5sYGFuaWlipCTO2zj7WbiQQHpiSWp2ampBahFMHxMHp1QD 4/7D7tqSJ+d85nu8K+MTYy/H8R0+D397/BM59W3Ou5m6ubsz/uR+/1wt3yolmi+5m/u7R4FD zsJNxssV9r/zOPaX+fMJTnubi/GTIhbdlDp1rmJRXXWhT8HNH0s4o5LsdtXGsc449EjotfFW h7eLp7pPFjdYbLafWy9pe+AMHrXlGf/spjUIbVFiKc5INNRiLipOBABYNGaotwIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170125010040epcas1p359f3535b91c1c07c76b941ccf22fa839 X-Msg-Generator: CA X-Sender-IP: 203.254.230.26 X-Local-Sender: =?UTF-8?B?7LWc7LCs7JqwG1RpemVuIFBsYXRmb3JtIExhYihTL1fshLw=?= =?UTF-8?B?7YSwKRvsgrzshLHsoITsnpAbUzUo7LGF7J6EKS/ssYXsnoQ=?= X-Global-Sender: =?UTF-8?B?Q2hhbndvbyBDaG9pG1RpemVuIFBsYXRmb3JtIExhYi4bU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtTNS9TZW5pb3IgRW5naW5lZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG1NUQUYbQzEwVjgxMTE=?= CMS-TYPE: 101P DLP-Filter: Pass X-CFilter-Loop: Reflected X-HopCount: 7 X-CMS-RootMailID: 20170124101629epcas1p375b21474f3b015515a2438e54afabeee X-RootMTR: 20170124101629epcas1p375b21474f3b015515a2438e54afabeee References: <1485253044-22795-1-git-send-email-ckeepax@opensource.wolfsonmicro.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Charles, On 2017년 01월 24일 19:17, Charles Keepax wrote: > As the HPDET can't be aborted mid way through we should not allow any new > insertion to be processed until the previous HPDET has finished. It is very > unlikely but with low enough debounce settings you could start a new HPDET > before the old one has completed, which results in an erroneous reading. > > Signed-off-by: Charles Keepax > --- > drivers/extcon/extcon-arizona.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c > index ed78b7c..218e378 100644 > --- a/drivers/extcon/extcon-arizona.c > +++ b/drivers/extcon/extcon-arizona.c > @@ -1049,6 +1049,39 @@ static void arizona_hpdet_work(struct work_struct *work) > mutex_unlock(&info->lock); > } > > +static int arizona_hpdet_wait(struct arizona_extcon_info *info) > +{ > + struct arizona *arizona = info->arizona; > + unsigned int val; > + int i, ret; > + > + for (i = 0; i < 15; i++) { I recommend that you define the separate constant definition instead of using interger value (15) directly as following: #define HPDET_WAIT_COUNT 15 > + ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, > + &val); > + if (ret) { > + dev_err(arizona->dev, > + "Failed to read HPDET state: %d\n", ret); > + return ret; > + } > + > + switch (info->hpdet_ip_version) { > + case 0: > + if (val & ARIZONA_HP_DONE) > + return 0; > + break; > + default: > + if (val & ARIZONA_HP_DONE_B) > + return 0; > + break; > + } > + > + msleep(20); ditto. #define HPDET_WAIT_DELAY_MS 20 > + } > + > + dev_err(arizona->dev, "HPDET did not appear to complete\n"); > + return -ETIMEDOUT; > +} > + > static irqreturn_t arizona_jackdet(int irq, void *data) > { > struct arizona_extcon_info *info = data; > @@ -1155,6 +1188,8 @@ static irqreturn_t arizona_jackdet(int irq, void *data) > "Removal report failed: %d\n", ret); > } > Sometimes I met this similiar case on specific h/w. I agree to add some delay according to the h/w. So, you better to add the detailed comment before calling the arizona_hpdet_wait() to make it easy to understand why it is necessary. > + arizona_hpdet_wait(info); Is not necessary to check the return value of this call? When arizona_hpdet_wait() return the -ETIMEDOUT, Does not this error affect the next some code? > + > regmap_update_bits(arizona->regmap, > ARIZONA_JACK_DETECT_DEBOUNCE, > ARIZONA_MICD_CLAMP_DB | ARIZONA_JD1_DB, > -- Best Regards, Chanwoo Choi Samsung Electronics