[PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached

Mark Cave-Ayland posted 9 patches 5 years ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>
There is a newer version of this series
[PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached
Posted by Mark Cave-Ayland 5 years ago
Some operating systems will generate a new device ID when a USB device is unplugged
and then replugged into the USB. If this is done whilst switching between multiple
applications over a virtual serial port, the change of device ID requires going
back into the OS/application to locate the new device accordingly.

Add a new always-plugged property that if specified will ensure that the device
always remains attached to the USB regardless of the state of the backend
chardev.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/usb/dev-serial.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 92c35615eb..919e25e1d9 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -97,6 +97,7 @@ struct USBSerialState {
     uint8_t event_chr;
     uint8_t error_chr;
     uint8_t event_trigger;
+    bool always_plugged;
     QEMUSerialSetParams params;
     int latency;        /* ms */
     CharBackend cs;
@@ -516,12 +517,12 @@ static void usb_serial_event(void *opaque, QEMUChrEvent event)
         s->event_trigger |= FTDI_BI;
         break;
     case CHR_EVENT_OPENED:
-        if (!s->dev.attached) {
+        if (!s->always_plugged && !s->dev.attached) {
             usb_device_attach(&s->dev, &error_abort);
         }
         break;
     case CHR_EVENT_CLOSED:
-        if (s->dev.attached) {
+        if (!s->always_plugged && s->dev.attached) {
             usb_device_detach(&s->dev);
         }
         break;
@@ -556,7 +557,8 @@ static void usb_serial_realize(USBDevice *dev, Error **errp)
                              usb_serial_event, NULL, s, NULL, true);
     usb_serial_handle_reset(dev);
 
-    if (qemu_chr_fe_backend_open(&s->cs) && !dev->attached) {
+    if (s->always_plugged || (qemu_chr_fe_backend_open(&s->cs) &&
+                              !dev->attached)) {
         usb_device_attach(dev, &error_abort);
     }
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
@@ -584,6 +586,7 @@ static const VMStateDescription vmstate_usb_serial = {
 
 static Property serial_properties[] = {
     DEFINE_PROP_CHR("chardev", USBSerialState, cs),
+    DEFINE_PROP_BOOL("always-plugged", USBSerialState, always_plugged, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.20.1


Re: [PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached
Posted by Samuel Thibault 5 years ago
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:58 +0000, a ecrit:
> Some operating systems will generate a new device ID when a USB device is unplugged
> and then replugged into the USB. If this is done whilst switching between multiple
> applications over a virtual serial port, the change of device ID requires going
> back into the OS/application to locate the new device accordingly.
> 
> Add a new always-plugged property that if specified will ensure that the device
> always remains attached to the USB regardless of the state of the backend
> chardev.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  hw/usb/dev-serial.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 92c35615eb..919e25e1d9 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -97,6 +97,7 @@ struct USBSerialState {
>      uint8_t event_chr;
>      uint8_t error_chr;
>      uint8_t event_trigger;
> +    bool always_plugged;
>      QEMUSerialSetParams params;
>      int latency;        /* ms */
>      CharBackend cs;
> @@ -516,12 +517,12 @@ static void usb_serial_event(void *opaque, QEMUChrEvent event)
>          s->event_trigger |= FTDI_BI;
>          break;
>      case CHR_EVENT_OPENED:
> -        if (!s->dev.attached) {
> +        if (!s->always_plugged && !s->dev.attached) {
>              usb_device_attach(&s->dev, &error_abort);
>          }
>          break;
>      case CHR_EVENT_CLOSED:
> -        if (s->dev.attached) {
> +        if (!s->always_plugged && s->dev.attached) {
>              usb_device_detach(&s->dev);
>          }
>          break;
> @@ -556,7 +557,8 @@ static void usb_serial_realize(USBDevice *dev, Error **errp)
>                               usb_serial_event, NULL, s, NULL, true);
>      usb_serial_handle_reset(dev);
>  
> -    if (qemu_chr_fe_backend_open(&s->cs) && !dev->attached) {
> +    if (s->always_plugged || (qemu_chr_fe_backend_open(&s->cs) &&
> +                              !dev->attached)) {
>          usb_device_attach(dev, &error_abort);
>      }
>      s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
> @@ -584,6 +586,7 @@ static const VMStateDescription vmstate_usb_serial = {
>  
>  static Property serial_properties[] = {
>      DEFINE_PROP_CHR("chardev", USBSerialState, cs),
> +    DEFINE_PROP_BOOL("always-plugged", USBSerialState, always_plugged, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 2.20.1
> 

-- 
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)

Re: [PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached
Posted by Gerd Hoffmann 5 years ago
>      case CHR_EVENT_OPENED:
> -        if (!s->dev.attached) {
> +        if (!s->always_plugged && !s->dev.attached) {
>              usb_device_attach(&s->dev, &error_abort);
>          }

Not needed (but doesn't hurt either).

>          break;
>      case CHR_EVENT_CLOSED:
> -        if (s->dev.attached) {
> +        if (!s->always_plugged && s->dev.attached) {
>              usb_device_detach(&s->dev);
>          }

Ok.

> -    if (qemu_chr_fe_backend_open(&s->cs) && !dev->attached) {
> +    if (s->always_plugged || (qemu_chr_fe_backend_open(&s->cs) &&
> +                              !dev->attached)) {

The dev->attached check should not be skipped, i.e. the logic should be
((always_plugged || open) && !attached).

take care,
  Gerd


Re: [PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached
Posted by Mark Cave-Ayland 5 years ago
On 27/10/2020 08:09, Gerd Hoffmann wrote:

>>       case CHR_EVENT_OPENED:
>> -        if (!s->dev.attached) {
>> +        if (!s->always_plugged && !s->dev.attached) {
>>               usb_device_attach(&s->dev, &error_abort);
>>           }
> 
> Not needed (but doesn't hurt either).

Okay I'll leave this as-is for now.

>>           break;
>>       case CHR_EVENT_CLOSED:
>> -        if (s->dev.attached) {
>> +        if (!s->always_plugged && s->dev.attached) {
>>               usb_device_detach(&s->dev);
>>           }
> 
> Ok.
> 
>> -    if (qemu_chr_fe_backend_open(&s->cs) && !dev->attached) {
>> +    if (s->always_plugged || (qemu_chr_fe_backend_open(&s->cs) &&
>> +                              !dev->attached)) {
> 
> The dev->attached check should not be skipped, i.e. the logic should be
> ((always_plugged || open) && !attached).

Let me test this, and if it works I'll post a v2 shortly.


ATB,

Mark.