linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-09 21:07 [PATCH 0/2] Use HCI device type to determine suitability of " David Scherba
@ 2010-08-09 21:07 ` David Scherba
  2010-08-09 21:59   ` Johan Hedberg
  0 siblings, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-09 21:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba

Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
---
 lib/hci_lib.h    |    5 +++++
 plugins/hciops.c |   23 +++++++++++++++--------
 src/adapter.c    |    3 ++-
 src/security.c   |    6 ++++--
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..c0786ab 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
 	return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
 }
 
+static inline int is_bredr_hci_device_type(uint8_t type)
+{
+  return (type >> 4) == HCI_BREDR;
+}
+
 /* HCI filter tools */
 static inline void hci_filter_clear(struct hci_filter *f)
 {
diff --git a/plugins/hciops.c b/plugins/hciops.c
index b38c056..705e9dd 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,8 @@ static void device_devup_setup(int index)
 	if (hci_devinfo(index, &di) < 0)
 		return;
 
-	if (hci_test_bit(HCI_RAW, &di.flags))
+	if (!is_bredr_hci_device_type(di.type) ||
+	    hci_test_bit(HCI_RAW, &di.flags))
 		return;
 
 	dd = hci_open_dev(index);
@@ -160,12 +161,17 @@ static void init_device(int index)
 					index, strerror(err), err);
 	}
 
-	/* Set link policy */
-	dr.dev_opt = main_opts.link_policy;
-	if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
-							errno != ENETDOWN) {
-		error("Can't set link policy on hci%d: %s (%d)",
-					index, strerror(errno), errno);
+	/* Set link policy for BR/EDR HCI devices */
+	if (hci_devinfo(index, &di) < 0)
+		goto fail;
+
+	if (is_bredr_hci_device_type(di.type)) {
+		dr.dev_opt = main_opts.link_policy;
+		if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+								errno != ENETDOWN) {
+			error("Can't set link policy on hci%d: %s (%d)",
+						index, strerror(errno), errno);
+		}
 	}
 
 	/* Start HCI device */
@@ -198,7 +204,8 @@ static void device_devreg_setup(int index)
 
 	devup = hci_test_bit(HCI_UP, &di.flags);
 
-	if (!hci_test_bit(HCI_RAW, &di.flags))
+	if (is_bredr_hci_device_type(di.type) &&
+	    !hci_test_bit(HCI_RAW, &di.flags))
 		manager_register_adapter(index, devup);
 }
 
diff --git a/src/adapter.c b/src/adapter.c
index 7d0e34d..acc1068 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2314,7 +2314,8 @@ int adapter_start(struct btd_adapter *adapter)
 	if (hci_devinfo(adapter->dev_id, &di) < 0)
 		return -errno;
 
-	if (hci_test_bit(HCI_RAW, &di.flags)) {
+	if (!is_bredr_hci_device_type(di.type) ||
+	    hci_test_bit(HCI_RAW, &di.flags)) {
 		dev->ignore = 1;
 		return -1;
 	}
diff --git a/src/security.c b/src/security.c
index ca394e1..6d07d86 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,8 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
 
 	ioctl(dev, HCIGETDEVINFO, (void *) di);
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (!is_bredr_hci_device_type(di->type) ||
+	    hci_test_bit(HCI_RAW, &di->flags))
 		return TRUE;
 
 	switch (eh->evt) {
@@ -1185,7 +1186,8 @@ void start_security_manager(int hdev)
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (!is_bredr_hci_device_type(di->type) ||
+	    hci_test_bit(HCI_RAW, &di->flags))
 		return;
 
 	bacpy(&cp.bdaddr, BDADDR_ANY);
-- 
1.7.0
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-09 21:07 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific " David Scherba
@ 2010-08-09 21:59   ` Johan Hedberg
  0 siblings, 0 replies; 12+ messages in thread
From: Johan Hedberg @ 2010-08-09 21:59 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer

Hi David,

On Mon, Aug 09, 2010, David Scherba wrote:
> +static inline int is_bredr_hci_device_type(uint8_t type)

Could you just call this ignore_device() which combines the AMP and RAW
checks.

> +{
> +  return (type >> 4) == HCI_BREDR;
> +}

Looks like the indentation is wrong inside the function (two spaces
instead of a tab).

> -	if (hci_test_bit(HCI_RAW, &di.flags))
> +	if (!is_bredr_hci_device_type(di.type) ||
> +	    hci_test_bit(HCI_RAW, &di.flags))

No spaces+tabs mixed indentation please. The second line should be
indented by at least two tabs more than the line above it (as many tabs
as you can as long as the line doesn't go beyond 79 characters). OTOH,
the second line goes away completely if you do the ignore_device
simplification as proposed above.

> -	if (!hci_test_bit(HCI_RAW, &di.flags))
> +	if (is_bredr_hci_device_type(di.type) &&
> +	    !hci_test_bit(HCI_RAW, &di.flags))

Same here.

> -	if (hci_test_bit(HCI_RAW, &di.flags)) {
> +	if (!is_bredr_hci_device_type(di.type) ||
> +	    hci_test_bit(HCI_RAW, &di.flags)) {

And here.

> -	if (hci_test_bit(HCI_RAW, &di->flags))
> +	if (!is_bredr_hci_device_type(di->type) ||
> +	    hci_test_bit(HCI_RAW, &di->flags))

And here.

> -	if (hci_test_bit(HCI_RAW, &di->flags))
> +	if (!is_bredr_hci_device_type(di->type) ||
> +	    hci_test_bit(HCI_RAW, &di->flags))

And here.

Johan

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

* [PATCH v2 0/2] Use HCI device type to determine suitability of HCI commands
@ 2010-08-10 15:15 David Scherba
  2010-08-10 15:15 ` [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device() David Scherba
  2010-08-10 15:15 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands David Scherba
  0 siblings, 2 replies; 12+ messages in thread
From: David Scherba @ 2010-08-10 15:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer

BlueZ assumes that HCI devices are either BR/EDR radios, or "RAW" (via
HCI_RAW).  At present, alternate radios are handled as RAW (reference
Marcel's 943da25d9 kernel commit).  More subtlety in BlueZ is useful as
AMP support is enhanced.

These patches remove the need to treat non-BR/EDR HCI devices as "RAW."

Cheers,
David

-- 
David Scherba
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

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

* [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
  2010-08-10 15:15 [PATCH v2 0/2] Use HCI device type to determine suitability of HCI commands David Scherba
@ 2010-08-10 15:15 ` David Scherba
  2010-08-10 19:20   ` Johan Hedberg
  2010-08-10 15:15 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands David Scherba
  1 sibling, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-10 15:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba

---
 plugins/hciops.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 5775cf1..3e3e172 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -122,7 +122,6 @@ static void device_devup_setup(int index)
 static void init_device(int index)
 {
 	struct hci_dev_req dr;
-	struct hci_dev_info di;
 	pid_t pid;
 	int dd, err;
 
@@ -175,13 +174,6 @@ static void init_device(int index)
 		goto fail;
 	}
 
-	if (hci_devinfo(index, &di) < 0)
-		goto fail;
-
-	if (hci_test_bit(HCI_RAW, &di.flags))
-		goto done;
-
-done:
 	hci_close_dev(dd);
 	exit(0);
 
-- 
1.7.0.2
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 15:15 [PATCH v2 0/2] Use HCI device type to determine suitability of HCI commands David Scherba
  2010-08-10 15:15 ` [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device() David Scherba
@ 2010-08-10 15:15 ` David Scherba
  2010-08-10 15:44   ` David Scherba
  1 sibling, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-10 15:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba

Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
---
 lib/hci_lib.h    |    5 +++++
 plugins/hciops.c |   20 +++++++++++++-------
 src/adapter.c    |    2 +-
 src/security.c   |    4 ++--
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..814672d 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
 	return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
 }
 
+static inline int ignore_device(struct hci_dev_info *di)
+{
+	return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
+}
+
 /* HCI filter tools */
 static inline void hci_filter_clear(struct hci_filter *f)
 {
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 3e3e172..54c5bc0 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,7 @@ static void device_devup_setup(int index)
 	if (hci_devinfo(index, &di) < 0)
 		return;
 
-	if (hci_test_bit(HCI_RAW, &di.flags))
+	if (ignore_device(&di))
 		return;
 
 	dd = hci_open_dev(index);
@@ -122,6 +122,7 @@ static void device_devup_setup(int index)
 static void init_device(int index)
 {
 	struct hci_dev_req dr;
+	struct hci_dev_info di;
 	pid_t pid;
 	int dd, err;
 
@@ -159,12 +160,17 @@ static void init_device(int index)
 					index, strerror(err), err);
 	}
 
-	/* Set link policy */
-	dr.dev_opt = main_opts.link_policy;
-	if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+	/* Set link policy for BR/EDR HCI devices */
+	if (hci_devinfo(index, &di) < 0)
+		goto fail;
+
+	if (!ignore_device(&di)) {
+		dr.dev_opt = main_opts.link_policy;
+		if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
 							errno != ENETDOWN) {
-		error("Can't set link policy on hci%d: %s (%d)",
-					index, strerror(errno), errno);
+			error("Can't set link policy on hci%d: %s (%d)",
+						index, strerror(errno), errno);
+		}
 	}
 
 	/* Start HCI device */
@@ -196,7 +202,7 @@ static void device_devreg_setup(int index)
 
 	devup = hci_test_bit(HCI_UP, &di.flags);
 
-	if (!hci_test_bit(HCI_RAW, &di.flags))
+	if (ignore_device(&di))
 		manager_register_adapter(index, devup);
 }
 
diff --git a/src/adapter.c b/src/adapter.c
index af44a59..fc1e123 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2300,7 +2300,7 @@ int adapter_start(struct btd_adapter *adapter)
 	if (hci_devinfo(adapter->dev_id, &di) < 0)
 		return -errno;
 
-	if (hci_test_bit(HCI_RAW, &di.flags)) {
+	if (ignore_device(&di)) {
 		dev->ignore = 1;
 		return -1;
 	}
diff --git a/src/security.c b/src/security.c
index ca394e1..667f1f1 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,7 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
 
 	ioctl(dev, HCIGETDEVINFO, (void *) di);
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return TRUE;
 
 	switch (eh->evt) {
@@ -1185,7 +1185,7 @@ void start_security_manager(int hdev)
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return;
 
 	bacpy(&cp.bdaddr, BDADDR_ANY);
-- 
1.7.0.2
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 15:15 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands David Scherba
@ 2010-08-10 15:44   ` David Scherba
  2010-08-10 15:45     ` David Scherba
  0 siblings, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-10 15:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer

On 8/10/2010 10:15 AM, David Scherba wrote:
> -	if (!hci_test_bit(HCI_RAW,&di.flags))
> +	if (ignore_device(&di))

Johan--

This change is functionally incorrect--sorry about that.  Updating patch 
2/2.

Cheers,
David

-- 
David Scherba
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

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

* [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 15:44   ` David Scherba
@ 2010-08-10 15:45     ` David Scherba
  2010-08-10 19:16       ` Johan Hedberg
  0 siblings, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-10 15:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba

Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
---
 lib/hci_lib.h    |    5 +++++
 plugins/hciops.c |   20 +++++++++++++-------
 src/adapter.c    |    2 +-
 src/security.c   |    4 ++--
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..814672d 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
 	return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
 }
 
+static inline int ignore_device(struct hci_dev_info *di)
+{
+	return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
+}
+
 /* HCI filter tools */
 static inline void hci_filter_clear(struct hci_filter *f)
 {
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 3e3e172..9c97c5a 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,7 @@ static void device_devup_setup(int index)
 	if (hci_devinfo(index, &di) < 0)
 		return;
 
-	if (hci_test_bit(HCI_RAW, &di.flags))
+	if (ignore_device(&di))
 		return;
 
 	dd = hci_open_dev(index);
@@ -122,6 +122,7 @@ static void device_devup_setup(int index)
 static void init_device(int index)
 {
 	struct hci_dev_req dr;
+	struct hci_dev_info di;
 	pid_t pid;
 	int dd, err;
 
@@ -159,12 +160,17 @@ static void init_device(int index)
 					index, strerror(err), err);
 	}
 
-	/* Set link policy */
-	dr.dev_opt = main_opts.link_policy;
-	if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+	/* Set link policy for BR/EDR HCI devices */
+	if (hci_devinfo(index, &di) < 0)
+		goto fail;
+
+	if (!ignore_device(&di)) {
+		dr.dev_opt = main_opts.link_policy;
+		if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
 							errno != ENETDOWN) {
-		error("Can't set link policy on hci%d: %s (%d)",
-					index, strerror(errno), errno);
+			error("Can't set link policy on hci%d: %s (%d)",
+						index, strerror(errno), errno);
+		}
 	}
 
 	/* Start HCI device */
@@ -196,7 +202,7 @@ static void device_devreg_setup(int index)
 
 	devup = hci_test_bit(HCI_UP, &di.flags);
 
-	if (!hci_test_bit(HCI_RAW, &di.flags))
+	if (!ignore_device(&di))
 		manager_register_adapter(index, devup);
 }
 
diff --git a/src/adapter.c b/src/adapter.c
index af44a59..fc1e123 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2300,7 +2300,7 @@ int adapter_start(struct btd_adapter *adapter)
 	if (hci_devinfo(adapter->dev_id, &di) < 0)
 		return -errno;
 
-	if (hci_test_bit(HCI_RAW, &di.flags)) {
+	if (ignore_device(&di)) {
 		dev->ignore = 1;
 		return -1;
 	}
diff --git a/src/security.c b/src/security.c
index ca394e1..667f1f1 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,7 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
 
 	ioctl(dev, HCIGETDEVINFO, (void *) di);
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return TRUE;
 
 	switch (eh->evt) {
@@ -1185,7 +1185,7 @@ void start_security_manager(int hdev)
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return;
 
 	bacpy(&cp.bdaddr, BDADDR_ANY);
-- 
1.7.0.2
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 15:45     ` David Scherba
@ 2010-08-10 19:16       ` Johan Hedberg
  2010-08-10 19:49         ` David Scherba
  2010-08-10 19:51         ` [PATCH 1/1] " David Scherba
  0 siblings, 2 replies; 12+ messages in thread
From: Johan Hedberg @ 2010-08-10 19:16 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer

Hi David,

On Tue, Aug 10, 2010, David Scherba wrote:
> --- a/lib/hci_lib.h
> +++ b/lib/hci_lib.h
> @@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
>  	return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
>  }
>  
> +static inline int ignore_device(struct hci_dev_info *di)
> +{
> +	return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
> +}

Since this is hci_lib.h (something I didn't realize in the previous
review) the function should follow the same namespace as all other
functions in the header file, i.e. hci_*. However, does the function
really need to be exported in this public library header file, i.e.
isn't there some private header file that could be used instead?

Johan

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

* Re: [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
  2010-08-10 15:15 ` [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device() David Scherba
@ 2010-08-10 19:20   ` Johan Hedberg
  0 siblings, 0 replies; 12+ messages in thread
From: Johan Hedberg @ 2010-08-10 19:20 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer

Hi David,

On Tue, Aug 10, 2010, David Scherba wrote:
> ---
>  plugins/hciops.c |    8 --------
>  1 files changed, 0 insertions(+), 8 deletions(-)

This patch is now upstream. Thanks.

Johan

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

* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 19:16       ` Johan Hedberg
@ 2010-08-10 19:49         ` David Scherba
  2010-08-10 19:51         ` [PATCH 1/1] " David Scherba
  1 sibling, 0 replies; 12+ messages in thread
From: David Scherba @ 2010-08-10 19:49 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, marcel, rshaffer

On 8/10/2010 2:16 PM, Johan Hedberg wrote:

>> +static inline int ignore_device(struct hci_dev_info *di)
>> +{
>> +	return hci_test_bit(HCI_RAW,&di->flags) || di->type>>  4 != HCI_BREDR;
>> +}
>
> Since this is hci_lib.h (something I didn't realize in the previous
> review) the function should follow the same namespace as all other
> functions in the header file, i.e. hci_*. However, does the function
> really need to be exported in this public library header file, i.e.
> isn't there some private header file that could be used instead?

It looks like src/hcid.h fits the bill, although the modified hcid.h 
will need to include the hci headers to have the appropriate defines... 
Modified patch coming up.

Thanks,
David

-- 
David Scherba
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

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

* [PATCH 1/1] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 19:16       ` Johan Hedberg
  2010-08-10 19:49         ` David Scherba
@ 2010-08-10 19:51         ` David Scherba
  2010-08-10 20:15           ` Johan Hedberg
  1 sibling, 1 reply; 12+ messages in thread
From: David Scherba @ 2010-08-10 19:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba

Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
---
 plugins/hciops.c |   20 +++++++++++++-------
 src/adapter.c    |    2 +-
 src/hcid.h       |    8 ++++++++
 src/security.c   |    4 ++--
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 3e3e172..9c97c5a 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,7 @@ static void device_devup_setup(int index)
 	if (hci_devinfo(index, &di) < 0)
 		return;
 
-	if (hci_test_bit(HCI_RAW, &di.flags))
+	if (ignore_device(&di))
 		return;
 
 	dd = hci_open_dev(index);
@@ -122,6 +122,7 @@ static void device_devup_setup(int index)
 static void init_device(int index)
 {
 	struct hci_dev_req dr;
+	struct hci_dev_info di;
 	pid_t pid;
 	int dd, err;
 
@@ -159,12 +160,17 @@ static void init_device(int index)
 					index, strerror(err), err);
 	}
 
-	/* Set link policy */
-	dr.dev_opt = main_opts.link_policy;
-	if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+	/* Set link policy for BR/EDR HCI devices */
+	if (hci_devinfo(index, &di) < 0)
+		goto fail;
+
+	if (!ignore_device(&di)) {
+		dr.dev_opt = main_opts.link_policy;
+		if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
 							errno != ENETDOWN) {
-		error("Can't set link policy on hci%d: %s (%d)",
-					index, strerror(errno), errno);
+			error("Can't set link policy on hci%d: %s (%d)",
+						index, strerror(errno), errno);
+		}
 	}
 
 	/* Start HCI device */
@@ -196,7 +202,7 @@ static void device_devreg_setup(int index)
 
 	devup = hci_test_bit(HCI_UP, &di.flags);
 
-	if (!hci_test_bit(HCI_RAW, &di.flags))
+	if (!ignore_device(&di))
 		manager_register_adapter(index, devup);
 }
 
diff --git a/src/adapter.c b/src/adapter.c
index af44a59..fc1e123 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2300,7 +2300,7 @@ int adapter_start(struct btd_adapter *adapter)
 	if (hci_devinfo(adapter->dev_id, &di) < 0)
 		return -errno;
 
-	if (hci_test_bit(HCI_RAW, &di.flags)) {
+	if (ignore_device(&di)) {
 		dev->ignore = 1;
 		return -1;
 	}
diff --git a/src/hcid.h b/src/hcid.h
index 9c5f1d6..50bdb65 100644
--- a/src/hcid.h
+++ b/src/hcid.h
@@ -23,6 +23,9 @@
  *
  */
 
+#include <bluetooth/hci.h>
+#include <bluetooth/hci_lib.h>
+
 /* When all services should trust a remote device */
 #define GLOBAL_TRUST "[all]"
 
@@ -99,3 +102,8 @@ void rfkill_exit(void);
 
 void __probe_servers(const char *adapter);
 void __remove_servers(const char *adapter);
+
+static inline int ignore_device(struct hci_dev_info *di)
+{
+	return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
+}
diff --git a/src/security.c b/src/security.c
index ca394e1..667f1f1 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,7 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
 
 	ioctl(dev, HCIGETDEVINFO, (void *) di);
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return TRUE;
 
 	switch (eh->evt) {
@@ -1185,7 +1185,7 @@ void start_security_manager(int hdev)
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (ignore_device(di))
 		return;
 
 	bacpy(&cp.bdaddr, BDADDR_ANY);
-- 
1.7.0.2
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH 1/1] Use HCI device type to gate BR/EDR-specific HCI commands
  2010-08-10 19:51         ` [PATCH 1/1] " David Scherba
@ 2010-08-10 20:15           ` Johan Hedberg
  0 siblings, 0 replies; 12+ messages in thread
From: Johan Hedberg @ 2010-08-10 20:15 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer

Hi David,

On Tue, Aug 10, 2010, David Scherba wrote:
> Use the hci_dev_info structure member 'type' to classify whether a HCI device
> is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
> ---
>  plugins/hciops.c |   20 +++++++++++++-------
>  src/adapter.c    |    2 +-
>  src/hcid.h       |    8 ++++++++
>  src/security.c   |    4 ++--
>  4 files changed, 24 insertions(+), 10 deletions(-)

Thanks. This one is now also pushed upstream.

Johan

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

end of thread, other threads:[~2010-08-10 20:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-10 15:15 [PATCH v2 0/2] Use HCI device type to determine suitability of HCI commands David Scherba
2010-08-10 15:15 ` [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device() David Scherba
2010-08-10 19:20   ` Johan Hedberg
2010-08-10 15:15 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands David Scherba
2010-08-10 15:44   ` David Scherba
2010-08-10 15:45     ` David Scherba
2010-08-10 19:16       ` Johan Hedberg
2010-08-10 19:49         ` David Scherba
2010-08-10 19:51         ` [PATCH 1/1] " David Scherba
2010-08-10 20:15           ` Johan Hedberg
  -- strict thread matches above, loose matches on Subject: below --
2010-08-09 21:07 [PATCH 0/2] Use HCI device type to determine suitability of " David Scherba
2010-08-09 21:07 ` [PATCH 2/2] Use HCI device type to gate BR/EDR-specific " David Scherba
2010-08-09 21:59   ` Johan Hedberg

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).