[PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[Huisong Li]

Simplify udc_lock and connect_lock holding using guards in
gadget_bind_driver() and gadget_unbind_driver().

Huisong Li (2):
  usb: gadget: udc: simplify udc_lock holding using guards
  usb: gadget: udc: simplify connect_lock holding using guards

 drivers/usb/gadget/udc/core.c | 77 ++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

-- 
2.33.0

[PATCH v1 1/2] usb: gadget: udc: simplify udc_lock holding using guards

[Huisong Li]

Use guard() and scoped_guard() to manage udc_lock to simplify the code
in gadget_bind_driver() and gadget_unbind_driver().

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/usb/gadget/udc/core.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index e8861eaad907..e3f27fb39e9e 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1614,14 +1614,13 @@ static int gadget_bind_driver(struct device *dev)
 			struct usb_gadget_driver, driver);
 	int ret = 0;
 
-	mutex_lock(&udc_lock);
-	if (driver->is_bound) {
-		mutex_unlock(&udc_lock);
-		return -ENXIO;		/* Driver binds to only one gadget */
+	scoped_guard(mutex, &udc_lock) {
+		/* Driver binds to only one gadget */
+		if (driver->is_bound)
+			return -ENXIO;
+		driver->is_bound = true;
+		udc->driver = driver;
 	}
-	driver->is_bound = true;
-	udc->driver = driver;
-	mutex_unlock(&udc_lock);
 
 	dev_dbg(&udc->dev, "binding gadget driver [%s]\n", driver->function);
 
@@ -1664,10 +1663,9 @@ static int gadget_bind_driver(struct device *dev)
 		dev_err(&udc->dev, "failed to start %s: %d\n",
 			driver->function, ret);
 
-	mutex_lock(&udc_lock);
+	guard(mutex)(&udc_lock);
 	udc->driver = NULL;
 	driver->is_bound = false;
-	mutex_unlock(&udc_lock);
 
 	return ret;
 }
@@ -1695,10 +1693,10 @@ static void gadget_unbind_driver(struct device *dev)
 	usb_gadget_udc_stop_locked(udc);
 	mutex_unlock(&udc->connect_lock);
 
-	mutex_lock(&udc_lock);
-	driver->is_bound = false;
-	udc->driver = NULL;
-	mutex_unlock(&udc_lock);
+	scoped_guard(mutex, &udc_lock) {
+		driver->is_bound = false;
+		udc->driver = NULL;
+	}
 
 	kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 }
-- 
2.33.0

[PATCH v1 2/2] usb: gadget: udc: simplify connect_lock holding using guards

[Huisong Li]

Use guard() and scoped_guard() to manage connect_lock to simplify the
code in gadget_bind_driver() and gadget_unbind_driver().

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/usb/gadget/udc/core.c | 53 ++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index e3f27fb39e9e..1d615a870907 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1630,31 +1630,26 @@ static int gadget_bind_driver(struct device *dev)
 	if (ret)
 		goto err_bind;
 
-	mutex_lock(&udc->connect_lock);
-	ret = usb_gadget_udc_start_locked(udc);
-	if (ret) {
-		mutex_unlock(&udc->connect_lock);
-		goto err_start;
+	scoped_guard(mutex, &udc->connect_lock) {
+		ret = usb_gadget_udc_start_locked(udc);
+		if (ret)
+			goto err_start;
+		usb_gadget_enable_async_callbacks(udc);
+		udc->allow_connect = true;
+		ret = usb_udc_connect_control_locked(udc);
+		if (ret) {
+			udc->allow_connect = false;
+			usb_gadget_disable_async_callbacks(udc);
+			if (gadget->irq)
+				synchronize_irq(gadget->irq);
+			usb_gadget_udc_stop_locked(udc);
+			goto err_start;
+		}
 	}
-	usb_gadget_enable_async_callbacks(udc);
-	udc->allow_connect = true;
-	ret = usb_udc_connect_control_locked(udc);
-	if (ret)
-		goto err_connect_control;
-
-	mutex_unlock(&udc->connect_lock);
 
 	kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 	return 0;
 
- err_connect_control:
-	udc->allow_connect = false;
-	usb_gadget_disable_async_callbacks(udc);
-	if (gadget->irq)
-		synchronize_irq(gadget->irq);
-	usb_gadget_udc_stop_locked(udc);
-	mutex_unlock(&udc->connect_lock);
-
  err_start:
 	driver->unbind(udc->gadget);
 
@@ -1680,18 +1675,18 @@ static void gadget_unbind_driver(struct device *dev)
 
 	udc->allow_connect = false;
 	cancel_work_sync(&udc->vbus_work);
-	mutex_lock(&udc->connect_lock);
-	usb_gadget_disconnect_locked(gadget);
-	usb_gadget_disable_async_callbacks(udc);
-	if (gadget->irq)
-		synchronize_irq(gadget->irq);
-	mutex_unlock(&udc->connect_lock);
+	scoped_guard(mutex, &udc->connect_lock) {
+		usb_gadget_disconnect_locked(gadget);
+		usb_gadget_disable_async_callbacks(udc);
+		if (gadget->irq)
+			synchronize_irq(gadget->irq);
+	}
 
 	udc->driver->unbind(gadget);
 
-	mutex_lock(&udc->connect_lock);
-	usb_gadget_udc_stop_locked(udc);
-	mutex_unlock(&udc->connect_lock);
+	scoped_guard(mutex, &udc->connect_lock) {
+		usb_gadget_udc_stop_locked(udc);
+	}
 
 	scoped_guard(mutex, &udc_lock) {
 		driver->is_bound = false;
-- 
2.33.0

Re: [PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[Greg KH]

On Mon, Apr 20, 2026 at 11:57:20AM +0800, Huisong Li wrote:
> Simplify udc_lock and connect_lock holding using guards in
> gadget_bind_driver() and gadget_unbind_driver().
> 
> Huisong Li (2):
>   usb: gadget: udc: simplify udc_lock holding using guards
>   usb: gadget: udc: simplify connect_lock holding using guards
> 
>  drivers/usb/gadget/udc/core.c | 77 ++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 42 deletions(-)

Only do this for new drivers, don't convert existing code that works
just fine for no valid reason other than wanting to clean stuff up.

Does this fix a bug?

thanks,

greg k-h

Re: [PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[lihuisong (C)]

On 4/20/2026 1:40 PM, Greg KH wrote:
> On Mon, Apr 20, 2026 at 11:57:20AM +0800, Huisong Li wrote:
>> Simplify udc_lock and connect_lock holding using guards in
>> gadget_bind_driver() and gadget_unbind_driver().
>>
>> Huisong Li (2):
>>    usb: gadget: udc: simplify udc_lock holding using guards
>>    usb: gadget: udc: simplify connect_lock holding using guards
>>
>>   drivers/usb/gadget/udc/core.c | 77 ++++++++++++++++-------------------
>>   1 file changed, 35 insertions(+), 42 deletions(-)
> Only do this for new drivers, don't convert existing code that works
> just fine for no valid reason other than wanting to clean stuff up.
>
> Does this fix a bug?
Here not to fix a bug. But prepare for fixing a bug in link[1].
I found that there are many unlock of udc_lock in error path after that 
patch.
So use the guard to simpliy them by the way.

[1]https://patchwork.kernel.org/project/linux-usb/patch/20260420040223.1232745-1-lihuisong@huawei.com/

/Huisong

Re: [PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[Greg KH]

On Mon, Apr 20, 2026 at 02:21:01PM +0800, lihuisong (C) wrote:
> 
> On 4/20/2026 1:40 PM, Greg KH wrote:
> > On Mon, Apr 20, 2026 at 11:57:20AM +0800, Huisong Li wrote:
> > > Simplify udc_lock and connect_lock holding using guards in
> > > gadget_bind_driver() and gadget_unbind_driver().
> > > 
> > > Huisong Li (2):
> > >    usb: gadget: udc: simplify udc_lock holding using guards
> > >    usb: gadget: udc: simplify connect_lock holding using guards
> > > 
> > >   drivers/usb/gadget/udc/core.c | 77 ++++++++++++++++-------------------
> > >   1 file changed, 35 insertions(+), 42 deletions(-)
> > Only do this for new drivers, don't convert existing code that works
> > just fine for no valid reason other than wanting to clean stuff up.
> > 
> > Does this fix a bug?
> Here not to fix a bug. But prepare for fixing a bug in link[1].
> I found that there are many unlock of udc_lock in error path after that
> patch.
> So use the guard to simpliy them by the way.
> 
> [1]https://patchwork.kernel.org/project/linux-usb/patch/20260420040223.1232745-1-lihuisong@huawei.com/

So those patches must be applied in order?  That was not obvious and I
would have gotten them wrong :(

The policy of these guard usages is not to add them just to "clean up"
the code, only use them to actually fix real bugs today, or in new code.
Otherwise the churn in the kernel tree would be huge for no valid
reason, and cause even more bugs with backports than this pattern causes
today.

thanks,

greg k-h

Re: [PATCH v1 0/2] usb: gadget: udc: simplify lock holding using guards

[lihuisong (C)]

On 4/20/2026 2:30 PM, Greg KH wrote:
> On Mon, Apr 20, 2026 at 02:21:01PM +0800, lihuisong (C) wrote:
>> On 4/20/2026 1:40 PM, Greg KH wrote:
>>> On Mon, Apr 20, 2026 at 11:57:20AM +0800, Huisong Li wrote:
>>>> Simplify udc_lock and connect_lock holding using guards in
>>>> gadget_bind_driver() and gadget_unbind_driver().
>>>>
>>>> Huisong Li (2):
>>>>     usb: gadget: udc: simplify udc_lock holding using guards
>>>>     usb: gadget: udc: simplify connect_lock holding using guards
>>>>
>>>>    drivers/usb/gadget/udc/core.c | 77 ++++++++++++++++-------------------
>>>>    1 file changed, 35 insertions(+), 42 deletions(-)
>>> Only do this for new drivers, don't convert existing code that works
>>> just fine for no valid reason other than wanting to clean stuff up.
>>>
>>> Does this fix a bug?
>> Here not to fix a bug. But prepare for fixing a bug in link[1].
>> I found that there are many unlock of udc_lock in error path after that
>> patch.
>> So use the guard to simpliy them by the way.
>>
>> [1]https://patchwork.kernel.org/project/linux-usb/patch/20260420040223.1232745-1-lihuisong@huawei.com/
> So those patches must be applied in order?  That was not obvious and I
> would have gotten them wrong :(
That patch in the link is not modified based on this series.
I originally thought that I would update that patch after this series is 
merged (because there are not functional changes).
I will drop this series if accept the unlock on multiple error path.
>
> The policy of these guard usages is not to add them just to "clean up"
> the code, only use them to actually fix real bugs today, or in new code.
> Otherwise the churn in the kernel tree would be huge for no valid
> reason, and cause even more bugs with backports than this pattern causes
> today.
got it.