Question about HCI_QUIRK_RESET_ON_CLOSE

Question about HCI_QUIRK_RESET_ON_CLOSE

[Tedd Ho-Jeong An]

Hi All,

I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?

What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?

Regards,

Tedd

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Marcel Holtmann]

Hi Tedd,

> I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
> If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?

Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear defined behavior on HCI_Reset when it comes to the host transport. Only Bluetooth 1.1 cleared that HCI_Reset is not suppose to reset the transport. For example with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, USB Connect endless cycles.

A certain set of drivers are required to set this quirk. However eventually they have to do the HCI_Reset since otherwise we end up in funky states. Check drivers/bluetooth/ for devices that require not to send the HCI_Reset on init.

> What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?

I have been thinking about this a while ago. Instead of using HCI_Reset we actually started to modify the kernel to clear out all its states when we are powering down the controller. A recent bluetooth-next kernel should just make sure we are no longer connectable and discoverable and also no longer advertising.

We needed this for UART based devices where the transport has no clear indication that it is down. With USB devices this problem normally never happens since we are bringing down USB as well.

Do you need something else?

Regards

Marcel

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Tedd Ho-Jeong An]

On Mon, 9 Jun 2014 17:45:01 +0200
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Tedd,
> 
> > I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
> > If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?
> 
> Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear defined behavior on HCI_Reset when it comes to the host transport. Only Bluetooth 1.1 cleared that HCI_Reset is not suppose to reset the transport. For example with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, USB Connect endless cycles.
> 
> A certain set of drivers are required to set this quirk. However eventually they have to do the HCI_Reset since otherwise we end up in funky states. Check drivers/bluetooth/ for devices that require not to send the HCI_Reset on init.
> 
> > What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?
> 
> I have been thinking about this a while ago. Instead of using HCI_Reset we actually started to modify the kernel to clear out all its states when we are powering down the controller. A recent bluetooth-next kernel should just make sure we are no longer connectable and discoverable and also no longer advertising.
> 
> We needed this for UART based devices where the transport has no clear indication that it is down. With USB devices this problem normally never happens since we are bringing down USB as well.
> 
> Do you need something else?

We are seeing an issue while turning on/off BT with inquiry, especially extended inquiry. If BT is turned off right after sending an extended inquiry, and next time when the BT is turned on the buffer is corrupted. We have seen this on Chromebook with 3.10 kernel.

If the changes are made to recent version, I am not sure whether I can push the changes to chromebook tree. If it is not acceptable, then I need to come up with something else like sending HCI_RESET upon closing the device.

Do you know which patches should I start to look at?

Regards,

Tedd

> 
> Regards
> 
> Marcel
> 

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Marcel Holtmann]

Hi Tedd,

>>> I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
>>> If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?
>> 
>> Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear defined behavior on HCI_Reset when it comes to the host transport. Only Bluetooth 1.1 cleared that HCI_Reset is not suppose to reset the transport. For example with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, USB Connect endless cycles.
>> 
>> A certain set of drivers are required to set this quirk. However eventually they have to do the HCI_Reset since otherwise we end up in funky states. Check drivers/bluetooth/ for devices that require not to send the HCI_Reset on init.
>> 
>>> What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?
>> 
>> I have been thinking about this a while ago. Instead of using HCI_Reset we actually started to modify the kernel to clear out all its states when we are powering down the controller. A recent bluetooth-next kernel should just make sure we are no longer connectable and discoverable and also no longer advertising.
>> 
>> We needed this for UART based devices where the transport has no clear indication that it is down. With USB devices this problem normally never happens since we are bringing down USB as well.
>> 
>> Do you need something else?
> 
> We are seeing an issue while turning on/off BT with inquiry, especially extended inquiry. If BT is turned off right after sending an extended inquiry, and next time when the BT is turned on the buffer is corrupted. We have seen this on Chromebook with 3.10 kernel.

that is actually a good question for Johan, are we also canceling any connection attempts, remote name requests and ongoing inquiry transaction. We might need to verify that we really clean all of these that have baseband activity.

> If the changes are made to recent version, I am not sure whether I can push the changes to chromebook tree. If it is not acceptable, then I need to come up with something else like sending HCI_RESET upon closing the device.

So why are you getting a buffer corruption is kinda interesting. Since you will get a new HCI_Reset when you power Bluetooth back on. Not sure what makes the difference if it comes at the beginning or at the end or both times.

Normally the HCI_Reset for UART based controllers is to ensure that all baseband resources are freed. That is what we are doing manually for each resources. Calling HCI_Reset is just the cheap way out ;)

The problem is that you can not know if sending HCI_Reset at the end is acceptable by all USB based devices. You need to quirk it. And right now it is just one quirk. We do not have separate quirks for opening HCI_Reset and closing HCI_Reset. That would need to be changed if we really need it. So far we did not.

But just a heads up, this also forces changes into hci_uart driver and its ioctl interface if we change this. So there is a bit of work to be done to do this cleanly and without breaking other controllers.

> Do you know which patches should I start to look at?

I think that 3.14 kernel and later have this feature enabled. What I can tell is that we cancel connection attempts since I found a follow up patch there. For inquiry and remote name request, I do not know.

Johan, did we ever checked this and in addition that we send a Discovery Stopped event as well.

Regards

Marcel

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Johan Hedberg]

Hi Marcel,

On Mon, Jun 09, 2014, Marcel Holtmann wrote:
> I think that 3.14 kernel and later have this feature enabled. What I
> can tell is that we cancel connection attempts since I found a follow
> up patch there. For inquiry and remote name request, I do not know.
> 
> Johan, did we ever checked this and in addition that we send a
> Discovery Stopped event as well.

I'm not completely sure about a proper emission of Discover Stopped, but
the function to look at is clean_up_hci_state() in mgmt.c. You'll see
that it stops inquiry scan & page scan, advertising, LE scanning,
cancels outgoing connection attempts and disconnects existing
connections.

However, as I just looked at the function we seem to (quite
surprisingly) have completely forgotten about inquiry (either normal or
periodic), so another patch to this function is needed as it seems.

Johan

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Marcel Holtmann]

Hi Johan,

>> I think that 3.14 kernel and later have this feature enabled. What I
>> can tell is that we cancel connection attempts since I found a follow
>> up patch there. For inquiry and remote name request, I do not know.
>> 
>> Johan, did we ever checked this and in addition that we send a
>> Discovery Stopped event as well.
> 
> I'm not completely sure about a proper emission of Discover Stopped, but
> the function to look at is clean_up_hci_state() in mgmt.c. You'll see
> that it stops inquiry scan & page scan, advertising, LE scanning,
> cancels outgoing connection attempts and disconnects existing
> connections.
> 
> However, as I just looked at the function we seem to (quite
> surprisingly) have completely forgotten about inquiry (either normal or
> periodic), so another patch to this function is needed as it seems.

the same might apply to remote name request that we also have to cancel. However we might should just run a normal stop discovery procedure. Since that should already today clear this all correctly.

Regards

Marcel

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Tedd Ho-Jeong An]

On Mon, 9 Jun 2014 19:50:24 +0200
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Tedd,
> 
> >>> I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
> >>> If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?
> >> 
> >> Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear defined behavior on HCI_Reset when it comes to the host transport. Only Bluetooth 1.1 cleared that HCI_Reset is not suppose to reset the transport. For example with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, USB Connect endless cycles.
> >> 
> >> A certain set of drivers are required to set this quirk. However eventually they have to do the HCI_Reset since otherwise we end up in funky states. Check drivers/bluetooth/ for devices that require not to send the HCI_Reset on init.
> >> 
> >>> What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?
> >> 
> >> I have been thinking about this a while ago. Instead of using HCI_Reset we actually started to modify the kernel to clear out all its states when we are powering down the controller. A recent bluetooth-next kernel should just make sure we are no longer connectable and discoverable and also no longer advertising.
> >> 
> >> We needed this for UART based devices where the transport has no clear indication that it is down. With USB devices this problem normally never happens since we are bringing down USB as well.
> >> 
> >> Do you need something else?
> > 
> > We are seeing an issue while turning on/off BT with inquiry, especially extended inquiry. If BT is turned off right after sending an extended inquiry, and next time when the BT is turned on the buffer is corrupted. We have seen this on Chromebook with 3.10 kernel.
> 
> that is actually a good question for Johan, are we also canceling any connection attempts, remote name requests and ongoing inquiry transaction. We might need to verify that we really clean all of these that have baseband activity.
> 
> > If the changes are made to recent version, I am not sure whether I can push the changes to chromebook tree. If it is not acceptable, then I need to come up with something else like sending HCI_RESET upon closing the device.
> 
> So why are you getting a buffer corruption is kinda interesting. Since you will get a new HCI_Reset when you power Bluetooth back on. Not sure what makes the difference if it comes at the beginning or at the end or both times.
> 
> Normally the HCI_Reset for UART based controllers is to ensure that all baseband resources are freed. That is what we are doing manually for each resources. Calling HCI_Reset is just the cheap way out ;)

Here is what we found. After the host sends extended inquiry and it is turned off, the device is still in inquiry state and updates the USB FIFO with inquiry results. Once the BT is turned on, then host sends the HCI_RESET but it returns with inquiry results in USB FIFO.

If the host sends HCI_RESET before BT off, at least the device state will be back to idle.

> 
> The problem is that you can not know if sending HCI_Reset at the end is acceptable by all USB based devices. You need to quirk it. And right now it is just one quirk. We do not have separate quirks for opening HCI_Reset and closing HCI_Reset. That would need to be changed if we really need it. So far we did not.
> 
Is it possible to have something like HCI_QUIRK_RESET_ON_INIT? Existing implementation will still be there and if a HW vendor wants this feature, they can set this up for init. 

> But just a heads up, this also forces changes into hci_uart driver and its ioctl interface if we change this. So there is a bit of work to be done to do this cleanly and without breaking other controllers.
> 
> > Do you know which patches should I start to look at?
> 
> I think that 3.14 kernel and later have this feature enabled. What I can tell is that we cancel connection attempts since I found a follow up patch there. For inquiry and remote name request, I do not know.
> 
> Johan, did we ever checked this and in addition that we send a Discovery Stopped event as well.
> 
> Regards
> 
> Marcel
> 

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Luiz Augusto von Dentz]

Hi Tedd, Marcel,

On Mon, Jun 9, 2014 at 10:21 PM, Tedd Ho-Jeong An <tedd.an@intel.com> wrote=
:
> On Mon, 9 Jun 2014 19:50:24 +0200
> Marcel Holtmann <marcel@holtmann.org> wrote:
>
>> Hi Tedd,
>>
>> >>> I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack ini=
tialization or device close exclusively.
>> >>> If the bit is set, HCI_RESET is sent to the device during device clo=
se but not during stack initialization and vice versa when it is not set. I=
 just wonder if there is any reason for doing it?
>> >>
>> >> Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear define=
d behavior on HCI_Reset when it comes to the host transport. Only Bluetooth=
 1.1 cleared that HCI_Reset is not suppose to reset the transport. For exam=
ple with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, =
USB Connect endless cycles.
>> >>
>> >> A certain set of drivers are required to set this quirk. However even=
tually they have to do the HCI_Reset since otherwise we end up in funky sta=
tes. Check drivers/bluetooth/ for devices that require not to send the HCI_=
Reset on init.
>> >>
>> >>> What do you recommend if HCI_RESET needs to be sent for both stack i=
nitialization case and device close?
>> >>
>> >> I have been thinking about this a while ago. Instead of using HCI_Res=
et we actually started to modify the kernel to clear out all its states whe=
n we are powering down the controller. A recent bluetooth-next kernel shoul=
d just make sure we are no longer connectable and discoverable and also no =
longer advertising.
>> >>
>> >> We needed this for UART based devices where the transport has no clea=
r indication that it is down. With USB devices this problem normally never =
happens since we are bringing down USB as well.
>> >>
>> >> Do you need something else?
>> >
>> > We are seeing an issue while turning on/off BT with inquiry, especiall=
y extended inquiry. If BT is turned off right after sending an extended inq=
uiry, and next time when the BT is turned on the buffer is corrupted. We ha=
ve seen this on Chromebook with 3.10 kernel.
>>
>> that is actually a good question for Johan, are we also canceling any co=
nnection attempts, remote name requests and ongoing inquiry transaction. We=
 might need to verify that we really clean all of these that have baseband =
activity.
>>
>> > If the changes are made to recent version, I am not sure whether I can=
 push the changes to chromebook tree. If it is not acceptable, then I need =
to come up with something else like sending HCI_RESET upon closing the devi=
ce.
>>
>> So why are you getting a buffer corruption is kinda interesting. Since y=
ou will get a new HCI_Reset when you power Bluetooth back on. Not sure what=
 makes the difference if it comes at the beginning or at the end or both ti=
mes.
>>
>> Normally the HCI_Reset for UART based controllers is to ensure that all =
baseband resources are freed. That is what we are doing manually for each r=
esources. Calling HCI_Reset is just the cheap way out ;)
>
> Here is what we found. After the host sends extended inquiry and it is tu=
rned off, the device is still in inquiry state and updates the USB FIFO wit=
h inquiry results. Once the BT is turned on, then host sends the HCI_RESET =
but it returns with inquiry results in USB FIFO.
>
> If the host sends HCI_RESET before BT off, at least the device state will=
 be back to idle.

I thought this was the purpose of HCI_QUIRK_RESET_ON_CLOSE so that
drivers can really force the controller into inactivity by requesting
the stack to issue a reset before before BT off. This all seems rather
strange if the controller keeps the radio active on BT off there is
something broken and sending a reset won't fix that.

>>
>> The problem is that you can not know if sending HCI_Reset at the end is =
acceptable by all USB based devices. You need to quirk it. And right now it=
 is just one quirk. We do not have separate quirks for opening HCI_Reset an=
d closing HCI_Reset. That would need to be changed if we really need it. So=
 far we did not.
>>
> Is it possible to have something like HCI_QUIRK_RESET_ON_INIT? Existing i=
mplementation will still be there and if a HW vendor wants this feature, th=
ey can set this up for init.
>
>> But just a heads up, this also forces changes into hci_uart driver and i=
ts ioctl interface if we change this. So there is a bit of work to be done =
to do this cleanly and without breaking other controllers.
>>
>> > Do you know which patches should I start to look at?
>>
>> I think that 3.14 kernel and later have this feature enabled. What I can=
 tell is that we cancel connection attempts since I found a follow up patch=
 there. For inquiry and remote name request, I do not know.
>>
>> Johan, did we ever checked this and in addition that we send a Discovery=
 Stopped event as well.
>>
>> Regards
>>
>> Marcel
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--=20
Luiz Augusto von Dentz

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Marcel Holtmann]

Hi Luiz,

>>>>>> I noticed that HCI_QUIRK_RESET_ON_CLOSE bit is used during stack initialization or device close exclusively.
>>>>>> If the bit is set, HCI_RESET is sent to the device during device close but not during stack initialization and vice versa when it is not set. I just wonder if there is any reason for doing it?
>>>>> 
>>>>> Bluetooth 1.0b and some Bluetooth 1.1 devices have not a clear defined behavior on HCI_Reset when it comes to the host transport. Only Bluetooth 1.1 cleared that HCI_Reset is not suppose to reset the transport. For example with USB. So you ended up seeing HCI_Reset, USB Reset, USB Disconnect, USB Connect endless cycles.
>>>>> 
>>>>> A certain set of drivers are required to set this quirk. However eventually they have to do the HCI_Reset since otherwise we end up in funky states. Check drivers/bluetooth/ for devices that require not to send the HCI_Reset on init.
>>>>> 
>>>>>> What do you recommend if HCI_RESET needs to be sent for both stack initialization case and device close?
>>>>> 
>>>>> I have been thinking about this a while ago. Instead of using HCI_Reset we actually started to modify the kernel to clear out all its states when we are powering down the controller. A recent bluetooth-next kernel should just make sure we are no longer connectable and discoverable and also no longer advertising.
>>>>> 
>>>>> We needed this for UART based devices where the transport has no clear indication that it is down. With USB devices this problem normally never happens since we are bringing down USB as well.
>>>>> 
>>>>> Do you need something else?
>>>> 
>>>> We are seeing an issue while turning on/off BT with inquiry, especially extended inquiry. If BT is turned off right after sending an extended inquiry, and next time when the BT is turned on the buffer is corrupted. We have seen this on Chromebook with 3.10 kernel.
>>> 
>>> that is actually a good question for Johan, are we also canceling any connection attempts, remote name requests and ongoing inquiry transaction. We might need to verify that we really clean all of these that have baseband activity.
>>> 
>>>> If the changes are made to recent version, I am not sure whether I can push the changes to chromebook tree. If it is not acceptable, then I need to come up with something else like sending HCI_RESET upon closing the device.
>>> 
>>> So why are you getting a buffer corruption is kinda interesting. Since you will get a new HCI_Reset when you power Bluetooth back on. Not sure what makes the difference if it comes at the beginning or at the end or both times.
>>> 
>>> Normally the HCI_Reset for UART based controllers is to ensure that all baseband resources are freed. That is what we are doing manually for each resources. Calling HCI_Reset is just the cheap way out ;)
>> 
>> Here is what we found. After the host sends extended inquiry and it is turned off, the device is still in inquiry state and updates the USB FIFO with inquiry results. Once the BT is turned on, then host sends the HCI_RESET but it returns with inquiry results in USB FIFO.
>> 
>> If the host sends HCI_RESET before BT off, at least the device state will be back to idle.
> 
> I thought this was the purpose of HCI_QUIRK_RESET_ON_CLOSE so that
> drivers can really force the controller into inactivity by requesting
> the stack to issue a reset before before BT off. This all seems rather
> strange if the controller keeps the radio active on BT off there is
> something broken and sending a reset won't fix that.
> 

the reset on close quirk only tell the Bluetooth core when to issue the HCI_Reset. Default is that we reset during init. However for Bluetooth 1.0b and some 1.1 devices we call the HCI_Reset at the end. That is all what the quirk is doing. It was never intended for a providing dedicated reset quirks for init and closing.

Back in the days it was just HCI_QUIRK_RESET, but luckily from Bluetooth 1.2 and forward this was default and we changed the quirk meaning to mark the bad sheep and not punish the good ones. Most modern controllers rely on the HCI_Reset in the beginning to set up basics for their baseband and radio pipeline. I have seen many controllers that will not work properly if you do not send HCI_Reset in the beginning.

So if you actually set HCI_QUIRK_RESET_ON_CLOSE, then it means that we are now no longer sending the HCI_Reset on init. Which means the controller does not work properly either.

The real fix here is that we cancel inquiry and remote name request operations that occupy the baseband and will cause HCI event along the line later on. We could also set the HCI event mask(s) to zero.

Regards

Marcel

Re: Question about HCI_QUIRK_RESET_ON_CLOSE

[Johan Hedberg]

Hi Marcel,

On Tue, Jun 10, 2014, Marcel Holtmann wrote:
> The real fix here is that we cancel inquiry and remote name request
> operations that occupy the baseband and will cause HCI event along the
> line later on. We could also set the HCI event mask(s) to zero.

I've just sent three patches that cover the inquiry and name request
parts.

Johan