[PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Remove magic and use standard HCI cmd timeout

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a2e15436c..e5a9a09 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
 		set_bit(HCI_INIT, &hdev->flags);
 		__hci_request(hdev, hci_reset_req, 0,
-			      msecs_to_jiffies(250));
+			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
-- 
1.7.9.5

[PATCHv1 2/4] Bluetooth: Add opcode to error message

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Sometimes HCI command sending timeouts and gives error message without
specifying which command causes error. Patch makes sure that opcode
is printed to help debugging.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e5a9a09..8f35f2e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1369,11 +1369,18 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr)
 }
 
 /* HCI command timer function */
-static void hci_cmd_timer(unsigned long arg)
+static void hci_cmd_timeout(unsigned long arg)
 {
 	struct hci_dev *hdev = (void *) arg;
 
-	BT_ERR("%s command tx timeout", hdev->name);
+	if (hdev->sent_cmd) {
+		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
+		u16 opcode = __le16_to_cpu(sent->opcode);
+
+		BT_ERR("%s command 0x%04x tx timeout", hdev->name, opcode);
+	} else
+		BT_ERR("%s command tx timeout", hdev->name);
+
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
 }
@@ -1673,7 +1680,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	init_waitqueue_head(&hdev->req_wait_q);
 
-	setup_timer(&hdev->cmd_timer, hci_cmd_timer, (unsigned long) hdev);
+	setup_timer(&hdev->cmd_timer, hci_cmd_timeout, (unsigned long) hdev);
 
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
-- 
1.7.9.5

[PATCHv1 3/4] Bluetooth: Add debug print specifier

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Some functions print opcode as "0xc03" others as "0x0c03". Patch
ensures that opcodes printed are the in the same format.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8f35f2e..c7be1bc 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2097,7 +2097,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	struct hci_command_hdr *hdr;
 	struct sk_buff *skb;
 
-	BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
+	BT_DBG("%s opcode 0x%04x plen %d", hdev->name, opcode, plen);
 
 	skb = bt_skb_alloc(len, GFP_ATOMIC);
 	if (!skb) {
@@ -2264,7 +2264,7 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)
 	if (hdr->opcode != cpu_to_le16(opcode))
 		return NULL;
 
-	BT_DBG("%s opcode 0x%x", hdev->name, opcode);
+	BT_DBG("%s opcode 0x%04x", hdev->name, opcode);
 
 	return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE;
 }
-- 
1.7.9.5

Re: [PATCHv1 3/4] Bluetooth: Add debug print specifier

[Marcel Holtmann]

Hi Andrei,

> Some functions print opcode as "0xc03" others as "0x0c03". Patch
> ensures that opcodes printed are the in the same format.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_core.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8f35f2e..c7be1bc 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2097,7 +2097,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
>  	struct hci_command_hdr *hdr;
>  	struct sk_buff *skb;
>  
> -	BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
> +	BT_DBG("%s opcode 0x%04x plen %d", hdev->name, opcode, plen);

please use 0x%4.4x to be consistent with our cases.

Regards

Marcel

[PATCHv1 4/4] Bluetooth: Fix not setting HCI_RESET flag for AMP

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Move reset function to common initialization section fixing
not setting HCI_RESET flag for amp_init.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index c7be1bc..88d996b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -188,12 +188,6 @@ static void bredr_init(struct hci_dev *hdev)
 
 	/* Mandatory initialization */
 
-	/* Reset */
-	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
-		set_bit(HCI_RESET, &hdev->flags);
-		hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-	}
-
 	/* Read Local Supported Features */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_FEATURES, 0, NULL);
 
@@ -234,9 +228,6 @@ static void amp_init(struct hci_dev *hdev)
 {
 	hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_BLOCK_BASED;
 
-	/* Reset */
-	hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-
 	/* Read Local Version */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
 
@@ -262,6 +253,10 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 	}
 	skb_queue_purge(&hdev->driver_init);
 
+	/* Reset */
+	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks))
+		hci_reset_req(hdev, 0);
+
 	switch (hdev->dev_type) {
 	case HCI_BREDR:
 		bredr_init(hdev);
-- 
1.7.9.5

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Marcel Holtmann]

Hi Andrei,

> Remove magic and use standard HCI cmd timeout
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index a2e15436c..e5a9a09 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
>  		set_bit(HCI_INIT, &hdev->flags);
>  		__hci_request(hdev, hci_reset_req, 0,
> -			      msecs_to_jiffies(250));
> +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
>  		clear_bit(HCI_INIT, &hdev->flags);
>  	}

And while you are at it, can we please move over to put the
msecs_to_jiffies into the #define itself. So we get more readable code
here. A bunch of location have already been changed, but seems we have a
few leftovers.

Regards

Marcel

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Andrei Emeltchenko]

Hi Marcel,

On Fri, Jun 08, 2012 at 05:31:56PM +0900, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Remove magic and use standard HCI cmd timeout
> > 
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > ---
> >  net/bluetooth/hci_core.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index a2e15436c..e5a9a09 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> >  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
> >  		set_bit(HCI_INIT, &hdev->flags);
> >  		__hci_request(hdev, hci_reset_req, 0,
> > -			      msecs_to_jiffies(250));
> > +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
> >  		clear_bit(HCI_INIT, &hdev->flags);
> >  	}
> 
> And while you are at it, can we please move over to put the
> msecs_to_jiffies into the #define itself. 

Do you mean convert those defines below?

/* HCI timeouts */
#define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
#define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
#define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
#define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
#define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */

Best regards 
Andrei Emeltchenko 

> So we get more readable code
> here. A bunch of location have already been changed, but seems we have a
> few leftovers.
> 
> Regards
> 
> Marcel
> 
> 

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Andrei Emeltchenko]

On Fri, Jun 08, 2012 at 11:53:17AM +0300, Andrei Emeltchenko wrote:
> Hi Marcel,
> 
> On Fri, Jun 08, 2012 at 05:31:56PM +0900, Marcel Holtmann wrote:
> > Hi Andrei,
> > 
> > > Remove magic and use standard HCI cmd timeout
> > > 
> > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > ---
> > >  net/bluetooth/hci_core.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > 
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index a2e15436c..e5a9a09 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> > >  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
> > >  		set_bit(HCI_INIT, &hdev->flags);
> > >  		__hci_request(hdev, hci_reset_req, 0,
> > > -			      msecs_to_jiffies(250));
> > > +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
> > >  		clear_bit(HCI_INIT, &hdev->flags);
> > >  	}
> > 
> > And while you are at it, can we please move over to put the
> > msecs_to_jiffies into the #define itself. 
> 
> Do you mean convert those defines below?
> 
> /* HCI timeouts */
> #define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
> #define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
> #define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
> #define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
> #define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */

I think I will convert them in the following patch.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Marcel Holtmann]

Hi Andrei,

> > > > Remove magic and use standard HCI cmd timeout
> > > > 
> > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > > ---
> > > >  net/bluetooth/hci_core.c |    2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > > 
> > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > > index a2e15436c..e5a9a09 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > > @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> > > >  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
> > > >  		set_bit(HCI_INIT, &hdev->flags);
> > > >  		__hci_request(hdev, hci_reset_req, 0,
> > > > -			      msecs_to_jiffies(250));
> > > > +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
> > > >  		clear_bit(HCI_INIT, &hdev->flags);
> > > >  	}
> > > 
> > > And while you are at it, can we please move over to put the
> > > msecs_to_jiffies into the #define itself. 
> > 
> > Do you mean convert those defines below?
> > 
> > /* HCI timeouts */
> > #define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
> > #define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
> > #define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
> > #define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
> > #define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */
> 
> I think I will convert them in the following patch.

yes, an additional patch is find. That is why I acked this one ;)

Regards

Marcel

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Marcel Holtmann]

Hi Andrei,

> > > Remove magic and use standard HCI cmd timeout
> > > 
> > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > ---
> > >  net/bluetooth/hci_core.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > 
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index a2e15436c..e5a9a09 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> > >  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
> > >  		set_bit(HCI_INIT, &hdev->flags);
> > >  		__hci_request(hdev, hci_reset_req, 0,
> > > -			      msecs_to_jiffies(250));
> > > +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
> > >  		clear_bit(HCI_INIT, &hdev->flags);
> > >  	}
> > 
> > And while you are at it, can we please move over to put the
> > msecs_to_jiffies into the #define itself. 
> 
> Do you mean convert those defines below?
> 
> /* HCI timeouts */
> #define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
> #define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
> #define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
> #define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
> #define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */

all of them where we do msecs_to_jiffies all the time anyway. See the
L2CAP code where I started to change this already.

Regards

Marcel

Re: [PATCHv1 1/4] Bluetooth: Use standard HCI cmd timeout for RESET

[Szymon Janc]

Hi,

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Remove magic and use standard HCI cmd timeout
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index a2e15436c..e5a9a09 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>  	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
>  		set_bit(HCI_INIT, &hdev->flags);
>  		__hci_request(hdev, hci_reset_req, 0,
> -			      msecs_to_jiffies(250));
> +			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
>  		clear_bit(HCI_INIT, &hdev->flags);
>  	}

In hci_dev_reset reset command is timeouted with HCI_INIT_TIMEOUT, maybe use same?

-- 
BR
Szymon Janc

[PATCHv2 1/5] Bluetooth: Use standard HCI cmd timeout for RESET

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Remove magic and use standard HCI cmd timeout

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a2e15436c..e5a9a09 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
 		set_bit(HCI_INIT, &hdev->flags);
 		__hci_request(hdev, hci_reset_req, 0,
-			      msecs_to_jiffies(250));
+			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
-- 
1.7.9.5

[PATCHv2 2/5] Bluetooth: Update HCI timeouts constants to use msecs_to_jiffies

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

The HCI constants are always used in form of jiffies. So just
include the conversion from msecs in the define itself. This has the
advantage of making the code where the timeout is used more readable
and avoiding unnecessary conversions.

The patch is similar to commit ba13ccd9 doing the same job for L2CAP

Reported-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 include/net/bluetooth/hci.h      |   10 +++++-----
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_core.c         |   25 +++++++++++--------------
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 23ef7ad..d66bc60 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -150,11 +150,11 @@ enum {
 #define HCIINQUIRY	_IOR('H', 240, int)
 
 /* HCI timeouts */
-#define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
-#define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
-#define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
-#define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
-#define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */
+#define HCI_DISCONN_TIMEOUT	msecs_to_jiffies(2000)	/* 2 seconds */
+#define HCI_PAIRING_TIMEOUT	msecs_to_jiffies(60000)	/* 60 seconds */
+#define HCI_INIT_TIMEOUT	msecs_to_jiffies(10000)	/* 10 seconds */
+#define HCI_CMD_TIMEOUT		msecs_to_jiffies(1000)	/* 1 seconds */
+#define HCI_ACL_TX_TIMEOUT	msecs_to_jiffies(45000)	/* 45 seconds */
 
 /* HCI data types */
 #define HCI_COMMAND_PKT		0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 288eaa8..d91d919 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -628,7 +628,7 @@ static inline void hci_conn_put(struct hci_conn *conn)
 		if (conn->type == ACL_LINK || conn->type == LE_LINK) {
 			del_timer(&conn->idle_timer);
 			if (conn->state == BT_CONNECTED) {
-				timeo = msecs_to_jiffies(conn->disc_timeout);
+				timeo = conn->disc_timeout;
 				if (!conn->out)
 					timeo *= 2;
 			} else {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e5a9a09..0334ebe 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -690,12 +690,11 @@ int hci_dev_open(__u16 dev)
 		set_bit(HCI_INIT, &hdev->flags);
 		hdev->init_last_cmd = 0;
 
-		ret = __hci_request(hdev, hci_init_req, 0,
-				    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+		ret = __hci_request(hdev, hci_init_req, 0, HCI_INIT_TIMEOUT);
 
 		if (lmp_host_le_capable(hdev))
 			ret = __hci_request(hdev, hci_le_init_req, 0,
-					    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+					    HCI_INIT_TIMEOUT);
 
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
@@ -782,8 +781,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	if (!test_bit(HCI_RAW, &hdev->flags) &&
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
 		set_bit(HCI_INIT, &hdev->flags);
-		__hci_request(hdev, hci_reset_req, 0,
-			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
+		__hci_request(hdev, hci_reset_req, 0, HCI_CMD_TIMEOUT);
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
@@ -872,8 +870,7 @@ int hci_dev_reset(__u16 dev)
 	hdev->acl_cnt = 0; hdev->sco_cnt = 0; hdev->le_cnt = 0;
 
 	if (!test_bit(HCI_RAW, &hdev->flags))
-		ret = __hci_request(hdev, hci_reset_req, 0,
-				    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+		ret = __hci_request(hdev, hci_reset_req, 0, HCI_INIT_TIMEOUT);
 
 done:
 	hci_req_unlock(hdev);
@@ -913,7 +910,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 	switch (cmd) {
 	case HCISETAUTH:
 		err = hci_request(hdev, hci_auth_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETENCRYPT:
@@ -925,23 +922,23 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 		if (!test_bit(HCI_AUTH, &hdev->flags)) {
 			/* Auth must be enabled first */
 			err = hci_request(hdev, hci_auth_req, dr.dev_opt,
-					  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+					  HCI_INIT_TIMEOUT);
 			if (err)
 				break;
 		}
 
 		err = hci_request(hdev, hci_encrypt_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETSCAN:
 		err = hci_request(hdev, hci_scan_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETLINKPOL:
 		err = hci_request(hdev, hci_linkpol_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETLINKMODE:
@@ -2583,7 +2580,7 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
 		/* ACL tx timeout must be longer than maximum
 		 * link supervision timeout (40.9 seconds) */
 		if (!cnt && time_after(jiffies, hdev->acl_last_tx +
-				       msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
+				       HCI_ACL_TX_TIMEOUT))
 			hci_link_tx_to(hdev, ACL_LINK);
 	}
 }
@@ -2967,7 +2964,7 @@ static void hci_cmd_work(struct work_struct *work)
 				del_timer(&hdev->cmd_timer);
 			else
 				mod_timer(&hdev->cmd_timer,
-				  jiffies + msecs_to_jiffies(HCI_CMD_TIMEOUT));
+					  jiffies + HCI_CMD_TIMEOUT);
 		} else {
 			skb_queue_head(&hdev->cmd_q, skb);
 			queue_work(hdev->workqueue, &hdev->cmd_work);
-- 
1.7.9.5

[PATCHv2 3/5] Bluetooth: Add opcode to error message

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Sometimes HCI command sending timeouts and gives error message without
specifying which command causes error. Patch makes sure that opcode
is printed to help debugging.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 0334ebe..146854f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1366,11 +1366,18 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr)
 }
 
 /* HCI command timer function */
-static void hci_cmd_timer(unsigned long arg)
+static void hci_cmd_timeout(unsigned long arg)
 {
 	struct hci_dev *hdev = (void *) arg;
 
-	BT_ERR("%s command tx timeout", hdev->name);
+	if (hdev->sent_cmd) {
+		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
+		u16 opcode = __le16_to_cpu(sent->opcode);
+
+		BT_ERR("%s command 0x%4.4x tx timeout", hdev->name, opcode);
+	} else
+		BT_ERR("%s command tx timeout", hdev->name);
+
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
 }
@@ -1670,7 +1677,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	init_waitqueue_head(&hdev->req_wait_q);
 
-	setup_timer(&hdev->cmd_timer, hci_cmd_timer, (unsigned long) hdev);
+	setup_timer(&hdev->cmd_timer, hci_cmd_timeout, (unsigned long) hdev);
 
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
-- 
1.7.9.5

Re: [PATCHv2 3/5] Bluetooth: Add opcode to error message

[Szymon Janc]

Hi,

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Sometimes HCI command sending timeouts and gives error message without
> specifying which command causes error. Patch makes sure that opcode
> is printed to help debugging.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_core.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 0334ebe..146854f 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1366,11 +1366,18 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr)
>  }
>  
>  /* HCI command timer function */
> -static void hci_cmd_timer(unsigned long arg)
> +static void hci_cmd_timeout(unsigned long arg)
>  {
>  	struct hci_dev *hdev = (void *) arg;
>  
> -	BT_ERR("%s command tx timeout", hdev->name);
> +	if (hdev->sent_cmd) {
> +		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> +		u16 opcode = __le16_to_cpu(sent->opcode);
> +
> +		BT_ERR("%s command 0x%4.4x tx timeout", hdev->name, opcode);
> +	} else
> +		BT_ERR("%s command tx timeout", hdev->name);
> +

Just a minor, 'else' should have braces as well.

>  	atomic_set(&hdev->cmd_cnt, 1);
>  	queue_work(hdev->workqueue, &hdev->cmd_work);
>  }
> @@ -1670,7 +1677,7 @@ struct hci_dev *hci_alloc_dev(void)
>  
>  	init_waitqueue_head(&hdev->req_wait_q);
>  
> -	setup_timer(&hdev->cmd_timer, hci_cmd_timer, (unsigned long) hdev);
> +	setup_timer(&hdev->cmd_timer, hci_cmd_timeout, (unsigned long) hdev);
>  
>  	hci_init_sysfs(hdev);
>  	discovery_init(hdev);
> 

-- 
BR
Szymon Janc

Re: [PATCHv2 3/5] Bluetooth: Add opcode to error message

[Andrei Emeltchenko]

Hi Szymon,

On Fri, Jun 08, 2012 at 01:17:42PM +0200, Szymon Janc wrote:
> > -	BT_ERR("%s command tx timeout", hdev->name);
> > +	if (hdev->sent_cmd) {
> > +		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> > +		u16 opcode = __le16_to_cpu(sent->opcode);
> > +
> > +		BT_ERR("%s command 0x%4.4x tx timeout", hdev->name, opcode);
> > +	} else
> > +		BT_ERR("%s command tx timeout", hdev->name);
> > +
> 
> Just a minor, 'else' should have braces as well.

Sure, thanks for pointing out. I will send updated version soon.

Best regards 
Andrei Emeltchenko 

[PATCHv2 4/5] Bluetooth: Correct debug print specifier for u16 objects

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Some functions print u16 objects as "0xc03" others as "0x0c03". Patch
ensures that opcodes printed are the in the same format and consistent
with bluetooth code.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 146854f..f1e46b7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -61,7 +61,7 @@ static void hci_notify(struct hci_dev *hdev, int event)
 
 void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
 {
-	BT_DBG("%s command 0x%04x result 0x%2.2x", hdev->name, cmd, result);
+	BT_DBG("%s command 0x%4.4x result 0x%2.2x", hdev->name, cmd, result);
 
 	/* If this is the init phase check if the completed command matches
 	 * the last init command, and if not just return.
@@ -2094,7 +2094,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	struct hci_command_hdr *hdr;
 	struct sk_buff *skb;
 
-	BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
+	BT_DBG("%s opcode 0x%4.4x plen %d", hdev->name, opcode, plen);
 
 	skb = bt_skb_alloc(len, GFP_ATOMIC);
 	if (!skb) {
@@ -2261,7 +2261,7 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)
 	if (hdr->opcode != cpu_to_le16(opcode))
 		return NULL;
 
-	BT_DBG("%s opcode 0x%x", hdev->name, opcode);
+	BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
 
 	return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE;
 }
@@ -2331,7 +2331,7 @@ void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags)
 	struct hci_conn *conn = chan->conn;
 	struct hci_dev *hdev = conn->hdev;
 
-	BT_DBG("%s chan %p flags 0x%x", hdev->name, chan, flags);
+	BT_DBG("%s chan %p flags 0x%4.4x", hdev->name, chan, flags);
 
 	skb->dev = (void *) hdev;
 
@@ -2831,7 +2831,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	flags  = hci_flags(handle);
 	handle = hci_handle(handle);
 
-	BT_DBG("%s len %d handle 0x%x flags 0x%x", hdev->name, skb->len,
+	BT_DBG("%s len %d handle 0x%4.4x flags 0x%4.4x", hdev->name, skb->len,
 	       handle, flags);
 
 	hdev->stat.acl_rx++;
@@ -2873,7 +2873,7 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 
 	handle = __le16_to_cpu(hdr->handle);
 
-	BT_DBG("%s len %d handle 0x%x", hdev->name, skb->len, handle);
+	BT_DBG("%s len %d handle 0x%4.4x", hdev->name, skb->len, handle);
 
 	hdev->stat.sco_rx++;
 
-- 
1.7.9.5

[PATCHv2 5/5] Bluetooth: Fix not setting HCI_RESET flag for AMP

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Move reset function to common initialization section fixing
not setting HCI_RESET flag for amp_init.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f1e46b7..83b4134 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -188,12 +188,6 @@ static void bredr_init(struct hci_dev *hdev)
 
 	/* Mandatory initialization */
 
-	/* Reset */
-	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
-		set_bit(HCI_RESET, &hdev->flags);
-		hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-	}
-
 	/* Read Local Supported Features */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_FEATURES, 0, NULL);
 
@@ -234,9 +228,6 @@ static void amp_init(struct hci_dev *hdev)
 {
 	hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_BLOCK_BASED;
 
-	/* Reset */
-	hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-
 	/* Read Local Version */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
 
@@ -262,6 +253,10 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 	}
 	skb_queue_purge(&hdev->driver_init);
 
+	/* Reset */
+	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks))
+		hci_reset_req(hdev, 0);
+
 	switch (hdev->dev_type) {
 	case HCI_BREDR:
 		bredr_init(hdev);
-- 
1.7.9.5

[PATCHv3 1/5] Bluetooth: Use standard HCI cmd timeout for RESET

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Remove magic and use standard HCI cmd timeout

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a2e15436c..e5a9a09 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -783,7 +783,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
 		set_bit(HCI_INIT, &hdev->flags);
 		__hci_request(hdev, hci_reset_req, 0,
-			      msecs_to_jiffies(250));
+			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
-- 
1.7.9.5

[PATCHv3 2/5] Bluetooth: Update HCI timeouts constants to use msecs_to_jiffies

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

The HCI constants are always used in form of jiffies. So just
include the conversion from msecs in the define itself. This has the
advantage of making the code where the timeout is used more readable
and avoiding unnecessary conversions.

The patch is similar to commit ba13ccd9 doing the same job for L2CAP

Reported-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 include/net/bluetooth/hci.h      |   10 +++++-----
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_core.c         |   25 +++++++++++--------------
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 23ef7ad..d66bc60 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -150,11 +150,11 @@ enum {
 #define HCIINQUIRY	_IOR('H', 240, int)
 
 /* HCI timeouts */
-#define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
-#define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
-#define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
-#define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
-#define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */
+#define HCI_DISCONN_TIMEOUT	msecs_to_jiffies(2000)	/* 2 seconds */
+#define HCI_PAIRING_TIMEOUT	msecs_to_jiffies(60000)	/* 60 seconds */
+#define HCI_INIT_TIMEOUT	msecs_to_jiffies(10000)	/* 10 seconds */
+#define HCI_CMD_TIMEOUT		msecs_to_jiffies(1000)	/* 1 seconds */
+#define HCI_ACL_TX_TIMEOUT	msecs_to_jiffies(45000)	/* 45 seconds */
 
 /* HCI data types */
 #define HCI_COMMAND_PKT		0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 288eaa8..d91d919 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -628,7 +628,7 @@ static inline void hci_conn_put(struct hci_conn *conn)
 		if (conn->type == ACL_LINK || conn->type == LE_LINK) {
 			del_timer(&conn->idle_timer);
 			if (conn->state == BT_CONNECTED) {
-				timeo = msecs_to_jiffies(conn->disc_timeout);
+				timeo = conn->disc_timeout;
 				if (!conn->out)
 					timeo *= 2;
 			} else {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e5a9a09..0334ebe 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -690,12 +690,11 @@ int hci_dev_open(__u16 dev)
 		set_bit(HCI_INIT, &hdev->flags);
 		hdev->init_last_cmd = 0;
 
-		ret = __hci_request(hdev, hci_init_req, 0,
-				    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+		ret = __hci_request(hdev, hci_init_req, 0, HCI_INIT_TIMEOUT);
 
 		if (lmp_host_le_capable(hdev))
 			ret = __hci_request(hdev, hci_le_init_req, 0,
-					    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+					    HCI_INIT_TIMEOUT);
 
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
@@ -782,8 +781,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	if (!test_bit(HCI_RAW, &hdev->flags) &&
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
 		set_bit(HCI_INIT, &hdev->flags);
-		__hci_request(hdev, hci_reset_req, 0,
-			      msecs_to_jiffies(HCI_CMD_TIMEOUT));
+		__hci_request(hdev, hci_reset_req, 0, HCI_CMD_TIMEOUT);
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
@@ -872,8 +870,7 @@ int hci_dev_reset(__u16 dev)
 	hdev->acl_cnt = 0; hdev->sco_cnt = 0; hdev->le_cnt = 0;
 
 	if (!test_bit(HCI_RAW, &hdev->flags))
-		ret = __hci_request(hdev, hci_reset_req, 0,
-				    msecs_to_jiffies(HCI_INIT_TIMEOUT));
+		ret = __hci_request(hdev, hci_reset_req, 0, HCI_INIT_TIMEOUT);
 
 done:
 	hci_req_unlock(hdev);
@@ -913,7 +910,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 	switch (cmd) {
 	case HCISETAUTH:
 		err = hci_request(hdev, hci_auth_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETENCRYPT:
@@ -925,23 +922,23 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 		if (!test_bit(HCI_AUTH, &hdev->flags)) {
 			/* Auth must be enabled first */
 			err = hci_request(hdev, hci_auth_req, dr.dev_opt,
-					  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+					  HCI_INIT_TIMEOUT);
 			if (err)
 				break;
 		}
 
 		err = hci_request(hdev, hci_encrypt_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETSCAN:
 		err = hci_request(hdev, hci_scan_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETLINKPOL:
 		err = hci_request(hdev, hci_linkpol_req, dr.dev_opt,
-				  msecs_to_jiffies(HCI_INIT_TIMEOUT));
+				  HCI_INIT_TIMEOUT);
 		break;
 
 	case HCISETLINKMODE:
@@ -2583,7 +2580,7 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
 		/* ACL tx timeout must be longer than maximum
 		 * link supervision timeout (40.9 seconds) */
 		if (!cnt && time_after(jiffies, hdev->acl_last_tx +
-				       msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
+				       HCI_ACL_TX_TIMEOUT))
 			hci_link_tx_to(hdev, ACL_LINK);
 	}
 }
@@ -2967,7 +2964,7 @@ static void hci_cmd_work(struct work_struct *work)
 				del_timer(&hdev->cmd_timer);
 			else
 				mod_timer(&hdev->cmd_timer,
-				  jiffies + msecs_to_jiffies(HCI_CMD_TIMEOUT));
+					  jiffies + HCI_CMD_TIMEOUT);
 		} else {
 			skb_queue_head(&hdev->cmd_q, skb);
 			queue_work(hdev->workqueue, &hdev->cmd_work);
-- 
1.7.9.5

[PATCHv3 3/5] Bluetooth: Add opcode to error message

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Sometimes HCI command sending timeouts and gives error message without
specifying which command causes error. Patch makes sure that opcode
is printed to help debugging.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 0334ebe..e71485f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1366,11 +1366,19 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr)
 }
 
 /* HCI command timer function */
-static void hci_cmd_timer(unsigned long arg)
+static void hci_cmd_timeout(unsigned long arg)
 {
 	struct hci_dev *hdev = (void *) arg;
 
-	BT_ERR("%s command tx timeout", hdev->name);
+	if (hdev->sent_cmd) {
+		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
+		u16 opcode = __le16_to_cpu(sent->opcode);
+
+		BT_ERR("%s command 0x%4.4x tx timeout", hdev->name, opcode);
+	} else {
+		BT_ERR("%s command tx timeout", hdev->name);
+	}
+
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
 }
@@ -1670,7 +1678,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	init_waitqueue_head(&hdev->req_wait_q);
 
-	setup_timer(&hdev->cmd_timer, hci_cmd_timer, (unsigned long) hdev);
+	setup_timer(&hdev->cmd_timer, hci_cmd_timeout, (unsigned long) hdev);
 
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
-- 
1.7.9.5

[PATCHv3 4/5] Bluetooth: Correct debug print specifier for u16 objects

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Some functions print u16 objects as "0xc03" others as "0x0c03". Patch
ensures that opcodes printed are the in the same format and consistent
with bluetooth code.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e71485f..4f804ff 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -61,7 +61,7 @@ static void hci_notify(struct hci_dev *hdev, int event)
 
 void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
 {
-	BT_DBG("%s command 0x%04x result 0x%2.2x", hdev->name, cmd, result);
+	BT_DBG("%s command 0x%4.4x result 0x%2.2x", hdev->name, cmd, result);
 
 	/* If this is the init phase check if the completed command matches
 	 * the last init command, and if not just return.
@@ -2095,7 +2095,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	struct hci_command_hdr *hdr;
 	struct sk_buff *skb;
 
-	BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
+	BT_DBG("%s opcode 0x%4.4x plen %d", hdev->name, opcode, plen);
 
 	skb = bt_skb_alloc(len, GFP_ATOMIC);
 	if (!skb) {
@@ -2262,7 +2262,7 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)
 	if (hdr->opcode != cpu_to_le16(opcode))
 		return NULL;
 
-	BT_DBG("%s opcode 0x%x", hdev->name, opcode);
+	BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
 
 	return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE;
 }
@@ -2332,7 +2332,7 @@ void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags)
 	struct hci_conn *conn = chan->conn;
 	struct hci_dev *hdev = conn->hdev;
 
-	BT_DBG("%s chan %p flags 0x%x", hdev->name, chan, flags);
+	BT_DBG("%s chan %p flags 0x%4.4x", hdev->name, chan, flags);
 
 	skb->dev = (void *) hdev;
 
@@ -2832,7 +2832,7 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	flags  = hci_flags(handle);
 	handle = hci_handle(handle);
 
-	BT_DBG("%s len %d handle 0x%x flags 0x%x", hdev->name, skb->len,
+	BT_DBG("%s len %d handle 0x%4.4x flags 0x%4.4x", hdev->name, skb->len,
 	       handle, flags);
 
 	hdev->stat.acl_rx++;
@@ -2874,7 +2874,7 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 
 	handle = __le16_to_cpu(hdr->handle);
 
-	BT_DBG("%s len %d handle 0x%x", hdev->name, skb->len, handle);
+	BT_DBG("%s len %d handle 0x%4.4x", hdev->name, skb->len, handle);
 
 	hdev->stat.sco_rx++;
 
-- 
1.7.9.5

[PATCHv3 5/5] Bluetooth: Fix not setting HCI_RESET flag for AMP

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Move reset function to common initialization section fixing
not setting HCI_RESET flag for amp_init.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4f804ff..7a6ff57 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -188,12 +188,6 @@ static void bredr_init(struct hci_dev *hdev)
 
 	/* Mandatory initialization */
 
-	/* Reset */
-	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
-		set_bit(HCI_RESET, &hdev->flags);
-		hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-	}
-
 	/* Read Local Supported Features */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_FEATURES, 0, NULL);
 
@@ -234,9 +228,6 @@ static void amp_init(struct hci_dev *hdev)
 {
 	hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_BLOCK_BASED;
 
-	/* Reset */
-	hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
-
 	/* Read Local Version */
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
 
@@ -262,6 +253,10 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 	}
 	skb_queue_purge(&hdev->driver_init);
 
+	/* Reset */
+	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks))
+		hci_reset_req(hdev, 0);
+
 	switch (hdev->dev_type) {
 	case HCI_BREDR:
 		bredr_init(hdev);
-- 
1.7.9.5

Re: [PATCHv3 5/5] Bluetooth: Fix not setting HCI_RESET flag for AMP

[Gustavo Padovan]

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-06-11 11:13:10 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Move reset function to common initialization section fixing
> not setting HCI_RESET flag for amp_init.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_core.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)

All 5 patches were applied to bluetooth-next. Thanks.

	Gustavo