RE: drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked 'ci->lock' (orig line 638)
From: Jun Li
Date: Fri Aug 07 2020 - 22:44:40 EST
Hi,
> -----Original Message-----
> From: kernel test robot <lkp@xxxxxxxxx>
> Sent: Friday, August 7, 2020 6:00 PM
> To: Jun Li <jun.li@xxxxxxx>
> Cc: kbuild-all@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Peter Chen
> <peter.chen@xxxxxxx>
> Subject: drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double
> unlocked 'ci->lock' (orig line 638)
>
> tree:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.o
> rg%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git&data=02%7C01%
> 7Cjun.li%40nxp.com%7Ce261827f14f6446b3e8308d83ab8c407%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C637323912694699319&sdata=ioDexF9tIACz%2BlnZC%2BzZsiFz
> iOeeLembxqr9PwE%2Fpgg%3D&reserved=0 master
> head: 86cfccb66937dd6cbf26ed619958b9e587e6a115
> commit: 05559f10ed797b79f7fa47313682c48919a2b111 usb: chipidea: add role switch
> class support
> date: 12 months ago
> config: parisc-randconfig-m031-20200807 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>
> smatch warnings:
> drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked
> 'ci->lock' (orig line 638)
>
> vim +657 drivers/usb/chipidea/core.c
>
> 615
> 616 static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> 617 {
> 618 struct ci_hdrc *ci = dev_get_drvdata(dev);
> 619 struct ci_hdrc_cable *cable = NULL;
> 620 enum usb_role current_role = ci_role_to_usb_role(ci);
> 621 unsigned long flags;
> 622
> 623 if (current_role == role)
> 624 return 0;
> 625
> 626 pm_runtime_get_sync(ci->dev);
> 627 /* Stop current role */
> 628 spin_lock_irqsave(&ci->lock, flags);
> 629 if (current_role == USB_ROLE_DEVICE)
> 630 cable = &ci->platdata->vbus_extcon;
> 631 else if (current_role == USB_ROLE_HOST)
> 632 cable = &ci->platdata->id_extcon;
> 633
> 634 if (cable) {
> 635 cable->changed = true;
> 636 cable->connected = false;
> 637 ci_irq(ci->irq, ci);
> > 638 spin_unlock_irqrestore(&ci->lock, flags);
> 639 if (ci->wq && role != USB_ROLE_NONE)
> 640 flush_workqueue(ci->wq);
> 641 spin_lock_irqsave(&ci->lock, flags);
Here we hold the lock again.
> 642 }
> 643
> 644 cable = NULL;
> 645
> 646 /* Start target role */
> 647 if (role == USB_ROLE_DEVICE)
> 648 cable = &ci->platdata->vbus_extcon;
> 649 else if (role == USB_ROLE_HOST)
> 650 cable = &ci->platdata->id_extcon;
> 651
> 652 if (cable) {
> 653 cable->changed = true;
> 654 cable->connected = true;
> 655 ci_irq(ci->irq, ci);
> 656 }
> > 657 spin_unlock_irqrestore(&ci->lock, flags);
So it's fine to unlock here.
Thanks
Li Jun
> 658 pm_runtime_put_sync(ci->dev);
> 659
> 660 return 0;
> 661 }
> 662
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org
> %2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=02%7C01%7Cjun.li%40n
> xp.com%7Ce261827f14f6446b3e8308d83ab8c407%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C637323912694709319&sdata=tAxmh6eq3yT3A89mY2RrGQxwdkUN4A9r708lFQJW
> gcI%3D&reserved=0