1. Patchew
  2. QEMU
  3. [PATCH 0/4] usb-serial: xHCI and timeout fixes
  4. View patch

[PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function

Jason Andryuk posted 4 patches 5 years, 6 months ago
Maintainers: Samuel Thibault <samuel.thibault@ens-lyon.org>, Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
[PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function
Posted by Jason Andryuk 5 years, 6 months ago 
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(-)

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
Re: [PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function
Posted by Gerd Hoffmann 5 years, 6 months ago 
  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
Re: [PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function
Posted by Jason Andryuk 5 years, 6 months ago 
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
Re: [PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function
Posted by Samuel Thibault 5 years, 6 months ago 
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
> 

-- 
Samuel
Tu as lu les docs. Tu es devenu un informaticien. Que tu le veuilles
ou non. Lire la doc, c'est le Premier et Unique Commandement de
l'informaticien.
-+- TP in: Guide du Linuxien pervers - "L'évangile selon St Thomas"

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.