Re: Bluetooth update for 2.4.23-pre2

Re: Bluetooth update for 2.4.23-pre2

[Max Krasnyansky]

At 09:14 AM 9/1/2003, Marcel Holtmann wrote:
>here are the outstanding Bluetooth updates for 2.4.23-pre. All of them
>are well tested in my -mh patches and I don't expect any other big
>changes for the 2.4 series.

Ok. I got problem with the following Changesets.

><marcel@holtmann.org> (03/08/21 1.1067)
>   [Bluetooth] Add notify() callback for host drivers
>   
>   This patch adds a notification callback to the hci_dev structure which
>   is used by the HCI core to tell the driver about connection creation
>   and clearing. It also notifies about changed voice setting. To assure
>   that the core always knows the correct voice setting, it is read at
>   device initialization and stored in the hci_dev structure.

--- 1.5/net/bluetooth/hci_conn.c        Fri Mar 21 04:26:29 2003
+++ 1.6/net/bluetooth/hci_conn.c        Thu Aug 21 06:11:42 2003
@@ -168,6 +168,9 @@
 
        hci_dev_hold(hdev);
 
+       if (hdev->notify)
+               hdev->notify(hdev, HCI_NOTIFY_CONN_ADD, (unsigned long) conn);
+

I believe I mentioned that before. Above patch basically means that driver has to keep 
its own counter of ACL and SCO connection (ie check conn->type and stuff). That does 
not make sense to me. Core already has that information. It's part of hci_conn_hash. 
Currently we single counter. So we probably need to split it. And if we do it that way 
there is no need for the third argument to hdev->notify().

><marcel@holtmann.org> (03/07/31 1.1019.1.7)
>   [Bluetooth] Set initial value of RFCOMM credits to zero
>   
>   The initial credits value must be zero, because this is default
>   for Bluetooth 1.0b without credit based flow control. Once the
>   other side signals us that it supports credit based flow control
>   we set the session wide credits value.

@@ -746,7 +746,7 @@
        pn->ack_timer   = 0;
        pn->max_retrans = 0;
 
-       if (d->credits) {
+       if (cr || d->credits) {
                pn->flow_ctrl = cr ? 0xf0 : 0xe0;
                pn->credits = RFCOMM_DEFAULT_CREDITS;
        } else {

We talked about this one too. With that change we're going to ask
for credits every time we send PN request even if other side already told 
us that they don't support CFC. This is not right. We need to negotiate CFC 
only once.

Max

[Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Marcel Holtmann]

Hi Max,

> Ok. I got problem with the following Changesets.
> 
> ><marcel@holtmann.org> (03/08/21 1.1067)
> >   [Bluetooth] Add notify() callback for host drivers
> >   
> >   This patch adds a notification callback to the hci_dev structure which
> >   is used by the HCI core to tell the driver about connection creation
> >   and clearing. It also notifies about changed voice setting. To assure
> >   that the core always knows the correct voice setting, it is read at
> >   device initialization and stored in the hci_dev structure.
> 
> --- 1.5/net/bluetooth/hci_conn.c        Fri Mar 21 04:26:29 2003
> +++ 1.6/net/bluetooth/hci_conn.c        Thu Aug 21 06:11:42 2003
> @@ -168,6 +168,9 @@
>  
>         hci_dev_hold(hdev);
>  
> +       if (hdev->notify)
> +               hdev->notify(hdev, HCI_NOTIFY_CONN_ADD, (unsigned long) conn);
> +
> 
> I believe I mentioned that before. Above patch basically means that driver has to keep 
> its own counter of ACL and SCO connection (ie check conn->type and stuff). That does 
> not make sense to me. Core already has that information. It's part of hci_conn_hash. 
> Currently we single counter. So we probably need to split it. And if we do it that way 
> there is no need for the third argument to hdev->notify().

you mention that, but I only thought of it as some kind of additional
modification so we don't have to count the number of ACL and SCO in the
driver itself.

I want to keep the the third parameter to be save in future if we need
to send other notifications down to the HCI driver for which we can use
it eventually. Bluetooth 1.2 and 2.0 will come with new stuff that may
need it.

For the CONN_ADD and CONN_DEL I think we should also send the "conn"
down to the driver so it can read from it if needed. If not, it has to
go by itself through the hash and I don't see any way that the driver
can find out the current added connection. Am I wrong?

> ><marcel@holtmann.org> (03/07/31 1.1019.1.7)
> >   [Bluetooth] Set initial value of RFCOMM credits to zero
> >   
> >   The initial credits value must be zero, because this is default
> >   for Bluetooth 1.0b without credit based flow control. Once the
> >   other side signals us that it supports credit based flow control
> >   we set the session wide credits value.
> 
> @@ -746,7 +746,7 @@
>         pn->ack_timer   = 0;
>         pn->max_retrans = 0;
>  
> -       if (d->credits) {
> +       if (cr || d->credits) {
>                 pn->flow_ctrl = cr ? 0xf0 : 0xe0;
>                 pn->credits = RFCOMM_DEFAULT_CREDITS;
>         } else {
> 
> We talked about this one too. With that change we're going to ask
> for credits every time we send PN request even if other side already told 
> us that they don't support CFC. This is not right. We need to negotiate CFC 
> only once.

Yes, you told me that. And I put a note on my TODO list that this needs
further attention. But your suggestions was to change the credits value
from uint to int and don't find the time to check if a type change may
not causes other problems. And as I also said a new flag may be a better
solution.

Anyway I took the patch in, because it fixes the problem and whithout we
have a bug with incoming 1.0b connections. The double negotiation only
takes place on the second outgoing connections to the same 1.0b device.
And how much 1.0b devices do you have or do you know of? My only 1.0b
device is a Ericsson T39m and even my old Nokia 6210 already supports
CFC. And my HBH-10 don't count, because it support only one RFCOMM
connection ;)

Regards

Marcel




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: Bluetooth update for 2.4.23-pre2

[Max Krasnyansky]

At 03:48 PM 9/12/2003, Marcel Holtmann wrote:
>> --- 1.5/net/bluetooth/hci_conn.c        Fri Mar 21 04:26:29 2003
>> +++ 1.6/net/bluetooth/hci_conn.c        Thu Aug 21 06:11:42 2003
>> @@ -168,6 +168,9 @@
>>  
>>         hci_dev_hold(hdev);
>>  
>> +       if (hdev->notify)
>> +               hdev->notify(hdev, HCI_NOTIFY_CONN_ADD, (unsigned long) conn);
>> +
>> 
>> I believe I mentioned that before. Above patch basically means that driver has to keep 
>> its own counter of ACL and SCO connection (ie check conn->type and stuff). That does 
>> not make sense to me. Core already has that information. It's part of hci_conn_hash. 
>> Currently we single counter. So we probably need to split it. And if we do it that way 
>> there is no need for the third argument to hdev->notify().
>
>you mention that, but I only thought of it as some kind of additional
>modification so we don't have to count the number of ACL and SCO in the
>driver itself.
>
>I want to keep the the third parameter to be save in future if we need
>to send other notifications down to the HCI driver for which we can use
>it eventually. Bluetooth 1.2 and 2.0 will come with new stuff that may
>need it.
Unlikely. In any case we can add it later of needed.

>For the CONN_ADD and CONN_DEL I think we should also send the "conn"
>down to the driver so it can read from it if needed.
But the only thing useful for the driver is the connection type. We might as 
well just pass that instead of a pointer to the whole struct. Anyway core should
count connections not the driver.

>If not, it has to go by itself through the hash and I don't see any way that the 
>driver can find out the current added connection. Am I wrong?
conn_hash has 'num' field which is total number of connections. So we just need to
keep two separate counters for ACL and SCO.
I'll try to find some time this week to implement that.

>> ><marcel@holtmann.org> (03/07/31 1.1019.1.7)
>> >   [Bluetooth] Set initial value of RFCOMM credits to zero
>> >   
>> We talked about this one too. With that change we're going to ask
>> for credits every time we send PN request even if other side already told 
>> us that they don't support CFC. This is not right. We need to negotiate CFC 
>> only once.
>
>Yes, you told me that. And I put a note on my TODO list that this needs
>further attention. But your suggestions was to change the credits value
>from uint to int and don't find the time to check if a type change may
>not causes other problems. And as I also said a new flag may be a better
>solution.
>Anyway I took the patch in, because it fixes the problem and whithout we
>have a bug with incoming 1.0b connections. The double negotiation only
>takes place on the second outgoing connections to the same 1.0b device.
>And how much 1.0b devices do you have or do you know of? My only 1.0b
>device is a Ericsson T39m and even my old Nokia 6210 already supports
>CFC. And my HBH-10 don't count, because it support only one RFCOMM
>connection ;)
What I'm saying is that we should fix the write way not just some hack that 
fixes first negotiation. It will pass certification with Ceticomm (or whatever 
the name of that company) but will fail if tester code does proper checking ie second 
connection, etc.
It should be a trivial fix. I take a look at it tonight.

Max

[Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Marcel Holtmann]

Hi Max,

> >I want to keep the the third parameter to be save in future if we need
> >to send other notifications down to the HCI driver for which we can use
> >it eventually. Bluetooth 1.2 and 2.0 will come with new stuff that may
> >need it.
> Unlikely. In any case we can add it later of needed.
> 
> >For the CONN_ADD and CONN_DEL I think we should also send the "conn"
> >down to the driver so it can read from it if needed.
> But the only thing useful for the driver is the connection type. We might as 
> well just pass that instead of a pointer to the whole struct. Anyway core should
> count connections not the driver.
> 
> >If not, it has to go by itself through the hash and I don't see any way that the 
> >driver can find out the current added connection. Am I wrong?
> conn_hash has 'num' field which is total number of connections. So we just need to
> keep two separate counters for ACL and SCO.
> I'll try to find some time this week to implement that.

I see the notify() interface and the seperate counting of ACL and SCO as
two different problems and I don't wanna mix them.

However I think it is nice to send the "conn" down to the driver, if you
thing it is two much, I drop it.

> >Yes, you told me that. And I put a note on my TODO list that this needs
> >further attention. But your suggestions was to change the credits value
> >from uint to int and don't find the time to check if a type change may
> >not causes other problems. And as I also said a new flag may be a better
> >solution.
> >Anyway I took the patch in, because it fixes the problem and whithout we
> >have a bug with incoming 1.0b connections. The double negotiation only
> >takes place on the second outgoing connections to the same 1.0b device.
> >And how much 1.0b devices do you have or do you know of? My only 1.0b
> >device is a Ericsson T39m and even my old Nokia 6210 already supports
> >CFC. And my HBH-10 don't count, because it support only one RFCOMM
> >connection ;)
> What I'm saying is that we should fix the write way not just some hack that 
> fixes first negotiation. It will pass certification with Ceticomm (or whatever 
> the name of that company) but will fail if tester code does proper checking ie second 
> connection, etc.
> It should be a trivial fix. I take a look at it tonight.

I looked at your fix and it looks fine to me. Can you please rebuild my
marcel-2.4 tree on the base of your current work? I will re-push my
other patches so we can sync up with Marcelo until the end of this week.
I want to finish this as soon as possible, because I am away for the
complete next week.

Regards

Marcel




-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Max Krasnyansky]

At 09:13 AM 10/9/2003, Marcel Holtmann wrote:
>Hi Max,
>
>> >I want to keep the the third parameter to be save in future if we need
>> >to send other notifications down to the HCI driver for which we can use
>> >it eventually. Bluetooth 1.2 and 2.0 will come with new stuff that may
>> >need it.
>> Unlikely. In any case we can add it later of needed.
>> 
>> >For the CONN_ADD and CONN_DEL I think we should also send the "conn"
>> >down to the driver so it can read from it if needed.
>> But the only thing useful for the driver is the connection type. We might as 
>> well just pass that instead of a pointer to the whole struct. Anyway core should
>> count connections not the driver.
>> 
>> >If not, it has to go by itself through the hash and I don't see any way that the 
>> >driver can find out the current added connection. Am I wrong?
>> conn_hash has 'num' field which is total number of connections. So we just need to
>> keep two separate counters for ACL and SCO.
>> I'll try to find some time this week to implement that.
>
>I see the notify() interface and the seperate counting of ACL and SCO as
>two different problems and I don't wanna mix them.
>However I think it is nice to send the "conn" down to the driver, if you
>thing it is two much, I drop it.
Well, both are needed only for SCO over USB.
So in some sense they are related :).

btw How about fixing it in 2.6 and leaving 2.4 alone ?
I don't think we will ever have decent Voice support in 2.4 anyway.
For example ALSA driver will definitely go into 2.6 not 2.4.

>> What I'm saying is that we should fix the write way not just some hack that 
>> fixes first negotiation. It will pass certification with Ceticomm (or whatever 
>> the name of that company) but will fail if tester code does proper checking ie second 
>> connection, etc.
>> It should be a trivial fix. I take a look at it tonight.
>
>I looked at your fix and it looks fine to me. Can you please rebuild my
>marcel-2.4 tree on the base of your current work? I will re-push my
>other patches so we can sync up with Marcelo until the end of this week.
>I want to finish this as soon as possible, because I am away for the
>complete next week.
Ok. I'll do that. Not sure about today though. Don't worry about sync up 
with Marcelo. I'll merge both trees and send a patch.

Max

Re: [Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Marcel Holtmann]

Hi Max,

> >I see the notify() interface and the seperate counting of ACL and SCO as
> >two different problems and I don't wanna mix them.
> >However I think it is nice to send the "conn" down to the driver, if you
> >thing it is two much, I drop it.
> Well, both are needed only for SCO over USB.
> So in some sense they are related :).
> 
> btw How about fixing it in 2.6 and leaving 2.4 alone ?
> I don't think we will ever have decent Voice support in 2.4 anyway.
> For example ALSA driver will definitely go into 2.6 not 2.4.

the ALSA support is not the point, because you can use the SCO socket
for 2.4 for now and it is working fine for me. I think we should drop
this for 2.4.23 and wait until 2.4.24-pre.

> >> What I'm saying is that we should fix the write way not just some hack that 
> >> fixes first negotiation. It will pass certification with Ceticomm (or whatever 
> >> the name of that company) but will fail if tester code does proper checking ie second 
> >> connection, etc.
> >> It should be a trivial fix. I take a look at it tonight.
> >
> >I looked at your fix and it looks fine to me. Can you please rebuild my
> >marcel-2.4 tree on the base of your current work? I will re-push my
> >other patches so we can sync up with Marcelo until the end of this week.
> >I want to finish this as soon as possible, because I am away for the
> >complete next week.
> Ok. I'll do that. Not sure about today though. Don't worry about sync up 
> with Marcelo. I'll merge both trees and send a patch.

I can push my other changes within 10 minutes if I have a clean tree. So
drop me a note if you have rebuild it it and then I push them.

Regards

Marcel




-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Jonathan Paisley]

On Thu, 9 Oct 2003 8:24pm +0200, Marcel Holtmann wrote:

> > btw How about fixing it in 2.6 and leaving 2.4 alone ?
> > I don't think we will ever have decent Voice support in 2.4 anyway.
> > For example ALSA driver will definitely go into 2.6 not 2.4.
>
> the ALSA support is not the point, because you can use the SCO socket
> for 2.4 for now and it is working fine for me. I think we should drop
> this for 2.4.23 and wait until 2.4.24-pre.

Have you had a chance to try or look at my snd-bluez-sco ALSA driver yet?
I'm interested to hear your thoughts on its design.

	http://www.dcs.gla.ac.uk/~jp/snd-bluez-sco/

Thanks.

Re: [Bluez-devel] Re: Bluetooth update for 2.4.23-pre2

[Marcel Holtmann]

Hi Jonathan,

> > the ALSA support is not the point, because you can use the SCO socket
> > for 2.4 for now and it is working fine for me. I think we should drop
> > this for 2.4.23 and wait until 2.4.24-pre.
> 
> Have you had a chance to try or look at my snd-bluez-sco ALSA driver yet?
> I'm interested to hear your thoughts on its design.
> 
> 	http://www.dcs.gla.ac.uk/~jp/snd-bluez-sco/

I have looked at your code, but never tested it. However it looks fine
and it seems that some people got success with it. What I don't like is
that it is build on top of the SCO sockets. I raised this a long time
ago and what I expect from a good Bluetooth audio integration is direct
interface to OSS and ALSA. I think of something we have done for RFCOMM,
which means that we integrate all stuff directly into the sco.o module.

	Linux 2.4	SCO socket + OSS interface
	Linux 2.6	SCO socket + OSS/ALSA interface

I am not an audio or ALSA expert, so don't expect to much from me in
this area. And the 2.6 looks like a good starting point to redesign our
SCO interface. I would say that you should go ahead and try to do it ;)

Regards

Marcel




-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel