Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section. Reviewed-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> --- V2: fixed whitespace issues, added Reviewed-by from Javier drivers/input/tablet/pegasus_notetaker.c | 86 +++++++++++++++++------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c index a68da2988f9c..8d6b71d59793 100644 --- a/drivers/input/tablet/pegasus_notetaker.c +++ b/drivers/input/tablet/pegasus_notetaker.c @@ -214,6 +214,28 @@ static void pegasus_init(struct work_struct *work) error); } +static int __pegasus_open(struct pegasus *pegasus) +{ + int error; + + guard(mutex)(&pegasus->pm_mutex); + + pegasus->irq->dev = pegasus->usbdev; + if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) + return -EIO; + + error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); + if (error) { + usb_kill_urb(pegasus->irq); + cancel_work_sync(&pegasus->init); + return error; + } + + pegasus->is_open = true; + + return 0; +} + static int pegasus_open(struct input_dev *dev) { struct pegasus *pegasus = input_get_drvdata(dev); @@ -223,39 +245,25 @@ static int pegasus_open(struct input_dev *dev) if (error) return error; - mutex_lock(&pegasus->pm_mutex); - pegasus->irq->dev = pegasus->usbdev; - if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) { - error = -EIO; - goto err_autopm_put; + error = __pegasus_open(pegasus); + if (error) { + usb_autopm_put_interface(pegasus->intf); + return error; } - error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); - if (error) - goto err_kill_urb; - - pegasus->is_open = true; - mutex_unlock(&pegasus->pm_mutex); return 0; - -err_kill_urb: - usb_kill_urb(pegasus->irq); - cancel_work_sync(&pegasus->init); -err_autopm_put: - mutex_unlock(&pegasus->pm_mutex); - usb_autopm_put_interface(pegasus->intf); - return error; } static void pegasus_close(struct input_dev *dev) { struct pegasus *pegasus = input_get_drvdata(dev); - mutex_lock(&pegasus->pm_mutex); - usb_kill_urb(pegasus->irq); - cancel_work_sync(&pegasus->init); - pegasus->is_open = false; - mutex_unlock(&pegasus->pm_mutex); + scoped_guard(mutex, &pegasus->pm_mutex) { + usb_kill_urb(pegasus->irq); + cancel_work_sync(&pegasus->init); + + pegasus->is_open = false; + } usb_autopm_put_interface(pegasus->intf); } @@ -411,10 +419,10 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message) { struct pegasus *pegasus = usb_get_intfdata(intf); - mutex_lock(&pegasus->pm_mutex); + guard(mutex)(&pegasus->pm_mutex); + usb_kill_urb(pegasus->irq); cancel_work_sync(&pegasus->init); - mutex_unlock(&pegasus->pm_mutex); return 0; } @@ -422,31 +430,33 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message) static int pegasus_resume(struct usb_interface *intf) { struct pegasus *pegasus = usb_get_intfdata(intf); - int retval = 0; - mutex_lock(&pegasus->pm_mutex); + guard(mutex)(&pegasus->pm_mutex); + if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0) - retval = -EIO; - mutex_unlock(&pegasus->pm_mutex); + return -EIO; - return retval; + return 0; } static int pegasus_reset_resume(struct usb_interface *intf) { struct pegasus *pegasus = usb_get_intfdata(intf); - int retval = 0; + int error; + + guard(mutex)(&pegasus->pm_mutex); - mutex_lock(&pegasus->pm_mutex); if (pegasus->is_open) { - retval = pegasus_set_mode(pegasus, PEN_MODE_XY, + error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); - if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0) - retval = -EIO; + if (error) + return error; + + if (usb_submit_urb(pegasus->irq, GFP_NOIO) < 0) + return -EIO; } - mutex_unlock(&pegasus->pm_mutex); - return retval; + return 0; } static const struct usb_device_id pegasus_ids[] = {