2005-04-06 13:08:54

by Catalin Drula

[permalink] [raw]
Subject: [Bluez-devel] [PATCH] H4 loss of synchronization recovery

Hi Marcel,

Attached you have the updated patch for recovering from
H4 synchronization loss (sending the reset command after a
hardware error event and reseting stack state).

Here's a summary of the changes:
- the procedure is only perfomed when the underlying transport
is HCI_UART
- the counters are reset not by a command, but from the hdev struct
- the "scan enable", "authentication enable", "encrypt mode" variables
are restored to their previous values from hdev->flags
- if there was an ongoing inquiry, it has been killed by the reset, so
the HCI_INQUIRY flag is cleared
- I added an hci_req_cancel

I have tested the patch with the buggy BT module in the iPaq h5550 and it
works as expected.

Catalin

diff -ur linux-2.6.11-mh2/include/net/bluetooth/hci_core.h linux-2.6.11-mh2-hwerr/include/net/bluetooth/hci_core.h
--- linux-2.6.11-mh2/include/net/bluetooth/hci_core.h 2005-03-24 13:08:31.000000000 +0100
+++ linux-2.6.11-mh2-hwerr/include/net/bluetooth/hci_core.h 2005-04-05 18:09:14.891362807 +0200
@@ -381,6 +381,7 @@
int hci_dev_close(__u16 dev);
int hci_dev_reset(__u16 dev);
int hci_dev_reset_stat(__u16 dev);
+int hci_dev_reset_h4_sync_loss(struct hci_dev *hdev);
int hci_dev_cmd(unsigned int cmd, void __user *arg);
int hci_get_dev_list(void __user *arg);
int hci_get_dev_info(void __user *arg);
diff -ur linux-2.6.11-mh2/include/net/bluetooth/hci.h linux-2.6.11-mh2-hwerr/include/net/bluetooth/hci.h
--- linux-2.6.11-mh2/include/net/bluetooth/hci.h 2005-03-24 13:02:39.000000000 +0100
+++ linux-2.6.11-mh2-hwerr/include/net/bluetooth/hci.h 2005-04-05 14:10:55.000000000 +0200
@@ -584,6 +584,11 @@
__u16 clock_offset;
} __attribute__ ((packed));

+#define HCI_EV_HARDWARE_ERROR 0x10
+struct hci_ev_hardware_error {
+ __u8 hwcode;
+} __attribute__ ((packed));
+
/* Internal events generated by Bluetooth stack */
#define HCI_EV_STACK_INTERNAL 0xFD
struct hci_ev_stack_internal {
diff -ur linux-2.6.11-mh2/net/bluetooth/hci_core.c linux-2.6.11-mh2-hwerr/net/bluetooth/hci_core.c
--- linux-2.6.11-mh2/net/bluetooth/hci_core.c 2005-03-24 13:02:43.000000000 +0100
+++ linux-2.6.11-mh2-hwerr/net/bluetooth/hci_core.c 2005-04-05 18:09:18.617875792 +0200
@@ -646,6 +646,81 @@
return ret;
}

+int hci_dev_reset_h4_sync_loss(struct hci_dev *hdev) {
+ int ret = 0;
+ __u8 auth = 0, scan = 0, encrypt = 0;
+
+ hci_req_cancel(hdev, ENODEV);
+ hci_req_lock(hdev);
+
+ /* Disable RX and TX tasks */
+ tasklet_disable(&hdev->rx_task);
+ tasklet_disable(&hdev->tx_task);
+
+ /* Flush connection hash */
+ hci_dev_lock_bh(hdev);
+ hci_conn_hash_flush(hdev);
+ hci_dev_unlock_bh(hdev);
+
+ /* Flush driver */
+ if (hdev->flush)
+ hdev->flush(hdev);
+
+ /* Disable cmd task */
+ tasklet_disable(&hdev->cmd_task);
+
+ /* Drop queues */
+ skb_queue_purge(&hdev->rx_q);
+ skb_queue_purge(&hdev->cmd_q);
+ skb_queue_purge(&hdev->raw_q);
+
+ /* Reset command counter */
+ atomic_set(&hdev->cmd_cnt, 1);
+
+ /* Drop last sent command */
+ if (hdev->sent_cmd) {
+ kfree_skb(hdev->sent_cmd);
+ hdev->sent_cmd = NULL;
+ }
+
+ /* Send reset command */
+ hci_send_cmd(hdev, OGF_HOST_CTL, OCF_RESET, 0, NULL);
+
+ /* Reset ACL and SCO counters */
+ hdev->acl_cnt = hdev->acl_pkts;
+ hdev->sco_cnt = hdev->sco_pkts;
+
+ /* Restore device flags and state */
+ if (test_bit(HCI_ISCAN, &hdev->flags))
+ scan |= SCAN_INQUIRY;
+ if (test_bit(HCI_PSCAN, &hdev->flags))
+ scan |= SCAN_PAGE;
+ hci_send_cmd(hdev, OGF_HOST_CTL, OCF_WRITE_SCAN_ENABLE, 1, &scan);
+
+ if (test_bit(HCI_AUTH, &hdev->flags))
+ auth |= AUTH_ENABLED;
+ else
+ auth |= AUTH_DISABLED;
+ hci_send_cmd(hdev, OGF_HOST_CTL, OCF_WRITE_AUTH_ENABLE, 1, &auth);
+
+ if (test_bit(HCI_ENCRYPT, &hdev->flags))
+ encrypt |= ENCRYPT_P2P;
+ else
+ encrypt |= ENCRYPT_DISABLED;
+ hci_send_cmd(hdev, OGF_HOST_CTL, OCF_WRITE_ENCRYPT_MODE, 1, &encrypt);
+
+ clear_bit(HCI_INQUIRY, &hdev->flags);
+
+ /* Enable tasks */
+ tasklet_enable(&hdev->rx_task);
+ tasklet_enable(&hdev->tx_task);
+ tasklet_enable(&hdev->cmd_task);
+
+ hci_req_unlock(hdev);
+
+ return ret;
+}
+
int hci_dev_cmd(unsigned int cmd, void __user *arg)
{
struct hci_dev *hdev;
diff -ur linux-2.6.11-mh2/net/bluetooth/hci_event.c linux-2.6.11-mh2-hwerr/net/bluetooth/hci_event.c
--- linux-2.6.11-mh2/net/bluetooth/hci_event.c 2005-03-24 13:08:31.000000000 +0100
+++ linux-2.6.11-mh2-hwerr/net/bluetooth/hci_event.c 2005-04-05 18:03:37.437462054 +0200
@@ -866,6 +866,19 @@
hci_dev_unlock(hdev);
}

+/* Hardware Error */
+static inline void hci_hardware_error_evt(struct hci_dev *hdev, struct sk_buff *skb) {
+ struct hci_ev_hardware_error *ev = (struct hci_ev_hardware_error *) skb->data;
+
+ BT_ERR("%s hardware error event with code 0x%.2x", hdev->name, ev->hwcode);
+
+ /* Perform recovery procedure if the transport is UART */
+ if (hdev->type == HCI_UART) {
+ BT_ERR("%s performing H4 synchronization recovery procedure", hdev->name);
+ hci_dev_reset_h4_sync_loss(hdev);
+ }
+}
+
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_event_hdr *hdr = (struct hci_event_hdr *) skb->data;
@@ -938,6 +951,10 @@
hci_clock_offset_evt(hdev, skb);
break;

+ case HCI_EV_HARDWARE_ERROR:
+ hci_hardware_error_evt(hdev, skb);
+ break;
+
case HCI_EV_CMD_STATUS:
cs = (struct hci_ev_cmd_status *) skb->data;
skb_pull(skb, sizeof(cs));



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel


2005-04-06 13:28:36

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] H4 loss of synchronization recovery

Hi Catalin,

> Attached you have the updated patch for recovering from
> H4 synchronization loss (sending the reset command after a
> hardware error event and reseting stack state).
>
> Here's a summary of the changes:
> - the procedure is only perfomed when the underlying transport
> is HCI_UART
> - the counters are reset not by a command, but from the hdev struct
> - the "scan enable", "authentication enable", "encrypt mode" variables
> are restored to their previous values from hdev->flags
> - if there was an ongoing inquiry, it has been killed by the reset, so
> the HCI_INQUIRY flag is cleared
> - I added an hci_req_cancel
>
> I have tested the patch with the buggy BT module in the iPaq h5550 and it
> works as expected.

please change some things for me to clean this patch.

1. The "{" after the function name must be at the next line.

2. Name the reset function hci_reset_dev().

It is not H:4 specific, even if you only call it in that case. And since
the parameter is hci_dev it should not start with hci_dev_*. In this
case you would expect a device id.

3. There is no need to preset "auth" and "encrypt".

4. For "auth" and "encrypt" use constructs like this:

auth = test_bit(HCI_AUTH, &hdev->flags) ? AUTH_ENABLED : AUTH_DISABLED

To decrease the stack size, you can also use the same variable for
"auth", "encrypt" and "scan".


And what do we do if the transport if BCSP? In this case the type is
also HCI_UART.

Regards

Marcel




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel