Use HCI device type to determine suitability of HCI commands
Subsystems (2)
v2 cover Use HCI device type to determine suitability of HCI commands
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
Signature · 3 lines
David Scherba Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
PATCH v2 1/2 Remove non-functional hci_devinfo calls in init_device()
--- plugins/hciops.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
plugins/hciops.c +0 -8
@@ -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 );
Signature · 5 lines
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.
Re: [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
Hi David,
Quoted 3 lines · David Scherba
--- plugins/hciops.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
This patch is now upstream. Thanks.
Johan
PATCH v2 2/2 Use HCI device type to gate BR/EDR-specific HCI commands
In reply to David ScherbaUse 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(-)
lib/hci_lib.h +5 -0
@@ -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) {
plugins/hciops.c +13 -7
@@ -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); }
src/adapter.c +1 -1
@@ -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 ; }
src/security.c +2 -2
@@ -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 );
Signature · 5 lines
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.
Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
Review
Quoted 2 lines · David Scherba
(quoted diff) +1
-1- 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
Signature · 3 lines
David Scherba Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
PATCH v2 2/2 Use HCI device type to gate BR/EDR-specific HCI commands
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(-)
lib/hci_lib.h +5 -0
@@ -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) {
plugins/hciops.c +13 -7
@@ -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); }
src/adapter.c +1 -1
@@ -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 ; }
src/security.c +2 -2
@@ -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 );
Signature · 5 lines
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.
Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
Hi David,
lib/hci_lib.h static inline int hci_test_bit(int nr, void *addr) · line 175
Quoted 10 lines · David Scherba
lib/hci_lib.h +4
-0@@ -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
Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
Quoted 10 lines · Johan Hedberg
lib/hci_lib.h static inline int hci_test_bit(int nr, void *addr) · line 175
Quoted 4 lines
lib/hci_lib.h +4
-0+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
Signature · 3 lines
David Scherba Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
PATCH 1/1 Use HCI device type to gate BR/EDR-specific HCI commands
In reply to Johan HedbergUse 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(-)
plugins/hciops.c +13 -7
@@ -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); }
src/adapter.c +1 -1
@@ -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 ; }
src/hcid.h +8 -0
@@ -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 ; +}
src/security.c +2 -2
@@ -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 );
Signature · 5 lines
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.
Re: [PATCH 1/1] Use HCI device type to gate BR/EDR-specific HCI commands
Hi David,
Quoted 8 lines · David Scherba
Use the
hci_dev_infostructure 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