Re: [PATCH v2 1/3] USB: serial: return errors from break handling

Johan Hovold
2023-06-06T11:34:20Z

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