Re: [PATCH] bt usb-device initialize as wakeup enable

Re: [PATCH] bt usb-device initialize as wakeup enable

[Marcel Holtmann]

Hi Sukumar,

> BT-Controller connected as platform non-root-hub device and
> usb-driver initialize such device with wakeup disabled,
> Ref. usb_new_device().
> 
> At present wakeup-capability get enabled by hid-input device from usb
> function driver(e.g. BT HID device) at runtime. Again some functional
> driver does not set usb-wakeup capability(e.g LE HID device implement
> as HID-over-GATT), and can't wakeup the host on USB.
> 
> Hence usb-wakeup capability enable form btusc driver as a generic
> solution for multiple profile usecase and required for USB remote
> wakeup (in-bus wakeup) while host is suspended.
> 
> Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> Signed-off-by: Amit K Bag <amit.k.bag@intel.com>#
> ---
> drivers/bluetooth/btusb.c | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e1124ba44154..7eeeeb6ffae6 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1083,6 +1083,10 @@ static int btusb_open(struct hci_dev *hdev)
> 	}
> 
> 	data->intf->needs_remote_wakeup = 1;
> +	/* device specific wakeup source enabled and required for USB
> +	 * remote wakeup while host is suspended
> +	 */
> +	device_init_wakeup(&data->udev->dev, true);
> 

is this the right location? Are we repeating this on every hciconfig up / btmgmt poweron operation? Or is this something that should be actually happen only once during USB probe callback?

Regards

Marcel

Re: [PATCH] bt usb-device initialize as wakeup enable

[Oliver Neukum]

Am Montag, den 14.08.2017, 08:10 +0200 schrieb Marcel Holtmann:
> Hi Sukumar,
> 
> > 
> > BT-Controller connected as platform non-root-hub device and
> > usb-driver initialize such device with wakeup disabled,
> > Ref. usb_new_device().
> > 
> > At present wakeup-capability get enabled by hid-input device from usb
> > function driver(e.g. BT HID device) at runtime. Again some functional
> > driver does not set usb-wakeup capability(e.g LE HID device implement
> > as HID-over-GATT), and can't wakeup the host on USB.
> > 
> > Hence usb-wakeup capability enable form btusc driver as a generic
> > solution for multiple profile usecase and required for USB remote
> > wakeup (in-bus wakeup) while host is suspended.
> > 
> > Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> > Signed-off-by: Amit K Bag <amit.k.bag@intel.com>#
> > ---
> > drivers/bluetooth/btusb.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index e1124ba44154..7eeeeb6ffae6 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -1083,6 +1083,10 @@ static int btusb_open(struct hci_dev *hdev)
> > 	}
> > 
> > 	data->intf->needs_remote_wakeup = 1;
> > +	/* device specific wakeup source enabled and required for USB
> > +	 * remote wakeup while host is suspended
> > +	 */
> > +	device_init_wakeup(&data->udev->dev, true);
> > 
> 
> is this the right location? Are we repeating this on every hciconfig up / btmgmt poweron operation? Or is this something that should be actually happen only once during USB probe callback?

Hi,

it would be more efficient to do that on probe(). But have you tested this for
side effects? Will other BT operations now wake up the host?

On a philosophical note it is kind of ugly to solve a generic problem
in btusb. Arguably wakeup capability should propagate up the device tree
if you request it. Have you discussed this with Rafael?

	Regards
		Oliver

RE: [PATCH] bt usb-device initialize as wakeup enable

[Ghorai, Sukumar]

Pj4gPg0KPj4gPiBCVC1Db250cm9sbGVyIGNvbm5lY3RlZCBhcyBwbGF0Zm9ybSBub24tcm9vdC1o
dWIgZGV2aWNlIGFuZA0KPj4gPiB1c2ItZHJpdmVyIGluaXRpYWxpemUgc3VjaCBkZXZpY2Ugd2l0
aCB3YWtldXAgZGlzYWJsZWQsIFJlZi4NCj4+ID4gdXNiX25ld19kZXZpY2UoKS4NCj4+ID4NCj4+
ID4gQXQgcHJlc2VudCB3YWtldXAtY2FwYWJpbGl0eSBnZXQgZW5hYmxlZCBieSBoaWQtaW5wdXQg
ZGV2aWNlIGZyb20NCj4+ID4gdXNiIGZ1bmN0aW9uIGRyaXZlcihlLmcuIEJUIEhJRCBkZXZpY2Up
IGF0IHJ1bnRpbWUuIEFnYWluIHNvbWUNCj4+ID4gZnVuY3Rpb25hbCBkcml2ZXIgZG9lcyBub3Qg
c2V0IHVzYi13YWtldXAgY2FwYWJpbGl0eShlLmcgTEUgSElEDQo+PiA+IGRldmljZSBpbXBsZW1l
bnQgYXMgSElELW92ZXItR0FUVCksIGFuZCBjYW4ndCB3YWtldXAgdGhlIGhvc3Qgb24gVVNCLg0K
Pj4gPg0KPj4gPiBIZW5jZSB1c2Itd2FrZXVwIGNhcGFiaWxpdHkgZW5hYmxlIGZvcm0gYnR1c2Mg
ZHJpdmVyIGFzIGEgZ2VuZXJpYw0KPj4gPiBzb2x1dGlvbiBmb3IgbXVsdGlwbGUgcHJvZmlsZSB1
c2VjYXNlIGFuZCByZXF1aXJlZCBmb3IgVVNCIHJlbW90ZQ0KPj4gPiB3YWtldXAgKGluLWJ1cyB3
YWtldXApIHdoaWxlIGhvc3QgaXMgc3VzcGVuZGVkLg0KPj4gPg0KPj4gPiBTaWduZWQtb2ZmLWJ5
OiBTdWt1bWFyIEdob3JhaSA8c3VrdW1hci5naG9yYWlAaW50ZWwuY29tPg0KPj4gPiBTaWduZWQt
b2ZmLWJ5OiBBbWl0IEsgQmFnIDxhbWl0LmsuYmFnQGludGVsLmNvbT4jDQo+PiA+IC0tLQ0KPj4g
PiBkcml2ZXJzL2JsdWV0b290aC9idHVzYi5jIHwgNCArKysrDQo+PiA+IDEgZmlsZSBjaGFuZ2Vk
LCA0IGluc2VydGlvbnMoKykNCj4+ID4NCj4+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmx1ZXRv
b3RoL2J0dXNiLmMgYi9kcml2ZXJzL2JsdWV0b290aC9idHVzYi5jDQo+PiA+IGluZGV4IGUxMTI0
YmE0NDE1NC4uN2VlZWViNmZmYWU2IDEwMDY0NA0KPj4gPiAtLS0gYS9kcml2ZXJzL2JsdWV0b290
aC9idHVzYi5jDQo+PiA+ICsrKyBiL2RyaXZlcnMvYmx1ZXRvb3RoL2J0dXNiLmMNCj4+ID4gQEAg
LTEwODMsNiArMTA4MywxMCBAQCBzdGF0aWMgaW50IGJ0dXNiX29wZW4oc3RydWN0IGhjaV9kZXYg
KmhkZXYpDQo+PiA+IAl9DQo+PiA+DQo+PiA+IAlkYXRhLT5pbnRmLT5uZWVkc19yZW1vdGVfd2Fr
ZXVwID0gMTsNCj4+ID4gKwkvKiBkZXZpY2Ugc3BlY2lmaWMgd2FrZXVwIHNvdXJjZSBlbmFibGVk
IGFuZCByZXF1aXJlZCBmb3IgVVNCDQo+PiA+ICsJICogcmVtb3RlIHdha2V1cCB3aGlsZSBob3N0
IGlzIHN1c3BlbmRlZA0KPj4gPiArCSAqLw0KPj4gPiArCWRldmljZV9pbml0X3dha2V1cCgmZGF0
YS0+dWRldi0+ZGV2LCB0cnVlKTsNCj4+ID4NCj4+DQo+PiBpcyB0aGlzIHRoZSByaWdodCBsb2Nh
dGlvbj8gQXJlIHdlIHJlcGVhdGluZyB0aGlzIG9uIGV2ZXJ5IGhjaWNvbmZpZyB1cCAvIGJ0bWdt
dA0KPnBvd2Vyb24gb3BlcmF0aW9uPyBPciBpcyB0aGlzIHNvbWV0aGluZyB0aGF0IHNob3VsZCBi
ZSBhY3R1YWxseSBoYXBwZW4gb25seSBvbmNlDQo+ZHVyaW5nIFVTQiBwcm9iZSBjYWxsYmFjaz8N
CltTdWt1bWFyXSBhZ3JlZS4uIGRldmljZV93YWtldXBfZW5hYmxlL19kaXNhYmxlKCkgbmVlZCB0
byBjYWxsIGF0DQpidHVzYl9vcGVuL19jbG9zZSgpIHJlc3BlY3RpdmVseS4gdGhpcyB3aWxsIHRv
Z2dsZSB0aGUgZmxhZyANCi9zeXMvYnVzL3VzYi9kZXZpY2VzL3h4L3Bvd2VyL3dha2V1cC4gDQpX
aWxsIGNvcnJlY3QgaW4gcGF0Y2ggdjIuDQoNCj4NCj5IaSwNCj4NCj5pdCB3b3VsZCBiZSBtb3Jl
IGVmZmljaWVudCB0byBkbyB0aGF0IG9uIHByb2JlKCkuIEJ1dCBoYXZlIHlvdSB0ZXN0ZWQgdGhp
cyBmb3Igc2lkZQ0KPmVmZmVjdHM/IFdpbGwgb3RoZXIgQlQgb3BlcmF0aW9ucyBub3cgd2FrZSB1
cCB0aGUgaG9zdD8NCj4NCj5PbiBhIHBoaWxvc29waGljYWwgbm90ZSBpdCBpcyBraW5kIG9mIHVn
bHkgdG8gc29sdmUgYSBnZW5lcmljIHByb2JsZW0gaW4gYnR1c2IuDQo+QXJndWFibHkgd2FrZXVw
IGNhcGFiaWxpdHkgc2hvdWxkIHByb3BhZ2F0ZSB1cCB0aGUgZGV2aWNlIHRyZWUgaWYgeW91IHJl
cXVlc3QgaXQuDQo+SGF2ZSB5b3UgZGlzY3Vzc2VkIHRoaXMgd2l0aCBSYWZhZWw/DQpbU3VrdW1h
cl0gV2UgbmVlZCB0aGUgYnQtY29udHJvbGxlciB0byBiZSB3YWtlLXVwIGVuYWJsZSwgd2hpbGUg
aG9zdCBpcyBzdXNwZW5kZWQuDQpBbmQgZXZlbiB3aGVuIG5vIHByb2ZpbGUvcHJvdG9jb2wgaXMg
ZW5hYmxlZCBhbmQgdG8gYWNjZXB0IHRoZSByZW1vdGUgY29ubmVjdGlvbiwgDQpmb3IgZXhhbXBs
ZS4NCg0KPg0KPglSZWdhcmRzDQo+CQlPbGl2ZXINCg0K

[PATCH] bt usb-device initialize as wakeup enable

[Sukumar Ghorai]

BT-Controller connected as platform non-root-hub device and
usb-driver initialize such device with wakeup disabled,
Ref. usb_new_device().

At present wakeup-capability get enabled by hid-input device from usb
function driver(e.g. BT HID device) at runtime. Again some functional
driver does not set usb-wakeup capability(e.g LE HID device implement
as HID-over-GATT), and can't wakeup the host on USB.

Hence usb-wakeup capability enable form btusc driver as a generic
solution for multiple profile usecase and required for USB remote
wakeup (in-bus wakeup) while host is suspended.

Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
Signed-off-by: Amit K Bag <amit.k.bag@intel.com>#
---
 drivers/bluetooth/btusb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index e1124ba44154..7eeeeb6ffae6 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1083,6 +1083,10 @@ static int btusb_open(struct hci_dev *hdev)
 	}
 
 	data->intf->needs_remote_wakeup = 1;
+	/* device specific wakeup source enabled and required for USB
+	 * remote wakeup while host is suspended
+	 */
+	device_init_wakeup(&data->udev->dev, true);
 
 	if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
 		goto done;
-- 
2.7.4