All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
@ 2011-11-04 17:16 Andre Guedes
  2011-11-04 17:16 ` [PATCH 2/3] Bluetooth: Create hci_cancel_inquiry() Andre Guedes
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-04 17:16 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds a function to hci_core to carry out inquiry.

All inquiry code from start_discovery() were replaced by a
hci_do_inquiry() call.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_core.c         |   17 +++++++++++++++++
 net/bluetooth/mgmt.c             |    9 +--------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f97792c..ae36ac0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
 void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
+int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b7f6b5b..098f26c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
 		}
 	}
 }
+
+int hci_do_inquiry(struct hci_dev *hdev, u8 length)
+{
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_cp_inquiry cp;
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_INQUIRY, &hdev->flags))
+		return -EINPROGRESS;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, 3);
+	cp.length  = length;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 747366a..25dda47 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1598,8 +1598,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 
 static int start_discovery(struct sock *sk, u16 index)
 {
-	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-	struct hci_cp_inquiry cp;
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1618,12 +1616,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, 3);
-	cp.length  = 0x08;
-	cp.num_rsp = 0x00;
-
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	err = hci_do_inquiry(hdev, 0x08);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] Bluetooth: Create hci_cancel_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
@ 2011-11-04 17:16 ` Andre Guedes
  2011-11-04 17:16 ` [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery Andre Guedes
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-04 17:16 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds a function to hci_core to cancel an ongoing inquiry.

According to the Bluetooth spec, the inquiry cancel command should
only be issued after the inquiry command has been issued, a command
status event has been received for the inquiry command, and before
the inquiry complete event occurs.

As HCI_INQUIRY flag is only set just after an inquiry command status
event occurs and it is cleared just after an inquiry complete event
occurs, the inquiry cancel command should be issued only if HCI_INQUIRY
flag is set.

Additionally, cancel inquiry related code from stop_discovery() were
replaced by a hci_cancel_inquiry() call.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_core.c         |   10 ++++++++++
 net/bluetooth/mgmt.c             |    2 +-
 3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ae36ac0..567e9b8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -971,5 +971,6 @@ void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
 void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
 int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+int hci_cancel_inquiry(struct hci_dev *hdev);
 
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 098f26c..b57cb17 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2577,3 +2577,13 @@ int hci_do_inquiry(struct hci_dev *hdev, u8 length)
 
 	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
 }
+
+int hci_cancel_inquiry(struct hci_dev *hdev)
+{
+	BT_DBG("%s", hdev->name);
+
+	if (!test_bit(HCI_INQUIRY, &hdev->flags))
+		return -EPERM;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 25dda47..73cf34b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1647,7 +1647,7 @@ static int stop_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+	err = hci_cancel_inquiry(hdev);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
  2011-11-04 17:16 ` [PATCH 2/3] Bluetooth: Create hci_cancel_inquiry() Andre Guedes
@ 2011-11-04 17:16 ` Andre Guedes
  2011-11-07 19:38   ` Gustavo Padovan
  2011-11-04 18:20 ` [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Anderson Lizardo
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andre Guedes @ 2011-11-04 17:16 UTC (permalink / raw)
  To: linux-bluetooth

By using periodic inquiry command we're not able to detect correctly
when the controller has started inquiry.

Today we have this workaround in inquiry result event handler
to set the HCI_INQUIRY flag when it sees the first inquiry result
event. This workaround isn't enough because the device may be
performing an inquiry but the HCI_INQUIRY flag is not set. For
instance, if there is no device in range, no inquiry result event
is generated, consequently, the HCI_INQUIRY flags isn't set when
it should so.

We rely on HCI_INQUIRY flag to implement the discovery procedure
properly. So, as we aren't able to clear/set the HCI_INQUIRY flag
in a reliable manner, periodic inquiry events shouldn't change
the HCI_INQUIRY flag.

Thus, due to that issue and in order to keep compatibility with
userspace, periodic inquiry events shouldn't send mgmt discovering
events.

In future, we might track if periodic inquiry is enabled or not.
By tracking this state we'll be able to do some improvements in
Discovery such as failing MGMT_OP_START_DISCOVERY command in case
periodic inquiry is on. We can also send no mgmt_device_found
event if periodic inquiry is on.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c |   43 +++++++++++--------------------------------
 1 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d8fa657..bfee273 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -58,9 +58,9 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
-	if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
-			test_bit(HCI_MGMT, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
+	clear_bit(HCI_INQUIRY, &hdev->flags);
+
+	mgmt_discovering(hdev->id, 0);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
 
@@ -76,10 +76,6 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
-	if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
-				test_bit(HCI_MGMT, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
-
 	hci_conn_check_pending(hdev);
 }
 
@@ -984,9 +980,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		return;
 	}
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags) &&
-				test_bit(HCI_MGMT, &hdev->flags))
-		mgmt_discovering(hdev->id, 1);
+	set_bit(HCI_INQUIRY, &hdev->flags);
+
+	mgmt_discovering(hdev->id, 1);
 }
 
 static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
@@ -1365,13 +1361,14 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	BT_DBG("%s status %d", hdev->name, status);
 
-	if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
-				test_bit(HCI_MGMT, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
-
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 
 	hci_conn_check_pending(hdev);
+
+	if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
+		return;
+
+	mgmt_discovering(hdev->id, 0);
 }
 
 static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1387,12 +1384,6 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
 
 	hci_dev_lock(hdev);
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	for (; num_rsp; num_rsp--, info++) {
 		bacpy(&data.bdaddr, &info->bdaddr);
 		data.pscan_rep_mode	= info->pscan_rep_mode;
@@ -2393,12 +2384,6 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
 
 	hci_dev_lock(hdev);
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	if ((skb->len - 1) / num_rsp != sizeof(struct inquiry_info_with_rssi)) {
 		struct inquiry_info_with_rssi_and_pscan_mode *info;
 		info = (void *) (skb->data + 1);
@@ -2561,12 +2546,6 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 	if (!num_rsp)
 		return;
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	hci_dev_lock(hdev);
 
 	for (; num_rsp; num_rsp--, info++) {
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
  2011-11-04 17:16 ` [PATCH 2/3] Bluetooth: Create hci_cancel_inquiry() Andre Guedes
  2011-11-04 17:16 ` [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery Andre Guedes
@ 2011-11-04 18:20 ` Anderson Lizardo
  2011-11-04 18:33   ` Andre Guedes
  2011-11-04 18:51 ` Andre Guedes
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Anderson Lizardo @ 2011-11-04 18:20 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Guedes,

On Fri, Nov 4, 2011 at 1:16 PM, Andre Guedes <andre.guedes@openbossa.org> wrote:
> -       err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +       err = hci_do_inquiry(hdev, 0x08);

In a later patch in your tree you introduce this new definition:

net/bluetooth/mgmt.c:#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */

Does it make sense to introduce it already here and use it above?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 18:20 ` [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Anderson Lizardo
@ 2011-11-04 18:33   ` Andre Guedes
  0 siblings, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-04 18:33 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth

Hi Lizardo,

On Nov 4, 2011, at 3:20 PM, Anderson Lizardo wrote:

> Hi Guedes,
> 
> On Fri, Nov 4, 2011 at 1:16 PM, Andre Guedes <andre.guedes@openbossa.org> wrote:
>> -       err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
>> +       err = hci_do_inquiry(hdev, 0x08);
> 
> In a later patch in your tree you introduce this new definition:
> 
> net/bluetooth/mgmt.c:#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> 
> Does it make sense to introduce it already here and use it above?

Sure, I'll add INQUIRY_LEN_BREDR macro to this patch and
resend it. Thanks.

Padovan, please don't consider this patch.

Andre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
                   ` (2 preceding siblings ...)
  2011-11-04 18:20 ` [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Anderson Lizardo
@ 2011-11-04 18:51 ` Andre Guedes
  2011-11-07  9:03 ` Andrei Emeltchenko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-04 18:51 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds a function to hci_core to carry out inquiry.

All inquiry code from start_discovery() were replaced by a
hci_do_inquiry() call.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_core.c         |   17 +++++++++++++++++
 net/bluetooth/mgmt.c             |   11 +++--------
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f97792c..ae36ac0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
 void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
+int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b7f6b5b..098f26c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
 		}
 	}
 }
+
+int hci_do_inquiry(struct hci_dev *hdev, u8 length)
+{
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_cp_inquiry cp;
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_INQUIRY, &hdev->flags))
+		return -EINPROGRESS;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, 3);
+	cp.length  = length;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 747366a..17c7fbb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,8 @@
 #define MGMT_VERSION	0
 #define MGMT_REVISION	1
 
+#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+
 struct pending_cmd {
 	struct list_head list;
 	__u16 opcode;
@@ -1598,8 +1600,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 
 static int start_discovery(struct sock *sk, u16 index)
 {
-	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-	struct hci_cp_inquiry cp;
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1618,12 +1618,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, 3);
-	cp.length  = 0x08;
-	cp.num_rsp = 0x00;
-
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
                   ` (3 preceding siblings ...)
  2011-11-04 18:51 ` Andre Guedes
@ 2011-11-07  9:03 ` Andrei Emeltchenko
  2011-11-07 13:58   ` Andre Guedes
  2011-11-07 13:59 ` Andre Guedes
  2011-11-07 14:45 ` Andre Guedes
  6 siblings, 1 reply; 16+ messages in thread
From: Andrei Emeltchenko @ 2011-11-07  9:03 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Andre,

On Fri, Nov 04, 2011 at 02:16:51PM -0300, Andre Guedes wrote:
> This patch adds a function to hci_core to carry out inquiry.
> 
> All inquiry code from start_discovery() were replaced by a
> hci_do_inquiry() call.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 ++
>  net/bluetooth/hci_core.c         |   17 +++++++++++++++++
>  net/bluetooth/mgmt.c             |    9 +--------
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index f97792c..ae36ac0 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>  void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
>  void hci_le_ltk_neg_reply(struct hci_conn *conn);
>  
> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
> +
>  #endif /* __HCI_CORE_H */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b7f6b5b..098f26c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
>  		}
>  	}
>  }
> +
> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
> +{
> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };

what are those numbers above?

> +	struct hci_cp_inquiry cp;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
> +		return -EINPROGRESS;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	memcpy(&cp.lap, lap, 3);

sizeof(lap) looks better

Best regards 
Andrei Emeltchenko 

> +	cp.length  = length;
> +
> +	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +}
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 747366a..25dda47 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1598,8 +1598,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
>  
>  static int start_discovery(struct sock *sk, u16 index)
>  {
> -	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> -	struct hci_cp_inquiry cp;
>  	struct pending_cmd *cmd;
>  	struct hci_dev *hdev;
>  	int err;
> @@ -1618,12 +1616,7 @@ static int start_discovery(struct sock *sk, u16 index)
>  		goto failed;
>  	}
>  
> -	memset(&cp, 0, sizeof(cp));
> -	memcpy(&cp.lap, lap, 3);
> -	cp.length  = 0x08;
> -	cp.num_rsp = 0x00;
> -
> -	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +	err = hci_do_inquiry(hdev, 0x08);
>  	if (err < 0)
>  		mgmt_pending_remove(cmd);
>  
> -- 
> 1.7.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07  9:03 ` Andrei Emeltchenko
@ 2011-11-07 13:58   ` Andre Guedes
  2011-11-07 14:09     ` Andrei Emeltchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Andre Guedes @ 2011-11-07 13:58 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Nov 7, 2011, at 6:03 AM, Andrei Emeltchenko wrote:

> Hi Andre,
> 
> On Fri, Nov 04, 2011 at 02:16:51PM -0300, Andre Guedes wrote:
>> This patch adds a function to hci_core to carry out inquiry.
>> 
>> All inquiry code from start_discovery() were replaced by a
>> hci_do_inquiry() call.
>> 
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |    2 ++
>> net/bluetooth/hci_core.c         |   17 +++++++++++++++++
>> net/bluetooth/mgmt.c             |    9 +--------
>> 3 files changed, 20 insertions(+), 8 deletions(-)
>> 
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index f97792c..ae36ac0 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>> void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
>> void hci_le_ltk_neg_reply(struct hci_conn *conn);
>> 
>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
>> +
>> #endif /* __HCI_CORE_H */
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index b7f6b5b..098f26c 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
>> 		}
>> 	}
>> }
>> +
>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
>> +{
>> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> 
> what are those numbers above?

This is the general inquiry LAP value defined in page 314 of
Core spec.

> 
>> +	struct hci_cp_inquiry cp;
>> +
>> +	BT_DBG("%s", hdev->name);
>> +
>> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
>> +		return -EINPROGRESS;
>> +
>> +	memset(&cp, 0, sizeof(cp));
>> +	memcpy(&cp.lap, lap, 3);
> 
> sizeof(lap) looks better

Ok, I'll change this.

BR,

Andre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
                   ` (4 preceding siblings ...)
  2011-11-07  9:03 ` Andrei Emeltchenko
@ 2011-11-07 13:59 ` Andre Guedes
  2011-11-07 23:53   ` Marcel Holtmann
  2011-11-07 14:45 ` Andre Guedes
  6 siblings, 1 reply; 16+ messages in thread
From: Andre Guedes @ 2011-11-07 13:59 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds a function to hci_core to carry out inquiry.

All inquiry code from start_discovery() were replaced by a
hci_do_inquiry() call.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_core.c         |   17 +++++++++++++++++
 net/bluetooth/mgmt.c             |   11 +++--------
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f97792c..ae36ac0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
 void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
+int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b7f6b5b..5b861c7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
 		}
 	}
 }
+
+int hci_do_inquiry(struct hci_dev *hdev, u8 length)
+{
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_cp_inquiry cp;
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_INQUIRY, &hdev->flags))
+		return -EINPROGRESS;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, sizeof(cp.lap));
+	cp.length  = length;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 747366a..17c7fbb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,8 @@
 #define MGMT_VERSION	0
 #define MGMT_REVISION	1
 
+#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+
 struct pending_cmd {
 	struct list_head list;
 	__u16 opcode;
@@ -1598,8 +1600,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 
 static int start_discovery(struct sock *sk, u16 index)
 {
-	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-	struct hci_cp_inquiry cp;
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1618,12 +1618,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, 3);
-	cp.length  = 0x08;
-	cp.num_rsp = 0x00;
-
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07 13:58   ` Andre Guedes
@ 2011-11-07 14:09     ` Andrei Emeltchenko
  2011-11-07 14:27       ` Andre Guedes
  0 siblings, 1 reply; 16+ messages in thread
From: Andrei Emeltchenko @ 2011-11-07 14:09 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Andre,

On Mon, Nov 07, 2011 at 10:58:29AM -0300, Andre Guedes wrote:
> Hi Andrei,
> 
> On Nov 7, 2011, at 6:03 AM, Andrei Emeltchenko wrote:
> 
> > Hi Andre,
> > 
> > On Fri, Nov 04, 2011 at 02:16:51PM -0300, Andre Guedes wrote:
> >> This patch adds a function to hci_core to carry out inquiry.
> >> 
> >> All inquiry code from start_discovery() were replaced by a
> >> hci_do_inquiry() call.
> >> 
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >> include/net/bluetooth/hci_core.h |    2 ++
> >> net/bluetooth/hci_core.c         |   17 +++++++++++++++++
> >> net/bluetooth/mgmt.c             |    9 +--------
> >> 3 files changed, 20 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index f97792c..ae36ac0 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
> >> void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
> >> void hci_le_ltk_neg_reply(struct hci_conn *conn);
> >> 
> >> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
> >> +
> >> #endif /* __HCI_CORE_H */
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index b7f6b5b..098f26c 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
> >> 		}
> >> 	}
> >> }
> >> +
> >> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
> >> +{
> >> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> > 
> > what are those numbers above?
> 
> This is the general inquiry LAP value defined in page 314 of
> Core spec.

Can we define those numbers (like HCI_GENERAL_INQ_LAP) or put comments?

Best regards 
Andrei Emeltchenko 

> 
> > 
> >> +	struct hci_cp_inquiry cp;
> >> +
> >> +	BT_DBG("%s", hdev->name);
> >> +
> >> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
> >> +		return -EINPROGRESS;
> >> +
> >> +	memset(&cp, 0, sizeof(cp));
> >> +	memcpy(&cp.lap, lap, 3);
> > 
> > sizeof(lap) looks better
> 
> Ok, I'll change this.
> 
> BR,
> 
> Andre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07 14:09     ` Andrei Emeltchenko
@ 2011-11-07 14:27       ` Andre Guedes
  0 siblings, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-07 14:27 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Nov 7, 2011, at 11:09 AM, Andrei Emeltchenko wrote:

> Hi Andre,
> 
> On Mon, Nov 07, 2011 at 10:58:29AM -0300, Andre Guedes wrote:
>> Hi Andrei,
>> 
>> On Nov 7, 2011, at 6:03 AM, Andrei Emeltchenko wrote:
>> 
>>> Hi Andre,
>>> 
>>> On Fri, Nov 04, 2011 at 02:16:51PM -0300, Andre Guedes wrote:
>>>> This patch adds a function to hci_core to carry out inquiry.
>>>> 
>>>> All inquiry code from start_discovery() were replaced by a
>>>> hci_do_inquiry() call.
>>>> 
>>>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>>>> ---
>>>> include/net/bluetooth/hci_core.h |    2 ++
>>>> net/bluetooth/hci_core.c         |   17 +++++++++++++++++
>>>> net/bluetooth/mgmt.c             |    9 +--------
>>>> 3 files changed, 20 insertions(+), 8 deletions(-)
>>>> 
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index f97792c..ae36ac0 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>>>> void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
>>>> void hci_le_ltk_neg_reply(struct hci_conn *conn);
>>>> 
>>>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
>>>> +
>>>> #endif /* __HCI_CORE_H */
>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>> index b7f6b5b..098f26c 100644
>>>> --- a/net/bluetooth/hci_core.c
>>>> +++ b/net/bluetooth/hci_core.c
>>>> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
>>>> 		}
>>>> 	}
>>>> }
>>>> +
>>>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
>>>> +{
>>>> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
>>> 
>>> what are those numbers above?
>> 
>> This is the general inquiry LAP value defined in page 314 of
>> Core spec.
> 
> Can we define those numbers (like HCI_GENERAL_INQ_LAP) or put comments?

Sure, I'll add a comment.

Andre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
                   ` (5 preceding siblings ...)
  2011-11-07 13:59 ` Andre Guedes
@ 2011-11-07 14:45 ` Andre Guedes
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-07 14:45 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds a function to hci_core to carry out inquiry.

All inquiry code from start_discovery() were replaced by a
hci_do_inquiry() call.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_core.c         |   18 ++++++++++++++++++
 net/bluetooth/mgmt.c             |   11 +++--------
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f97792c..ae36ac0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
 void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
+int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b7f6b5b..e6e9913 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2560,3 +2560,21 @@ static void hci_cmd_task(unsigned long arg)
 		}
 	}
 }
+
+int hci_do_inquiry(struct hci_dev *hdev, u8 length)
+{
+	/* General inquiry access code (GIAC) */
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_cp_inquiry cp;
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_INQUIRY, &hdev->flags))
+		return -EINPROGRESS;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, sizeof(cp.lap));
+	cp.length  = length;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 747366a..17c7fbb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,8 @@
 #define MGMT_VERSION	0
 #define MGMT_REVISION	1
 
+#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+
 struct pending_cmd {
 	struct list_head list;
 	__u16 opcode;
@@ -1598,8 +1600,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 
 static int start_discovery(struct sock *sk, u16 index)
 {
-	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-	struct hci_cp_inquiry cp;
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1618,12 +1618,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, 3);
-	cp.length  = 0x08;
-	cp.num_rsp = 0x00;
-
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery
  2011-11-04 17:16 ` [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery Andre Guedes
@ 2011-11-07 19:38   ` Gustavo Padovan
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo Padovan @ 2011-11-07 19:38 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Andre,

* Andre Guedes <andre.guedes@openbossa.org> [2011-11-04 14:16:53 -0300]:

> By using periodic inquiry command we're not able to detect correctly
> when the controller has started inquiry.
> 
> Today we have this workaround in inquiry result event handler
> to set the HCI_INQUIRY flag when it sees the first inquiry result
> event. This workaround isn't enough because the device may be
> performing an inquiry but the HCI_INQUIRY flag is not set. For
> instance, if there is no device in range, no inquiry result event
> is generated, consequently, the HCI_INQUIRY flags isn't set when
> it should so.
> 
> We rely on HCI_INQUIRY flag to implement the discovery procedure
> properly. So, as we aren't able to clear/set the HCI_INQUIRY flag
> in a reliable manner, periodic inquiry events shouldn't change
> the HCI_INQUIRY flag.
> 
> Thus, due to that issue and in order to keep compatibility with
> userspace, periodic inquiry events shouldn't send mgmt discovering
> events.
> 
> In future, we might track if periodic inquiry is enabled or not.
> By tracking this state we'll be able to do some improvements in
> Discovery such as failing MGMT_OP_START_DISCOVERY command in case
> periodic inquiry is on. We can also send no mgmt_device_found
> event if periodic inquiry is on.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c |   43 +++++++++++--------------------------------
>  1 files changed, 11 insertions(+), 32 deletions(-)

The three patches were applied, thanks.

	Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07 13:59 ` Andre Guedes
@ 2011-11-07 23:53   ` Marcel Holtmann
  2011-11-08 14:47     ` Gustavo Padovan
  2011-11-08 17:40     ` Andre Guedes
  0 siblings, 2 replies; 16+ messages in thread
From: Marcel Holtmann @ 2011-11-07 23:53 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Andre,

> This patch adds a function to hci_core to carry out inquiry.
> 
> All inquiry code from start_discovery() were replaced by a
> hci_do_inquiry() call.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 ++
>  net/bluetooth/hci_core.c         |   17 +++++++++++++++++
>  net/bluetooth/mgmt.c             |   11 +++--------
>  3 files changed, 22 insertions(+), 8 deletions(-)

in general I am fine with this patch, besides some tiny nitpicks.

> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index f97792c..ae36ac0 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>  void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
>  void hci_le_ltk_neg_reply(struct hci_conn *conn);
>  
> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
> +

I think the name hci_send_inquiry would be better. Since you are just
sending the command and not handling the whole inquiry.

>  #endif /* __HCI_CORE_H */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b7f6b5b..5b861c7 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
>  		}
>  	}
>  }
> +
> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
> +{
> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> +	struct hci_cp_inquiry cp;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
> +		return -EINPROGRESS;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	memcpy(&cp.lap, lap, sizeof(cp.lap));
> +	cp.length  = length;
> +
> +	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +}
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 747366a..17c7fbb 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -32,6 +32,8 @@
>  #define MGMT_VERSION	0
>  #define MGMT_REVISION	1
>  
> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> +

Please add an extra tab between the comment and the define. It is too
easy to get confused otherwise.

>  struct pending_cmd {
>  	struct list_head list;
>  	__u16 opcode;
> @@ -1598,8 +1600,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
>  
>  static int start_discovery(struct sock *sk, u16 index)
>  {
> -	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> -	struct hci_cp_inquiry cp;
>  	struct pending_cmd *cmd;
>  	struct hci_dev *hdev;
>  	int err;
> @@ -1618,12 +1618,7 @@ static int start_discovery(struct sock *sk, u16 index)
>  		goto failed;
>  	}
>  
> -	memset(&cp, 0, sizeof(cp));
> -	memcpy(&cp.lap, lap, 3);
> -	cp.length  = 0x08;
> -	cp.num_rsp = 0x00;
> -
> -	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
>  	if (err < 0)
>  		mgmt_pending_remove(cmd);
>  

Otherwise.

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

Regards

Marcel



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07 23:53   ` Marcel Holtmann
@ 2011-11-08 14:47     ` Gustavo Padovan
  2011-11-08 17:40     ` Andre Guedes
  1 sibling, 0 replies; 16+ messages in thread
From: Gustavo Padovan @ 2011-11-08 14:47 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Andre Guedes, linux-bluetooth

Hi Andre,

* Marcel Holtmann <marcel@holtmann.org> [2011-11-08 08:53:57 +0900]:

> Hi Andre,
> 
> > This patch adds a function to hci_core to carry out inquiry.
> > 
> > All inquiry code from start_discovery() were replaced by a
> > hci_do_inquiry() call.
> > 
> > Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> > ---
> >  include/net/bluetooth/hci_core.h |    2 ++
> >  net/bluetooth/hci_core.c         |   17 +++++++++++++++++
> >  net/bluetooth/mgmt.c             |   11 +++--------
> >  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> in general I am fine with this patch, besides some tiny nitpicks.
> 
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index f97792c..ae36ac0 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
> >  void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
> >  void hci_le_ltk_neg_reply(struct hci_conn *conn);
> >  
> > +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
> > +
> 
> I think the name hci_send_inquiry would be better. Since you are just
> sending the command and not handling the whole inquiry.
> 
> >  #endif /* __HCI_CORE_H */
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index b7f6b5b..5b861c7 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
> >  		}
> >  	}
> >  }
> > +
> > +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
> > +{
> > +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
> > +	struct hci_cp_inquiry cp;
> > +
> > +	BT_DBG("%s", hdev->name);
> > +
> > +	if (test_bit(HCI_INQUIRY, &hdev->flags))
> > +		return -EINPROGRESS;
> > +
> > +	memset(&cp, 0, sizeof(cp));
> > +	memcpy(&cp.lap, lap, sizeof(cp.lap));
> > +	cp.length  = length;
> > +
> > +	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
> > +}
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 747366a..17c7fbb 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -32,6 +32,8 @@
> >  #define MGMT_VERSION	0
> >  #define MGMT_REVISION	1
> >  
> > +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> > +
> 
> Please add an extra tab between the comment and the define. It is too
> easy to get confused otherwise.

Please send a delta patch to this one, since I already applied it. Thanks.

	Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] Bluetooth: Create hci_do_inquiry()
  2011-11-07 23:53   ` Marcel Holtmann
  2011-11-08 14:47     ` Gustavo Padovan
@ 2011-11-08 17:40     ` Andre Guedes
  1 sibling, 0 replies; 16+ messages in thread
From: Andre Guedes @ 2011-11-08 17:40 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

On Nov 7, 2011, at 8:53 PM, Marcel Holtmann wrote:

> Hi Andre,
> 
>> This patch adds a function to hci_core to carry out inquiry.
>> 
>> All inquiry code from start_discovery() were replaced by a
>> hci_do_inquiry() call.
>> 
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |    2 ++
>> net/bluetooth/hci_core.c         |   17 +++++++++++++++++
>> net/bluetooth/mgmt.c             |   11 +++--------
>> 3 files changed, 22 insertions(+), 8 deletions(-)
> 
> in general I am fine with this patch, besides some tiny nitpicks.
> 
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index f97792c..ae36ac0 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -970,4 +970,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>> void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
>> void hci_le_ltk_neg_reply(struct hci_conn *conn);
>> 
>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length);
>> +
> 
> I think the name hci_send_inquiry would be better. Since you are just
> sending the command and not handling the whole inquiry.

In LE scan patch (which I'll send to ML soon), we define a function called
hci_do_le_scan() which checks the current state, send some commands and
setup a timer to disable le scan after a given timeout. In this case the "do"
makes more sense. So, in order to keep things lined up, I chose a similar
name to inquiry function. This way hci_core provides the following helper
functions to carry out device discovery:

hci_do_inquiry();
hci_do_le_scan();
hci_cancel_inquiry();
hci_cancel_le_scan();

We may rename hci_do_inquiry to hci_send_inquiry but we won't keep theses
names lined up. So, can we keep this way or you still think we should
rename it?

>> #endif /* __HCI_CORE_H */
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index b7f6b5b..5b861c7 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2560,3 +2560,20 @@ static void hci_cmd_task(unsigned long arg)
>> 		}
>> 	}
>> }
>> +
>> +int hci_do_inquiry(struct hci_dev *hdev, u8 length)
>> +{
>> +	u8 lap[3] = { 0x33, 0x8b, 0x9e };
>> +	struct hci_cp_inquiry cp;
>> +
>> +	BT_DBG("%s", hdev->name);
>> +
>> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
>> +		return -EINPROGRESS;
>> +
>> +	memset(&cp, 0, sizeof(cp));
>> +	memcpy(&cp.lap, lap, sizeof(cp.lap));
>> +	cp.length  = length;
>> +
>> +	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
>> +}
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 747366a..17c7fbb 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -32,6 +32,8 @@
>> #define MGMT_VERSION	0
>> #define MGMT_REVISION	1
>> 
>> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>> +
> 
> Please add an extra tab between the comment and the define. It is too
> easy to get confused otherwise.

Ok, I'll fix it.

> 
>> struct pending_cmd {
>> 	struct list_head list;
>> 	__u16 opcode;
>> @@ -1598,8 +1600,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
>> 
>> static int start_discovery(struct sock *sk, u16 index)
>> {
>> -	u8 lap[3] = { 0x33, 0x8b, 0x9e };
>> -	struct hci_cp_inquiry cp;
>> 	struct pending_cmd *cmd;
>> 	struct hci_dev *hdev;
>> 	int err;
>> @@ -1618,12 +1618,7 @@ static int start_discovery(struct sock *sk, u16 index)
>> 		goto failed;
>> 	}
>> 
>> -	memset(&cp, 0, sizeof(cp));
>> -	memcpy(&cp.lap, lap, 3);
>> -	cp.length  = 0x08;
>> -	cp.num_rsp = 0x00;
>> -
>> -	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
>> +	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
>> 	if (err < 0)
>> 		mgmt_pending_remove(cmd);
>> 
> 
> Otherwise.
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> Regards
> 
> Marcel
> 
> 

BR,

Andre

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2011-11-08 17:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-04 17:16 [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Andre Guedes
2011-11-04 17:16 ` [PATCH 2/3] Bluetooth: Create hci_cancel_inquiry() Andre Guedes
2011-11-04 17:16 ` [PATCH 3/3] Bluetooth: Periodic Inquiry and Discovery Andre Guedes
2011-11-07 19:38   ` Gustavo Padovan
2011-11-04 18:20 ` [PATCH 1/3] Bluetooth: Create hci_do_inquiry() Anderson Lizardo
2011-11-04 18:33   ` Andre Guedes
2011-11-04 18:51 ` Andre Guedes
2011-11-07  9:03 ` Andrei Emeltchenko
2011-11-07 13:58   ` Andre Guedes
2011-11-07 14:09     ` Andrei Emeltchenko
2011-11-07 14:27       ` Andre Guedes
2011-11-07 13:59 ` Andre Guedes
2011-11-07 23:53   ` Marcel Holtmann
2011-11-08 14:47     ` Gustavo Padovan
2011-11-08 17:40     ` Andre Guedes
2011-11-07 14:45 ` Andre Guedes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.