USB: serial: iuu_phoenix: fix led-activity helpers
Subsystems (2)
Johan Hovold
maintainerPATCH 1/1 USB: serial: iuu_phoenix: fix led-activity helpers
The set-led command is eight bytes long and starts with a command byte followed by six bytes of RGB data and ends with a byte encoding a frequency (see iuu_led() and iuu_rgbf_fill_buffer()).
The led activity helpers had a few long-standing bugs which corrupted the command packets by inserting a second command byte and thereby offsetting the RGB data and dropping the frequency in non-xmas mode.
In xmas mode, a related off-by-one error left the frequency field uninitialised.
---
George,
Thanks for reporting this issue and sorry for not getting back to you sooner about this. It was sort buried in that long RFC series of yours which was about something unrelated (and your patch did two distinct logical changes in one patch which is generally frowned upon):
https://lore.kernel.org/r/[email protected]
Let me know if you prefer to respin your fix, and then include also the fix of the related issue in iuu_led_activity_off().
Otherwise I'll apply this one next week.
Johan
drivers/usb/serial/iuu_phoenix.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/usb/serial/iuu_phoenix.c +8 -6
@@ -350,10 +350,11 @@ static void iuu_led_activity_on(struct urb *urb) {struct usb_serial_port *port = urb->context;char *buf_ptr = port->write_urb->transfer_buffer; - *buf_ptr++ =IUU_SET_LED ; +if (xmas) { -get_random_bytes (buf_ptr,6 ); - *(buf_ptr+7 ) =1 ; + buf_ptr[0 ] =IUU_SET_LED ; +get_random_bytes (buf_ptr +1 ,6 ); + buf_ptr[7 ] =1 ; }else {iuu_rgbf_fill_buffer (buf_ptr,255 ,255 ,0 ,0 ,0 ,0 ,255 ); }
@@ -370,13 +371,14 @@ static void iuu_led_activity_off(struct urb *urb) {struct usb_serial_port *port = urb->context;char *buf_ptr = port->write_urb->transfer_buffer; +if (xmas) {iuu_rxcmd (urb);return ; - }else { - *buf_ptr++ =IUU_SET_LED ; -iuu_rgbf_fill_buffer (buf_ptr,0 ,0 ,255 ,255 ,0 ,0 ,255 ); } + +iuu_rgbf_fill_buffer (buf_ptr,0 ,0 ,255 ,255 ,0 ,0 ,255 ); + usb_fill_bulk_urb(port->write_urb, port->serial->dev, usb_sndbulkpipe(port->serial->dev, port->bulk_out_endpointAddress),
Signature · 1 line
2.26.2
Quoted 8 lines · Johan Hovold
The set-led command is eight bytes long and starts with a command byte followed by six bytes of RGB data and ends with a byte encoding a frequency (see
iuu_led()andiuu_rgbf_fill_buffer()).The led activity helpers had a few long-standing bugs which corrupted the command packets by inserting a second command byte and thereby offsetting the RGB data and dropping the frequency in non-xmas mode.
Quoted 6 lines
In xmas mode, a related off-by-one error left the frequency field uninitialised.
Fixes 60a8fc017103 ("USB: add iuu_phoenix driver")Reported-by George Spelvin <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Johan Hovold
maintainerQuoted 9 lines · Greg Kroah-Hartman
Quoted 8 lines · Johan Hovold
The set-led command is eight bytes long and starts with a command byte followed by six bytes of RGB data and ends with a byte encoding a frequency (see
iuu_led()andiuu_rgbf_fill_buffer()).The led activity helpers had a few long-standing bugs which corrupted the command packets by inserting a second command byte and thereby offsetting the RGB data and dropping the frequency in non-xmas mode.
Quoted 8 lines
Quoted 6 lines
In xmas mode, a related off-by-one error left the frequency field uninitialised.
Fixes 60a8fc017103 ("USB: add iuu_phoenix driver")Reported-by George Spelvin <[email protected]>Signed-off-by Johan Hovold <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>
Thanks for reviewing. Now applied for -next.
Johan