2003-08-02 01:20:45

by Marcel Holtmann

[permalink] [raw]
Subject: [Bluez-devel] HCI USB driver and SCO support

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


Attachments:
patch-2.4.22-pre10-hci-notify (5.57 kB)
patch-2.4.22-pre10-hci-usb (3.75 kB)
Download all attachments

2003-08-06 11:07:21

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2003-08-06 10:23:34

by James Courtier-Dutton

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

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

2003-08-06 08:41:46

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2003-08-05 23:02:59

by James Courtier-Dutton

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

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.

2003-08-05 22:56:27

by James Courtier-Dutton

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

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.

2003-08-05 22:17:14

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2003-08-05 17:44:31

by Max Krasnyansky

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

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

2003-08-05 17:48:47

by Max Krasnyansky

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

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

2003-08-04 09:35:11

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2003-08-02 09:19:03

by James Courtier-Dutton

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

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