* [bug report] usb: gadget: u_serial: Implement remote wakeup capability
@ 2025-04-30 8:09 Dan Carpenter
2025-04-30 10:36 ` Prashanth K
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-04-30 8:09 UTC (permalink / raw)
To: Prashanth K; +Cc: linux-usb
Hello Prashanth K,
Commit 3baea29dc0a7 ("usb: gadget: u_serial: Implement remote wakeup
capability") from Apr 24, 2025 (linux-next), leads to the following
Smatch static checker warning:
drivers/usb/gadget/function/u_serial.c:1511 gserial_suspend() warn: double unlock 'global &serial_port_lock' (orig line 1505)
drivers/usb/gadget/function/u_serial.c:1514 gserial_suspend() warn: double unlock 'flags' (orig line 1505)
drivers/usb/gadget/function/u_serial.c
1501 }
1502
1503 if (port->write_busy || port->write_started) {
1504 /* Wakeup to host if there are ongoing transfers */
1505 spin_unlock_irqrestore(&serial_port_lock, flags);
^^^^^^^^^^^^^^^^ ^^^^^
We unlock here.
1506 if (!gserial_wakeup_host(gser))
Assume gserial_wakeup_host() fails so we don't return on the next line.
1507 return;
1508 }
1509
1510 spin_lock(&port->port_lock);
--> 1511 spin_unlock(&serial_port_lock);
^^^^^^^^^^^^^^^^^
Double unlock.
1512 port->suspended = true;
1513 port->start_delayed = true;
1514 spin_unlock_irqrestore(&port->port_lock, flags);
^^^^^
IRQs were already enabled. Which is probably fine, I don't know.
1515 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] usb: gadget: u_serial: Implement remote wakeup capability
2025-04-30 8:09 [bug report] usb: gadget: u_serial: Implement remote wakeup capability Dan Carpenter
@ 2025-04-30 10:36 ` Prashanth K
2025-04-30 10:43 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Prashanth K @ 2025-04-30 10:36 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-usb
On 30-04-25 01:39 pm, Dan Carpenter wrote:
> Hello Prashanth K,
>
> Commit 3baea29dc0a7 ("usb: gadget: u_serial: Implement remote wakeup
> capability") from Apr 24, 2025 (linux-next), leads to the following
> Smatch static checker warning:
>
> drivers/usb/gadget/function/u_serial.c:1511 gserial_suspend() warn: double unlock 'global &serial_port_lock' (orig line 1505)
> drivers/usb/gadget/function/u_serial.c:1514 gserial_suspend() warn: double unlock 'flags' (orig line 1505)
>
> drivers/usb/gadget/function/u_serial.c
> 1501 }
> 1502
> 1503 if (port->write_busy || port->write_started) {
> 1504 /* Wakeup to host if there are ongoing transfers */
> 1505 spin_unlock_irqrestore(&serial_port_lock, flags);
> ^^^^^^^^^^^^^^^^ ^^^^^
> We unlock here.
>
> 1506 if (!gserial_wakeup_host(gser))
>
> Assume gserial_wakeup_host() fails so we don't return on the next line.
>
> 1507 return;
> 1508 }
> 1509
> 1510 spin_lock(&port->port_lock);
> --> 1511 spin_unlock(&serial_port_lock);
> ^^^^^^^^^^^^^^^^^
> Double unlock.
>
> 1512 port->suspended = true;
> 1513 port->start_delayed = true;
> 1514 spin_unlock_irqrestore(&port->port_lock, flags);
> ^^^^^
> IRQs were already enabled. Which is probably fine, I don't know.
>
> 1515 }
>
> regards,
> dan carpenter
Can you send a patch, or should i fix it? Let me know.
I think we can rearrange the locks also, something like this,
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 41dee7c8cc7c..1f182fce071a 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1500,15 +1500,16 @@ void gserial_suspend(struct gserial *gser)
return;
}
+ spin_lock(&port->port_lock);
+ spin_unlock(&serial_port_lock);
if (port->write_busy || port->write_started) {
/* Wakeup to host if there are ongoing transfers */
- spin_unlock_irqrestore(&serial_port_lock, flags);
+ spin_unlock_irqrestore(&port->port_lock, flags);
if (!gserial_wakeup_host(gser))
return;
+ spin_lock_irqsave(&port->port_lock, flags);
}
- spin_lock(&port->port_lock);
- spin_unlock(&serial_port_lock);
port->suspended = true;
port->start_delayed = true;
spin_unlock_irqrestore(&port->port_lock, flags);
Regards,
Prashanth K
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [bug report] usb: gadget: u_serial: Implement remote wakeup capability
2025-04-30 10:36 ` Prashanth K
@ 2025-04-30 10:43 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-04-30 10:43 UTC (permalink / raw)
To: Prashanth K; +Cc: linux-usb
On Wed, Apr 30, 2025 at 04:06:00PM +0530, Prashanth K wrote:
>
>
> On 30-04-25 01:39 pm, Dan Carpenter wrote:
> > Hello Prashanth K,
> >
> > Commit 3baea29dc0a7 ("usb: gadget: u_serial: Implement remote wakeup
> > capability") from Apr 24, 2025 (linux-next), leads to the following
> > Smatch static checker warning:
> >
> > drivers/usb/gadget/function/u_serial.c:1511 gserial_suspend() warn: double unlock 'global &serial_port_lock' (orig line 1505)
> > drivers/usb/gadget/function/u_serial.c:1514 gserial_suspend() warn: double unlock 'flags' (orig line 1505)
> >
> > drivers/usb/gadget/function/u_serial.c
> > 1501 }
> > 1502
> > 1503 if (port->write_busy || port->write_started) {
> > 1504 /* Wakeup to host if there are ongoing transfers */
> > 1505 spin_unlock_irqrestore(&serial_port_lock, flags);
> > ^^^^^^^^^^^^^^^^ ^^^^^
> > We unlock here.
> >
> > 1506 if (!gserial_wakeup_host(gser))
> >
> > Assume gserial_wakeup_host() fails so we don't return on the next line.
> >
> > 1507 return;
> > 1508 }
> > 1509
> > 1510 spin_lock(&port->port_lock);
> > --> 1511 spin_unlock(&serial_port_lock);
> > ^^^^^^^^^^^^^^^^^
> > Double unlock.
> >
> > 1512 port->suspended = true;
> > 1513 port->start_delayed = true;
> > 1514 spin_unlock_irqrestore(&port->port_lock, flags);
> > ^^^^^
> > IRQs were already enabled. Which is probably fine, I don't know.
> >
> > 1515 }
> >
> > regards,
> > dan carpenter
>
> Can you send a patch, or should i fix it? Let me know.
Could you could send the patch? Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-30 10:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-30 8:09 [bug report] usb: gadget: u_serial: Implement remote wakeup capability Dan Carpenter
2025-04-30 10:36 ` Prashanth K
2025-04-30 10:43 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox