| Message ID | 20200312125524.7812-2-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb-serial: xHCI and timeout fixes | expand |
Jason Andryuk, le jeu. 12 mars 2020 08:55:20 -0400, a ecrit: > We'll be adding a loop, so move the code into a helper function. breaks > are replaced with returns. > > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> > --- > hw/usb/dev-serial.c | 77 +++++++++++++++++++++++++-------------------- > 1 file changed, 43 insertions(+), 34 deletions(-) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index daac75b7ae..71fa786bd8 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -358,13 +358,53 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p, > } > } > > +static void usb_serial_token_in(USBSerialState *s, USBPacket *p) > +{ > + int first_len, len; > + uint8_t header[2]; > + > + first_len = RECV_BUF - s->recv_ptr; > + len = p->iov.size; > + if (len <= 2) { > + p->status = USB_RET_NAK; > + return; > + } > + header[0] = usb_get_modem_lines(s) | 1; > + /* We do not have the uart details */ > + /* handle serial break */ > + if (s->event_trigger && s->event_trigger & FTDI_BI) { > + s->event_trigger &= ~FTDI_BI; > + header[1] = FTDI_BI; > + usb_packet_copy(p, header, 2); > + return; > + } else { > + header[1] = 0; > + } > + len -= 2; > + if (len > s->recv_used) > + len = s->recv_used; > + if (!len) { > + p->status = USB_RET_NAK; > + return; > + } > + if (first_len > len) > + first_len = len; > + usb_packet_copy(p, header, 2); > + usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len); > + if (len > first_len) > + usb_packet_copy(p, s->recv_buf, len - first_len); > + s->recv_used -= len; > + s->recv_ptr = (s->recv_ptr + len) % RECV_BUF; > + > + return; > +} > + > static void usb_serial_handle_data(USBDevice *dev, USBPacket *p) > { > USBSerialState *s = (USBSerialState *)dev; > uint8_t devep = p->ep->nr; > struct iovec *iov; > - uint8_t header[2]; > - int i, first_len, len; > + int i; > > switch (p->pid) { > case USB_TOKEN_OUT: > @@ -382,38 +422,7 @@ static void usb_serial_handle_data(USBDevice *dev, USBPacket *p) > case USB_TOKEN_IN: > if (devep != 1) > goto fail; > - first_len = RECV_BUF - s->recv_ptr; > - len = p->iov.size; > - if (len <= 2) { > - p->status = USB_RET_NAK; > - break; > - } > - header[0] = usb_get_modem_lines(s) | 1; > - /* We do not have the uart details */ > - /* handle serial break */ > - if (s->event_trigger && s->event_trigger & FTDI_BI) { > - s->event_trigger &= ~FTDI_BI; > - header[1] = FTDI_BI; > - usb_packet_copy(p, header, 2); > - break; > - } else { > - header[1] = 0; > - } > - len -= 2; > - if (len > s->recv_used) > - len = s->recv_used; > - if (!len) { > - p->status = USB_RET_NAK; > - break; > - } > - if (first_len > len) > - first_len = len; > - usb_packet_copy(p, header, 2); > - usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len); > - if (len > first_len) > - usb_packet_copy(p, s->recv_buf, len - first_len); > - s->recv_used -= len; > - s->recv_ptr = (s->recv_ptr + len) % RECV_BUF; > + usb_serial_token_in(s, p); > break; > > default: > -- > 2.24.1 >
Hi, > + if (len > s->recv_used) > + len = s->recv_used; scripts/checkpatch.pl flags a codestyle error here. > - if (len > s->recv_used) > - len = s->recv_used; Which is strictly speaking not your fault as you are just moving around existing code. It's common practice though that codestyle is fixed up too when touching code. Any chance you can make sure the patches pass checkpatch & resend? thanks, Gerd
On Mon, Mar 16, 2020 at 7:40 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > + if (len > s->recv_used) > > + len = s->recv_used; > > scripts/checkpatch.pl flags a codestyle error here. > > > - if (len > s->recv_used) > > - len = s->recv_used; > > Which is strictly speaking not your fault as you are just moving around > existing code. It's common practice though that codestyle is fixed up > too when touching code. Any chance you can make sure the patches pass > checkpatch & resend? Sure. Will do. Regards, Jason
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index daac75b7ae..71fa786bd8 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -358,13 +358,53 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p, } } +static void usb_serial_token_in(USBSerialState *s, USBPacket *p) +{ + int first_len, len; + uint8_t header[2]; + + first_len = RECV_BUF - s->recv_ptr; + len = p->iov.size; + if (len <= 2) { + p->status = USB_RET_NAK; + return; + } + header[0] = usb_get_modem_lines(s) | 1; + /* We do not have the uart details */ + /* handle serial break */ + if (s->event_trigger && s->event_trigger & FTDI_BI) { + s->event_trigger &= ~FTDI_BI; + header[1] = FTDI_BI; + usb_packet_copy(p, header, 2); + return; + } else { + header[1] = 0; + } + len -= 2; + if (len > s->recv_used) + len = s->recv_used; + if (!len) { + p->status = USB_RET_NAK; + return; + } + if (first_len > len) + first_len = len; + usb_packet_copy(p, header, 2); + usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len); + if (len > first_len) + usb_packet_copy(p, s->recv_buf, len - first_len); + s->recv_used -= len; + s->recv_ptr = (s->recv_ptr + len) % RECV_BUF; + + return; +} + static void usb_serial_handle_data(USBDevice *dev, USBPacket *p) { USBSerialState *s = (USBSerialState *)dev; uint8_t devep = p->ep->nr; struct iovec *iov; - uint8_t header[2]; - int i, first_len, len; + int i; switch (p->pid) { case USB_TOKEN_OUT: @@ -382,38 +422,7 @@ static void usb_serial_handle_data(USBDevice *dev, USBPacket *p) case USB_TOKEN_IN: if (devep != 1) goto fail; - first_len = RECV_BUF - s->recv_ptr; - len = p->iov.size; - if (len <= 2) { - p->status = USB_RET_NAK; - break; - } - header[0] = usb_get_modem_lines(s) | 1; - /* We do not have the uart details */ - /* handle serial break */ - if (s->event_trigger && s->event_trigger & FTDI_BI) { - s->event_trigger &= ~FTDI_BI; - header[1] = FTDI_BI; - usb_packet_copy(p, header, 2); - break; - } else { - header[1] = 0; - } - len -= 2; - if (len > s->recv_used) - len = s->recv_used; - if (!len) { - p->status = USB_RET_NAK; - break; - } - if (first_len > len) - first_len = len; - usb_packet_copy(p, header, 2); - usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len); - if (len > first_len) - usb_packet_copy(p, s->recv_buf, len - first_len); - s->recv_used -= len; - s->recv_ptr = (s->recv_ptr + len) % RECV_BUF; + usb_serial_token_in(s, p); break; default:
We'll be adding a loop, so move the code into a helper function. breaks are replaced with returns. Signed-off-by: Jason Andryuk <jandryuk@gmail.com> --- hw/usb/dev-serial.c | 77 +++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 34 deletions(-)