[Bluez-devel] HCI USB driver and SCO support

[Bluez-devel] HCI USB driver and SCO support

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

Hi Max,

the patch from Jonathan Paisley is working fine. I have tested it on
2.4.18 to 2.4.21 and we can safely use two ISOC TX and RX URB's by
default. But the configuration of the ISOC endpoint must be adjusted on
demand. It depends on the voice setting and number of SCO connections,
otherwise you won't hear any sound. At the moment we don't have access
to these information from within the driver, so I started to extend the
HCI core.

The first patch introduces hdev->notify and reads the voice setting on
device init and stores them in hdev->voice_setting for later use. With
these modification a driver can be notified about added or deleted
connections (ACL + SCO) and if the voice setting was changed.

The second patch is for the HCI USB driver and it shows how to use
hdev->notify. It also contains Jonathans modifications to use two ISOC
TX and RX URB's and it keeps track of the number of ACL and SCO
connections.

In the next step, we must choose the correct ISOC configuration. The
default one should be 0, because we don't use any SCO channel. It is
also a good idea to start the ISOC and BULK transfers only if they are
needed.

Comments?

Regards

Marcel


[-- Attachment #2: patch-2.4.22-pre10-hci-notify --]
[-- Type: text/x-patch, Size: 5886 bytes --]

diff -urN linux-2.4.22-pre10/include/net/bluetooth/hci.h linux-2.4.22-pre10-notify/include/net/bluetooth/hci.h
--- linux-2.4.22-pre10/include/net/bluetooth/hci.h	Sat Aug  2 02:33:57 2003
+++ linux-2.4.22-pre10-notify/include/net/bluetooth/hci.h	Sat Aug  2 02:34:37 2003
@@ -42,6 +42,11 @@
 #define HCI_DEV_SUSPEND	5
 #define HCI_DEV_RESUME	6
 
+/* HCI notify events */
+#define HCI_CONN_ADD		1
+#define HCI_CONN_DEL		2
+#define HCI_VOICE_SETTING	3
+
 /* HCI device types */
 #define HCI_VHCI	0
 #define HCI_USB		1
@@ -269,6 +274,19 @@
 	__u8 	dev_class[3];
 } __attribute__ ((packed)) write_class_of_dev_cp;
 #define WRITE_CLASS_OF_DEV_CP_SIZE 3
+
+#define OCF_READ_VOICE_SETTING	0x0025
+typedef struct {
+	__u8	status;
+	__u16	voice_setting;
+} __attribute__ ((packed)) read_voice_setting_rp;
+#define READ_VOICE_SETTING_RP_SIZE 3
+
+#define OCF_WRITE_VOICE_SETTING	0x0026
+typedef struct {
+	__u16	voice_setting;
+} __attribute__ ((packed)) write_voice_setting_cp;
+#define WRITE_VOICE_SETTING_CP_SIZE 2
 
 #define OCF_HOST_BUFFER_SIZE	0x0033
 typedef struct {
diff -urN linux-2.4.22-pre10/include/net/bluetooth/hci_core.h linux-2.4.22-pre10-notify/include/net/bluetooth/hci_core.h
--- linux-2.4.22-pre10/include/net/bluetooth/hci_core.h	Fri Jun 13 16:51:39 2003
+++ linux-2.4.22-pre10-notify/include/net/bluetooth/hci_core.h	Sat Aug  2 02:34:37 2003
@@ -68,11 +68,12 @@
 	__u8	 	type;
 	bdaddr_t	bdaddr;
 	__u8		features[8];
+	__u16		voice_setting;
 
 	__u16		pkt_type;
 	__u16		link_policy;
 	__u16		link_mode;
-	
+
 	atomic_t 	cmd_cnt;
 	unsigned int 	acl_cnt;
 	unsigned int 	sco_cnt;
@@ -116,6 +117,7 @@
 	int (*flush)(struct hci_dev *hdev);
 	int (*send)(struct sk_buff *skb);
 	void (*destruct)(struct hci_dev *hdev);
+	void (*notify)(struct hci_dev *hdev, unsigned int evt, unsigned long arg);
 	int (*ioctl)(struct hci_dev *hdev, unsigned int cmd, unsigned long arg);
 };
 
diff -urN linux-2.4.22-pre10/net/bluetooth/af_bluetooth.c linux-2.4.22-pre10-notify/net/bluetooth/af_bluetooth.c
--- linux-2.4.22-pre10/net/bluetooth/af_bluetooth.c	Sat Aug  2 02:33:57 2003
+++ linux-2.4.22-pre10-notify/net/bluetooth/af_bluetooth.c	Sat Aug  2 02:35:01 2003
@@ -27,7 +27,7 @@
  *
  * $Id: af_bluetooth.c,v 1.8 2002/07/22 20:32:54 maxk Exp $
  */
-#define VERSION "2.3"
+#define VERSION "2.4"
 
 #include <linux/config.h>
 #include <linux/module.h>
diff -urN linux-2.4.22-pre10/net/bluetooth/hci_conn.c linux-2.4.22-pre10-notify/net/bluetooth/hci_conn.c
--- linux-2.4.22-pre10/net/bluetooth/hci_conn.c	Fri Jun 13 16:51:39 2003
+++ linux-2.4.22-pre10-notify/net/bluetooth/hci_conn.c	Sat Aug  2 02:34:37 2003
@@ -168,6 +168,9 @@
 
 	hci_dev_hold(hdev);
 
+	if (hdev->notify)
+		hdev->notify(hdev, HCI_CONN_ADD, (unsigned long) conn);
+
 	tasklet_disable(&hdev->tx_task);
 	conn_hash_add(hdev, conn);
 	tasklet_enable(&hdev->tx_task);
@@ -203,6 +206,9 @@
 	tasklet_enable(&hdev->tx_task);
 
 	skb_queue_purge(&conn->data_q);
+
+	if (hdev->notify)
+		hdev->notify(hdev, HCI_CONN_DEL, (unsigned long) conn);
 
 	hci_dev_put(hdev);
 
diff -urN linux-2.4.22-pre10/net/bluetooth/hci_core.c linux-2.4.22-pre10-notify/net/bluetooth/hci_core.c
--- linux-2.4.22-pre10/net/bluetooth/hci_core.c	Sat Aug  2 02:33:57 2003
+++ linux-2.4.22-pre10-notify/net/bluetooth/hci_core.c	Sat Aug  2 02:34:37 2003
@@ -240,6 +240,9 @@
 	/* Read BD Address */
 	hci_send_cmd(hdev, OGF_INFO_PARAM, OCF_READ_BD_ADDR, 0, NULL);
 
+        /* Read Voice Setting */
+	hci_send_cmd(hdev, OGF_HOST_CTL, OCF_READ_VOICE_SETTING, 0, NULL);
+
 	/* Optional initialization */
 
 	/* Clear Event Filters */
diff -urN linux-2.4.22-pre10/net/bluetooth/hci_event.c linux-2.4.22-pre10-notify/net/bluetooth/hci_event.c
--- linux-2.4.22-pre10/net/bluetooth/hci_event.c	Sat Aug  2 02:33:57 2003
+++ linux-2.4.22-pre10-notify/net/bluetooth/hci_event.c	Sat Aug  2 02:34:37 2003
@@ -123,6 +123,8 @@
 static void hci_cc_host_ctl(struct hci_dev *hdev, __u16 ocf, struct sk_buff *skb)
 {
 	__u8 status, param;
+	__u16 setting;
+	read_voice_setting_rp *vs;
 	void *sent;
 
 	BT_DBG("%s ocf 0x%x", hdev->name, ocf);
@@ -198,6 +200,7 @@
 		sent = hci_sent_cmd_data(hdev, OGF_HOST_CTL, OCF_WRITE_SCAN_ENABLE);
 		if (!sent)
 			break;
+
 		status = *((__u8 *) skb->data);
 		param  = *((__u8 *) sent);
 
@@ -215,6 +218,46 @@
 		hci_req_complete(hdev, status);
 		break;
 
+	case OCF_READ_VOICE_SETTING:
+		vs = (read_voice_setting_rp *) skb->data;
+
+		if (vs->status) { 
+			BT_DBG("%s READ_VOICE_SETTING failed %d", hdev->name, vc->status);
+			break;
+		}
+
+		setting = __le16_to_cpu(vs->voice_setting);
+
+		if (hdev->voice_setting != setting ) {
+			hdev->voice_setting = setting;
+
+			BT_DBG("%s: voice setting 0x%04x", hdev->name, setting);
+
+			if (hdev->notify)
+				hdev->notify(hdev, HCI_VOICE_SETTING, 0);
+		}
+
+		break;
+
+	case OCF_WRITE_VOICE_SETTING:
+		sent = hci_sent_cmd_data(hdev, OGF_HOST_CTL, OCF_WRITE_VOICE_SETTING);
+		if (!sent)
+			break;
+
+		status = *((__u8 *) skb->data);
+		setting = __le16_to_cpu(get_unaligned((__u16 *) sent));
+
+		if (!status && hdev->voice_setting != setting) {
+			hdev->voice_setting = setting;
+
+			BT_DBG("%s: voice setting 0x%04x", hdev->name, setting);
+
+			if (hdev->notify)
+				hdev->notify(hdev, HCI_VOICE_SETTING, 0);
+		}
+		hci_req_complete(hdev, status);
+		break;
+
 	case OCF_HOST_BUFFER_SIZE:
 		status = *((__u8 *) skb->data);
 		if (status) {
@@ -282,7 +325,8 @@
 		hdev->sco_pkts = hdev->sco_cnt = __le16_to_cpu(bs->sco_max_pkt);
 
 		BT_DBG("%s mtu: acl %d, sco %d max_pkt: acl %d, sco %d", hdev->name,
-		    hdev->acl_mtu, hdev->sco_mtu, hdev->acl_pkts, hdev->sco_pkts);
+			hdev->acl_mtu, hdev->sco_mtu, hdev->acl_pkts, hdev->sco_pkts);
+
 		break;
 
 	case OCF_READ_BD_ADDR:

[-- Attachment #3: patch-2.4.22-pre10-hci-usb --]
[-- Type: text/x-patch, Size: 3965 bytes --]

diff -urN linux-2.4.22-pre10/drivers/bluetooth/hci_usb.c linux-2.4.22-pre10-usb/drivers/bluetooth/hci_usb.c
--- linux-2.4.22-pre10/drivers/bluetooth/hci_usb.c	Sat Aug  2 02:33:45 2003
+++ linux-2.4.22-pre10-usb/drivers/bluetooth/hci_usb.c	Sat Aug  2 02:54:11 2003
@@ -30,7 +30,7 @@
  *
  * $Id: hci_usb.c,v 1.8 2002/07/18 17:23:09 maxk Exp $    
  */
-#define VERSION "2.4"
+#define VERSION "2.5"
 
 #include <linux/config.h>
 #include <linux/module.h>
@@ -302,7 +302,8 @@
 
 #ifdef CONFIG_BLUEZ_USB_SCO
 		if (husb->isoc_iface)
-			hci_usb_isoc_rx_submit(husb);
+			for (i = 0; i < HCI_MAX_ISOC_RX; i++)
+				hci_usb_isoc_rx_submit(husb);
 #endif
 	} else {
 		clear_bit(HCI_RUNNING, &hdev->flags);
@@ -522,7 +523,7 @@
 #ifdef CONFIG_BLUEZ_USB_SCO
 		/* Process SCO queue */
 		q = __transmit_q(husb, HCI_SCODATA_PKT);
-		if (!atomic_read(__pending_tx(husb, HCI_SCODATA_PKT)) &&
+		if (atomic_read(__pending_tx(husb, HCI_SCODATA_PKT)) < HCI_MAX_ISOC_TX &&
 				(skb = skb_dequeue(q))) {
 			if (hci_usb_send_isoc(husb, skb) < 0)
 				skb_queue_head(q, skb);
@@ -769,6 +770,44 @@
 	kfree(husb);
 }
 
+static void hci_usb_notify(struct hci_dev *hdev, unsigned int evt, unsigned long arg)
+{
+	struct hci_usb *husb = (struct hci_usb *) hdev->driver_data;
+	struct hci_conn *conn;
+
+	BT_DBG("%s cmd %d arg %ld", hdev->name, evt, arg);
+
+	switch (evt) {
+	case HCI_CONN_ADD:
+		conn = (struct hci_conn *) arg;
+
+		BT_DBG("%s add conn type %d", hdev->name, conn->type);
+
+		if (conn->type == ACL_LINK)
+			atomic_inc(&husb->acl_num_conn);
+		else
+			atomic_inc(&husb->sco_num_conn);
+
+		break;
+
+	case HCI_CONN_DEL:
+		conn = (struct hci_conn *) arg;
+
+		BT_DBG("%s del conn type %d", hdev->name, conn->type);
+
+		if (conn->type == ACL_LINK)
+			atomic_dec(&husb->acl_num_conn);
+		else
+			atomic_dec(&husb->sco_num_conn);
+
+		break;
+
+	case HCI_VOICE_SETTING:
+		BT_DBG("%s voice setting 0x%04x", hdev->name, hdev->voice_setting);
+		break;
+	}
+}
+
 static void *hci_usb_probe(struct usb_device *udev, unsigned int ifnum, const struct usb_device_id *id)
 {
 	struct usb_endpoint_descriptor *bulk_out_ep[HCI_MAX_IFACE_NUM];
@@ -901,11 +940,12 @@
 	hdev->type  = HCI_USB;
 	hdev->driver_data = husb;
 
-	hdev->open  = hci_usb_open;
-	hdev->close = hci_usb_close;
-	hdev->flush = hci_usb_flush;
-	hdev->send  = hci_usb_send_frame;
+	hdev->open     = hci_usb_open;
+	hdev->close    = hci_usb_close;
+	hdev->flush    = hci_usb_flush;
+	hdev->send     = hci_usb_send_frame;
 	hdev->destruct = hci_usb_destruct;
+	hdev->notify   = hci_usb_notify;
 
 	if (hci_register_dev(hdev) < 0) {
 		BT_ERR("Can't register HCI device");
diff -urN linux-2.4.22-pre10/drivers/bluetooth/hci_usb.h linux-2.4.22-pre10-usb/drivers/bluetooth/hci_usb.h
--- linux-2.4.22-pre10/drivers/bluetooth/hci_usb.h	Fri Jun 13 16:51:32 2003
+++ linux-2.4.22-pre10-usb/drivers/bluetooth/hci_usb.h	Sat Aug  2 02:41:19 2003
@@ -41,6 +41,9 @@
 #define HCI_MAX_BULK_TX     	4
 #define HCI_MAX_BULK_RX     	1
 
+#define HCI_MAX_ISOC_RX		2
+#define HCI_MAX_ISOC_TX		2
+
 #define HCI_MAX_ISOC_FRAMES     10
 
 struct _urb_queue {
@@ -120,13 +123,16 @@
 	struct usb_endpoint_descriptor	*isoc_in_ep;
 
 	struct sk_buff_head	transmit_q[4];
-	struct sk_buff		*reassembly[4]; // Reassembly buffers
+	struct sk_buff		*reassembly[4];	/* Reassembly buffers */
 
 	rwlock_t		completion_lock;
 
-	atomic_t		pending_tx[4];  // Number of pending requests 
-	struct _urb_queue	pending_q[4];   // Pending requests
-	struct _urb_queue	completed_q[4]; // Completed requests
+	atomic_t		pending_tx[4];  /* Number of pending requests */
+	struct _urb_queue	pending_q[4];	/* Pending requests */
+	struct _urb_queue	completed_q[4];	/* Completed requests */
+
+	atomic_t		acl_num_conn;	/* Number of ACL connections */
+	atomic_t		sco_num_conn;	/* Number of SCO connections */
 };
 
 /* States  */

Re: [Bluez-devel] HCI USB driver and SCO support

[James Courtier-Dutton]

Marcel Holtmann wrote:
> Hi Max,
> 
> the patch from Jonathan Paisley is working fine. I have tested it on
> 2.4.18 to 2.4.21 and we can safely use two ISOC TX and RX URB's by
> default. But the configuration of the ISOC endpoint must be adjusted on
> demand. It depends on the voice setting and number of SCO connections,
> otherwise you won't hear any sound. At the moment we don't have access
> to these information from within the driver, so I started to extend the
> HCI core.
> 
> The first patch introduces hdev->notify and reads the voice setting on
> device init and stores them in hdev->voice_setting for later use. With
> these modification a driver can be notified about added or deleted
> connections (ACL + SCO) and if the voice setting was changed.
> 
> The second patch is for the HCI USB driver and it shows how to use
> hdev->notify. It also contains Jonathans modifications to use two ISOC
> TX and RX URB's and it keeps track of the number of ACL and SCO
> connections.
> 
> In the next step, we must choose the correct ISOC configuration. The
> default one should be 0, because we don't use any SCO channel. It is
> also a good idea to start the ISOC and BULK transfers only if they are
> needed.
> 
> Comments?
> 
> Regards
> 
> Marcel
> 
If we are going to use the SCO channels for sound output from 
applications, it would be very useful if we could provide some sort of 
ring buffer support, with hardware pointer updates.
Currently, there are no callbacks from sco to the application layer, 
telling it how many sound samples have actually left the PC for the 
Bluetooth device. This is a requirement if we are to support any sound 
card simulation layer above the SCO layer.
Until this is resolved, we will not be able to implement alsa or oss 
support.

Next point:
The current method for allocation of the URB is incompatible with kernel 
2.6.
bluez currently allocates it's own urb as a structure inside _urb.
In 2.6, the requirement is to get the usb subsystem to allocate the urb, 
  and then set the urb->context pointer to point to any bluez specific 
state information.
If the urb allocation is adjusted, it will then work on both 2.4 and 2.6

Cheers
James


Cheers
James

Re: [Bluez-devel] HCI USB driver and SCO support

[Marcel Holtmann]

Hi James,

> If we are going to use the SCO channels for sound output from 
> applications, it would be very useful if we could provide some sort of 
> ring buffer support, with hardware pointer updates.
> Currently, there are no callbacks from sco to the application layer, 
> telling it how many sound samples have actually left the PC for the 
> Bluetooth device. This is a requirement if we are to support any sound 
> card simulation layer above the SCO layer.
> Until this is resolved, we will not be able to implement alsa or oss 
> support.

I don't care about this at the moment, because the current problem is
the hci_usb driver. We need to adjust the alternate setting for the ISOC
interface on demand. And this setting depends on the voice setting (8 or
16 bit) and the number of SCO connections. If the used alternate setting
is not correct, you don't get any correct audio data over the SCO link.

> The current method for allocation of the URB is incompatible with kernel 
> 2.6.
> bluez currently allocates it's own urb as a structure inside _urb.
> In 2.6, the requirement is to get the usb subsystem to allocate the urb, 
>   and then set the urb->context pointer to point to any bluez specific 
> state information.
> If the urb allocation is adjusted, it will then work on both 2.4 and 2.6

There was a discussion between Max and Greg about this topic on the
Linux USB mailing list. You should read this first.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] HCI USB driver and SCO support

[Max Krasnyansky]

At 02:35 AM 8/4/2003, Marcel Holtmann wrote:
>Hi James,
>
>> If we are going to use the SCO channels for sound output from 
>> applications, it would be very useful if we could provide some sort of 
>> ring buffer support, with hardware pointer updates.
>> Currently, there are no callbacks from sco to the application layer, 
>> telling it how many sound samples have actually left the PC for the 
>> Bluetooth device. This is a requirement if we are to support any sound 
>> card simulation layer above the SCO layer.
>> Until this is resolved, we will not be able to implement alsa or oss 
>> support.
>
>I don't care about this at the moment, because the current problem is
>the hci_usb driver. We need to adjust the alternate setting for the ISOC
>interface on demand. And this setting depends on the voice setting (8 or
>16 bit) and the number of SCO connections. If the used alternate setting
>is not correct, you don't get any correct audio data over the SCO link.
Yep. I agree. Let's fix the driver/core interaction first.

>> The current method for allocation of the URB is incompatible with kernel 
>> 2.6.
>> bluez currently allocates it's own urb as a structure inside _urb.
>> In 2.6, the requirement is to get the usb subsystem to allocate the urb, 
>>   and then set the urb->context pointer to point to any bluez specific 
>> state information.
>> If the urb allocation is adjusted, it will then work on both 2.4 and 2.6
>
>There was a discussion between Max and Greg about this topic on the
>Linux USB mailing list. You should read this first.
Yeah, I was going to add a few fields to 'struct urb' :).

Max

Re: [Bluez-devel] HCI USB driver and SCO support

[James Courtier-Dutton]

Max Krasnyansky wrote:
> At 02:35 AM 8/4/2003, Marcel Holtmann wrote:
> 
>>Hi James,
>>
>>
>>>If we are going to use the SCO channels for sound output from 
>>>applications, it would be very useful if we could provide some sort of 
>>>ring buffer support, with hardware pointer updates.
>>>Currently, there are no callbacks from sco to the application layer, 
>>>telling it how many sound samples have actually left the PC for the 
>>>Bluetooth device. This is a requirement if we are to support any sound 
>>>card simulation layer above the SCO layer.
>>>Until this is resolved, we will not be able to implement alsa or oss 
>>>support.
>>
>>I don't care about this at the moment, because the current problem is
>>the hci_usb driver. We need to adjust the alternate setting for the ISOC
>>interface on demand. And this setting depends on the voice setting (8 or
>>16 bit) and the number of SCO connections. If the used alternate setting
>>is not correct, you don't get any correct audio data over the SCO link.
> 
> Yep. I agree. Let's fix the driver/core interaction first.
> 
> 

I would like to help with bluez developement, but it seems that bluez 
developement is not done on kernel 2.6, so I cannot help.
All kernel usb development is done on kernel 2.6, so why developement of 
the bluez hci_usb driver is not also done on kernel 2.6 is a mystery to me.

Re: [Bluez-devel] HCI USB driver and SCO support

[Max Krasnyansky]

At 06:20 PM 8/1/2003, Marcel Holtmann wrote:
>Hi Max,
>
>the patch from Jonathan Paisley is working fine. I have tested it on
>2.4.18 to 2.4.21 and we can safely use two ISOC TX and RX URB's by
>default. 
Yep, I was going to apply his patch.

>But the configuration of the ISOC endpoint must be adjusted on
>demand. It depends on the voice setting and number of SCO connections,
>otherwise you won't hear any sound. At the moment we don't have access
>to these information from within the driver, so I started to extend the
>HCI core.
>
>The first patch introduces hdev->notify and reads the voice setting on
>device init and stores them in hdev->voice_setting for later use. With
>these modification a driver can be notified about added or deleted
>connections (ACL + SCO) and if the voice setting was changed.
I like the idea in general.

HCI_CONN_ADD should be something like HCI_NOTIFY_CONN_ADD.

>The second patch is for the HCI USB driver and it shows how to use
>hdev->notify. It also contains Jonathans modifications to use two ISOC
>TX and RX URB's and it keeps track of the number of ACL and SCO
>connections.
There is no need to count number of connections in the drivert. Core already 
has that counter (conn_hash.num). We just need to extend it to count ACL and 
SCO separately.

>In the next step, we must choose the correct ISOC configuration. The
>default one should be 0, because we don't use any SCO channel. It is
>also a good idea to start the ISOC and BULK transfers only if they are
>needed.
I think that's the way to go. Driver shouldn't start any BULK or ISOC transfers 
until core tells it so.

btw This stuff should go into 2.6 and not 2.4. 

Max

Re: [Bluez-devel] HCI USB driver and SCO support

[Marcel Holtmann]

Hi Max,

> >the patch from Jonathan Paisley is working fine. I have tested it on
> >2.4.18 to 2.4.21 and we can safely use two ISOC TX and RX URB's by
> >default. 
> Yep, I was going to apply his patch.

but without choosing the correct ISOC alternate setting it is useless :(

> >But the configuration of the ISOC endpoint must be adjusted on
> >demand. It depends on the voice setting and number of SCO connections,
> >otherwise you won't hear any sound. At the moment we don't have access
> >to these information from within the driver, so I started to extend the
> >HCI core.
> >
> >The first patch introduces hdev->notify and reads the voice setting on
> >device init and stores them in hdev->voice_setting for later use. With
> >these modification a driver can be notified about added or deleted
> >connections (ACL + SCO) and if the voice setting was changed.
> I like the idea in general.
> 
> HCI_CONN_ADD should be something like HCI_NOTIFY_CONN_ADD.

I will change this and push it to my Bitkeeper repository.

> >The second patch is for the HCI USB driver and it shows how to use
> >hdev->notify. It also contains Jonathans modifications to use two ISOC
> >TX and RX URB's and it keeps track of the number of ACL and SCO
> >connections.
> There is no need to count number of connections in the drivert. Core already 
> has that counter (conn_hash.num). We just need to extend it to count ACL and 
> SCO separately.

Is it worth to change this? The only driver who needs it at the moment
will be hci_usb.o and it can count the stuff by itself.

> >In the next step, we must choose the correct ISOC configuration. The
> >default one should be 0, because we don't use any SCO channel. It is
> >also a good idea to start the ISOC and BULK transfers only if they are
> >needed.
> I think that's the way to go. Driver shouldn't start any BULK or ISOC transfers 
> until core tells it so.

I have some successful results with starting and stopping ISOC URB's,
but this code was not based on the hdev->notify architecture.

> btw This stuff should go into 2.6 and not 2.4. 

At the moment I switched my development machine back to 2.4, because the
2.6 was out of date (hint) and I have to do the next round of my -mh
patches.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] HCI USB driver and SCO support

[James Courtier-Dutton]

Marcel Holtmann wrote:
> Hi Max,
> 
> 
>>>the patch from Jonathan Paisley is working fine. I have tested it on
>>>2.4.18 to 2.4.21 and we can safely use two ISOC TX and RX URB's by
>>>default. 
>>
>>Yep, I was going to apply his patch.
> 
> 
> but without choosing the correct ISOC alternate setting it is useless :(

I think the "ISOC alternate setting" are USB specific, so I think work 
should be contained in hci_usb, and not require core modifications.
To this end, I think it would be helpful to separate INT and BULK 
traffic from SCO traffic.
SCO is realtime, INT and BULK are not, so they require different buffer 
handling.
To this end, INT and BULK work well with queues, and SCO are better 
suited to ring buffers.

Re: [Bluez-devel] HCI USB driver and SCO support

[Marcel Holtmann]

Hi James,

> > but without choosing the correct ISOC alternate setting it is useless :(
> 
> I think the "ISOC alternate setting" are USB specific, so I think work 
> should be contained in hci_usb, and not require core modifications.

you basicly have two options

	1) Parse events and commands inside the driver
	2) Let the core send a notify event to the driver

I already have done 1) for the number of SCO connections. It is very
easy and it didn't blow-up the hci_usb code very much. After getting
dynamic starting and stoping of ISOC URB's working I realized that all
this won't help, if we don't adjust the ISOC alternate setting on
demand. As I already said, the ISOC alternate setting depends on the
number of SCO connections and the current voice setting (8 or 16 bit).
So I have to parse the read_voice_setting and write_voice_setting
commands and results. If you try this by yourself you will see, that
this is not a job, which should be done in the driver. Keep the driver
quite stupid and let it be what it is meant to be - a host transport
driver.

And after knowing all this, the only way that makes sense at the moment
is 2). Look at my patch and you will see that it is a clean extension to
the core, because it only notify the driver about special events. What
to do with this information is up to the driver.

> To this end, I think it would be helpful to separate INT and BULK 
> traffic from SCO traffic.
> SCO is realtime, INT and BULK are not, so they require different buffer 
> handling.
> To this end, INT and BULK work well with queues, and SCO are better 
> suited to ring buffers.

This is not quite correct. The hci_usb driver and the HCI core have to
handle SCO traffic, but they don't have to know much about it. It is a
packet type which have to be send and received. Nothing more, nothing
less. The hci_usb driver uses ISOC endpoints to transfer SCO packets and
BULK endpoints to transfer ACL packets. All other buffering which is
needing have to be done in the sco module.

May I have to repeat myself - let us first finish the hci_usb driver
with full SCO support. Max has agreed on my core change and I will push
it for 2.4 and 2.5/2.6, so the hci_usb driver can be fixed very easy.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] HCI USB driver and SCO support

[James Courtier-Dutton]

Marcel Holtmann wrote:
> Hi James,
> 
> 
>>>but without choosing the correct ISOC alternate setting it is useless :(
>>
>>I think the "ISOC alternate setting" are USB specific, so I think work 
>>should be contained in hci_usb, and not require core modifications.
> 
> 
> you basicly have two options
> 
> 	1) Parse events and commands inside the driver
> 	2) Let the core send a notify event to the driver
> 
> I already have done 1) for the number of SCO connections. It is very
> easy and it didn't blow-up the hci_usb code very much. After getting
> dynamic starting and stoping of ISOC URB's working I realized that all
> this won't help, if we don't adjust the ISOC alternate setting on
> demand. As I already said, the ISOC alternate setting depends on the
> number of SCO connections and the current voice setting (8 or 16 bit).
> So I have to parse the read_voice_setting and write_voice_setting
> commands and results. If you try this by yourself you will see, that
> this is not a job, which should be done in the driver. Keep the driver
> quite stupid and let it be what it is meant to be - a host transport
> driver.
> 
> And after knowing all this, the only way that makes sense at the moment
> is 2). Look at my patch and you will see that it is a clean extension to
> the core, because it only notify the driver about special events. What
> to do with this information is up to the driver.

I thought of this just after I sent the email. Sorry to trouble your on 
that point.

> 
> 
>>To this end, I think it would be helpful to separate INT and BULK 
>>traffic from SCO traffic.
>>SCO is realtime, INT and BULK are not, so they require different buffer 
>>handling.
>>To this end, INT and BULK work well with queues, and SCO are better 
>>suited to ring buffers.
> 
> 
> This is not quite correct. The hci_usb driver and the HCI core have to
> handle SCO traffic, but they don't have to know much about it. It is a
> packet type which have to be send and received. Nothing more, nothing
> less. The hci_usb driver uses ISOC endpoints to transfer SCO packets and
> BULK endpoints to transfer ACL packets. All other buffering which is
> needing have to be done in the sco module.
> 
> May I have to repeat myself - let us first finish the hci_usb driver
> with full SCO support. Max has agreed on my core change and I will push
> it for 2.4 and 2.5/2.6, so the hci_usb driver can be fixed very easy.
> 
> Regards
> 
> Marcel
> 
> 

I think you will find out later that SCO traffic needs a bit more than 
just packet send/receive. But I guess that I can add those features once 
you have fixed your stuff in hci_usb on kernel 2.6.

Cheers
James

Re: [Bluez-devel] HCI USB driver and SCO support

[Marcel Holtmann]

Hi James,

> >>To this end, I think it would be helpful to separate INT and BULK 
> >>traffic from SCO traffic.
> >>SCO is realtime, INT and BULK are not, so they require different buffer 
> >>handling.
> >>To this end, INT and BULK work well with queues, and SCO are better 
> >>suited to ring buffers.
> > 
> > 
> > This is not quite correct. The hci_usb driver and the HCI core have to
> > handle SCO traffic, but they don't have to know much about it. It is a
> > packet type which have to be send and received. Nothing more, nothing
> > less. The hci_usb driver uses ISOC endpoints to transfer SCO packets and
> > BULK endpoints to transfer ACL packets. All other buffering which is
> > needing have to be done in the sco module.
> > 
> > May I have to repeat myself - let us first finish the hci_usb driver
> > with full SCO support. Max has agreed on my core change and I will push
> > it for 2.4 and 2.5/2.6, so the hci_usb driver can be fixed very easy.
> 
> I think you will find out later that SCO traffic needs a bit more than 
> just packet send/receive. But I guess that I can add those features once 
> you have fixed your stuff in hci_usb on kernel 2.6.

I already know that SCO traffic needs a bit more than that, but not in
the HCI host drivers. Maybe we have to modify the HCI core for better
SCO support, but there is no need to push SCO buffering or anything else
into the driver. The driver is not meant for any other logic than
transporting HCI packets from the hardware to the HCI core and vice
versa. All drivers uses some kind of queuing, but this should not be a
problem, because it is only for serializing the HCI packets, before they
are send to the hardware.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel