From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751313AbcFDR2I (ORCPT ); Sat, 4 Jun 2016 13:28:08 -0400 Received: from mail1.bemta8.messagelabs.com ([216.82.243.200]:54230 "EHLO mail1.bemta8.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751197AbcFDR1w (ORCPT ); Sat, 4 Jun 2016 13:27:52 -0400 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBIsWRWlGSWpSXmKPExsVywNY2U3cKf3C 4wdoVIhYL25awWFzeNYfNgcnj8ya5AMYo1sy8pPyKBNaMyRPuMxY8VarYu28WYwPjGdkuRi4O IYFdjBKnlv5jh3D2M0q82trLAuFcYpS4NusRG4SzllFi8/MPUJktjBLLJjYBOZwcbAIGEjPv3 GUHsUUEzrNIPL2V28XIwcEsoCZx47slSFhYIFBi5pNzrCA2i4CKROPHZWDlvAJeEvv3bQCLSw jISZw8NhnM5gSKX3mwlAnEFhLwlNizv4cZwlaTOHwW5CAOoPpgiYVT1CDGCEqcnPkE7BpmAQm Jgy9eME9gFJqFJDULSWoBI9MqRo3i1KKy1CJdI0O9pKLM9IyS3MTMHF1DAwu93NTi4sT01JzE pGK95PzcTYzAEK5nYGDcwbjzvPshRkkOJiVR3qnzg8KF+JLyUyozEosz4otKc1KLDzHKcHAoS fDW8wWHCwkWpaanVqRl5gCjCSYtwcGjJMJrBJLmLS5IzC3OTIdInWJUlBLn7QZJCIAkMkrz4N pgEXyJUVZKmJeRgYFBiKcgtSg3swRV/hWjOAejkjDvFpApPJl5JXDTXwEtZgJaXPDIH2RxSSJ CSqqBUXGOylXn33vvX+Z6YfTBzfR+WcINfdevj/PknEya7/qK+Gt+SJyv/ksvevbR4NXR/pZH Twgb32ZzW2669/s2AYXCY05uKt9UJB4fdXbfxraoi+tWb59vhzavxWblPo+N8j+PRS1fzFR/J WrCN8fz7gesX3jo328/rMDAb2YuFjv54ee/ZxoMlViKMxINtZiLihMBPa5DeNsCAAA= X-Env-Sender: David.Kershner@unisys.com X-Msg-Ref: server-12.tower-75.messagelabs.com!1465061267!21397762!4 X-Originating-IP: [192.61.61.105] X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked From: David Kershner To: , , , , , , , , , , , , , , , , , , , , , CC: Tim Sell Subject: [PATCH v3 10/30] staging: unisys: visorinput: remove unnecessary locking Date: Sat, 4 Jun 2016 13:27:10 -0400 Message-ID: <1465061250-18551-11-git-send-email-david.kershner@unisys.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1465061250-18551-1-git-send-email-david.kershner@unisys.com> References: <1465061250-18551-1-git-send-email-david.kershner@unisys.com> X-OriginalArrivalTime: 04 Jun 2016 17:27:36.0701 (UTC) FILETIME=[630AEAD0:01D1BE86] MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Tim Sell Locking in the _interrupt() function is NOT necessary so long as we ensure that interrupts have been stopped whenever we need to pause or resume the device, which we now do. While a device is paused, we ensure that interrupts stay disabled, i.e. that the _interrupt() function will NOT be called, yet remember the desired state in devdata->interrupts_enabled if open() or close() are called are called while the device is paused. Then when the device is resumed, we restore the actual state of interrupts (i.e., whether _interrupt() is going to be called or not) to the desired state in devdata->interrupts_enabled. Signed-off-by: Tim Sell Signed-off-by: David Kershner --- drivers/staging/unisys/visorinput/visorinput.c | 57 +++++++++++++++++++++----- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c index 12a3570..9c00710 100644 --- a/drivers/staging/unisys/visorinput/visorinput.c +++ b/drivers/staging/unisys/visorinput/visorinput.c @@ -66,6 +66,7 @@ struct visorinput_devdata { struct rw_semaphore lock_visor_dev; /* lock for dev */ struct input_dev *visorinput_dev; bool paused; + bool interrupts_enabled; unsigned int keycode_table_bytes; /* size of following array */ /* for keyboard devices: visorkbd_keycode[] + visorkbd_ext_keycode[] */ unsigned char keycode_table[0]; @@ -228,7 +229,21 @@ static int visorinput_open(struct input_dev *visorinput_dev) return -EINVAL; } dev_dbg(&visorinput_dev->dev, "%s opened\n", __func__); + + /* + * If we're not paused, really enable interrupts. + * Regardless of whether we are paused, set a flag indicating + * interrupts should be enabled so when we resume, interrupts + * will really be enabled. + */ + down_write(&devdata->lock_visor_dev); + devdata->interrupts_enabled = true; + if (devdata->paused) + goto out_unlock; visorbus_enable_channel_interrupts(devdata->dev); + +out_unlock: + up_write(&devdata->lock_visor_dev); return 0; } @@ -243,7 +258,22 @@ static void visorinput_close(struct input_dev *visorinput_dev) return; } dev_dbg(&visorinput_dev->dev, "%s closed\n", __func__); + + /* + * If we're not paused, really disable interrupts. + * Regardless of whether we are paused, set a flag indicating + * interrupts should be disabled so when we resume we will + * not re-enable them. + */ + + down_write(&devdata->lock_visor_dev); + devdata->interrupts_enabled = false; + if (devdata->paused) + goto out_unlock; visorbus_disable_channel_interrupts(devdata->dev); + +out_unlock: + up_write(&devdata->lock_visor_dev); } /* @@ -438,10 +468,8 @@ visorinput_remove(struct visor_device *dev) * in visorinput_channel_interrupt() */ - down_write(&devdata->lock_visor_dev); dev_set_drvdata(&dev->device, NULL); unregister_client_input(devdata->visorinput_dev); - up_write(&devdata->lock_visor_dev); kfree(devdata); } @@ -529,13 +557,7 @@ visorinput_channel_interrupt(struct visor_device *dev) if (!devdata) return; - down_write(&devdata->lock_visor_dev); - if (devdata->paused) /* don't touch device/channel when paused */ - goto out_locked; - visorinput_dev = devdata->visorinput_dev; - if (!visorinput_dev) - goto out_locked; while (visorchannel_signalremove(dev->visorchannel, 0, &r)) { scancode = r.activity.arg1; @@ -611,8 +633,6 @@ visorinput_channel_interrupt(struct visor_device *dev) break; } } -out_locked: - up_write(&devdata->lock_visor_dev); } static int @@ -632,6 +652,14 @@ visorinput_pause(struct visor_device *dev, rc = -EBUSY; goto out_locked; } + if (devdata->interrupts_enabled) + visorbus_disable_channel_interrupts(dev); + + /* + * due to above, at this time no thread of execution will be + * in visorinput_channel_interrupt() + */ + devdata->paused = true; complete_func(dev, 0); rc = 0; @@ -659,6 +687,15 @@ visorinput_resume(struct visor_device *dev, } devdata->paused = false; complete_func(dev, 0); + + /* + * Re-establish calls to visorinput_channel_interrupt() if that is + * the desired state that we've kept track of in interrupts_enabled + * while the device was paused. + */ + if (devdata->interrupts_enabled) + visorbus_enable_channel_interrupts(dev); + rc = 0; out_locked: up_write(&devdata->lock_visor_dev); -- 1.9.1