From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753219AbcEYDx5 (ORCPT ); Tue, 24 May 2016 23:53:57 -0400 Received: from mail1.bemta8.messagelabs.com ([216.82.243.198]:21680 "EHLO mail1.bemta8.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755123AbcEYDqd (ORCPT ); Tue, 24 May 2016 23:46:33 -0400 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBIsWRWlGSWpSXmKPExsVywNY2U1dEwTX c4N5OfouFbUtYLC7vmsPmwOTxeZNcAGMUa2ZeUn5FAmvG6sWWBVuUKr5uWcvawDhftouRi0NI YDejxKPnj5ggnAOMEkdfv2eHcC4zSnxdsQTKWcco8Xb6NBYIZwujxLn9TUAZTg42AQOJmXfug tkiAudZJJ7eyu1i5OBgFlCTuPHdEiQsLOAnsWzzKxYQm0VAVeLEjctgNq+Ap8SM9VOYQWwJAT mJk8cms4LYnAJeEhsavjGB2EJANe9vN7BC2GoSh88+YoOoD5bovTKBHWKOoMTJmU/AZjILSEg cfPGCeQKj0CwkqVlIUgsYmVYxahSnFpWlFukamuolFWWmZ5TkJmbm6BoaWOjlphYXJ6an5iQm Fesl5+duYgSGcD0DA+MOxgPP3Q8xSnIwKYnyHnziEi7El5SfUpmRWJwRX1Sak1p8iFGGg0NJg neBnGu4kGBRanpqRVpmDjCaYNISHDxKIrxvQNK8xQWJucWZ6RCpU4yKUuK8eSAJAZBERmkeXB ssgi8xykoJ8zIyMDAI8RSkFuVmlqDKv2IU52BUEub9AjKFJzOvBG76K6DFTECL/b84gywuSUR ISTUw6r3R/Wr10WXZVtZFxZGVn7OsbWZOrIzgl/2sm3R5lqTLjz8Fkid6r+rWHTF/u0048viH oOr11z7u9hU6a/PKXPrX+XuV9i6TWF4e3e+h8rjp6wHX9j+SPmdmLWT917G+fH6Mim3DXcNbU 15ZeXrMmuj4KGb+F8bmQOZ9z7huu1jf5l8UOkM+QImlOCPRUIu5qDgRAFpjURzbAgAA X-Env-Sender: David.Kershner@unisys.com X-Msg-Ref: server-10.tower-75.messagelabs.com!1464147984!13999676!19 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 07/24] staging: unisys: visorinput: remove unnecessary locking Date: Tue, 24 May 2016 23:45:45 -0400 Message-ID: <1464147962-26650-8-git-send-email-david.kershner@unisys.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1464147962-26650-1-git-send-email-david.kershner@unisys.com> References: <1464147962-26650-1-git-send-email-david.kershner@unisys.com> X-OriginalArrivalTime: 25 May 2016 03:46:24.0764 (UTC) FILETIME=[028C7BC0:01D1B638] 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: 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 d67cd763..f633985 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