Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH v3 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-31  6:32 UTC (permalink / raw)
  To: Linux Bluetooth; +Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu
In-Reply-To: <1490941952-9048-1-git-send-email-huxm@marvell.com>

Sanity check of interrupt number in interrupt handler is unnecessary and
confusion, remove it.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: fix apply error in v1
v3: use Reported-by (Guenter)
---
 drivers/bluetooth/btmrvl_sdio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 95e40ec..eb794f0 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -64,11 +64,9 @@ static irqreturn_t btmrvl_wake_irq_bt(int irq, void *priv)
 	struct btmrvl_sdio_card *card = priv;
 	struct btmrvl_plt_wake_cfg *cfg = card->plt_wake_cfg;
 
-	if (cfg->irq_bt >= 0) {
-		pr_info("%s: wake by bt", __func__);
-		cfg->wake_by_bt = true;
-		disable_irq_nosync(irq);
-	}
+	pr_info("%s: wake by bt", __func__);
+	cfg->wake_by_bt = true;
+	disable_irq_nosync(irq);
 
 	pm_wakeup_event(&card->func->dev, 0);
 	pm_system_wakeup();
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH v3 1/2] Bluetooth: btmrvl: disable platform wakeup interrupt in suspend failure path
From: Xinming Hu @ 2017-03-31  6:32 UTC (permalink / raw)
  To: Linux Bluetooth; +Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu

Host sleep handshake with device might been fail, disable platform wakeup
interrupt in this case.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: same as v1
v3: use Reported-by (Guenter)
---
 drivers/bluetooth/btmrvl_sdio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 2424ea2..95e40ec 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -1630,6 +1630,13 @@ static int btmrvl_sdio_suspend(struct device *dev)
 	if (priv->adapter->hs_state != HS_ACTIVATED) {
 		if (btmrvl_enable_hs(priv)) {
 			BT_ERR("HS not activated, suspend failed!");
+			/* Disable platform specific wakeup interrupt */
+			if (card->plt_wake_cfg &&
+			    card->plt_wake_cfg->irq_bt >= 0) {
+				disable_irq_wake(card->plt_wake_cfg->irq_bt);
+				disable_irq(card->plt_wake_cfg->irq_bt);
+			}
+
 			priv->adapter->is_suspending = false;
 			return -EBUSY;
 		}
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH v2 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-31  6:31 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Linux Bluetooth, Amitkumar Karwar, Cathy Luo, Guenter Roeck
In-Reply-To: <21885C64-CE14-49C9-AB01-7EA44027B245@holtmann.org>

Hi Marcel,=0A=
________________________________________=0A=
=0A=
=0A=
>Hi Xinming,=0A=
=0A=
>> Sanity check of interrupt number in interrupt handler is unnecessary and=
=0A=
>> confusion, remove it.=0A=
>>=0A=
>> Signed-off-by: Xinming Hu <huxm@marvell.com>=0A=
>> Signed-off-by: Guenter Roeck <groeck@chromium.org>=0A=
>> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>=0A=
>> ---=0A=
>> v2: fix apply error in v1=0A=
>> ---=0A=
>> drivers/bluetooth/btmrvl_sdio.c | 8 +++-----=0A=
>> 1 file changed, 3 insertions(+), 5 deletions(-)=0A=
=0A=
>patch has been applied to bluetooth-next tree.=0A=
=0A=
Thanks ! we might need send V3 patches based on Guenter's comments.=0A=
=0A=
Regards,=0A=
Simon=0A=
=0A=
>Regards=0A=
=0A=
>Marcel=0A=
=0A=

^ permalink raw reply

* Re: [EXT] Re: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Guenter Roeck @ 2017-03-30 15:00 UTC (permalink / raw)
  To: Xinming Hu
  Cc: Linux Bluetooth, Marcel Holtmann, Amitkumar Karwar, Cathy Luo,
	Guenter Roeck
In-Reply-To: <aa6d3cab51e14de2b467e9da07296275@SC-EXCH02.marvell.com>

On Thu, Mar 30, 2017 at 6:17 AM, Xinming Hu <huxm@marvell.com> wrote:
> Hi Guenter,
>
>> -----Original Message-----
>> From: Guenter Roeck [mailto:groeck@google.com]
>> Sent: 2017=E5=B9=B43=E6=9C=8830=E6=97=A5 20:58
>> To: Xinming Hu
>> Cc: Linux Bluetooth; Marcel Holtmann; Amitkumar Karwar; Cathy Luo; Guent=
er
>> Roeck
>> Subject: [EXT] Re: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wak=
eup
>> interrupt number sanity check
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> On Thu, Mar 30, 2017 at 5:52 AM, Xinming Hu <huxm@marvell.com> wrote:
>> > Sanity check of interrupt number in interrupt handler is unnecessary
>> > and confusion, remove it.
>> >
>> > Signed-off-by: Xinming Hu <huxm@marvell.com>
>> > Signed-off-by: Guenter Roeck <groeck@chromium.org>
>>
>> Do you possibly mean "Reported-by:" ?
>
> The patch is prepared based on your suggestions on https://issuetracker.g=
oogle.com/issues/35647652
>

This means that either "Reported-by:" or "Suggested-by:" would be
appropriate, but not "Signed-off-by:".

Thanks,
Guenter

> Thanks
> Simon
>>
>> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
>> > ---
>> >  drivers/bluetooth/btmrvl_sdio.c | 9 +++------
>> >  1 file changed, 3 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/bluetooth/btmrvl_sdio.c
>> > b/drivers/bluetooth/btmrvl_sdio.c index 6166888..7a26961 100644
>> > --- a/drivers/bluetooth/btmrvl_sdio.c
>> > +++ b/drivers/bluetooth/btmrvl_sdio.c
>> > @@ -62,12 +62,9 @@ static irqreturn_t btmrvl_wake_irq_bt(int irq, void
>> > *priv)  {
>> >         struct btmrvl_plt_wake_cfg *cfg =3D priv;
>> >
>> > -       if (cfg->irq_bt >=3D 0) {
>> > -               pr_info("%s: wake by bt", __func__);
>> > -               cfg->wake_by_bt =3D true;
>> > -               disable_irq_nosync(irq);
>> > -       }
>> > -
>> > +       pr_info("%s: wake by bt", __func__);
>> > +       cfg->wake_by_bt =3D true;
>> > +       disable_irq_nosync(irq);
>> >         return IRQ_HANDLED;
>> >  }
>> >
>> > --
>> > 1.8.1.4
>> >

^ permalink raw reply

* Re: [PATCH v2 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Marcel Holtmann @ 2017-03-30 14:44 UTC (permalink / raw)
  To: Xinming Hu; +Cc: Linux Bluetooth, Amitkumar Karwar, Cathy Luo, Guenter Roeck
In-Reply-To: <1490884636-18684-2-git-send-email-huxm@marvell.com>

Hi Xinming,

> Sanity check of interrupt number in interrupt handler is unnecessary and
> confusion, remove it.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: fix apply error in v1
> ---
> drivers/bluetooth/btmrvl_sdio.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* RE: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-30 14:38 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Linux Bluetooth, Amitkumar Karwar, Cathy Luo, Guenter Roeck
In-Reply-To: <19C158FE-9A3F-4B4C-9173-ECAC99304B4A@holtmann.org>

SGkgTWFyY2VsLA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGxpbnV4
LWJsdWV0b290aC1vd25lckB2Z2VyLmtlcm5lbC5vcmcNCj4gW21haWx0bzpsaW51eC1ibHVldG9v
dGgtb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgTWFyY2VsDQo+IEhvbHRtYW5u
DQo+IFNlbnQ6IDIwMTfE6jPUwjMwyNUgMjI6MjENCj4gVG86IFhpbm1pbmcgSHUNCj4gQ2M6IExp
bnV4IEJsdWV0b290aDsgQW1pdGt1bWFyIEthcndhcjsgQ2F0aHkgTHVvOyBHdWVudGVyIFJvZWNr
DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMi8yXSBCbHVldG9vdGg6IGJ0bXJ2bDogcmVtb3ZlIHVu
bmVjZXNzYXJ5IHdha2V1cA0KPiBpbnRlcnJ1cHQgbnVtYmVyIHNhbml0eSBjaGVjaw0KPiANCj4g
SGkgWGlubWluZywNCj4gDQo+ID4gU2FuaXR5IGNoZWNrIG9mIGludGVycnVwdCBudW1iZXIgaW4g
aW50ZXJydXB0IGhhbmRsZXIgaXMgdW5uZWNlc3NhcnkNCj4gPiBhbmQgY29uZnVzaW9uLCByZW1v
dmUgaXQuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBYaW5taW5nIEh1IDxodXhtQG1hcnZlbGwu
Y29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IEd1ZW50ZXIgUm9lY2sgPGdyb2Vja0BjaHJvbWl1bS5v
cmc+DQo+ID4gU2lnbmVkLW9mZi1ieTogQW1pdGt1bWFyIEthcndhciA8YWthcndhckBtYXJ2ZWxs
LmNvbT4NCj4gPiAtLS0NCj4gPiBkcml2ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5jIHwgOSAr
KystLS0tLS0NCj4gPiAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9u
cygtKQ0KPiANCj4gcGF0Y2ggZG9lcyBub3QgYXBwbHkgdG8gYmx1ZXRvb3RoLW5leHQgdHJlZS4N
Cj4gDQo+IGVycm9yOiBkcml2ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5jOiBwYXRjaCBkb2Vz
IG5vdCBhcHBseSBQYXRjaCBmYWlsZWQgYXQNCj4gMDAwMiBCbHVldG9vdGg6IGJ0bXJ2bDogcmVt
b3ZlIHVubmVjZXNzYXJ5IHdha2V1cCBpbnRlcnJ1cHQgbnVtYmVyIHNhbml0eQ0KPiBjaGVjaw0K
DQpSZXNvbHZlZCBpbiB2Mi4NCg0KVGhhbmtzDQpTaW1vbg0KPiANCj4gUmVnYXJkcw0KPiANCj4g
TWFyY2VsDQo+IA0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0
aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtYmx1ZXRvb3RoIiBpbg0KPiB0aGUgYm9keSBvZiBh
IG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZyBNb3JlIG1ham9yZG9tbyBpbmZv
DQo+IGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCg==

^ permalink raw reply

* [PATCH v2 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-30 14:37 UTC (permalink / raw)
  To: Linux Bluetooth
  Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu,
	Guenter Roeck
In-Reply-To: <1490884636-18684-1-git-send-email-huxm@marvell.com>

Sanity check of interrupt number in interrupt handler is unnecessary and
confusion, remove it.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: fix apply error in v1
---
 drivers/bluetooth/btmrvl_sdio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 95e40ec..eb794f0 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -64,11 +64,9 @@ static irqreturn_t btmrvl_wake_irq_bt(int irq, void *priv)
 	struct btmrvl_sdio_card *card = priv;
 	struct btmrvl_plt_wake_cfg *cfg = card->plt_wake_cfg;
 
-	if (cfg->irq_bt >= 0) {
-		pr_info("%s: wake by bt", __func__);
-		cfg->wake_by_bt = true;
-		disable_irq_nosync(irq);
-	}
+	pr_info("%s: wake by bt", __func__);
+	cfg->wake_by_bt = true;
+	disable_irq_nosync(irq);
 
 	pm_wakeup_event(&card->func->dev, 0);
 	pm_system_wakeup();
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH v2 1/2] Bluetooth: btmrvl: disable platform wakeup interrupt in suspend failure path
From: Xinming Hu @ 2017-03-30 14:37 UTC (permalink / raw)
  To: Linux Bluetooth
  Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu,
	Guenter Roeck

Host sleep handshake with device might been fail, disable platform wakeup
interrupt in this case.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: same as v1
---
 drivers/bluetooth/btmrvl_sdio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 2424ea2..95e40ec 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -1630,6 +1630,13 @@ static int btmrvl_sdio_suspend(struct device *dev)
 	if (priv->adapter->hs_state != HS_ACTIVATED) {
 		if (btmrvl_enable_hs(priv)) {
 			BT_ERR("HS not activated, suspend failed!");
+			/* Disable platform specific wakeup interrupt */
+			if (card->plt_wake_cfg &&
+			    card->plt_wake_cfg->irq_bt >= 0) {
+				disable_irq_wake(card->plt_wake_cfg->irq_bt);
+				disable_irq(card->plt_wake_cfg->irq_bt);
+			}
+
 			priv->adapter->is_suspending = false;
 			return -EBUSY;
 		}
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH 1/2] Bluetooth: btmrvl: disable platform wakeup interrupt in suspend failure path
From: Marcel Holtmann @ 2017-03-30 14:21 UTC (permalink / raw)
  To: Xinming Hu; +Cc: Linux Bluetooth, Amitkumar Karwar, Cathy Luo, Guenter Roeck
In-Reply-To: <1490878364-2735-1-git-send-email-huxm@marvell.com>

Hi Xinming,

> Host sleep handshake with device might been fail, disable platform wakeup
> interrupt in this case.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> drivers/bluetooth/btmrvl_sdio.c | 7 +++++++
> 1 file changed, 7 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Marcel Holtmann @ 2017-03-30 14:20 UTC (permalink / raw)
  To: Xinming Hu; +Cc: Linux Bluetooth, Amitkumar Karwar, Cathy Luo, Guenter Roeck
In-Reply-To: <1490878364-2735-2-git-send-email-huxm@marvell.com>

Hi Xinming,

> Sanity check of interrupt number in interrupt handler is unnecessary and
> confusion, remove it.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> drivers/bluetooth/btmrvl_sdio.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)

patch does not apply to bluetooth-next tree.

error: drivers/bluetooth/btmrvl_sdio.c: patch does not apply
Patch failed at 0002 Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check

Regards

Marcel


^ permalink raw reply

* RE: [EXT] Re: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-30 13:17 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Linux Bluetooth, Marcel Holtmann, Amitkumar Karwar, Cathy Luo,
	Guenter Roeck
In-Reply-To: <CABXOdTeG0G0Fjww49U0fbyPzpo9czP21YoTdUwyQ+ar4A756cg@mail.gmail.com>

SGkgR3VlbnRlciwNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBHdWVu
dGVyIFJvZWNrIFttYWlsdG86Z3JvZWNrQGdvb2dsZS5jb21dDQo+IFNlbnQ6IDIwMTflubQz5pyI
MzDml6UgMjA6NTgNCj4gVG86IFhpbm1pbmcgSHUNCj4gQ2M6IExpbnV4IEJsdWV0b290aDsgTWFy
Y2VsIEhvbHRtYW5uOyBBbWl0a3VtYXIgS2Fyd2FyOyBDYXRoeSBMdW87IEd1ZW50ZXINCj4gUm9l
Y2sNCj4gU3ViamVjdDogW0VYVF0gUmU6IFtQQVRDSCAyLzJdIEJsdWV0b290aDogYnRtcnZsOiBy
ZW1vdmUgdW5uZWNlc3Nhcnkgd2FrZXVwDQo+IGludGVycnVwdCBudW1iZXIgc2FuaXR5IGNoZWNr
DQo+IA0KPiBFeHRlcm5hbCBFbWFpbA0KPiANCj4gLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiBPbiBUaHUsIE1h
ciAzMCwgMjAxNyBhdCA1OjUyIEFNLCBYaW5taW5nIEh1IDxodXhtQG1hcnZlbGwuY29tPiB3cm90
ZToNCj4gPiBTYW5pdHkgY2hlY2sgb2YgaW50ZXJydXB0IG51bWJlciBpbiBpbnRlcnJ1cHQgaGFu
ZGxlciBpcyB1bm5lY2Vzc2FyeQ0KPiA+IGFuZCBjb25mdXNpb24sIHJlbW92ZSBpdC4NCj4gPg0K
PiA+IFNpZ25lZC1vZmYtYnk6IFhpbm1pbmcgSHUgPGh1eG1AbWFydmVsbC5jb20+DQo+ID4gU2ln
bmVkLW9mZi1ieTogR3VlbnRlciBSb2VjayA8Z3JvZWNrQGNocm9taXVtLm9yZz4NCj4gDQo+IERv
IHlvdSBwb3NzaWJseSBtZWFuICJSZXBvcnRlZC1ieToiID8NCg0KVGhlIHBhdGNoIGlzIHByZXBh
cmVkIGJhc2VkIG9uIHlvdXIgc3VnZ2VzdGlvbnMgb24gaHR0cHM6Ly9pc3N1ZXRyYWNrZXIuZ29v
Z2xlLmNvbS9pc3N1ZXMvMzU2NDc2NTINCg0KVGhhbmtzDQpTaW1vbg0KPiANCj4gPiBTaWduZWQt
b2ZmLWJ5OiBBbWl0a3VtYXIgS2Fyd2FyIDxha2Fyd2FyQG1hcnZlbGwuY29tPg0KPiA+IC0tLQ0K
PiA+ICBkcml2ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5jIHwgOSArKystLS0tLS0NCj4gPiAg
MSBmaWxlIGNoYW5nZWQsIDMgaW5zZXJ0aW9ucygrKSwgNiBkZWxldGlvbnMoLSkNCj4gPg0KPiA+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5jDQo+ID4gYi9kcml2
ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5jIGluZGV4IDYxNjY4ODguLjdhMjY5NjEgMTAwNjQ0
DQo+ID4gLS0tIGEvZHJpdmVycy9ibHVldG9vdGgvYnRtcnZsX3NkaW8uYw0KPiA+ICsrKyBiL2Ry
aXZlcnMvYmx1ZXRvb3RoL2J0bXJ2bF9zZGlvLmMNCj4gPiBAQCAtNjIsMTIgKzYyLDkgQEAgc3Rh
dGljIGlycXJldHVybl90IGJ0bXJ2bF93YWtlX2lycV9idChpbnQgaXJxLCB2b2lkDQo+ID4gKnBy
aXYpICB7DQo+ID4gICAgICAgICBzdHJ1Y3QgYnRtcnZsX3BsdF93YWtlX2NmZyAqY2ZnID0gcHJp
djsNCj4gPg0KPiA+IC0gICAgICAgaWYgKGNmZy0+aXJxX2J0ID49IDApIHsNCj4gPiAtICAgICAg
ICAgICAgICAgcHJfaW5mbygiJXM6IHdha2UgYnkgYnQiLCBfX2Z1bmNfXyk7DQo+ID4gLSAgICAg
ICAgICAgICAgIGNmZy0+d2FrZV9ieV9idCA9IHRydWU7DQo+ID4gLSAgICAgICAgICAgICAgIGRp
c2FibGVfaXJxX25vc3luYyhpcnEpOw0KPiA+IC0gICAgICAgfQ0KPiA+IC0NCj4gPiArICAgICAg
IHByX2luZm8oIiVzOiB3YWtlIGJ5IGJ0IiwgX19mdW5jX18pOw0KPiA+ICsgICAgICAgY2ZnLT53
YWtlX2J5X2J0ID0gdHJ1ZTsNCj4gPiArICAgICAgIGRpc2FibGVfaXJxX25vc3luYyhpcnEpOw0K
PiA+ICAgICAgICAgcmV0dXJuIElSUV9IQU5ETEVEOw0KPiA+ICB9DQo+ID4NCj4gPiAtLQ0KPiA+
IDEuOC4xLjQNCj4gPg0K

^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Guenter Roeck @ 2017-03-30 12:57 UTC (permalink / raw)
  To: Xinming Hu
  Cc: Linux Bluetooth, Marcel Holtmann, Amitkumar Karwar, Cathy Luo,
	Guenter Roeck
In-Reply-To: <1490878364-2735-2-git-send-email-huxm@marvell.com>

On Thu, Mar 30, 2017 at 5:52 AM, Xinming Hu <huxm@marvell.com> wrote:
> Sanity check of interrupt number in interrupt handler is unnecessary and
> confusion, remove it.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>

Do you possibly mean "Reported-by:" ?

> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
>  drivers/bluetooth/btmrvl_sdio.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> index 6166888..7a26961 100644
> --- a/drivers/bluetooth/btmrvl_sdio.c
> +++ b/drivers/bluetooth/btmrvl_sdio.c
> @@ -62,12 +62,9 @@ static irqreturn_t btmrvl_wake_irq_bt(int irq, void *priv)
>  {
>         struct btmrvl_plt_wake_cfg *cfg = priv;
>
> -       if (cfg->irq_bt >= 0) {
> -               pr_info("%s: wake by bt", __func__);
> -               cfg->wake_by_bt = true;
> -               disable_irq_nosync(irq);
> -       }
> -
> +       pr_info("%s: wake by bt", __func__);
> +       cfg->wake_by_bt = true;
> +       disable_irq_nosync(irq);
>         return IRQ_HANDLED;
>  }
>
> --
> 1.8.1.4
>

^ permalink raw reply

* [PATCH 2/2] Bluetooth: btmrvl: remove unnecessary wakeup interrupt number sanity check
From: Xinming Hu @ 2017-03-30 12:52 UTC (permalink / raw)
  To: Linux Bluetooth
  Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu,
	Guenter Roeck
In-Reply-To: <1490878364-2735-1-git-send-email-huxm@marvell.com>

Sanity check of interrupt number in interrupt handler is unnecessary and
confusion, remove it.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/bluetooth/btmrvl_sdio.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 6166888..7a26961 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -62,12 +62,9 @@ static irqreturn_t btmrvl_wake_irq_bt(int irq, void *priv)
 {
 	struct btmrvl_plt_wake_cfg *cfg = priv;
 
-	if (cfg->irq_bt >= 0) {
-		pr_info("%s: wake by bt", __func__);
-		cfg->wake_by_bt = true;
-		disable_irq_nosync(irq);
-	}
-
+	pr_info("%s: wake by bt", __func__);
+	cfg->wake_by_bt = true;
+	disable_irq_nosync(irq);
 	return IRQ_HANDLED;
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH 1/2] Bluetooth: btmrvl: disable platform wakeup interrupt in suspend failure path
From: Xinming Hu @ 2017-03-30 12:52 UTC (permalink / raw)
  To: Linux Bluetooth
  Cc: Marcel Holtmann, Amitkumar Karwar, Cathy Luo, Xinming Hu,
	Guenter Roeck

Host sleep handshake with device might been fail, disable platform wakeup
interrupt in this case.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/bluetooth/btmrvl_sdio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 08e01f0..6166888 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -1625,6 +1625,13 @@ static int btmrvl_sdio_suspend(struct device *dev)
 	if (priv->adapter->hs_state != HS_ACTIVATED) {
 		if (btmrvl_enable_hs(priv)) {
 			BT_ERR("HS not activated, suspend failed!");
+			/* Disable platform specific wakeup interrupt */
+			if (card->plt_wake_cfg &&
+			    card->plt_wake_cfg->irq_bt >= 0) {
+				disable_irq_wake(card->plt_wake_cfg->irq_bt);
+				disable_irq(card->plt_wake_cfg->irq_bt);
+			}
+
 			priv->adapter->is_suspending = false;
 			return -EBUSY;
 		}
-- 
1.8.1.4

^ permalink raw reply related

* Re: [RFC V1 04/16] Bluetooth: hci_ldisc: Add HCI RESET comment to hci_unregister_dev() call
From: Marcel Holtmann @ 2017-03-30 10:11 UTC (permalink / raw)
  To: Dean Jenkins; +Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth
In-Reply-To: <1490723429-28870-5-git-send-email-Dean_Jenkins@mentor.com>

Hi Dean,

> This commit relates to the HCI UART quirk HCI_QUIRK_RESET_ON_CLOSE
> which is enabled by default and can be disabled by setting hdev_flags
> flag HCI_UART_RESET_ON_INIT via ioctl HCIUARTSETFLAGS from userland.
> 
> The implementation of HCI_QUIRK_RESET_ON_CLOSE is broken for the BCSP
> Data Link protocol layer because it can be seen that
> hci_unregister_dev() takes 2 seconds to run due to BCSP communications
> failure. Suspect that sending of the HCI RESET command is broken
> for the other Data Link protocols as well.
> 
> Therefore, add a comment to inform that the call to
> hci_unregister_dev() in hci_uart_tty_close() may send a HCI RESET
> command. This transmission needs the HCI UART driver to remain
> operational including having the Data Link protocol being bound to
> the HCI UART driver. If the transmission fails, then
> hci_unregister_dev() waits HCI_CMD_TIMEOUT (2) seconds for the
> timeout to occur which is undesirable.
> 
> The current software has a call to hci_unregister_dev(hdev) in
> hci_uart_tty_close() which can cause an attempt at sending the
> command HCI RESET to occur through the following callstack:
> 
> hci_uart_tty_close()
> hci_unregister_dev()
> hci_dev_do_close()
> __hci_req_sync() to queue up command HCI RESET
> which adds a work-item to the hdev->workqueue and waits 2 seconds
> for a response
> 
> Subsequently
> hdev->workqueue runs and processes the work-item by calling
> hci_cmd_work()
> hci_send_frame()
> hci_uart_send_frame() to enqueue into the Data Link protocol layer
> 
> No protocol response comes so hci_unregister_dev() takes 2 seconds to
> run!
> 
> In fact, this hidden transmission of the HCI RESET command is most
> likely the root cause of why hci_uart_tty_close() crashed in the past
> and was "fixed" with multiple workarounds in the past.
> 
> The underlying design flaw is that hci_uart_tty_close() is interfering
> with various aspects of transmitting and receiving HCI messages
> whilst hci_unregister_dev() is trying to use the Data Link protocol
> to send the HCI RESET command. Consequently, the design flaw
> causes the Data Link protocol to retransmit as no reply comes from
> the Bluetooth Radio module over the UART link.
> 
> Over the many years of "fixes", this caused the logic in
> hci_uart_tty_close() to become over-complex.
> 
> Signed-off-by: Dean Jenkins <Dean_Jenkins@mentor.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 1887ad4..c78c9dc 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -499,6 +499,12 @@ static void hci_uart_tty_close(struct tty_struct *tty)
> 	if (test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags)) {
> 		if (hdev) {
> 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
> +				/* Note hci_unregister_dev() may try to send
> +				 * a HCI RESET command. If the transmission
> +				 * fails then hci_unregister_dev() waits
> +				 * HCI_CMD_TIMEOUT (2) seconds for the timeout
> +				 * to occur.
> +				 */
> 				hci_unregister_dev(hdev);

lets put { } around this if statement. I know it is no needed, but we generally do that for clarity.

Regards

Marcel


^ permalink raw reply

* Re: [RFC V1 07/16] Bluetooth: hci_ldisc: Add protocol check to hci_uart_tx_wakeup()
From: Marcel Holtmann @ 2017-03-30 10:11 UTC (permalink / raw)
  To: Dean Jenkins; +Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth
In-Reply-To: <1490723429-28870-8-git-send-email-Dean_Jenkins@mentor.com>

Hi Dean,

> Before attempting to schedule a work-item onto hu->write_work in
> hci_uart_tx_wakeup(), check that the Data Link protocol layer is
> still bound to the HCI UART driver.
> 
> Failure to perform this protocol check causes a race condition between
> the work queue hu->write_work running hci_uart_write_work() and the
> Data Link protocol layer being unbound (closed) in hci_uart_tty_close().
> 
> Note hci_uart_tty_close() does have a "cancel_work_sync(&hu->write_work)"
> but it is ineffective because it cannot prevent work-items being added
> to hu->write_work after cancel_work_sync() has run.
> 
> Therefore, add a check for HCI_UART_PROTO_READY into hci_uart_tx_wakeup()
> which prevents scheduling of the work queue when HCI_UART_PROTO_READY
> is in the clear state. However, note a small race condition remains
> because the hci_uart_tx_wakeup() thread can run in parallel with the
> hci_uart_tty_close() thread so it is possible that a schedule of
> hu->write_work can occur when HCI_UART_PROTO_READY is cleared. A complete
> solution needs locking of the threads which is implemented in a future
> commit.
> 
> Signed-off-by: Dean Jenkins <Dean_Jenkins@mentor.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index c4b801b..a1bb4d64b9 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -125,15 +125,19 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
> 
> int hci_uart_tx_wakeup(struct hci_uart *hu)
> {
> +	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags))
> +		goto no_schedule;
> +
> 	if (test_and_set_bit(HCI_UART_SENDING, &hu->tx_state)) {
> 		set_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
> -		return 0;
> +		goto no_schedule;
> 	}
> 
> 	BT_DBG("");
> 
> 	schedule_work(&hu->write_work);
> 
> +no_schedule:
> 	return 0;
> }

so instead of calling no_schedule label, just keep calling return 0 directly.

Regards

Marcel


^ permalink raw reply

* Re: [RFC V1 00/16] hci_ldisc hci_uart_tty_close() fixes
From: Marcel Holtmann @ 2017-03-30 10:11 UTC (permalink / raw)
  To: Dean Jenkins; +Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth
In-Reply-To: <1490723429-28870-1-git-send-email-Dean_Jenkins@mentor.com>

Hi Dean,

> This is RFC patchset V1 which reorganises hci_uart_tty_close() to overcome a
> design flaw. I would like some comments on the changes.
> 
> Design Flaw
> ===========
> 
> An example callstack is as follows
> 
> Have Bluetooth running using a BCSP based UART Bluetooth Radio Module.
> 
> Now kill the userland hciattach program by doing
> killall hciattach

is there any chance we can convert BCSP support to run fully inside the kernel with the new parts we have put in. And with that then also use btattach. The split of some parts of BCSP in userspace seems never been a good idea.

I am a bit reluctant to change major hci_ldisc pieces because of just one broken protocol. Running BCSP fully in the kernel seems a better solution to deal with some of these issues.

Regards

Marcel


^ permalink raw reply

* Re: [EXT] Unexpected SMP Command 0x17
From: Marcel Holtmann @ 2017-03-30  6:13 UTC (permalink / raw)
  To: Wong, Joshua Weng Onn
  Cc: Szymon Janc, Avinash Kadam, Bluez mailing list, Wong, Mun choy,
	Zulqarnain, Adam
In-Reply-To: <E3B98393E6037849B63EA428240A6513607C80@PGSMSX103.gar.corp.intel.com>

Hi Joshua,

>>>> Yes, for secure connection the LTK is generated locally.
>>>> But issue here is observed that after Pairing is complete the key
>>>> distribution is not completed from Master.
>>>> 
>>>> i.e. After Slave sends the  "Signature key:" but Master doesn't
>>>> share any key.  Attached logs.
>>> I get that and that is clear from the logs. Something is stalling here
>>> and because of that, you run into the 30 seconds SMP timeout. We just
>>> need to know if the 4.9 kernel is doing this correctly. If so, then
>>> you can bi-sect that patch that fixes. Without proof that 4.9 is also
>>> broken, nobody will even bother to chase this down.
>> 
>> I think the problem here is race between ACL data and HCI events on USB
>> dongle...  We get initial slave keys but those get dropped due to encryption
>> changed event not being received yet. Since keys were silently dropped we later
>> on get unexpected SMP PDU and ignoring remaining keys as well which
>> eventually leads to SMP timeout.
>> 
>> If this is USB dongle (using btusd) then only (AFAIK) solution would be to have a
>> workaround for this inside chip (it would delay ACL data received right after
>> encryption change giving host time to handle encpryption change event).
>> Bluetooth specification for USB transport is unfortunatelly kinda broken.
>> 
>> --
>> pozdrawiam
>> Szymon Janc
> 
> Thank you for your reply. Your inputs are valuable to us in helping to debug the issue. Yes, we are indeed using the btusb kernel module and it is using a USB interface (Bluetooth over USB).
> 
> I noticed that when btmgmt settings are set to turn 'bredr off', the 'ssp' mode also turns off. Is this behavior expected to occur?
> My current settings are 'powered connectable discoverable bondable le secure-conn’

the SSP (Secure Simple Pairing) is a BR/EDR only feature. So when you disable BR/EDR, it will be disabled as well.

Regards

Marcel


^ permalink raw reply

* [Bug 64671] hci0 command 0x1009 tx timeout, bluez can't find adapter
From: bugzilla-daemon @ 2017-03-30  5:29 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <bug-64671-62941@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=64671

--- Comment #10 from fademind@gmail.com ---
(In reply to Maxim Moseychuk from comment #8)
> 
> Can someone test my patch? This bug not always reproduce, and i need help
> for test 1-2 week.
> 
> I can send raw patch, branch on github or PKGBUILD for archlinux.

Please send me your patch. Thanks

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* RE: [EXT] Re: Unexpected SMP Command 0x17
From: Wong, Joshua Weng Onn @ 2017-03-30  0:00 UTC (permalink / raw)
  To: Szymon Janc
  Cc: Avinash Kadam, Bluez mailing list, Wong, Mun choy,
	Zulqarnain, Adam, Marcel Holtmann
In-Reply-To: <1589262.TPhyyzNXdL@ix>

Hi Szymon,

> On Monday, 27 March 2017 16:55:05 CEST Marcel Holtmann wrote:
> > Hi Avinash,
> >
> > please also refrain from top posting.
> >
> > > Yes, for secure connection the LTK is generated locally.
> > > But issue here is observed that after Pairing is complete the key
> > > distribution is not completed from Master.
> > >
> > > i.e. After Slave sends the  "Signature key:" but Master doesn't
> > > share any key.  Attached logs.
> > I get that and that is clear from the logs. Something is stalling here
> > and because of that, you run into the 30 seconds SMP timeout. We just
> > need to know if the 4.9 kernel is doing this correctly. If so, then
> > you can bi-sect that patch that fixes. Without proof that 4.9 is also
> > broken, nobody will even bother to chase this down.
>=20
> I think the problem here is race between ACL data and HCI events on USB
> dongle...  We get initial slave keys but those get dropped due to encrypt=
ion
> changed event not being received yet. Since keys were silently dropped we=
 later
> on get unexpected SMP PDU and ignoring remaining keys as well which
> eventually leads to SMP timeout.
>=20
> If this is USB dongle (using btusd) then only (AFAIK) solution would be t=
o have a
> workaround for this inside chip (it would delay ACL data received right a=
fter
> encryption change giving host time to handle encpryption change event).
> Bluetooth specification for USB transport is unfortunatelly kinda broken.
>=20
> --
> pozdrawiam
> Szymon Janc

Thank you for your reply. Your inputs are valuable to us in helping to debu=
g the issue. Yes, we are indeed using the btusb kernel module and it is usi=
ng a USB interface (Bluetooth over USB).

I noticed that when btmgmt settings are set to turn 'bredr off', the 'ssp' =
mode also turns off. Is this behavior expected to occur?
My current settings are 'powered connectable discoverable bondable le secur=
e-conn'

Thank you.

Best regards,
Joshua



^ permalink raw reply

* [Bug 64671] hci0 command 0x1009 tx timeout, bluez can't find adapter
From: bugzilla-daemon @ 2017-03-29 22:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <bug-64671-62941@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=64671

--- Comment #9 from MOHAMMAD RASIM (mohammad.rasim96@gmail.com) ---
(In reply to Maxim Moseychuk from comment #8)
> (In reply to Ongun Kanat from comment #7)
> > I've got same behavior on 4.9.8 kernel on PC with Atheros AR3012 card.
> 
> (In reply to MOHAMMAD RASIM from comment #6)
> > I have the same issue and the same dmesg error.
> > 
> > My adapter is:
> > Broadcom Corporation BCM4313 802.11bgn Wireless Network Adapter [14e4:4727]
> > (rev 01)
> > I had the problem on archlinux but now I'm running linux mint and have the
> > same problem.
> > 
> > Current kernel:
> > Linux mohammad-mint 4.4.0-21-generic #37-Ubuntu SMP Mon Apr 18 18:33:37 UTC
> > 2016 x86_64 x86_64 x86_64 GNU/Linux
> 
> Can someone test my patch? This bug not always reproduce, and i need help
> for test 1-2 week.
> 
> I can send raw patch, branch on github or PKGBUILD for archlinux.

I can test the patch can you send it please?

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [PATCHv3 00/10] Nokia H4+ support
From: Marcel Holtmann @ 2017-03-29 21:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
	Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
	Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
	linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, David S. Miller
In-Reply-To: <CAL_JsqJcQ9ANtt71KRitOJ9spKD1SaeoL4RDftiCEPs8TjLJGA@mail.gmail.com>

Hi Rob,

>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>> they were queued by Tony. I also changed the patch order, so that the serdev
>> patches come first. All of them have Acked-by from Rob, so I think it makes
>> sense to merge them to serdev subsystem (now) and provide an immutable branch
>> for the bluetooth subsystem.
> 
> Greg doesn't read cover letters generally and since the serdev patches
> are Cc rather than To him, he's probably not planning to pick them up.

I wonder actually if we should merge all of these via bluetooth-next tree with proper Ack from Greg. However it would be good to also get buy in from Dave for merging this ultimately through net-next.

Regards

Marcel

^ permalink raw reply

* Re: [PATCHv3 00/10] Nokia H4+ support
From: Rob Herring @ 2017-03-29 21:17 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, Samuel Thibault,
	Pavel Machek, Tony Lindgren, Greg Kroah-Hartman, Jiri Slaby,
	Mark Rutland, open list:BLUETOOTH DRIVERS,
	linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170328155939.31566-1-sre@kernel.org>

On Tue, Mar 28, 2017 at 10:59 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> they were queued by Tony. I also changed the patch order, so that the serdev
> patches come first. All of them have Acked-by from Rob, so I think it makes
> sense to merge them to serdev subsystem (now) and provide an immutable branch
> for the bluetooth subsystem.

Greg doesn't read cover letters generally and since the serdev patches
are Cc rather than To him, he's probably not planning to pick them up.

Rob

^ permalink raw reply

* Re: [PATCH 0/2] Bluetooth: fix hci-uart crashes
From: Marcel Holtmann @ 2017-03-29 18:57 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-kernel
In-Reply-To: <20170329161528.17843-1-johan@kernel.org>

Hi Johan,

> Not every tty has a class device and this could be used to trigger
> NULL-pointer dereferences in two hci-uart drivers that lacked the
> required sanity checks.
> 
> Johan
> 
> 
> Johan Hovold (2):
>  Bluetooth: hci_bcm: add missing tty-device sanity check
>  Bluetooth: hci_intel: add missing tty-device sanity check
> 
> drivers/bluetooth/hci_bcm.c   |  5 ++++-
> drivers/bluetooth/hci_intel.c | 13 ++++++++++++-
> 2 files changed, 16 insertions(+), 2 deletions(-)

bot patches have been applied to bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* [PATCH 2/2] Bluetooth: hci_intel: add missing tty-device sanity check
From: Johan Hovold @ 2017-03-29 16:15 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: linux-bluetooth, linux-kernel, Johan Hovold, stable, Loic Poulain
In-Reply-To: <20170329161528.17843-1-johan@kernel.org>

Make sure to check the tty-device pointer before looking up the sibling
platform device to avoid dereferencing a NULL-pointer when the tty is
one end of a Unix98 pty.

Fixes: 74cdad37cd24 ("Bluetooth: hci_intel: Add runtime PM support")
Fixes: 1ab1f239bf17 ("Bluetooth: hci_intel: Add support for platform driver")
Cc: stable <stable@vger.kernel.org>     # 4.3
Cc: Loic Poulain <loic.poulain@intel.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/bluetooth/hci_intel.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 9e271286c5e5..73306384af6c 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -307,6 +307,9 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
 	struct list_head *p;
 	int err = -ENODEV;
 
+	if (!hu->tty->dev)
+		return err;
+
 	mutex_lock(&intel_device_list_lock);
 
 	list_for_each(p, &intel_device_list) {
@@ -379,6 +382,9 @@ static void intel_busy_work(struct work_struct *work)
 	struct intel_data *intel = container_of(work, struct intel_data,
 						busy_work);
 
+	if (!intel->hu->tty->dev)
+		return;
+
 	/* Link is busy, delay the suspend */
 	mutex_lock(&intel_device_list_lock);
 	list_for_each(p, &intel_device_list) {
@@ -889,6 +895,8 @@ static int intel_setup(struct hci_uart *hu)
 	list_for_each(p, &intel_device_list) {
 		struct intel_device *dev = list_entry(p, struct intel_device,
 						      list);
+		if (!hu->tty->dev)
+			break;
 		if (hu->tty->dev->parent == dev->pdev->dev.parent) {
 			if (device_may_wakeup(&dev->pdev->dev)) {
 				set_bit(STATE_LPM_ENABLED, &intel->flags);
@@ -1056,6 +1064,9 @@ static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 
 	BT_DBG("hu %p skb %p", hu, skb);
 
+	if (!hu->tty->dev)
+		goto out_enqueue;
+
 	/* Be sure our controller is resumed and potential LPM transaction
 	 * completed before enqueuing any packet.
 	 */
@@ -1072,7 +1083,7 @@ static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 		}
 	}
 	mutex_unlock(&intel_device_list_lock);
-
+out_enqueue:
 	skb_queue_tail(&intel->txq, skb);
 
 	return 0;
-- 
2.12.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox