2015-04-20 10:35:13

by Loic Poulain

[permalink] [raw]
Subject: bluetooth-next HCI user channel issue

Hi Marcel,

Seems there is a regression in bluetooth-next.
I'm unable to communicate with my Intel WP chip via HCI User channel.
To test user channel, I use bluemoon:
$ hciconfig hci0 down
$ bluemoon -i hci0
Bluemoon configuration utility ver 5.30

Then bluemoon hangs up, no HCI event...

HCI packet seems successfully sent, but no HCI response:
[ 491.525914] Bluetooth: hci0
[ 491.525918] Bluetooth: hci0
[ 491.525924] Bluetooth: hci0
[ 491.525927] Bluetooth: hci0
[ 491.525964] Bluetooth: hci0
[ 491.526114] Bluetooth: hci0 urb ffff8800c1f320c0 status 0 count 3

When I kill bluemoon, I get same kind of issue (cmd timeout):
[ 895.603661] Bluetooth: hci0 urb ffff8802f89d5b40 status 0 count 3
[ 897.606323] Bluetooth: hci0 command 0xfc3f tx timeout
[ 897.606350] Bluetooth: hci0
[ 897.607011] Bluetooth: hci0 urb ffff88030c2c83c0 status 0 count 244
[ 899.612007] Bluetooth: hci0 command 0x0c52 tx timeout

After that, device still usable by other sockets (hciconfig, hcitool...)

I wonder if you reproduce this issue on your side?

Regards,
Loic

--
Intel Open Source Technology Center
http://oss.intel.com/



2015-04-20 17:43:36

by Marcel Holtmann

[permalink] [raw]
Subject: Re: bluetooth-next HCI user channel issue

Hi Loic,

> Seems there is a regression in bluetooth-next.
> I'm unable to communicate with my Intel WP chip via HCI User channel.
> To test user channel, I use bluemoon:
> $ hciconfig hci0 down
> $ bluemoon -i hci0
> Bluemoon configuration utility ver 5.30
>
> Then bluemoon hangs up, no HCI event...
>
> HCI packet seems successfully sent, but no HCI response:
> [ 491.525914] Bluetooth: hci0
> [ 491.525918] Bluetooth: hci0
> [ 491.525924] Bluetooth: hci0
> [ 491.525927] Bluetooth: hci0
> [ 491.525964] Bluetooth: hci0
> [ 491.526114] Bluetooth: hci0 urb ffff8800c1f320c0 status 0 count 3
>
> When I kill bluemoon, I get same kind of issue (cmd timeout):
> [ 895.603661] Bluetooth: hci0 urb ffff8802f89d5b40 status 0 count 3
> [ 897.606323] Bluetooth: hci0 command 0xfc3f tx timeout
> [ 897.606350] Bluetooth: hci0
> [ 897.607011] Bluetooth: hci0 urb ffff88030c2c83c0 status 0 count 244
> [ 899.612007] Bluetooth: hci0 command 0x0c52 tx timeout
>
> After that, device still usable by other sockets (hciconfig, hcitool...)
>
> I wonder if you reproduce this issue on your side?

can you git bisect this to pin point the patch that broke this for you.

Regards

Marcel


2015-05-21 12:22:23

by Marcel Holtmann

[permalink] [raw]
Subject: Re: bluetooth-next HCI user channel issue

Hi Loic,

>>> any chance you did git bisect this and tell us which patch broken this?
>>
>> I'll give you a answer/status before the end of next week.
>>
>> Regards,
>> Loic
>>
>
> The following patch introduces the regression:
> commit bfbd45e9acd2ef90ccc31ea02e08f82af392dbec
> Bluetooth: Add device shutdown routine for Intel Bluetooth device

perfect. Thanks for bi-secting this.

The real offending commit is actually the one before introducing hdev->shutdown() call. Which is commit a44fecbd52a4. We are checking for HCI_UNREGISTER set, but we should also be checking for HCI user channel. If the dev_do_close is triggered from an open HCI user channel, we should not call hdev->shutdown(). That is a function that should only be called if we are indeed running BlueZ as stack.

I however think the patch is a bit trickier since the flag has already been cleared at that point. So it might needs an interim flag or placing of the hdev->shutdown() at a different location.

> This patch sends the 0xFC3F vendor command from the shutdown callback.
> Author says:
> "For backward compatibility of this command with old firmware, this
> command was supported before, but it behaves same as HCI_RESET
> internally. So, it won't be the problem even if the system doesn't have
> the latest firmware patch."
>
> However this patch introduces a HCI user channel regression:
> - When hci interface is down/up with hciconfig, no problem.
> - When hci interface is re-open via HCI user channel (bluemoon), first
> command hangs.
>
> After investigating, this is due to the "first HCI command" bug.
> There is the following workaround in btusb_setup_intel:
> /* The controller has a bug with the first HCI command sent to it
> * returning number of completed commands as zero. This would stall the
> * command processing in the Bluetooth core.
> *
> * As a workaround, send HCI Reset command first which will reset the
> * number of completed commands and allow normal command processing
> * from now on.
> */
>
> Seems that the 0xFC3F command puts the device in the same kind of state
> (Intel init state?).
>
> So we need to apply the same workaround by sending HCI_RESET before any
> other command, either in the open callback (QUIRK) or in the shutdown
> one (after 0xFC3F).

My feeling is that this should be in the shutdown callback. So sending another HCI Reset afterwards.

Tedd, would that actually work and have the right affect?

> We don't see the issue with "hciconfig up" because the first command is
> always HCI_RESET.

And btw. I think we need both patches. Not calling hdev->shutdown() when leaving HCI user channel and adding the extra HCI Reset to the Intel shutdown so that we can go from normal stack operation into a HCI user channel and are not caught by the first HCI command bug.

Regards

Marcel


2015-05-21 10:18:27

by Loic Poulain

[permalink] [raw]
Subject: Re: bluetooth-next HCI user channel issue

Hi Marcel,

>> any chance you did git bisect this and tell us which patch broken this?
>
> I'll give you a answer/status before the end of next week.
>
> Regards,
> Loic
>

The following patch introduces the regression:
commit bfbd45e9acd2ef90ccc31ea02e08f82af392dbec
Bluetooth: Add device shutdown routine for Intel Bluetooth device

This patch sends the 0xFC3F vendor command from the shutdown callback.
Author says:
"For backward compatibility of this command with old firmware, this
command was supported before, but it behaves same as HCI_RESET
internally. So, it won't be the problem even if the system doesn't have
the latest firmware patch."

However this patch introduces a HCI user channel regression:
- When hci interface is down/up with hciconfig, no problem.
- When hci interface is re-open via HCI user channel (bluemoon), first
command hangs.

After investigating, this is due to the "first HCI command" bug.
There is the following workaround in btusb_setup_intel:
/* The controller has a bug with the first HCI command sent to it
* returning number of completed commands as zero. This would stall the
* command processing in the Bluetooth core.
*
* As a workaround, send HCI Reset command first which will reset the
* number of completed commands and allow normal command processing
* from now on.
*/

Seems that the 0xFC3F command puts the device in the same kind of state
(Intel init state?).

So we need to apply the same workaround by sending HCI_RESET before any
other command, either in the open callback (QUIRK) or in the shutdown
one (after 0xFC3F).

We don't see the issue with "hciconfig up" because the first command is
always HCI_RESET.

Regards,
Loic

--
Intel Open Source Technology Center
http://oss.intel.com/

2015-05-15 13:28:04

by Loic Poulain

[permalink] [raw]
Subject: Re: bluetooth-next HCI user channel issue


Hi Marcel,

On 13/05/2015 23:10, Marcel Holtmann wrote:
> Hi Loic,
>
>>> Seems there is a regression in bluetooth-next.
>>> I'm unable to communicate with my Intel WP chip via HCI User channel.
>>> To test user channel, I use bluemoon:
>>> $ hciconfig hci0 down
>>> $ bluemoon -i hci0
>>> Bluemoon configuration utility ver 5.30
>>>
>>> Then bluemoon hangs up, no HCI event...
>>>
>>> HCI packet seems successfully sent, but no HCI response:
>>> [ 491.525914] Bluetooth: hci0
>>> [ 491.525918] Bluetooth: hci0
>>> [ 491.525924] Bluetooth: hci0
>>> [ 491.525927] Bluetooth: hci0
>>> [ 491.525964] Bluetooth: hci0
>>> [ 491.526114] Bluetooth: hci0 urb ffff8800c1f320c0 status 0 count 3
>>>
>>> When I kill bluemoon, I get same kind of issue (cmd timeout):
>>> [ 895.603661] Bluetooth: hci0 urb ffff8802f89d5b40 status 0 count 3
>>> [ 897.606323] Bluetooth: hci0 command 0xfc3f tx timeout
>>> [ 897.606350] Bluetooth: hci0
>>> [ 897.607011] Bluetooth: hci0 urb ffff88030c2c83c0 status 0 count 244
>>> [ 899.612007] Bluetooth: hci0 command 0x0c52 tx timeout
>>>
>>> After that, device still usable by other sockets (hciconfig, hcitool...)
>>>
>>> I wonder if you reproduce this issue on your side?
>>
>> can you git bisect this to pin point the patch that broke this for you.
>
> any chance you did git bisect this and tell us which patch broken this?

I'll give you a answer/status before the end of next week.

Regards,
Loic

--
Intel Open Source Technology Center
http://oss.intel.com/

2015-05-13 21:10:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: bluetooth-next HCI user channel issue

Hi Loic,

>> Seems there is a regression in bluetooth-next.
>> I'm unable to communicate with my Intel WP chip via HCI User channel.
>> To test user channel, I use bluemoon:
>> $ hciconfig hci0 down
>> $ bluemoon -i hci0
>> Bluemoon configuration utility ver 5.30
>>
>> Then bluemoon hangs up, no HCI event...
>>
>> HCI packet seems successfully sent, but no HCI response:
>> [ 491.525914] Bluetooth: hci0
>> [ 491.525918] Bluetooth: hci0
>> [ 491.525924] Bluetooth: hci0
>> [ 491.525927] Bluetooth: hci0
>> [ 491.525964] Bluetooth: hci0
>> [ 491.526114] Bluetooth: hci0 urb ffff8800c1f320c0 status 0 count 3
>>
>> When I kill bluemoon, I get same kind of issue (cmd timeout):
>> [ 895.603661] Bluetooth: hci0 urb ffff8802f89d5b40 status 0 count 3
>> [ 897.606323] Bluetooth: hci0 command 0xfc3f tx timeout
>> [ 897.606350] Bluetooth: hci0
>> [ 897.607011] Bluetooth: hci0 urb ffff88030c2c83c0 status 0 count 244
>> [ 899.612007] Bluetooth: hci0 command 0x0c52 tx timeout
>>
>> After that, device still usable by other sockets (hciconfig, hcitool...)
>>
>> I wonder if you reproduce this issue on your side?
>
> can you git bisect this to pin point the patch that broke this for you.

any chance you did git bisect this and tell us which patch broken this?

Regards

Marcel