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

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

[chen kris]

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

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

[Marcel Holtmann]

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

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

[Anderson Lizardo]

Hi,

On Mon, Oct 15, 2012 at 3:02 PM, Marcel Holtmann <marcel@holtmann.org> 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

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

[chen kris]

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
<anderson.lizardo@openbossa.org> wrote:
> Hi,
>
> On Mon, Oct 15, 2012 at 3:02 PM, Marcel Holtmann <marcel@holtmann.org> 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