[Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[list subscribe]

[-- Attachment #1.1: Type: text/plain, Size: 7395 bytes --]

Hi,

As per the discussion on #bluez channel , I am attaching a patch which
dynamically manages the the alternate settings for SCO channels.
The patch includes the following :

1. It declares a work queue(config_work), which is scheduled when
             1. There is a change in number of SCO channels OR
             2. There is a change in the Voice setting.

2. On invocation it determines the alternate setting based on the table
given in HCI - usb spec docs. The  table is also mentioned  in the patch.

3. It then empties all the queued isochronous URBs , except those which are
already submitted.

4. It then sets the alternate setting by calling   usb_set_interface().

I have tested this patch using "scotest".
I am using 2.6.21-rc7 with sco-flowcontrol-v4.2.diff patch.

Let me know if  any changes are necessary.
Any suggestions are welcome.



Alok.


--- hci_usb.c.orig    2007-04-16 19:37:41.000000000 +0530
+++ hci_usb.c    2007-04-24 01:41:31.000000000 +0530
@@ -47,6 +47,8 @@

 #include <linux/usb.h>

+#include <linux/workqueue.h>
+
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -57,6 +59,21 @@
 #define BT_DBG(D...)
 #endif

+/* The number of SCO channels  */
+int NoSco=0;
+/* The  Voice setting  */
+__u16 VocSetting=0;
+/* if bit=0,its 8bit encoding else its 16bit */
+__u16 bit = 0x20;
+
+struct hci_usb *hUSB;
+struct usb_device *usbdevice;
+struct usb_interface *isocIface;
+/* The Workque Function */
+static void set_alternate_config(struct work_struct *work);
+static DECLARE_WORK(config_work, set_alternate_config);
+
+
 #ifndef CONFIG_BT_HCIUSB_ZERO_PACKET
 #undef  URB_ZERO_PACKET
 #define URB_ZERO_PACKET 0
@@ -840,6 +857,10 @@
 static void hci_usb_notify(struct hci_dev *hdev, unsigned int evt)
 {
     BT_DBG("%s evt %d", hdev->name, evt);
+    NoSco = hdev->conn_hash.sco_num;
+    VocSetting = hdev->voice_setting;
+    hUSB = (struct hci_usb *) hdev->driver_data;
+    schedule_work(&config_work);
 }

 static int hci_usb_probe(struct usb_interface *intf, const struct
usb_device_id *id)
@@ -854,7 +875,7 @@
     struct hci_usb *husb;
     struct hci_dev *hdev;
     int i, e, size, isoc_ifnum, isoc_alts;
-
+    usbdevice = interface_to_usbdev(intf);
     BT_DBG("udev %p intf %p", udev, intf);

     if (!id->driver_info) {
@@ -926,9 +947,11 @@
     isoc_ifnum = 1;

 #ifdef CONFIG_BT_HCIUSB_SCO
-    if (isoc && !(id->driver_info & (HCI_BROKEN_ISOC | HCI_SNIFFER)))
+    if (isoc && !(id->driver_info & (HCI_BROKEN_ISOC | HCI_SNIFFER))){
         isoc_iface = usb_ifnum_to_if(udev, isoc_ifnum);
-
+            isocIface = usb_ifnum_to_if(udev, isoc_ifnum);
+    }
+
     if (isoc_iface) {
         int a;
         struct usb_host_endpoint *isoc_out_ep = NULL;
@@ -1142,6 +1165,121 @@
     return 0;
 }

+
+/*Set the alternate setting based on the number of SCO channels and the
+  voice setting.This function is invoked when there is a change in Number
of
+  SCO channels or when the voice encoding changes.*/
+static void set_alternate_config(struct work_struct *work)
+{
+    int isocIfnum=1, isocAlt=0;
+    struct usb_host_endpoint  *ep;
+    struct usb_host_interface *uif;
+    struct _urb *_urb,*_tmp;
+    struct _urb_queue *q = &hUSB->pending_q[isoc];
+    unsigned long flags;
+    atomic_t temp;/*Holds the number of URBs we need to skip(which are
submitted)*/
+    struct list_head inprocess;/*This list holds the already submitted URBs
*/
+
+    /*Change the alternate setting only if the number of SCO channels are
more than 1 */
+    if(NoSco > 0){
+        /* The alternate setting selection is based on the following table
*/
+        /*   No. of SCO channels    Bit-Encoding    Alternate Setting
Max. Packet Size */
+        /*           1                 8bit
1                  9 */
+        /*           1                16bit
2                 17 */
+        /*           2                 8bit
2                 17 */
+        /*           2                16bit
4                 33 */
+        /*           3                 8bit
3                 25 */
+        /*           3                16bit
5                 49 */
+        switch(NoSco)
+        {
+        case 1:
+            if(VocSetting && bit)
+                isocAlt=2;
+            else
+                isocAlt=1;
+            break;
+        case 2:
+            if(VocSetting && bit)
+                isocAlt=4;
+            else
+                isocAlt=2;
+            break;
+        case 3:
+            if(VocSetting && bit)
+                isocAlt=5;
+            else
+                isocAlt=3;
+            break;
+        }
+
+        /*Stop Current TX */
+        clear_bit(HCI_USB_TX_WAKEUP, &hUSB->state);
+        INIT_LIST_HEAD(&inprocess);
+        temp = hUSB->pending_tx[isoc];
+        /*  We cannot purge URBs which have been submitted. inprocess is a
*/
+        /*  temporary list which holds the currently submitted URBs. */
+        /*  This list is later merged with the emptyed pending queue.  */
+
+        while ((_urb = _urb_dequeue(q))) {
+            /*Dequeue all the submitted URBs and put them in the temporary
list*/
+            if(!atomic_dec_and_test(&temp)){
+                _urb->queue = q;
+                list_add(&_urb->list, &inprocess);
+            }
+            else{
+                /*Unlink all the rest of URBs and put them into the
completed queue.*/
+                _urb_unlink(_urb);
+                _urb_queue_tail(__completed_q(hUSB,HCI_SCODATA_PKT), _urb);
+            }
+        }
+        /*merge the inprocess queue with the pending queue*/
+        spin_lock_irqsave(&q->lock, flags);
+        list_for_each_entry_safe(_urb, _tmp, &inprocess, list) {
+            list_move_tail(&_urb->list, &q->head);
+        }
+        spin_unlock_irqrestore(&q->lock, flags);
+
+        /* Set the setting and the in/out endpoints */
+        if (isocIface) {
+            int e;
+            struct usb_host_endpoint *out = NULL;
+            struct usb_host_endpoint *in = NULL;
+            uif = &isocIface->altsetting[isocAlt];
+            for (e = 0; e < uif->desc.bNumEndpoints; e++) {
+                ep = &uif->endpoint[e];
+                switch (ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
{
+                case USB_ENDPOINT_XFER_ISOC:
+                    if (ep->desc.bEndpointAddress & USB_DIR_IN)
+                        in  = ep;
+                    else
+                        out = ep;
+                    break;
+                }
+            }
+
+            if (!in || !out)
+                BT_DBG("Isoc endpoints not found");
+            else {
+                BT_DBG("isoc ifnum %d alts %d", isocIfnum, isocAlt);
+                if (usb_set_interface(usbdevice, isocIfnum, isocAlt)) {
+                    BT_ERR("Can't set isoc interface settings");
+                    hUSB->isoc_iface = isocIface;
+                    usb_driver_release_interface(&hci_usb_driver,
isocIface);
+                    hUSB->isoc_iface = NULL;
+                } else {
+                    hUSB->isoc_iface  = isocIface;
+                    hUSB->isoc_in_ep  = in;
+                    hUSB->isoc_out_ep  = out;
+                }
+            }
+        }
+        set_bit(HCI_USB_TX_WAKEUP, &hUSB->state);
+    }
+
+}
+
+
+
 static struct usb_driver hci_usb_driver = {
     .name        = "hci_usb",
     .probe        = hci_usb_probe,

[-- Attachment #1.2: Type: text/html, Size: 15709 bytes --]

[-- Attachment #2: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Jose Vasconcellos]

Hi Alok,

This is a good start. There are a few other things to consider:

* Are there error conditions in hci_usb_tx_complete that need to be handled?
* How do you make sure data for different channels is interleaved?

Jose

list subscribe wrote:
> Hi,
>
> As per the discussion on #bluez channel , I am attaching a patch which 
> dynamically manages the the alternate settings for SCO channels.
> The patch includes the following :
>
> 1. It declares a work queue(config_work), which is scheduled when
>              1. There is a change in number of SCO channels OR
>              2. There is a change in the Voice setting.
>
> 2. On invocation it determines the alternate setting based on the 
> table given in HCI - usb spec docs. The  table is also mentioned  in 
> the patch.
>
> 3. It then empties all the queued isochronous URBs , except those 
> which are already submitted.
>
> 4. It then sets the alternate setting by calling   usb_set_interface().
>
> I have tested this patch using "scotest".
> I am using 2.6.21-rc7 with sco-flowcontrol-v4.2.diff patch.
>
> Let me know if  any changes are necessary.
> Any suggestions are welcome.
>
>
>
> Alok. 
>
>
> --- hci_usb.c.orig    2007-04-16 19:37:41.000000000 +0530
> +++ hci_usb.c    2007-04-24 01:41:31.000000000 +0530
> @@ -47,6 +47,8 @@
>  
>  #include <linux/usb.h>
>  
> +#include <linux/workqueue.h>
> +
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>  
> @@ -57,6 +59,21 @@
>  #define BT_DBG(D...)
>  #endif
>  
> +/* The number of SCO channels  */
> +int NoSco=0;
> +/* The  Voice setting  */
> +__u16 VocSetting=0;
> +/* if bit=0,its 8bit encoding else its 16bit */
> +__u16 bit = 0x20;
> +
> +struct hci_usb *hUSB;
> +struct usb_device *usbdevice;
> +struct usb_interface *isocIface;
> +/* The Workque Function */
> +static void set_alternate_config(struct work_struct *work);
> +static DECLARE_WORK(config_work, set_alternate_config);
> +
> +
>  #ifndef CONFIG_BT_HCIUSB_ZERO_PACKET
>  #undef  URB_ZERO_PACKET
>  #define URB_ZERO_PACKET 0
> @@ -840,6 +857,10 @@
>  static void hci_usb_notify(struct hci_dev *hdev, unsigned int evt)
>  {
>      BT_DBG("%s evt %d", hdev->name, evt);
> +    NoSco = hdev->conn_hash.sco_num;
> +    VocSetting = hdev->voice_setting;
> +    hUSB = (struct hci_usb *) hdev->driver_data;
> +    schedule_work(&config_work);
>  }
>  
>  static int hci_usb_probe(struct usb_interface *intf, const struct 
> usb_device_id *id)
> @@ -854,7 +875,7 @@
>      struct hci_usb *husb;
>      struct hci_dev *hdev;
>      int i, e, size, isoc_ifnum, isoc_alts;
> -
> +    usbdevice = interface_to_usbdev(intf);
>      BT_DBG("udev %p intf %p", udev, intf);
>  
>      if (!id->driver_info) {
> @@ -926,9 +947,11 @@
>      isoc_ifnum = 1;
>  
>  #ifdef CONFIG_BT_HCIUSB_SCO
> -    if (isoc && !(id->driver_info & (HCI_BROKEN_ISOC | HCI_SNIFFER)))
> +    if (isoc && !(id->driver_info & (HCI_BROKEN_ISOC | HCI_SNIFFER))){
>          isoc_iface = usb_ifnum_to_if(udev, isoc_ifnum);
> -
> +            isocIface = usb_ifnum_to_if(udev, isoc_ifnum);
> +    }
> +   
>      if (isoc_iface) {
>          int a;
>          struct usb_host_endpoint *isoc_out_ep = NULL;
> @@ -1142,6 +1165,121 @@
>      return 0;
>  }
>  
> +
> +/*Set the alternate setting based on the number of SCO channels and the
> +  voice setting.This function is invoked when there is a change in 
> Number of
> +  SCO channels or when the voice encoding changes.*/
> +static void set_alternate_config(struct work_struct *work)
> +{
> +    int isocIfnum=1, isocAlt=0;
> +    struct usb_host_endpoint  *ep;
> +    struct usb_host_interface *uif;
> +    struct _urb *_urb,*_tmp;
> +    struct _urb_queue *q = &hUSB->pending_q[isoc];
> +    unsigned long flags;
> +    atomic_t temp;/*Holds the number of URBs we need to skip(which 
> are submitted)*/
> +    struct list_head inprocess;/*This list holds the already 
> submitted URBs */
> +   
> +    /*Change the alternate setting only if the number of SCO channels 
> are more than 1 */
> +    if(NoSco > 0){
> +        /* The alternate setting selection is based on the following 
> table */
> +        /*   No. of SCO channels    Bit-Encoding    Alternate 
> Setting   Max. Packet Size */
> +        /*           1                 8bit                 
> 1                  9 */
> +        /*           1                16bit                 
> 2                 17 */
> +        /*           2                 8bit                 
> 2                 17 */
> +        /*           2                16bit                 
> 4                 33 */
> +        /*           3                 8bit                 
> 3                 25 */
> +        /*           3                16bit                 
> 5                 49 */
> +        switch(NoSco)
> +        {
> +        case 1:
> +            if(VocSetting && bit)
> +                isocAlt=2;
> +            else
> +                isocAlt=1;
> +            break;
> +        case 2:
> +            if(VocSetting && bit)
> +                isocAlt=4;
> +            else
> +                isocAlt=2;
> +            break;
> +        case 3:
> +            if(VocSetting && bit)
> +                isocAlt=5;
> +            else
> +                isocAlt=3;
> +            break;
> +        }
> +
> +        /*Stop Current TX */
> +        clear_bit(HCI_USB_TX_WAKEUP, &hUSB->state);
> +        INIT_LIST_HEAD(&inprocess);
> +        temp = hUSB->pending_tx[isoc];
> +        /*  We cannot purge URBs which have been submitted. inprocess 
> is a */
> +        /*  temporary list which holds the currently submitted URBs. */
> +        /*  This list is later merged with the emptyed pending queue.  */
> +  
> +        while ((_urb = _urb_dequeue(q))) {
> +            /*Dequeue all the submitted URBs and put them in the 
> temporary list*/ 
> +            if(!atomic_dec_and_test(&temp)){
> +                _urb->queue = q;
> +                list_add(&_urb->list, &inprocess);
> +            }
> +            else{
> +                /*Unlink all the rest of URBs and put them into the 
> completed queue.*/
> +                _urb_unlink(_urb);
> +                _urb_queue_tail(__completed_q(hUSB,HCI_SCODATA_PKT), 
> _urb);
> +            }
> +        }
> +        /*merge the inprocess queue with the pending queue*/
> +        spin_lock_irqsave(&q->lock, flags);
> +        list_for_each_entry_safe(_urb, _tmp, &inprocess, list) {
> +            list_move_tail(&_urb->list, &q->head);
> +        }
> +        spin_unlock_irqrestore(&q->lock, flags);
> +   
> +        /* Set the setting and the in/out endpoints */
> +        if (isocIface) {
> +            int e;
> +            struct usb_host_endpoint *out = NULL;
> +            struct usb_host_endpoint *in = NULL;
> +            uif = &isocIface->altsetting[isocAlt];
> +            for (e = 0; e < uif->desc.bNumEndpoints; e++) {
> +                ep = &uif->endpoint[e];
> +                switch (ep->desc.bmAttributes & 
> USB_ENDPOINT_XFERTYPE_MASK) {
> +                case USB_ENDPOINT_XFER_ISOC:
> +                    if (ep->desc.bEndpointAddress & USB_DIR_IN)
> +                        in  = ep;
> +                    else
> +                        out = ep;
> +                    break;
> +                }
> +            }
> +       
> +            if (!in || !out)
> +                BT_DBG("Isoc endpoints not found");
> +            else {
> +                BT_DBG("isoc ifnum %d alts %d", isocIfnum, isocAlt);
> +                if (usb_set_interface(usbdevice, isocIfnum, isocAlt)) {
> +                    BT_ERR("Can't set isoc interface settings");
> +                    hUSB->isoc_iface = isocIface;
> +                    usb_driver_release_interface(&hci_usb_driver, 
> isocIface);
> +                    hUSB->isoc_iface = NULL;
> +                } else {
> +                    hUSB->isoc_iface  = isocIface;
> +                    hUSB->isoc_in_ep  = in;
> +                    hUSB->isoc_out_ep  = out;
> +                }
> +            }
> +        }
> +        set_bit(HCI_USB_TX_WAKEUP, &hUSB->state);
> +    }
> +
> +}
> + 
> +
> +
>  static struct usb_driver hci_usb_driver = {
>      .name        = "hci_usb",
>      .probe        = hci_usb_probe,
> ------------------------------------------------------------------------

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[list subscribe]

[-- Attachment #1.1: Type: text/plain, Size: 691 bytes --]

Jose,



Hi Alok,
>
> This is a good start. There are a few other things to consider:
>
> * Are there error conditions in hci_usb_tx_complete that need to be
> handled?


I am not sure what you mean.  Since we don't flush submitted  URBs, these
URBs are handled like normal ones. If their status indicates error the
error_tx count will be incremented. What error conditions are u pointing at?


* How do you make sure data for different channels is interleaved?


Well , since we know that there can be max 3 SCO channels , we can have 3
separate queues, and use a round robin method to send out the packets from
the queues. i am not sure about this idea though.  What do u think?

Jose
>
>

[-- Attachment #1.2: Type: text/html, Size: 1250 bytes --]

[-- Attachment #2: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Jose Vasconcellos]

list subscribe wrote:
> Jose,
>
>     Hi Alok,
>
>     This is a good start. There are a few other things to consider:
>
>     * Are there error conditions in hci_usb_tx_complete that need to
>     be handled?
>
>
> I am not sure what you mean.  Since we don't flush submitted  URBs, 
> these URBs are handled like normal ones. If their status indicates 
> error the error_tx count will be incremented. What error conditions 
> are u pointing at?
The outstanding urbs are dequeued and put on a temporary list and 
resubmitted
after changing the configuration. But there may be an active URB that is 
being
transmitted that will be reported with an error status. 
hci_usb_tx_complete will
release this urb so it can't be resubmitted. This may also result in a 
small glitch
when switching to an alternate.
>
>     * How do you make sure data for different channels is interleaved?
>
>
> Well , since we know that there can be max 3 SCO channels , we can 
> have 3 separate queues, and use a round robin method to send out the 
> packets from the queues. i am not sure about this idea though.  What 
> do u think?
I think this is the right approach; I found that it's very important to 
do the
interleaving properly. There is an issue if one of the queues is empty
(i.e. buffer underrun). What to do in that case? Send silence?

Jose


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Marcel Holtmann]

Hi Jose,

> >     * How do you make sure data for different channels is interleaved?
> >
> >
> > Well , since we know that there can be max 3 SCO channels , we can 
> > have 3 separate queues, and use a round robin method to send out the 
> > packets from the queues. i am not sure about this idea though.  What 
> > do u think?
> I think this is the right approach; I found that it's very important to 
> do the
> interleaving properly. There is an issue if one of the queues is empty
> (i.e. buffer underrun). What to do in that case? Send silence?

this is not the job of the transport driver. The job of the transport
driver is to send the SCO packets to the hardware. The Bluetooth core
has to take care of the correct ordering.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[alok]

Jose,

On Tue, 2007-04-24 at 09:27 -0600, Jose Vasconcellos wrote:
> list subscribe wrote:
> > Jose,
> >
> >     Hi Alok,
> >
> >     This is a good start. There are a few other things to consider:
> >
> >     * Are there error conditions in hci_usb_tx_complete that need to
> >     be handled?
> >
> >
> > I am not sure what you mean.  Since we don't flush submitted  URBs, 
> > these URBs are handled like normal ones. If their status indicates 
> > error the error_tx count will be incremented. What error conditions 
> > are u pointing at?
> The outstanding urbs are dequeued and put on a temporary list and 
> resubmitted
> after changing the configuration. But there may be an active URB that is 
> being
> transmitted that will be reported with an error status. 
> hci_usb_tx_complete will
> release this urb so it can't be resubmitted. This may also result in a 
> small glitch
> when switching to an alternate.

The submitted URBs are dequeued and put into a temporary list but are
not resubmitted later . Rather after flushing the pending queue , the
temporary list is appended to pending_queue. 
So after the setting is changed, we have some previous submitted URBs
which are in the process of transmission followed by URBs which use the
changed settings in the pending queue. 

If any urb reports an error, only the tx error count is incremented
(hdev->stat.err_tx++). 
And all _urbs are unlinked (whether their urbs return error or not) and
queued to the complete_q queue.  

I hope this answers your question.

> >
> >     * How do you make sure data for different channels is interleaved?
> >
> >
> > Well , since we know that there can be max 3 SCO channels , we can 
> > have 3 separate queues, and use a round robin method to send out the 
> > packets from the queues. i am not sure about this idea though.  What 
> > do u think?
> I think this is the right approach; I found that it's very important to 
> do the
> interleaving properly. There is an issue if one of the queues is empty
> (i.e. buffer underrun). What to do in that case? Send silence?
If u hit a underrun on one queue, u ignore that sco channel.What
difference is it going to make if we sent silence packets on that
channel?
I am not sure about this. I have no expertise in audio. if someone can
justify this solution i can implement it. 

Alok.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Jose Vasconcellos]

alok wrote:
> Jose,
>
> On Tue, 2007-04-24 at 09:27 -0600, Jose Vasconcellos wrote:
>   
>> list subscribe wrote:
>>     
>>> Jose,
>>>
>>>     Hi Alok,
>>>
>>>     This is a good start. There are a few other things to consider:
>>>
>>>     * Are there error conditions in hci_usb_tx_complete that need to
>>>     be handled?
>>>
>>>
>>> I am not sure what you mean.  Since we don't flush submitted  URBs, 
>>> these URBs are handled like normal ones. If their status indicates 
>>> error the error_tx count will be incremented. What error conditions 
>>> are u pointing at?
>>>       
>> The outstanding urbs are dequeued and put on a temporary list and 
>> resubmitted
>> after changing the configuration. But there may be an active URB that is 
>> being
>> transmitted that will be reported with an error status. 
>> hci_usb_tx_complete will
>> release this urb so it can't be resubmitted. This may also result in a 
>> small glitch
>> when switching to an alternate.
>>     
>
> The submitted URBs are dequeued and put into a temporary list but are
> not resubmitted later . Rather after flushing the pending queue , the
> temporary list is appended to pending_queue. 
> So after the setting is changed, we have some previous submitted URBs
> which are in the process of transmission followed by URBs which use the
> changed settings in the pending queue. 
>
> If any urb reports an error, only the tx error count is incremented
> (hdev->stat.err_tx++). 
> And all _urbs are unlinked (whether their urbs return error or not) and
> queued to the complete_q queue.  
>
> I hope this answers your question.
>   
My point is that it's possible that the active URBs that was cancelled
will end up in hci_usb_tx_complete with an error status. You must not
add this URB to the pending queue since the complete routine will
have released it.

>>>     * How do you make sure data for different channels is interleaved?
>>>
>>>
>>> Well , since we know that there can be max 3 SCO channels , we can 
>>> have 3 separate queues, and use a round robin method to send out the 
>>> packets from the queues. i am not sure about this idea though.  What
>>> do u think?
>>>       
>> I think this is the right approach; I found that it's very important to 
>> do the
>> interleaving properly. There is an issue if one of the queues is empty
>> (i.e. buffer underrun). What to do in that case? Send silence?
>>     
> If u hit a underrun on one queue, u ignore that sco channel.What
> difference is it going to make if we sent silence packets on that
> channel?
> I am not sure about this. I have no expertise in audio. if someone can
> justify this solution i can implement it. 
>   
Since we're dealing with voice samples you may get some glitches.
Maybe this is not a big deal.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH]Dynamic Alternate Setting patch (hci_usb.c)

[Marcel Holtmann]

Hi,

> As per the discussion on #bluez channel , I am attaching a patch which
> dynamically manages the the alternate settings for SCO channels.
> The patch includes the following :
> 
> 1. It declares a work queue(config_work), which is scheduled when
>              1. There is a change in number of SCO channels OR 
>              2. There is a change in the Voice setting. 
> 
> 2. On invocation it determines the alternate setting based on the
> table given in HCI - usb spec docs. The  table is also mentioned  in
> the patch. 
> 
> 3. It then empties all the queued isochronous URBs , except those
> which are already submitted. 
> 
> 4. It then sets the alternate setting by calling
> usb_set_interface().
> 
> I have tested this patch using "scotest". 
> I am using 2.6.21-rc7 with sco-flowcontrol-v4.2.diff patch. 
> 
> Let me know if  any changes are necessary. 

having global static variables doesn't work. It will break horrible in
case you attach a second Bluetooth dongle. Also the voice setting is
stored in hci_dev and always present. So no need to copy these
information. What you need to store is the current alternate setting and
then calculate the new one. Change the setting and then store it.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel