Input: cm109 - use guard notation when acquiring mutex and spinlock

Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Link: https://lore.kernel.org/r/20240904044244.1042174-4-dmitry.torokhov@gmail.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 728325a..0cfe5d4 100644 (file)
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -355,6 +355,35 @@ static void cm109_submit_buzz_toggle(struct cm109_dev *dev)
                        __func__, error);
 }
 
+static void cm109_submit_ctl(struct cm109_dev *dev)
+{
+       int error;
+
+       guard(spinlock_irqsave)(&dev->ctl_submit_lock);
+
+       dev->irq_urb_pending = 0;
+
+       if (unlikely(dev->shutdown))
+               return;
+
+       if (dev->buzzer_state)
+               dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
+       else
+               dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
+
+       dev->ctl_data->byte[HID_OR1] = dev->keybit;
+       dev->ctl_data->byte[HID_OR2] = dev->keybit;
+
+       dev->buzzer_pending = 0;
+       dev->ctl_urb_pending = 1;
+
+       error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
+       if (error)
+               dev_err(&dev->intf->dev,
+                       "%s: usb_submit_urb (urb_ctl) failed %d\n",
+                       __func__, error);
+}
+
 /*
  * IRQ handler
  */
@@ -362,8 +391,6 @@ static void cm109_urb_irq_callback(struct urb *urb)
 {
        struct cm109_dev *dev = urb->context;
        const int status = urb->status;
-       int error;
-       unsigned long flags;
 
        dev_dbg(&dev->intf->dev, "### URB IRQ: [0x%02x 0x%02x 0x%02x 0x%02x] keybit=0x%02x\n",
             dev->irq_data->byte[0],
@@ -401,32 +428,7 @@ static void cm109_urb_irq_callback(struct urb *urb)
        }
 
  out:
-
-       spin_lock_irqsave(&dev->ctl_submit_lock, flags);
-
-       dev->irq_urb_pending = 0;
-
-       if (likely(!dev->shutdown)) {
-
-               if (dev->buzzer_state)
-                       dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
-               else
-                       dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
-
-               dev->ctl_data->byte[HID_OR1] = dev->keybit;
-               dev->ctl_data->byte[HID_OR2] = dev->keybit;
-
-               dev->buzzer_pending = 0;
-               dev->ctl_urb_pending = 1;
-
-               error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
-               if (error)
-                       dev_err(&dev->intf->dev,
-                               "%s: usb_submit_urb (urb_ctl) failed %d\n",
-                               __func__, error);
-       }
-
-       spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+       cm109_submit_ctl(dev);
 }
 
 static void cm109_urb_ctl_callback(struct urb *urb)
@@ -434,7 +436,6 @@ static void cm109_urb_ctl_callback(struct urb *urb)
        struct cm109_dev *dev = urb->context;
        const int status = urb->status;
        int error;
-       unsigned long flags;
 
        dev_dbg(&dev->intf->dev, "### URB CTL: [0x%02x 0x%02x 0x%02x 0x%02x]\n",
             dev->ctl_data->byte[0],
@@ -449,35 +450,31 @@ static void cm109_urb_ctl_callback(struct urb *urb)
                                    __func__, status);
        }
 
-       spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+       guard(spinlock_irqsave)(&dev->ctl_submit_lock);
 
        dev->ctl_urb_pending = 0;
 
-       if (likely(!dev->shutdown)) {
-
-               if (dev->buzzer_pending || status) {
-                       dev->buzzer_pending = 0;
-                       dev->ctl_urb_pending = 1;
-                       cm109_submit_buzz_toggle(dev);
-               } else if (likely(!dev->irq_urb_pending)) {
-                       /* ask for key data */
-                       dev->irq_urb_pending = 1;
-                       error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
-                       if (error)
-                               dev_err(&dev->intf->dev,
-                                       "%s: usb_submit_urb (urb_irq) failed %d\n",
-                                       __func__, error);
-               }
-       }
+       if (unlikely(dev->shutdown))
+               return;
 
-       spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+       if (dev->buzzer_pending || status) {
+               dev->buzzer_pending = 0;
+               dev->ctl_urb_pending = 1;
+               cm109_submit_buzz_toggle(dev);
+       } else if (likely(!dev->irq_urb_pending)) {
+               /* ask for key data */
+               dev->irq_urb_pending = 1;
+               error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
+               if (error)
+                       dev_err(&dev->intf->dev,
+                               "%s: usb_submit_urb (urb_irq) failed %d\n",
+                               __func__, error);
+       }
 }
 
 static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
 {
-       unsigned long flags;
-
-       spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+       guard(spinlock_irqsave)(&dev->ctl_submit_lock);
 
        if (dev->ctl_urb_pending) {
                /* URB completion will resubmit */
@@ -486,8 +483,6 @@ static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
                dev->ctl_urb_pending = 1;
                cm109_submit_buzz_toggle(dev);
        }
-
-       spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
 }
 
 static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on)
@@ -556,32 +551,30 @@ static int cm109_input_open(struct input_dev *idev)
                return error;
        }
 
-       mutex_lock(&dev->pm_mutex);
-
-       dev->buzzer_state = 0;
-       dev->key_code = -1;     /* no keys pressed */
-       dev->keybit = 0xf;
+       scoped_guard(mutex, &dev->pm_mutex) {
+               dev->buzzer_state = 0;
+               dev->key_code = -1;     /* no keys pressed */
+               dev->keybit = 0xf;
 
-       /* issue INIT */
-       dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
-       dev->ctl_data->byte[HID_OR1] = dev->keybit;
-       dev->ctl_data->byte[HID_OR2] = dev->keybit;
-       dev->ctl_data->byte[HID_OR3] = 0x00;
+               /* issue INIT */
+               dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
+               dev->ctl_data->byte[HID_OR1] = dev->keybit;
+               dev->ctl_data->byte[HID_OR2] = dev->keybit;
+               dev->ctl_data->byte[HID_OR3] = 0x00;
 
-       dev->ctl_urb_pending = 1;
-       error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
-       if (error) {
-               dev->ctl_urb_pending = 0;
-               dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
-                       __func__, error);
-       } else {
-               dev->open = 1;
+               dev->ctl_urb_pending = 1;
+               error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
+               if (!error) {
+                       dev->open = 1;
+                       return 0;
+               }
        }
 
-       mutex_unlock(&dev->pm_mutex);
+       dev->ctl_urb_pending = 0;
+       usb_autopm_put_interface(dev->intf);
 
-       if (error)
-               usb_autopm_put_interface(dev->intf);
+       dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
+               __func__, error);
 
        return error;
 }
@@ -590,17 +583,15 @@ static void cm109_input_close(struct input_dev *idev)
 {
        struct cm109_dev *dev = input_get_drvdata(idev);
 
-       mutex_lock(&dev->pm_mutex);
-
-       /*
-        * Once we are here event delivery is stopped so we
-        * don't need to worry about someone starting buzzer
-        * again
-        */
-       cm109_stop_traffic(dev);
-       dev->open = 0;
-
-       mutex_unlock(&dev->pm_mutex);
+       scoped_guard(mutex, &dev->pm_mutex) {
+               /*
+                * Once we are here event delivery is stopped so we
+                * don't need to worry about someone starting buzzer
+                * again
+                */
+               cm109_stop_traffic(dev);
+               dev->open = 0;
+       }
 
        usb_autopm_put_interface(dev->intf);
 }
@@ -823,9 +814,9 @@ static int cm109_usb_suspend(struct usb_interface *intf, pm_message_t message)
 
        dev_info(&intf->dev, "cm109: usb_suspend (event=%d)\n", message.event);
 
-       mutex_lock(&dev->pm_mutex);
+       guard(mutex)(&dev->pm_mutex);
+
        cm109_stop_traffic(dev);
-       mutex_unlock(&dev->pm_mutex);
 
        return 0;
 }
@@ -836,9 +827,9 @@ static int cm109_usb_resume(struct usb_interface *intf)
 
        dev_info(&intf->dev, "cm109: usb_resume\n");
 
-       mutex_lock(&dev->pm_mutex);
+       guard(mutex)(&dev->pm_mutex);
+
        cm109_restore_state(dev);
-       mutex_unlock(&dev->pm_mutex);
 
        return 0;
 }