From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754231AbcDKLMR (ORCPT ); Mon, 11 Apr 2016 07:12:17 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:44268 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753714AbcDKLMN (ORCPT ); Mon, 11 Apr 2016 07:12:13 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee68d-f79e86d0000012da-9b-570b8686164d Content-transfer-encoding: 8BIT Message-id: <570B8686.3050306@samsung.com> Date: Mon, 11 Apr 2016 20:12:06 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Grygorii Strashko , Roger Quadros , myungjoo.ham@samsung.com Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] extcon: usb-gpio: Don't miss event during suspend/resume References: <1459951299-20578-1-git-send-email-rogerq@ti.com> <57075F23.8080703@ti.com> <570AF062.5000609@samsung.com> <570B6248.6080905@ti.com> In-reply-to: <570B6248.6080905@ti.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpmkeLIzCtJLcpLzFFi42JZI2JSpNvWxh1uMP2TmsXKKSwWl3fNYbO4 3biCzaLnkZYDi0ffllWMHsdvbGfy+LxJLoA5issmJTUnsyy1SN8ugStj7p2nLAWX5SpOz7rA 1MB4WKKLkZNDQsBE4sjTiawQtpjEhXvr2UBsIYEVjBLrftXD1PTtP8sKEV/KKLH9cTqIzSsg KPFj8j2WLkYODmYBeYkjl7IhTHWJKVNyuxi5gKofMErs73nFAlGuJXHqQBOYzSKgKvHp1Upm EJsNKL7/xQ02kF5RgQiJ7hOVIGERgWyJ1p/7wcqZBRQkft3bBHaBsECAxLt9N9kh5k9hlLh6 4AZYEaeAmsS+u91MIAkJgVXsEq9enmKDWCYg8W3yIbA7JQRkJTYdYIZ4S1Li4IobLBMYxWYh +WYWwjezEL5ZwMi8ilE0tSC5oDgpvchQrzgxt7g0L10vOT93EyMwbk7/e9a7g/H2AetDjAIc jEo8vA7XuMKFWBPLiitzDzGaAt0wkVlKNDkfGJ15JfGGxmZGFqYmpsZG5pZmSuK8ilI/g4UE 0hNLUrNTUwtSi+KLSnNSiw8xMnFwSjUwHjexXO/zsvDYolydywyvrgXKO21amnBes0t/u+ad aau0OtmeifrMCsx4fL07rytzS8y0fZG+HqFP33eVCz95/qa83kpzdn1jT+Epr6uRJw43ClyW zZ0/u2amxp8t8Sp9x+cWFh2Pnb5dJ1w5a4vKn1ybawffmKnf0DdxOyF7sVZnncA7nZDrSizF GYmGWsxFxYkAwlMXdJYCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrIIsWRmVeSWpSXmKPExsVy+t9jAd22Nu5wg4N3FS1WTmGxuLxrDpvF 7cYVbBY9j7QcWDz6tqxi9Dh+YzuTx+dNcgHMUQ2MNhmpiSmpRQqpecn5KZl56bZK3sHxzvGm ZgaGuoaWFuZKCnmJuam2Si4+AbpumTlAy5QUyhJzSoFCAYnFxUr6dpgmhIa46VrANEbo+oYE wfUYGaCBhDWMGXPvPGUpuCxXcXrWBaYGxsMSXYycHBICJhJ9+8+yQthiEhfurWcDsYUEljJK bH+cDmLzCghK/Jh8j6WLkYODWUBe4silbAhTXWLKlNwuRi6g6geMEvt7XrFAlGtJnDrQBGaz CKhKfHq1khnEZgOK739xgw2kV1QgQqL7RCVIWEQgW6L1536wcmYBBYlf9zaBXSMsECDxbt9N doj5Uxglrh64AVbEKaAmse9uN9MERoFZSK6bhXDdLITrFjAyr2KUSC1ILihOSs81zEst1ytO zC0uzUvXS87P3cQIjs5nUjsYD+5yP8QowMGoxMP74jJXuBBrYllxZe4hRgkOZiUR3r/V3OFC vCmJlVWpRfnxRaU5qcWHGE2B3pvILCWanA9MHHkl8YbGJmZGlkbmhhZGxuZK4ryP/68LExJI TyxJzU5NLUgtgulj4uCUamDMlTrJyfV0ifS5d9oOR7u1ve8mvK+MfD41Lnm3bjWDh9vxfYYW q0oKuneHRHWxtXmX6jIwJ3218T3ZfGLvt/3Fs2xmvNgr9srnp63Nmw3SwVfPLs3S3ZD8TO/l yQmqP6bvyObffXJxHcOUoyUajIc/hqiFadcz/Z7VV36sK36jsc7hhg724nNKLMUZiYZazEXF iQAaldmW5AIAAA== 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 On 2016년 04월 11일 17:37, Grygorii Strashko wrote: > On 04/11/2016 03:31 AM, Chanwoo Choi wrote: >> Hi Roger, >> >> On 2016년 04월 08일 16:34, Roger Quadros wrote: >>> Pin state might have changed during suspend/resume while >>> our interrupts were disabled and if device doesn't support wakeup. >>> >>> Scan for change during resume for such case. >>> >>> Signed-off-by: Roger Quadros >>> --- >>> v2: >>> - only check for state change during resume if device wakeup is not supported >>> >>> drivers/extcon/extcon-usb-gpio.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c >>> index bc61d11..118f8ab 100644 >>> --- a/drivers/extcon/extcon-usb-gpio.c >>> +++ b/drivers/extcon/extcon-usb-gpio.c >>> @@ -185,6 +185,8 @@ static int usb_extcon_resume(struct device *dev) >>> int ret = 0; >>> >>> enable_irq(info->id_irq); >>> + if (!device_may_wakeup(dev)) >>> + usb_extcon_detect_cable(&info->wq_detcable.work); >> >> The device_may_wakeup() check the following two states: >> - dev->power.can_wakeup - device_init_wakeup() function set the this field. >> - dev->power.wakeup - device_wakeup_enable() function set the this field. >> >> The probe function of extcon-usb-gpio.c always call the 'device_init_wakeup(dev,true). >> But, anywhere in extcon-usb-gpio.c don't handle the device_wakeup_enable() for dev->power.wakeup. > > > device_init_wakeup() > |-> device_wakeup_enable() > >> >> In the extcon-usb-gpio.c, device_may_wakeup(dev) return always 'false'. >> If you use the only device_may_wakeup(), >> device_may_wakeup() is not able to check whether interrupt is wakeup source or not. >> > > This check is correct and it also will take into account wake up settings changes > which can be made through sysfs: /sys/.../devX/power/wakeup > To Grygorii, You're right. I was mistaken. Again, I analyzed the sequence about wakeup. Thanks for your reply. 1. Register device as wakeup_source. device_init_wakeup(dev, true) on probe() device_wakeup_enable(dev) device_source_register(const char *name) struct wakeup_source *ws; ws = wakeup_source_create(name) if (ws) wakeup_source_add(ws); ... list_add_rcu(&ws->entry, &wakeup_sources); ... return ws; 2. Register the interrupt as wake_irq dev_pm_set_wake_irq(struct device *dev, int irq) on probe() struct wake_irq *wirq; wirq->dev = dev; wirq->irq = irq; dev_pm_attach_wake_irq(dev, irq, wirq); device_wakeup_attach_irq(*dev, *wakeirq) struct wakeup_source *ws; ws = dev->power.wakeup; ws->wakeirq = wakeirq; 3. Enable irq wake if device is already registed to wakeup_sources. dpm_suspend_noirq() device_wakeup_arm_wake_irqs() list_for_each_entry_rcu(ws, &wakeup_sources, entry) { if (ws->wakeirq) dev_pm_arm_wake_irq(sw->wakeirq); if (device_may_wakeup(wirq->dev)) enable_irq_wake(wirq->irq); To Roger, How about using the queue_delayed_work() instead of direct call function? Because the spent time of wakeup from suspend state should be fast. So, I think that you better to use the queue_delayed_work(). diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c index 118f8ab3be73..f6cbdfe31519 100644 --- a/drivers/extcon/extcon-usb-gpio.c +++ b/drivers/extcon/extcon-usb-gpio.c @@ -186,7 +186,9 @@ static int usb_extcon_resume(struct device *dev) enable_irq(info->id_irq); if (!device_may_wakeup(dev)) - usb_extcon_detect_cable(&info->wq_detcable.work); + queue_delayed_work(system_power_efficient_wq, + &info->wq_detcable, + info->debounce_jiffies); return ret; } Thanks, Chanwoo Choi