Hi Oliver,
On Tue, Jun 06, 2023 at 01:13:30PM +0200, Oliver Neukum wrote:
> On 04.06.23 14:35, Johan Hovold wrote:
> > @@ -1077,15 +1077,19 @@ static void pl2303_set_break(struct usb_serial_port *port, bool enable)
> > result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
> > BREAK_REQUEST, BREAK_REQUEST_TYPE, state,
> > 0, NULL, 0, 100);
> > - if (result)
> > + if (result) {
> > dev_err(&port->dev, "error sending break = %d\n", result);
> > + return result;
> > + }
> > +
> > + return 0;
> > }
And thanks for taking a look.
> this code was always fishy, but I am afraid it worked by accident albeit
> spamming the logs.
> If I may quote from the kerneldoc of usb_control_msg():
>
> * usb_control_msg - Builds a control urb, sends it off and waits for completion
>
> [..]
>
> * Return: If successful, the number of bytes transferred. Otherwise, a negative
> * error number.
> */
>
> You need to test for < 0, not != 0
No, the current code is just fine as there is no data stage so
usb_control_msg() returns 0 or negative errno.
Johan
