2012-10-15 15:01:48

by Bluefrog

Subject: Is it incorrect in checking of "NO_RESET" bit in hci_core.c:hci_dev_do_close()?

static int hci_dev_do_close(struct hci_dev *hdev)
{
...

/* Reset device */
skb_queue_purge(&hdev->cmd_q);
atomic_set(&hdev->cmd_cnt, 1);
if (!test_bit(HCI_RAW, &hdev->flags) &&
test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
set_bit(HCI_INIT, &hdev->flags);
__hci_request(hdev, hci_reset_req, 0,
msecs_to_jiffies(250));
clear_bit(HCI_INIT, &hdev->flags);
}
...
}

I guess the condition should be as following:

if (!test_bit(HCI_RAW, &hdev->flags) &&
*! *test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))

it means the code should sent HCI_RESET, only if both of the following
conditions are met:
1. HCI_RAW is NOT set;
2. HCI_QUIRK_NO_RESET is *NOT* set.

thanks!

Bluefrog2012

2012-10-16 09:49:27

by Bluefrog

Subject: Re: Is it incorrect in checking of "NO_RESET" bit in hci_core.c:hci_dev_do_close()?

Hi, Marcel

Thanks!

Hi, Anderson

Sorry, the kernel source I used is compat-wireless-3.5-rc3-2-snpc.

I checked newer kernel code, it do have the new flag name.

now I know the meaning and logic of this flag, thanks!

Kris


On Tue, Oct 16, 2012 at 2:09 AM, Anderson Lizardo
<[email protected]> wrote:
> Hi,
>
> On Mon, Oct 15, 2012 at 3:02 PM, Marcel Holtmann <[email protected]> wrote:
>> Hi Chen,
>>
>>> static int hci_dev_do_close(struct hci_dev *hdev)
>>> {
>>> ...
>>>
>>> /* Reset device */
>>> skb_queue_purge(&hdev->cmd_q);
>>> atomic_set(&hdev->cmd_cnt, 1);
>>> if (!test_bit(HCI_RAW, &hdev->flags) &&
>>> test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
>>> set_bit(HCI_INIT, &hdev->flags);
>>> __hci_request(hdev, hci_reset_req, 0,
>>> msecs_to_jiffies(250));
>>> clear_bit(HCI_INIT, &hdev->flags);
>>> }
>>> ...
>>> }
>>>
>>> I guess the condition should be as following:
>>>
>>> if (!test_bit(HCI_RAW, &hdev->flags) &&
>>> *! *test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
>>>
>>> it means the code should sent HCI_RESET, only if both of the following
>>> conditions are met:
>>> 1. HCI_RAW is NOT set;
>>> 2. HCI_QUIRK_NO_RESET is *NOT* set.
>>
>> the name is confusing, when closing the device and NO_RESET is set, then
>> it needs to send the HCI Reset.
>
> I was almost sure this flag has been renamed to something like
> "HCI_QUIRK_RESET_ON_CLOSE" on recent kernels?
>
> Regards,
> --
> Anderson Lizardo
> Instituto Nokia de Tecnologia - INdT
> Manaus - Brazil

2012-10-15 18:09:59

by Anderson Lizardo

Subject: Re: Is it incorrect in checking of "NO_RESET" bit in hci_core.c:hci_dev_do_close()?

Hi,

On Mon, Oct 15, 2012 at 3:02 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Chen,
>
>> static int hci_dev_do_close(struct hci_dev *hdev)
>> {
>> ...
>>
>> /* Reset device */
>> skb_queue_purge(&hdev->cmd_q);
>> atomic_set(&hdev->cmd_cnt, 1);
>> if (!test_bit(HCI_RAW, &hdev->flags) &&
>> test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
>> set_bit(HCI_INIT, &hdev->flags);
>> __hci_request(hdev, hci_reset_req, 0,
>> msecs_to_jiffies(250));
>> clear_bit(HCI_INIT, &hdev->flags);
>> }
>> ...
>> }
>>
>> I guess the condition should be as following:
>>
>> if (!test_bit(HCI_RAW, &hdev->flags) &&
>> *! *test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
>>
>> it means the code should sent HCI_RESET, only if both of the following
>> conditions are met:
>> 1. HCI_RAW is NOT set;
>> 2. HCI_QUIRK_NO_RESET is *NOT* set.
>
> the name is confusing, when closing the device and NO_RESET is set, then
> it needs to send the HCI Reset.

I was almost sure this flag has been renamed to something like
"HCI_QUIRK_RESET_ON_CLOSE" on recent kernels?

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2012-10-15 18:02:07

by Marcel Holtmann

Subject: Re: Is it incorrect in checking of "NO_RESET" bit in hci_core.c:hci_dev_do_close()?

Hi Chen,

> static int hci_dev_do_close(struct hci_dev *hdev)
> {
> ...
>
> /* Reset device */
> skb_queue_purge(&hdev->cmd_q);
> atomic_set(&hdev->cmd_cnt, 1);
> if (!test_bit(HCI_RAW, &hdev->flags) &&
> test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
> set_bit(HCI_INIT, &hdev->flags);
> __hci_request(hdev, hci_reset_req, 0,
> msecs_to_jiffies(250));
> clear_bit(HCI_INIT, &hdev->flags);
> }
> ...
> }
>
> I guess the condition should be as following:
>
> if (!test_bit(HCI_RAW, &hdev->flags) &&
> *! *test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
>
> it means the code should sent HCI_RESET, only if both of the following
> conditions are met:
> 1. HCI_RAW is NOT set;
> 2. HCI_QUIRK_NO_RESET is *NOT* set.

the name is confusing, when closing the device and NO_RESET is set, then
it needs to send the HCI Reset.

Regards

Marcel