H5 Specification says:
If a SYNC message is received while in the Active State, it is
assumed that the peer device has reset. The local device should
therefore perform a full reset of the upper stack, and start Link
Establishment again at the Uninitialized State. Upon entering the
Active State, the first packet sent shall have its SEQ and ACK
numbers set to zero.
This patch resets the HCI H5 driver data/state to unitialized and
reports an HCI hardware error event to notify the upper stack that
HCI synchronization has been lost. H5 will be re-synchronized and
upper stack should generate an HCI Reset command.
Signed-off-by: Loic Poulain <[email protected]>
---
drivers/bluetooth/hci_h5.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index caacb42..5c5e9ac 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -168,6 +168,36 @@ wakeup:
hci_uart_tx_wakeup(hu);
}
+static void h5_peer_reset(struct hci_uart *hu)
+{
+ struct h5 *h5 = hu->priv;
+ struct sk_buff *skb;
+ const unsigned char hard_err[] = { 0x10, 0x01, 0x00 };
+
+ BT_ERR("Peer device has reset");
+
+ h5->state = H5_UNINITIALIZED;
+
+ del_timer(&h5->timer);
+
+ skb_queue_purge(&h5->rel);
+ skb_queue_purge(&h5->unrel);
+ skb_queue_purge(&h5->unack);
+
+ h5->tx_seq = 0;
+ h5->tx_ack = 0;
+
+ skb = bt_skb_alloc(3, GFP_ATOMIC);
+ if (!skb)
+ return;
+
+ bt_cb(skb)->pkt_type = HCI_EVENT_PKT;
+ memcpy(skb_put(skb, 3), hard_err, 3);
+
+ /* Send Hardware Error to upper stack */
+ hci_recv_frame(hu->hdev, skb);
+}
+
static int h5_open(struct hci_uart *hu)
{
struct h5 *h5;
@@ -283,8 +313,12 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
conf_req[2] = h5_cfg_field(h5);
if (memcmp(data, sync_req, 2) == 0) {
+ if (h5->state == H5_ACTIVE)
+ h5_peer_reset(hu);
h5_link_control(hu, sync_rsp, 2);
} else if (memcmp(data, sync_rsp, 2) == 0) {
+ if (h5->state == H5_ACTIVE)
+ h5_peer_reset(hu);
h5->state = H5_INITIALIZED;
h5_link_control(hu, conf_req, 3);
} else if (memcmp(data, conf_req, 2) == 0) {
--
1.8.3.2
Hi Marcel,
On Fri, Oct 31, 2014, Marcel Holtmann wrote:
> > So, hardware error seems the best way to warn host stack about the H5 controller
> > reset.
>
> I am actually fine with doing it this way.
>
> > Regarding the "upper stack", I know that Bluedroid restarts on hardware error, but
> > I ignore the Bluez behavior.
>
> This is something we need to fix then. First and foremost this has to
> work with the Bluetooth subsystem in the kernel as well and not just a
> stack on top of HCI User Channel.
Agreed. I've now applied this patch to bluetooth-next so we can continue
towards making the higher layers do the right thing as well.
Johan
Hi Loic,
please do not top-post on this mailing list. It breaks the flow of things since everybody is doing inline quoting.
> This is just the expected behavior as described int the specification:
>
> UART Spec says:
> If the UART synchronization is lost in the communication from Host to Host
> Controller, then the Host Controller shall send a Hardware Error Event to tell
> the Host about the synchronization error. The Host Controller will then expect to
> receive an HCI_Reset command from the Host in order to perform a reset. The
> Host Controller will also use the HCI_Reset command in the byte stream from
> Host to Host Controller to re-synchronize.
>
> H5 part says:
> If a SYNC message is received while in the Active State, it is assumed that the
> peer device has reset. The local device should therefore perform a full reset of
> the upper stack, and start Link Establishment again at the Uninitialized State.
>
> So, hardware error seems the best way to warn host stack about the H5 controller
> reset.
I am actually fine with doing it this way.
> Regarding the "upper stack", I know that Bluedroid restarts on hardware error, but
> I ignore the Bluez behavior.
This is something we need to fix then. First and foremost this has to work with the Bluetooth subsystem in the kernel as well and not just a stack on top of HCI User Channel.
Regards
Marcel
Hi Johan,
This is just the expected behavior as described int the specification:
UART Spec says:
If the UART synchronization is lost in the communication from Host to Host
Controller, then the Host Controller shall send a Hardware Error Event
to tell
the Host about the synchronization error. The Host Controller will then
expect to
receive an HCI_Reset command from the Host in order to perform a reset. The
Host Controller will also use the HCI_Reset command in the byte stream from
Host to Host Controller to re-synchronize.
H5 part says:
If a SYNC message is received while in the Active State, it is assumed
that the
peer device has reset. The local device should therefore perform a full
reset of
the upper stack, and start Link Establishment again at the Uninitialized
State.
So, hardware error seems the best way to warn host stack about the H5
controller
reset.
Regarding the "upper stack", I know that Bluedroid restarts on hardware
error, but
I ignore the Bluez behavior.
Regards,
Loic
On 31/10/2014 09:39, Johan Hedberg wrote:
> Hi Loic,
>
> On Wed, Oct 08, 2014, Loic Poulain wrote:
>> This patch resets the HCI H5 driver data/state to unitialized and
>> reports an HCI hardware error event to notify the upper stack that
>> HCI synchronization has been lost. H5 will be re-synchronized and
>> upper stack should generate an HCI Reset command.
> I couldn't find anywhere where we'd currently do this "upper stack"
> behavior that you describe (there . Did you have some follow-up patches to
> implement that?
>
> Johan
--
Intel Open Source Technology Center
http://oss.intel.com/
Hi Loic,
On Wed, Oct 08, 2014, Loic Poulain wrote:
> This patch resets the HCI H5 driver data/state to unitialized and
> reports an HCI hardware error event to notify the upper stack that
> HCI synchronization has been lost. H5 will be re-synchronized and
> upper stack should generate an HCI Reset command.
I couldn't find anywhere where we'd currently do this "upper stack"
behavior that you describe (there . Did you have some follow-up patches to
implement that?
Johan