Commit ce81843b authored by Manish Mandlik's avatar Manish Mandlik Committed by Marcel Holtmann
Browse files

Bluetooth: Fix Advertisement Monitor Suspend/Resume 



During system suspend, advertisement monitoring is disabled by setting
the HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable to False. This
disables the monitoring during suspend, however, if the controller is
monitoring a device, it sends HCI_VS_MSFT_LE_Monitor_Device_Event to
indicate that the monitoring has been stopped for that particular
device. This event may occur after suspend depending on the
low_threshold_timeout and peer device advertisement frequency, which
causes early wake up.

Right way to disable the monitoring for suspend is by removing all the
monitors before suspend and re-monitor after resume to ensure no events
are received during suspend. This patch fixes this suspend/resume issue.

Following tests are performed:
- Add monitors before suspend and make sure DeviceFound gets triggered
- Suspend the system and verify that all monitors are removed by kernel
  but not Released by bluetoothd
- Wake up and verify that all monitors are added again and DeviceFound
  gets triggered

Signed-off-by: default avatarManish Mandlik <mmandlik@google.com>
Reviewed-by: default avatarArchie Pusaka <apusaka@google.com>
Reviewed-by: default avatarMiao-chen Chou <mcchou@google.com>
Signed-off-by: default avatarMarcel Holtmann <marcel@holtmann.org>
parent 6fc16533

net/bluetooth/hci_request.c

+9 −6
Original line number Diff line number Diff line
@@ -1105,21 +1105,24 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode) 
	}

}



static void hci_req_add_set_adv_filter_enable(struct hci_request *req,

					      bool enable)

static void hci_req_prepare_adv_monitor_suspend(struct hci_request *req,

						bool suspending)

{

	struct hci_dev *hdev = req->hdev;



	switch (hci_get_adv_monitor_offload_ext(hdev)) {

	case HCI_ADV_MONITOR_EXT_MSFT:

		msft_req_add_set_filter_enable(req, enable);

		if (suspending)

			msft_suspend(hdev);

		else

			msft_resume(hdev);

		break;

	default:

		return;

	}



	/* No need to block when enabling since it's on resume path */

	if (hdev->suspended && !enable)

	if (hdev->suspended && suspending)

		set_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);

}
@@ -1186,7 +1189,7 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next) 
		}



		/* Disable advertisement filters */

		hci_req_add_set_adv_filter_enable(&req, false);

		hci_req_prepare_adv_monitor_suspend(&req, true);



		/* Prevent disconnects from causing scanning to be re-enabled */

		hdev->scanning_paused = true;
@@ -1228,7 +1231,7 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next) 
		/* Reset passive/background scanning to normal */

		__hci_update_background_scan(&req);

		/* Enable all of the advertisement filters */

		hci_req_add_set_adv_filter_enable(&req, true);

		hci_req_prepare_adv_monitor_suspend(&req, false);



		/* Unpause directed advertising */

		hdev->advertising_paused = false;

net/bluetooth/msft.c

+102 −15
Original line number Diff line number Diff line
@@ -94,11 +94,14 @@ struct msft_data { 
	__u16 pending_add_handle;

	__u16 pending_remove_handle;

	__u8 reregistering;

	__u8 suspending;

	__u8 filter_enabled;

};



static int __msft_add_monitor_pattern(struct hci_dev *hdev,

				      struct adv_monitor *monitor);

static int __msft_remove_monitor(struct hci_dev *hdev,

				 struct adv_monitor *monitor, u16 handle);



bool msft_monitor_supported(struct hci_dev *hdev)

{
@@ -154,7 +157,7 @@ static bool read_supported_features(struct hci_dev *hdev, 
}



/* This function requires the caller holds hdev->lock */

static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)

static void reregister_monitor(struct hci_dev *hdev, int handle)

{

	struct adv_monitor *monitor;

	struct msft_data *msft = hdev->msft_data;
@@ -182,6 +185,69 @@ static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle) 
	}

}



/* This function requires the caller holds hdev->lock */

static void remove_monitor_on_suspend(struct hci_dev *hdev, int handle)

{

	struct adv_monitor *monitor;

	struct msft_data *msft = hdev->msft_data;

	int err;



	while (1) {

		monitor = idr_get_next(&hdev->adv_monitors_idr, &handle);

		if (!monitor) {

			/* All monitors have been removed */

			msft->suspending = false;

			hci_update_background_scan(hdev);

			return;

		}



		msft->pending_remove_handle = (u16)handle;

		err = __msft_remove_monitor(hdev, monitor, handle);



		/* If success, return and wait for monitor removed callback */

		if (!err)

			return;



		/* Otherwise free the monitor and keep removing */

		hci_free_adv_monitor(hdev, monitor);

		handle++;

	}

}



/* This function requires the caller holds hdev->lock */

void msft_suspend(struct hci_dev *hdev)

{

	struct msft_data *msft = hdev->msft_data;



	if (!msft)

		return;



	if (msft_monitor_supported(hdev)) {

		msft->suspending = true;

		/* Quitely remove all monitors on suspend to avoid waking up

		 * the system.

		 */

		remove_monitor_on_suspend(hdev, 0);

	}

}



/* This function requires the caller holds hdev->lock */

void msft_resume(struct hci_dev *hdev)

{

	struct msft_data *msft = hdev->msft_data;



	if (!msft)

		return;



	if (msft_monitor_supported(hdev)) {

		msft->reregistering = true;

		/* Monitors are removed on suspend, so we need to add all

		 * monitors on resume.

		 */

		reregister_monitor(hdev, 0);

	}

}



void msft_do_open(struct hci_dev *hdev)

{

	struct msft_data *msft = hdev->msft_data;
@@ -214,7 +280,7 @@ void msft_do_open(struct hci_dev *hdev) 
		/* Monitors get removed on power off, so we need to explicitly

		 * tell the controller to re-monitor.

		 */

		reregister_monitor_on_restart(hdev, 0);

		reregister_monitor(hdev, 0);

	}

}
@@ -382,8 +448,7 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev, 


	/* If in restart/reregister sequence, keep registering. */

	if (msft->reregistering)

		reregister_monitor_on_restart(hdev,

					      msft->pending_add_handle + 1);

		reregister_monitor(hdev, msft->pending_add_handle + 1);



	hci_dev_unlock(hdev);
@@ -420,13 +485,25 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, 
	if (handle_data) {

		monitor = idr_find(&hdev->adv_monitors_idr,

				   handle_data->mgmt_handle);

		if (monitor)



		if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)

			monitor->state = ADV_MONITOR_STATE_REGISTERED;



		/* Do not free the monitor if it is being removed due to

		 * suspend. It will be re-monitored on resume.

		 */

		if (monitor && !msft->suspending)

			hci_free_adv_monitor(hdev, monitor);



		list_del(&handle_data->list);

		kfree(handle_data);

	}



	/* If in suspend/remove sequence, keep removing. */

	if (msft->suspending)

		remove_monitor_on_suspend(hdev,

					  msft->pending_remove_handle + 1);



	/* If remove all monitors is required, we need to continue the process

	 * here because the earlier it was paused when waiting for the

	 * response from controller.
@@ -445,6 +522,7 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, 
	hci_dev_unlock(hdev);



done:

	if (!msft->suspending)

		hci_remove_adv_monitor_complete(hdev, status);

}
@@ -578,15 +656,15 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor) 
	if (!msft)

		return -EOPNOTSUPP;



	if (msft->reregistering)

	if (msft->reregistering || msft->suspending)

		return -EBUSY;



	return __msft_add_monitor_pattern(hdev, monitor);

}



/* This function requires the caller holds hdev->lock */

int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,

			u16 handle)

static int __msft_remove_monitor(struct hci_dev *hdev,

				 struct adv_monitor *monitor, u16 handle)

{

	struct msft_cp_le_cancel_monitor_advertisement cp;

	struct msft_monitor_advertisement_handle_data *handle_data;
@@ -594,12 +672,6 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, 
	struct msft_data *msft = hdev->msft_data;

	int err = 0;



	if (!msft)

		return -EOPNOTSUPP;



	if (msft->reregistering)

		return -EBUSY;



	handle_data = msft_find_handle_data(hdev, monitor->handle, true);



	/* If no matched handle, just remove without telling controller */
@@ -619,6 +691,21 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, 
	return err;

}



/* This function requires the caller holds hdev->lock */

int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,

			u16 handle)

{

	struct msft_data *msft = hdev->msft_data;



	if (!msft)

		return -EOPNOTSUPP;



	if (msft->reregistering || msft->suspending)

		return -EBUSY;



	return __msft_remove_monitor(hdev, monitor, handle);

}



void msft_req_add_set_filter_enable(struct hci_request *req, bool enable)

{

	struct hci_dev *hdev = req->hdev;

net/bluetooth/msft.h

+5 −0
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, 
			u16 handle);

void msft_req_add_set_filter_enable(struct hci_request *req, bool enable);

int msft_set_filter_enable(struct hci_dev *hdev, bool enable);

void msft_suspend(struct hci_dev *hdev);

void msft_resume(struct hci_dev *hdev);

bool msft_curve_validity(struct hci_dev *hdev);



#else
@@ -59,6 +61,9 @@ static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable) 
	return -EOPNOTSUPP;

}



static inline void msft_suspend(struct hci_dev *hdev) {}

static inline void msft_resume(struct hci_dev *hdev) {}



static inline bool msft_curve_validity(struct hci_dev *hdev)

{

	return false;