[Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

Use usb_legacy_register handling to create bt-dongle device. This allows
to disable usb-bt-dongle device using CONFIG_BLUETOOTH option.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 hw/bt/core.c           | 25 ++++++++++++++++
 hw/bt/hci.c            | 51 ++++++++++++++++++++++++++++++++
 hw/usb/Makefile.objs   |  3 --
 hw/usb/dev-bluetooth.c | 10 ++++++-
 include/hw/bt.h        |  3 ++
 include/hw/usb.h       |  3 --
 vl.c                   | 79 ++------------------------------------------------
 7 files changed, 91 insertions(+), 83 deletions(-)

diff --git a/hw/bt/core.c b/hw/bt/core.c
index 49012e0..ef27b15 100644
--- a/hw/bt/core.c
+++ b/hw/bt/core.c
@@ -119,3 +119,28 @@ void bt_device_done(struct bt_device_s *dev)
 
     *p = dev->next;
 }
+
+static struct bt_vlan_s {
+    struct bt_scatternet_s net;
+    int id;
+    struct bt_vlan_s *next;
+} *first_bt_vlan;
+
+/* find or alloc a new bluetooth "VLAN" */
+struct bt_scatternet_s *qemu_find_bt_vlan(int id)
+{
+    struct bt_vlan_s **pvlan, *vlan;
+    for (vlan = first_bt_vlan; vlan != NULL; vlan = vlan->next) {
+        if (vlan->id == id) {
+            return &vlan->net;
+        }
+    }
+    vlan = g_malloc0(sizeof(struct bt_vlan_s));
+    vlan->id = id;
+    pvlan = &first_bt_vlan;
+    while (*pvlan != NULL) {
+        pvlan = &(*pvlan)->next;
+    }
+    *pvlan = vlan;
+    return &vlan->net;
+}
diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index d1c0604..d69ab53 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -429,6 +429,24 @@ static const uint8_t bt_event_reserved_mask[8] = {
     0xff, 0x9f, 0xfb, 0xff, 0x07, 0x18, 0x00, 0x00,
 };
 
+
+static void null_hci_send(struct HCIInfo *hci, const uint8_t *data, int len)
+{
+}
+
+static int null_hci_addr_set(struct HCIInfo *hci, const uint8_t *bd_addr)
+{
+    return -ENOTSUP;
+}
+
+struct HCIInfo null_hci = {
+    .cmd_send = null_hci_send,
+    .sco_send = null_hci_send,
+    .acl_send = null_hci_send,
+    .bdaddr_set = null_hci_addr_set,
+};
+
+
 static inline uint8_t *bt_hci_event_start(struct bt_hci_s *hci,
                 int evt, int len)
 {
@@ -2176,6 +2194,39 @@ struct HCIInfo *bt_new_hci(struct bt_scatternet_s *net)
     return &s->info;
 }
 
+struct HCIInfo *hci_init(const char *str)
+{
+    char *endp;
+    struct bt_scatternet_s *vlan = 0;
+
+    if (!strcmp(str, "null")) {
+        /* null */
+        return &null_hci;
+    } else if (!strncmp(str, "host", 4) && (str[4] == '\0' || str[4] == ':')) {
+        /* host[:hciN] */
+        return bt_host_hci(str[4] ? str + 5 : "hci0");
+    } else if (!strncmp(str, "hci", 3)) {
+        /* hci[,vlan=n] */
+        if (str[3]) {
+            if (!strncmp(str + 3, ",vlan=", 6)) {
+                vlan = qemu_find_bt_vlan(strtol(str + 9, &endp, 0));
+                if (*endp) {
+                    vlan = 0;
+                }
+            }
+        } else {
+            vlan = qemu_find_bt_vlan(0);
+        }
+        if (vlan) {
+            return bt_new_hci(vlan);
+        }
+    }
+
+    fprintf(stderr, "qemu: Unknown bluetooth HCI `%s'.\n", str);
+
+    return 0;
+}
+
 static void bt_hci_done(struct HCIInfo *info)
 {
     struct bt_hci_s *hci = hci_from_info(info);
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index f9695e7..a3eac3e 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -18,9 +18,6 @@ common-obj-$(CONFIG_USB_STORAGE_UAS)  += dev-uas.o
 common-obj-$(CONFIG_USB_AUDIO)        += dev-audio.o
 common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
-
-# FIXME: make configurable too
-CONFIG_USB_BLUETOOTH := y
 common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index f2fc2a8..7f292b1 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -511,10 +511,17 @@ static int usb_bt_initfn(USBDevice *dev)
     return 0;
 }
 
-USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci)
+static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline)
 {
     USBDevice *dev;
     struct USBBtState *s;
+    HCIInfo *hci;
+
+    if (*cmdline) {
+        hci = hci_init(cmdline);
+    } else {
+        hci = bt_new_hci(qemu_find_bt_vlan(0));
+    }
 
     if (!hci)
         return NULL;
@@ -566,6 +573,7 @@ static const TypeInfo bt_info = {
 static void usb_bt_register_types(void)
 {
     type_register_static(&bt_info);
+    usb_legacy_register("usb-bt-dongle", "bt", usb_bt_init);
 }
 
 type_init(usb_bt_register_types)
diff --git a/include/hw/bt.h b/include/hw/bt.h
index 830af94..49a9d03 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -108,12 +108,15 @@ struct bt_device_s {
     uint16_t clkoff;	/* Note: Always little-endian */
 };
 
+extern struct HCIInfo null_hci;
 /* bt.c */
 void bt_device_init(struct bt_device_s *dev, struct bt_scatternet_s *net);
 void bt_device_done(struct bt_device_s *dev);
+struct bt_scatternet_s *qemu_find_bt_vlan(int id);
 
 /* bt-hci.c */
 struct HCIInfo *bt_new_hci(struct bt_scatternet_s *net);
+struct HCIInfo *hci_init(const char *str);
 
 /* bt-vhci.c */
 void bt_vhci_init(struct HCIInfo *info);
diff --git a/include/hw/usb.h b/include/hw/usb.h
index 901b0da..695d853 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -442,9 +442,6 @@ int set_usb_string(uint8_t *buf, const char *str);
 USBDevice *usb_host_device_open(USBBus *bus, const char *devname);
 void usb_host_info(Monitor *mon, const QDict *qdict);
 
-/* usb-bt.c */
-USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci);
-
 /* usb ports of the VM */
 
 #define VM_USB_HUB_SIZE 8
diff --git a/vl.c b/vl.c
index dfbc071..846a1b0 100644
--- a/vl.c
+++ b/vl.c
@@ -843,45 +843,6 @@ static int nb_hcis;
 static int cur_hci;
 static struct HCIInfo *hci_table[MAX_NICS];
 
-static struct bt_vlan_s {
-    struct bt_scatternet_s net;
-    int id;
-    struct bt_vlan_s *next;
-} *first_bt_vlan;
-
-/* find or alloc a new bluetooth "VLAN" */
-static struct bt_scatternet_s *qemu_find_bt_vlan(int id)
-{
-    struct bt_vlan_s **pvlan, *vlan;
-    for (vlan = first_bt_vlan; vlan != NULL; vlan = vlan->next) {
-        if (vlan->id == id)
-            return &vlan->net;
-    }
-    vlan = g_malloc0(sizeof(struct bt_vlan_s));
-    vlan->id = id;
-    pvlan = &first_bt_vlan;
-    while (*pvlan != NULL)
-        pvlan = &(*pvlan)->next;
-    *pvlan = vlan;
-    return &vlan->net;
-}
-
-static void null_hci_send(struct HCIInfo *hci, const uint8_t *data, int len)
-{
-}
-
-static int null_hci_addr_set(struct HCIInfo *hci, const uint8_t *bd_addr)
-{
-    return -ENOTSUP;
-}
-
-static struct HCIInfo null_hci = {
-    .cmd_send = null_hci_send,
-    .sco_send = null_hci_send,
-    .acl_send = null_hci_send,
-    .bdaddr_set = null_hci_addr_set,
-};
-
 struct HCIInfo *qemu_next_hci(void)
 {
     if (cur_hci == nb_hcis)
@@ -890,36 +851,6 @@ struct HCIInfo *qemu_next_hci(void)
     return hci_table[cur_hci++];
 }
 
-static struct HCIInfo *hci_init(const char *str)
-{
-    char *endp;
-    struct bt_scatternet_s *vlan = 0;
-
-    if (!strcmp(str, "null"))
-        /* null */
-        return &null_hci;
-    else if (!strncmp(str, "host", 4) && (str[4] == '\0' || str[4] == ':'))
-        /* host[:hciN] */
-        return bt_host_hci(str[4] ? str + 5 : "hci0");
-    else if (!strncmp(str, "hci", 3)) {
-        /* hci[,vlan=n] */
-        if (str[3]) {
-            if (!strncmp(str + 3, ",vlan=", 6)) {
-                vlan = qemu_find_bt_vlan(strtol(str + 9, &endp, 0));
-                if (*endp)
-                    vlan = 0;
-            }
-        } else
-            vlan = qemu_find_bt_vlan(0);
-        if (vlan)
-           return bt_new_hci(vlan);
-    }
-
-    fprintf(stderr, "qemu: Unknown bluetooth HCI `%s'.\n", str);
-
-    return 0;
-}
-
 static int bt_hci_parse(const char *str)
 {
     struct HCIInfo *hci;
@@ -1526,8 +1457,10 @@ static void configure_msg(QemuOpts *opts)
 
 static int usb_device_add(const char *devname)
 {
-    const char *p;
     USBDevice *dev = NULL;
+#ifndef CONFIG_LINUX
+    const char *p;
+#endif
 
     if (!usb_enabled(false)) {
         return -1;
@@ -1545,13 +1478,7 @@ static int usb_device_add(const char *devname)
         dev = usb_host_device_open(usb_bus_find(-1), p);
     } else
 #endif
-    if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
-        dev = usb_bt_init(usb_bus_find(-1),
-                          devname[2] ? hci_init(p)
-                                     : bt_new_hci(qemu_find_bt_vlan(0)));
-    } else {
         return -1;
-    }
     if (!dev)
         return -1;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[Gerd Hoffmann]

> diff --git a/hw/bt/core.c b/hw/bt/core.c
> index 49012e0..ef27b15 100644
> --- a/hw/bt/core.c
> +++ b/hw/bt/core.c
> @@ -119,3 +119,28 @@ void bt_device_done(struct bt_device_s *dev)
>  
>      *p = dev->next;
>  }
> +
> +static struct bt_vlan_s {
> +    struct bt_scatternet_s net;
> +    int id;
> +    struct bt_vlan_s *next;
> +} *first_bt_vlan;
> +
> +/* find or alloc a new bluetooth "VLAN" */
> +struct bt_scatternet_s *qemu_find_bt_vlan(int id)
> +{
> +    struct bt_vlan_s **pvlan, *vlan;
> +    for (vlan = first_bt_vlan; vlan != NULL; vlan = vlan->next) {
> +        if (vlan->id == id) {
> +            return &vlan->net;
> +        }
> +    }
> +    vlan = g_malloc0(sizeof(struct bt_vlan_s));
> +    vlan->id = id;
> +    pvlan = &first_bt_vlan;
> +    while (*pvlan != NULL) {
> +        pvlan = &(*pvlan)->next;
> +    }
> +    *pvlan = vlan;
> +    return &vlan->net;
> +}

This (and some other bits) are pure code motion from vl.c, correct?
Can you split this into a separate patch please?  That'll simplify the
review o the actual code changes.

It also doesn't make much sense to compile hw/bt/ with
CONFIG_USB_BLUETOOTH=n.  It's basically dead code then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[Miroslav Rezanina]

----- Original Message -----
> From: "Gerd Hoffmann" <kraxel@redhat.com>
> To: mrezanin@redhat.com
> Cc: qemu-devel@nongnu.org
> Sent: Tuesday, September 3, 2013 9:41:11 AM
> Subject: Re: [Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable
> 
> > diff --git a/hw/bt/core.c b/hw/bt/core.c
> > index 49012e0..ef27b15 100644
> > --- a/hw/bt/core.c
> > +++ b/hw/bt/core.c
> > @@ -119,3 +119,28 @@ void bt_device_done(struct bt_device_s *dev)
> >  
> >      *p = dev->next;
> >  }
> > +
> > +static struct bt_vlan_s {
> > +    struct bt_scatternet_s net;
> > +    int id;
> > +    struct bt_vlan_s *next;
> > +} *first_bt_vlan;
> > +
> > +/* find or alloc a new bluetooth "VLAN" */
> > +struct bt_scatternet_s *qemu_find_bt_vlan(int id)
> > +{
> > +    struct bt_vlan_s **pvlan, *vlan;
> > +    for (vlan = first_bt_vlan; vlan != NULL; vlan = vlan->next) {
> > +        if (vlan->id == id) {
> > +            return &vlan->net;
> > +        }
> > +    }
> > +    vlan = g_malloc0(sizeof(struct bt_vlan_s));
> > +    vlan->id = id;
> > +    pvlan = &first_bt_vlan;
> > +    while (*pvlan != NULL) {
> > +        pvlan = &(*pvlan)->next;
> > +    }
> > +    *pvlan = vlan;
> > +    return &vlan->net;
> > +}
> 
> This (and some other bits) are pure code motion from vl.c, correct?
> Can you split this into a separate patch please?  That'll simplify the
> review o the actual code changes.

Yes, this is pure code motion. I'll split the code to separate patches.
> 
> It also doesn't make much sense to compile hw/bt/ with
> CONFIG_USB_BLUETOOTH=n.  It's basically dead code then.
> 

Is this true? So -bt option is not useable without usb-bt-dongle? 

> cheers,
>   Gerd
> 
> 
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team

Re: [Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[Gerd Hoffmann]

> > This (and some other bits) are pure code motion from vl.c, correct?
> > Can you split this into a separate patch please?  That'll simplify the
> > review o the actual code changes.
> 
> Yes, this is pure code motion. I'll split the code to separate patches.
> > 
> > It also doesn't make much sense to compile hw/bt/ with
> > CONFIG_USB_BLUETOOTH=n.  It's basically dead code then.
> > 
> 
> Is this true? So -bt option is not useable without usb-bt-dongle? 

Ahem, well, double-checked:  No.

n800+n810 emulation (qemu-system-arm) has a bluetooth hci too.

So we need a separate CONFIG_BLUETOOTH for hw/bt/, so we can enable it
for both arm emulation and for CONFIG_USB_BLUETOOTH=y.  Maybe it makes
sense to wait until we have kconfig.

cheers,
  Gerd

Re: [Qemu-devel] [PATCHv2] Make usb-bt-dongle configurable

[Paolo Bonzini]

Il 03/09/2013 09:26, mrezanin@redhat.com ha scritto:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> Use usb_legacy_register handling to create bt-dongle device. This allows
> to disable usb-bt-dongle device using CONFIG_BLUETOOTH option.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---

Looks good to me, just one small improvement I can suggest:

>  static int bt_hci_parse(const char *str)
>  {
>      struct HCIInfo *hci;
> @@ -1526,8 +1457,10 @@ static void configure_msg(QemuOpts *opts)
>  
>  static int usb_device_add(const char *devname)
>  {
> -    const char *p;
>      USBDevice *dev = NULL;
> +#ifndef CONFIG_LINUX
> +    const char *p;
> +#endif
>  
>      if (!usb_enabled(false)) {
>          return -1;
> @@ -1545,13 +1478,7 @@ static int usb_device_add(const char *devname)
>          dev = usb_host_device_open(usb_bus_find(-1), p);
>      } else
>  #endif
> -    if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
> -        dev = usb_bt_init(usb_bus_find(-1),
> -                          devname[2] ? hci_init(p)
> -                                     : bt_new_hci(qemu_find_bt_vlan(0)));
> -    } else {
>          return -1;

You can remove this "return" too.

Paolo

> -    }
>      if (!dev)
>          return -1;
>  
>