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

[Johan Hovold <johan@kernel.org>]

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