| Message ID | 20201026083401.13231-7-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | Accepted |
| Commit | 66007a95674d4b8e616245541faad6cdf5e9f70d |
| Headers | show |
| Series | dev-serial: minor fixes and improvements | expand |
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)
> 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
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.
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(), };
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(-)