Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+78 -87
+78 -87
drivers/input/misc/cm109.c
··· 355 355 __func__, error); 356 356 } 357 357 358 + static void cm109_submit_ctl(struct cm109_dev *dev) 359 + { 360 + int error; 361 + 362 + guard(spinlock_irqsave)(&dev->ctl_submit_lock); 363 + 364 + dev->irq_urb_pending = 0; 365 + 366 + if (unlikely(dev->shutdown)) 367 + return; 368 + 369 + if (dev->buzzer_state) 370 + dev->ctl_data->byte[HID_OR0] |= BUZZER_ON; 371 + else 372 + dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON; 373 + 374 + dev->ctl_data->byte[HID_OR1] = dev->keybit; 375 + dev->ctl_data->byte[HID_OR2] = dev->keybit; 376 + 377 + dev->buzzer_pending = 0; 378 + dev->ctl_urb_pending = 1; 379 + 380 + error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC); 381 + if (error) 382 + dev_err(&dev->intf->dev, 383 + "%s: usb_submit_urb (urb_ctl) failed %d\n", 384 + __func__, error); 385 + } 386 + 358 387 /* 359 388 * IRQ handler 360 389 */ ··· 391 362 { 392 363 struct cm109_dev *dev = urb->context; 393 364 const int status = urb->status; 394 - int error; 395 - unsigned long flags; 396 365 397 366 dev_dbg(&dev->intf->dev, "### URB IRQ: [0x%02x 0x%02x 0x%02x 0x%02x] keybit=0x%02x\n", 398 367 dev->irq_data->byte[0], ··· 428 401 } 429 402 430 403 out: 431 - 432 - spin_lock_irqsave(&dev->ctl_submit_lock, flags); 433 - 434 - dev->irq_urb_pending = 0; 435 - 436 - if (likely(!dev->shutdown)) { 437 - 438 - if (dev->buzzer_state) 439 - dev->ctl_data->byte[HID_OR0] |= BUZZER_ON; 440 - else 441 - dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON; 442 - 443 - dev->ctl_data->byte[HID_OR1] = dev->keybit; 444 - dev->ctl_data->byte[HID_OR2] = dev->keybit; 445 - 446 - dev->buzzer_pending = 0; 447 - dev->ctl_urb_pending = 1; 448 - 449 - error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC); 450 - if (error) 451 - dev_err(&dev->intf->dev, 452 - "%s: usb_submit_urb (urb_ctl) failed %d\n", 453 - __func__, error); 454 - } 455 - 456 - spin_unlock_irqrestore(&dev->ctl_submit_lock, flags); 404 + cm109_submit_ctl(dev); 457 405 } 458 406 459 407 static void cm109_urb_ctl_callback(struct urb *urb) ··· 436 434 struct cm109_dev *dev = urb->context; 437 435 const int status = urb->status; 438 436 int error; 439 - unsigned long flags; 440 437 441 438 dev_dbg(&dev->intf->dev, "### URB CTL: [0x%02x 0x%02x 0x%02x 0x%02x]\n", 442 439 dev->ctl_data->byte[0], ··· 450 449 __func__, status); 451 450 } 452 451 453 - spin_lock_irqsave(&dev->ctl_submit_lock, flags); 452 + guard(spinlock_irqsave)(&dev->ctl_submit_lock); 454 453 455 454 dev->ctl_urb_pending = 0; 456 455 457 - if (likely(!dev->shutdown)) { 456 + if (unlikely(dev->shutdown)) 457 + return; 458 458 459 - if (dev->buzzer_pending || status) { 460 - dev->buzzer_pending = 0; 461 - dev->ctl_urb_pending = 1; 462 - cm109_submit_buzz_toggle(dev); 463 - } else if (likely(!dev->irq_urb_pending)) { 464 - /* ask for key data */ 465 - dev->irq_urb_pending = 1; 466 - error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC); 467 - if (error) 468 - dev_err(&dev->intf->dev, 469 - "%s: usb_submit_urb (urb_irq) failed %d\n", 470 - __func__, error); 471 - } 459 + if (dev->buzzer_pending || status) { 460 + dev->buzzer_pending = 0; 461 + dev->ctl_urb_pending = 1; 462 + cm109_submit_buzz_toggle(dev); 463 + } else if (likely(!dev->irq_urb_pending)) { 464 + /* ask for key data */ 465 + dev->irq_urb_pending = 1; 466 + error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC); 467 + if (error) 468 + dev_err(&dev->intf->dev, 469 + "%s: usb_submit_urb (urb_irq) failed %d\n", 470 + __func__, error); 472 471 } 473 - 474 - spin_unlock_irqrestore(&dev->ctl_submit_lock, flags); 475 472 } 476 473 477 474 static void cm109_toggle_buzzer_async(struct cm109_dev *dev) 478 475 { 479 - unsigned long flags; 480 - 481 - spin_lock_irqsave(&dev->ctl_submit_lock, flags); 476 + guard(spinlock_irqsave)(&dev->ctl_submit_lock); 482 477 483 478 if (dev->ctl_urb_pending) { 484 479 /* URB completion will resubmit */ ··· 483 486 dev->ctl_urb_pending = 1; 484 487 cm109_submit_buzz_toggle(dev); 485 488 } 486 - 487 - spin_unlock_irqrestore(&dev->ctl_submit_lock, flags); 488 489 } 489 490 490 491 static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on) ··· 551 556 return error; 552 557 } 553 558 554 - mutex_lock(&dev->pm_mutex); 559 + scoped_guard(mutex, &dev->pm_mutex) { 560 + dev->buzzer_state = 0; 561 + dev->key_code = -1; /* no keys pressed */ 562 + dev->keybit = 0xf; 555 563 556 - dev->buzzer_state = 0; 557 - dev->key_code = -1; /* no keys pressed */ 558 - dev->keybit = 0xf; 564 + /* issue INIT */ 565 + dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF; 566 + dev->ctl_data->byte[HID_OR1] = dev->keybit; 567 + dev->ctl_data->byte[HID_OR2] = dev->keybit; 568 + dev->ctl_data->byte[HID_OR3] = 0x00; 559 569 560 - /* issue INIT */ 561 - dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF; 562 - dev->ctl_data->byte[HID_OR1] = dev->keybit; 563 - dev->ctl_data->byte[HID_OR2] = dev->keybit; 564 - dev->ctl_data->byte[HID_OR3] = 0x00; 565 - 566 - dev->ctl_urb_pending = 1; 567 - error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL); 568 - if (error) { 569 - dev->ctl_urb_pending = 0; 570 - dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n", 571 - __func__, error); 572 - } else { 573 - dev->open = 1; 570 + dev->ctl_urb_pending = 1; 571 + error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL); 572 + if (!error) { 573 + dev->open = 1; 574 + return 0; 575 + } 574 576 } 575 577 576 - mutex_unlock(&dev->pm_mutex); 578 + dev->ctl_urb_pending = 0; 579 + usb_autopm_put_interface(dev->intf); 577 580 578 - if (error) 579 - usb_autopm_put_interface(dev->intf); 581 + dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n", 582 + __func__, error); 580 583 581 584 return error; 582 585 } ··· 583 590 { 584 591 struct cm109_dev *dev = input_get_drvdata(idev); 585 592 586 - mutex_lock(&dev->pm_mutex); 587 - 588 - /* 589 - * Once we are here event delivery is stopped so we 590 - * don't need to worry about someone starting buzzer 591 - * again 592 - */ 593 - cm109_stop_traffic(dev); 594 - dev->open = 0; 595 - 596 - mutex_unlock(&dev->pm_mutex); 593 + scoped_guard(mutex, &dev->pm_mutex) { 594 + /* 595 + * Once we are here event delivery is stopped so we 596 + * don't need to worry about someone starting buzzer 597 + * again 598 + */ 599 + cm109_stop_traffic(dev); 600 + dev->open = 0; 601 + } 597 602 598 603 usb_autopm_put_interface(dev->intf); 599 604 } ··· 814 823 815 824 dev_info(&intf->dev, "cm109: usb_suspend (event=%d)\n", message.event); 816 825 817 - mutex_lock(&dev->pm_mutex); 826 + guard(mutex)(&dev->pm_mutex); 827 + 818 828 cm109_stop_traffic(dev); 819 - mutex_unlock(&dev->pm_mutex); 820 829 821 830 return 0; 822 831 } ··· 827 836 828 837 dev_info(&intf->dev, "cm109: usb_resume\n"); 829 838 830 - mutex_lock(&dev->pm_mutex); 839 + guard(mutex)(&dev->pm_mutex); 840 + 831 841 cm109_restore_state(dev); 832 - mutex_unlock(&dev->pm_mutex); 833 842 834 843 return 0; 835 844 }