* [PATCH] Bluetooth: Use proper timer for hci command timout
@ 2011-02-07 8:26 Ville Tervo
2011-02-10 19:03 ` Gustavo F. Padovan
0 siblings, 1 reply; 2+ messages in thread
From: Ville Tervo @ 2011-02-07 8:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ville Tervo
Use proper timer instead of hci command flow control to timeout
failed hci commands. Otherwise stack ends up sending commands
when flow control is used to block new commands.
2010-09-01 18:29:41.592132 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
bdaddr 00:16:CF:E1:C7:D7 mode 2 clkoffset 0x0000
2010-09-01 18:29:41.592681 > HCI Event: Command Status (0x0f) plen 4
Remote Name Request (0x01|0x0019) status 0x00 ncmd 0
2010-09-01 18:29:51.022033 < HCI Command: Remote Name Request Cancel (0x01|0x001a) plen 6
bdaddr 00:16:CF:E1:C7:D7
Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
include/net/bluetooth/hci.h | 3 +++
include/net/bluetooth/hci_core.h | 12 +++++++++++-
net/bluetooth/hci_core.c | 21 +++++++++++++++------
net/bluetooth/hci_event.c | 6 ++++++
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 4bee030..e3f0c7d 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -119,6 +119,7 @@ enum {
#define HCI_PAIRING_TIMEOUT (60000) /* 60 seconds */
#define HCI_IDLE_TIMEOUT (6000) /* 6 seconds */
#define HCI_INIT_TIMEOUT (10000) /* 10 seconds */
+#define HCI_CMD_TIMEOUT (1000) /* 1 secs */
/* HCI data types */
#define HCI_COMMAND_PKT 0x01
@@ -242,6 +243,8 @@ enum {
#define HCI_AT_GENERAL_BONDING_MITM 0x05
/* ----- HCI Commands ---- */
+#define HCI_OP_NOP 0x0000
+
#define HCI_OP_INQUIRY 0x0401
struct hci_cp_inquiry {
__u8 lap[3];
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6163bff..42105f9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -128,7 +128,6 @@ struct hci_dev {
unsigned int acl_pkts;
unsigned int sco_pkts;
- unsigned long cmd_last_tx;
unsigned long acl_last_tx;
unsigned long sco_last_tx;
@@ -138,6 +137,7 @@ struct hci_dev {
struct work_struct power_off;
struct timer_list off_timer;
+ struct timer_list cmd_timer;
struct tasklet_struct cmd_task;
struct tasklet_struct rx_task;
struct tasklet_struct tx_task;
@@ -417,6 +417,16 @@ static inline void hci_conn_put(struct hci_conn *conn)
}
}
+static inline void hci_start_cmd_timer(struct timer_list *timer)
+{
+ mod_timer(timer, jiffies + msecs_to_jiffies(HCI_CMD_TIMEOUT));
+}
+
+static inline void hci_stop_cmd_timer(struct timer_list *timer)
+{
+ del_timer(timer);
+}
+
/* ----- HCI Devices ----- */
static inline void __hci_dev_put(struct hci_dev *d)
{
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2f00322..dd8ca64 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -41,6 +41,7 @@
#include <linux/interrupt.h>
#include <linux/notifier.h>
#include <linux/rfkill.h>
+#include <linux/timer.h>
#include <net/sock.h>
#include <asm/system.h>
@@ -611,6 +612,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
/* Drop last sent command */
if (hdev->sent_cmd) {
+ del_timer_sync(&hdev->cmd_timer);
kfree_skb(hdev->sent_cmd);
hdev->sent_cmd = NULL;
}
@@ -1054,6 +1056,16 @@ int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr)
return 0;
}
+/* HCI command timer function */
+static void hci_cmd_timer(unsigned long arg)
+{
+ struct hci_dev *hdev = (void *) arg;
+
+ BT_ERR("%s command tx timeout", hdev->name);
+ atomic_set(&hdev->cmd_cnt, 1);
+ tasklet_schedule(&hdev->cmd_task);
+}
+
/* Register HCI device */
int hci_register_dev(struct hci_dev *hdev)
{
@@ -1100,6 +1112,8 @@ int hci_register_dev(struct hci_dev *hdev)
skb_queue_head_init(&hdev->cmd_q);
skb_queue_head_init(&hdev->raw_q);
+ setup_timer(&hdev->cmd_timer, hci_cmd_timer, (unsigned long) hdev);
+
for (i = 0; i < NUM_REASSEMBLY; i++)
hdev->reassembly[i] = NULL;
@@ -1936,11 +1950,6 @@ static void hci_cmd_task(unsigned long arg)
BT_DBG("%s cmd %d", hdev->name, atomic_read(&hdev->cmd_cnt));
- if (!atomic_read(&hdev->cmd_cnt) && time_after(jiffies, hdev->cmd_last_tx + HZ)) {
- BT_ERR("%s command tx timeout", hdev->name);
- atomic_set(&hdev->cmd_cnt, 1);
- }
-
/* Send queued commands */
if (atomic_read(&hdev->cmd_cnt)) {
skb = skb_dequeue(&hdev->cmd_q);
@@ -1953,7 +1962,7 @@ static void hci_cmd_task(unsigned long arg)
if (hdev->sent_cmd) {
atomic_dec(&hdev->cmd_cnt);
hci_send_frame(skb);
- hdev->cmd_last_tx = jiffies;
+ hci_start_cmd_timer(&hdev->cmd_timer);
} else {
skb_queue_head(&hdev->cmd_q, skb);
tasklet_schedule(&hdev->cmd_task);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index cee46cb..021f889 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1672,6 +1672,9 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
break;
}
+ if (ev->opcode != HCI_OP_NOP)
+ hci_stop_cmd_timer(&hdev->cmd_timer);
+
if (ev->ncmd) {
atomic_set(&hdev->cmd_cnt, 1);
if (!skb_queue_empty(&hdev->cmd_q))
@@ -1743,6 +1746,9 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
break;
}
+ if (ev->opcode != HCI_OP_NOP)
+ hci_stop_cmd_timer(&hdev->cmd_timer);
+
if (ev->ncmd) {
atomic_set(&hdev->cmd_cnt, 1);
if (!skb_queue_empty(&hdev->cmd_q))
--
1.7.2.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] Bluetooth: Use proper timer for hci command timout
2011-02-07 8:26 [PATCH] Bluetooth: Use proper timer for hci command timout Ville Tervo
@ 2011-02-10 19:03 ` Gustavo F. Padovan
0 siblings, 0 replies; 2+ messages in thread
From: Gustavo F. Padovan @ 2011-02-10 19:03 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth
Hi Ville,
* Ville Tervo <ville.tervo@nokia.com> [2011-02-07 10:26:47 +0200]:
> Use proper timer instead of hci command flow control to timeout
> failed hci commands. Otherwise stack ends up sending commands
> when flow control is used to block new commands.
>
> 2010-09-01 18:29:41.592132 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
> bdaddr 00:16:CF:E1:C7:D7 mode 2 clkoffset 0x0000
> 2010-09-01 18:29:41.592681 > HCI Event: Command Status (0x0f) plen 4
> Remote Name Request (0x01|0x0019) status 0x00 ncmd 0
> 2010-09-01 18:29:51.022033 < HCI Command: Remote Name Request Cancel (0x01|0x001a) plen 6
> bdaddr 00:16:CF:E1:C7:D7
>
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
> include/net/bluetooth/hci.h | 3 +++
> include/net/bluetooth/hci_core.h | 12 +++++++++++-
> net/bluetooth/hci_core.c | 21 +++++++++++++++------
> net/bluetooth/hci_event.c | 6 ++++++
> 4 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 4bee030..e3f0c7d 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -119,6 +119,7 @@ enum {
> #define HCI_PAIRING_TIMEOUT (60000) /* 60 seconds */
> #define HCI_IDLE_TIMEOUT (6000) /* 6 seconds */
> #define HCI_INIT_TIMEOUT (10000) /* 10 seconds */
> +#define HCI_CMD_TIMEOUT (1000) /* 1 secs */
Just to be keep it similar to the other defines change the comment to "1
second"
>
> /* HCI data types */
> #define HCI_COMMAND_PKT 0x01
> @@ -242,6 +243,8 @@ enum {
> #define HCI_AT_GENERAL_BONDING_MITM 0x05
>
> /* ----- HCI Commands ---- */
> +#define HCI_OP_NOP 0x0000
> +
> #define HCI_OP_INQUIRY 0x0401
> struct hci_cp_inquiry {
> __u8 lap[3];
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 6163bff..42105f9 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -128,7 +128,6 @@ struct hci_dev {
> unsigned int acl_pkts;
> unsigned int sco_pkts;
>
> - unsigned long cmd_last_tx;
> unsigned long acl_last_tx;
> unsigned long sco_last_tx;
>
> @@ -138,6 +137,7 @@ struct hci_dev {
> struct work_struct power_off;
> struct timer_list off_timer;
>
> + struct timer_list cmd_timer;
> struct tasklet_struct cmd_task;
> struct tasklet_struct rx_task;
> struct tasklet_struct tx_task;
> @@ -417,6 +417,16 @@ static inline void hci_conn_put(struct hci_conn *conn)
> }
> }
>
> +static inline void hci_start_cmd_timer(struct timer_list *timer)
> +{
> + mod_timer(timer, jiffies + msecs_to_jiffies(HCI_CMD_TIMEOUT));
> +}
> +
> +static inline void hci_stop_cmd_timer(struct timer_list *timer)
> +{
> + del_timer(timer);
> +}
We don't really need this function. Just call del_timer directly.
Otherwise looks good. :-)
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-02-10 19:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-07 8:26 [PATCH] Bluetooth: Use proper timer for hci command timout Ville Tervo
2011-02-10 19:03 ` Gustavo F. Padovan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).