Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marcel,

On Oct 15, 2011, at 3:18 PM, Marcel Holtmann wrote:

> Hi Andre,
> 
>>>>>> This patch adds to hci_core the infra-structure to carry out the
>>>>>> LE scan. Functions were created to init the LE scan and cancel
>>>>>> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
>>>>>> 
>>>>>> Also, the HCI_LE_SCAN flag was created to inform if the controller
>>>>>> is performing LE scan. The flag is set/cleared when the controller
>>>>>> starts/stops scanning.
>>>>>> 
>>>>>> Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
>>>>>> ---
>>>>>> include/net/bluetooth/hci.h      |    2 +
>>>>>> include/net/bluetooth/hci_core.h |    5 +++
>>>>>> net/bluetooth/hci_core.c         |   69 ++++++++++++++++++++++++++++++++++++++
>>>>>> net/bluetooth/hci_event.c        |    4 ++
>>>>>> 4 files changed, 80 insertions(+), 0 deletions(-)
>>>>>> 
>>>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>>>> index 11537b8..7520544 100644
>>>>>> --- a/include/net/bluetooth/hci.h
>>>>>> +++ b/include/net/bluetooth/hci.h
>>>>>> @@ -86,6 +86,8 @@ enum {
>>>>>> 	HCI_DEBUG_KEYS,
>>>>>> 
>>>>>> 	HCI_RESET,
>>>>>> +
>>>>>> +	HCI_LE_SCAN,
>>>>> 
>>>>> I remember that Marcel commented against this here. Let's hide this
>>>>> internally.
>>>> 
>>>> If I got it right, Marcel agreed in keeping this flag here, but we
>>>> won't export it to userspace now (we'll keep it hidden internally).
>>> 
>>> hdev->flags is exported to userspace, it is not internal. We need to create
>>> other place in struct hci_dev to keep this info. See some of Marcel's reply on
>>> this.
>> 
>> When I said "export to userspace" I meant to add this flag to userspace
>> header (lib/hci.h). Even though hdev->flags is exported to userspace,
>> if the flag is not defined at that header the userspace cannot decode it
>> properly. This way the flag is hidden internally in kernel.
>> 
>> Marcel, could you clarify this? Are you fine with we keep this HCI_LE_SCAN
>> in hdev->flags?
> 
> so the hdev->flags has to stay as it is right now since otherwise you
> need to change every single driver that sets HCI_RUNNING.
> 
> And I do not wanna export any single bit to the user that it does not
> need to see. And right now it only needs HCI_UP to HCI_RAW, because they
> have been there since ever. And deprecated them takes a bit longer. But
> even they will be deprecated in the long run. Any other bit that you
> guys introduced goes against this and needs to be fixed.
> 
> The one part I do not get right now is why is it so hard to introduce a
> new flags variable for this. I mentioned this really early on and you
> keep sending patches for it and we keep discussing this.
> 
> So I make this pretty simple, call this hdev->mgmt_flags and start
> changing these NOW. These flags need to stay internal. Mixing it with
> anything that is exported to userspace is a really bad idea.

Ok, I'll implement a new flags variable and add the LE_SCAN flag to it.
Then we can start moving other flags to this new flags variable.

Thanks,

Andre

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux