Re: [PATCH v6 2/3] Bluetooth: Add le_scan_restart

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

 



On Fri, Jan 9, 2015 at 12:22 PM, Jakub Pawlowski <jpawlowski@xxxxxxxxxx> wrote:
> Hi Johan,
>
> On Fri, Jan 9, 2015 at 11:26 AM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
>> Hi Jakub,
>>
>> On Wed, Jan 07, 2015, Jakub Pawlowski wrote:
>>> -             cancel_delayed_work(&hdev->le_scan_disable);
>>> +             if (!bt_cb(hdev->sent_cmd)->req.marker)
>>> +                     cancel_delayed_work(&hdev->le_scan_disable);
>>>
>>>               clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
>>
>> Modifying the generic HCI request framework for this single purpose
>> doesn't make much sense to me. Also, the details of the bt_cb(skb)->req
>> struct are considered an implementation detail of the HCI request
>> specific handling and should not be directly touched by other code.
>
> Description of cb field of skbuff says it can be used by every layer:
> " @cb: Control buffer. Free for use by every layer. Put private vars
> here" ( http://lxr.free-electrons.com/source/include/linux/skbuff.h#L445
> )
> I fought I can just add something there, and use it.
> I agree that this field doesn't fit struct hci_req_ctrl , but that was
> only place where there are free bits, otherwise the struct gets too
> big and don't fit cb field.
>
>>
>> It looks to me like this marker has the same purpose as the flag you had
>> in some earlier revision of this set? For such a use-case a flag would
>> still be the best option.
>
> Yes, it have same meaning as this eariler version flag. The problem
> with flag was that I can't set it when I create request, it need to be
> set when the request is being executed, otherwise I can get race
> condition. In case of this marker field, there is no option to have
> race condition.
>
>> However, I'm wondering whether we can remove
>> the need for this completely. Can't you just let the code cancel the
>> delayed work and the re-schedule it again once the scanning gets
>> re-enabled (with some extra code decrease the timeout some appropriate
>> amount for each time scanning gets restarted).
>
> 1. I can't get remaining time from workqueue, so I'll have to remember
> when it was started.
> 2. When I do restart I'll still need some flag to know wether it was
> my internal restart, or wether someone just stopped and then started
> the scan, so I'll still need some flag, and way to set it.
>
> I have one more idea on how to make it work, I'll send patch soon.
>

Ok I figured out how you wanted it, it's much simpler now, I'll send
patches now.
Thanks!
>>
>> Johan
--
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