2013-03-27 23:04:54

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v2 0/3] Fix Inquiry command support in HCI request framework

Hi all,

This v2 implements the following comments about the previous patch set:
- Consider commands (which don't send Command Complete Event) as completed
based on Command Status Event.
- Remove hci_req_cmd_status function.

Thus, for those HCI commands which don't send Command Complete Event, the HCI
request framework considers they have completed when the Command Status Event
is received.

Regards,

Andre

Andre Guedes (3):
Bluetooth: Fix HCI request framework
Bluetooth: Fix hci_inquiry
Bluetooth: Remove hci_req_cmd_status

include/net/bluetooth/hci_core.h | 1 -
net/bluetooth/hci_core.c | 39 +++++++++++++--------------------------
net/bluetooth/hci_event.c | 11 ++++++-----
3 files changed, 19 insertions(+), 32 deletions(-)

--
1.8.1.2



2013-03-27 23:04:57

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v2 3/3] Bluetooth: Remove hci_req_cmd_status

This patch removes the hci_req_cmd_status function since it is not
used anymore. The HCI request framework now considers the HCI command
has complete once the Command Status or Command Complete Event is
received.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 -
net/bluetooth/hci_core.c | 26 --------------------------
2 files changed, 27 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 358a698..0e7ee89 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1055,7 +1055,6 @@ void hci_req_init(struct hci_request *req, struct hci_dev *hdev);
int hci_req_run(struct hci_request *req, hci_req_complete_t complete);
void hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param);
void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
-void hci_req_cmd_status(struct hci_dev *hdev, u16 opcode, u8 status);

int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1239929..a199d63 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3322,32 +3322,6 @@ call_complete:
req_complete(hdev, status);
}

-void hci_req_cmd_status(struct hci_dev *hdev, u16 opcode, u8 status)
-{
- hci_req_complete_t req_complete = NULL;
-
- BT_DBG("opcode 0x%04x status 0x%02x", opcode, status);
-
- if (status) {
- hci_req_cmd_complete(hdev, opcode, status);
- return;
- }
-
- /* No need to handle success status if there are more commands */
- if (!hci_req_is_complete(hdev))
- return;
-
- if (hdev->sent_cmd)
- req_complete = bt_cb(hdev->sent_cmd)->req.complete;
-
- /* If the request doesn't have a complete callback or there
- * are other commands/requests in the hdev queue we consider
- * this request as completed.
- */
- if (!req_complete || !skb_queue_empty(&hdev->cmd_q))
- hci_req_cmd_complete(hdev, opcode, status);
-}
-
static void hci_rx_work(struct work_struct *work)
{
struct hci_dev *hdev = container_of(work, struct hci_dev, rx_work);
--
1.8.1.2


2013-03-27 23:04:56

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v2 2/3] Bluetooth: Fix hci_inquiry

Since the HCI request framework was properly fixed, the hci_req_sync
call, in hci_inquiry, will return as soon as the HCI command completes
(not the Inquiry procedure). However, in inquiry ioctl implementation,
we want to sleep the user process until the inquiry procedure finishes.

This patch changes hci_inquiry so, in case the HCI Inquiry command
was executed successfully, it waits the HCI_INQUIRY flag to be cleared.
This way, the user process will sleep until the inquiry procedure
finishes.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_core.c | 13 +++++++++++++
net/bluetooth/hci_event.c | 5 +++++
2 files changed, 18 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index cfcad54..1239929 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -818,6 +818,12 @@ static void hci_inq_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_INQUIRY, sizeof(cp), &cp);
}

+static int wait_inquiry(void *word)
+{
+ schedule();
+ return signal_pending(current);
+}
+
int hci_inquiry(void __user *arg)
{
__u8 __user *ptr = arg;
@@ -849,6 +855,13 @@ int hci_inquiry(void __user *arg)
timeo);
if (err < 0)
goto done;
+
+ /* Wait until Inquiry procedure finishes (HCI_INQUIRY flag is
+ * cleared). If it is interrupted by a signal, return -EINTR.
+ */
+ if (wait_on_bit(&hdev->flags, HCI_INQUIRY, wait_inquiry,
+ TASK_INTERRUPTIBLE))
+ return -EINTR;
}

/* for unlimited number of responses we will use buffer with
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8efb9c0..7e7fbca 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -48,6 +48,8 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
}

clear_bit(HCI_INQUIRY, &hdev->flags);
+ smp_mb__after_clear_bit(); /* wake_up_bit advises about this barrier */
+ wake_up_bit(&hdev->flags, HCI_INQUIRY);

hci_dev_lock(hdev);
hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
@@ -1603,6 +1605,9 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
return;

+ smp_mb__after_clear_bit(); /* wake_up_bit advises about this barrier */
+ wake_up_bit(&hdev->flags, HCI_INQUIRY);
+
if (!test_bit(HCI_MGMT, &hdev->dev_flags))
return;

--
1.8.1.2


2013-03-27 23:04:55

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v2 1/3] Bluetooth: Fix HCI request framework

Some HCI commands don't send a Command Complete Event once the HCI
command has completed so they require some special handling from the
HCI request framework. These HCI commands, however, send a Command
Status Event to indicate that the command has been received, and
that the controller is currently performing the task for the command.

So, in order to properly handle those HCI commands, the HCI request
framework should consider the HCI command has completed once the
Command Status Event is received.

This way, we fix some issues regarding the Inquiry command support,
as well as add support for all those HCI commands which would require
some special handling from the HCI request framework.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1385807..8efb9c0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -53,8 +53,6 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
hci_dev_unlock(hdev);

- hci_req_cmd_complete(hdev, HCI_OP_INQUIRY, status);
-
hci_conn_check_pending(hdev);
}

@@ -1600,8 +1598,6 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- hci_req_cmd_complete(hdev, HCI_OP_INQUIRY, status);
-
hci_conn_check_pending(hdev);

if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
@@ -2462,7 +2458,7 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (opcode != HCI_OP_NOP)
del_timer(&hdev->cmd_timer);

- hci_req_cmd_status(hdev, opcode, ev->status);
+ hci_req_cmd_complete(hdev, opcode, ev->status);

if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) {
atomic_set(&hdev->cmd_cnt, 1);
--
1.8.1.2


2013-04-04 08:26:13

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Fix Inquiry command support in HCI request framework

Hi Andre,

On Wed, Mar 27, 2013, Andre Guedes wrote:
> This v2 implements the following comments about the previous patch set:
> - Consider commands (which don't send Command Complete Event) as completed
> based on Command Status Event.
> - Remove hci_req_cmd_status function.
>
> Thus, for those HCI commands which don't send Command Complete Event, the HCI
> request framework considers they have completed when the Command Status Event
> is received.
>
> Regards,
>
> Andre
>
> Andre Guedes (3):
> Bluetooth: Fix HCI request framework
> Bluetooth: Fix hci_inquiry
> Bluetooth: Remove hci_req_cmd_status
>
> include/net/bluetooth/hci_core.h | 1 -
> net/bluetooth/hci_core.c | 39 +++++++++++++--------------------------
> net/bluetooth/hci_event.c | 11 ++++++-----
> 3 files changed, 19 insertions(+), 32 deletions(-)

All patches in this set have now been pushed to the bluetooth-next
tree. Thanks.

Johan

2013-04-02 07:07:46

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Fix Inquiry command support in HCI request framework

Hi Andre,

On Wed, Mar 27, 2013, Andre Guedes wrote:
> This v2 implements the following comments about the previous patch set:
> - Consider commands (which don't send Command Complete Event) as completed
> based on Command Status Event.
> - Remove hci_req_cmd_status function.
>
> Thus, for those HCI commands which don't send Command Complete Event, the HCI
> request framework considers they have completed when the Command Status Event
> is received.
>
> Regards,
>
> Andre
>
> Andre Guedes (3):
> Bluetooth: Fix HCI request framework
> Bluetooth: Fix hci_inquiry
> Bluetooth: Remove hci_req_cmd_status
>
> include/net/bluetooth/hci_core.h | 1 -
> net/bluetooth/hci_core.c | 39 +++++++++++++--------------------------
> net/bluetooth/hci_event.c | 11 ++++++-----
> 3 files changed, 19 insertions(+), 32 deletions(-)

I did a bit of testing and these patches seem to work fine. I'm not
really familiar with the smp_mb__after_clear_bit() usage, but I'll
assume you did your homework to ensure it's correct. Other than that the
patches seem fine to me.

Acked-by: Johan Hedberg <[email protected]>

Johan