Re: [PATCH v8 2/2] Bluetooth: Add restarting to service discovery

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Jakub,

On Wed, Jan 14, 2015, Jakub Pawlowski wrote:
> +static void restart_le_scan(struct hci_dev *hdev)
> +{
> +	/* If controller is not scanning we are done. */
> +	if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> +		return;

Since this function is only called from mgmt_device_found() can this
condition actually ever be true?

> +
> +	queue_delayed_work(hdev->workqueue, &hdev->le_scan_restart,
> +			   msecs_to_jiffies(DISCOV_LE_RESTART_DELAY));
> +}

What you should probably be checking for however is that scan_end <
jiffies + msecs_to_jiffies(DISCOV_LE_RESTART_DELAY) since then you know
that the scanning will have stopped by the tame the restart work gets
activated. Or am I thinking wrong here?

>  	if (hdev->discovery.rssi != HCI_RSSI_INVALID &&
> -	    (rssi < hdev->discovery.rssi || rssi == HCI_RSSI_INVALID))
> +	    (rssi == HCI_RSSI_INVALID ||
> +	    (rssi < hdev->discovery.rssi && !test_bit(
> +			HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks))))

What I meant by splitting this is that the !test_bit would start on a
new line. That way you wouldn't need to break the line right after (

Johan