* [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command
@ 2012-02-27 23:41 johan.hedberg
2012-02-27 23:41 ` [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands johan.hedberg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: johan.hedberg @ 2012-02-27 23:41 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
If the local host features indicate that LE is already in the state that
is desired there's no point in sending the HCI command to try to change
the setting.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 27830f4..5373bc1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1260,7 +1260,7 @@ static int set_le(struct sock *sk, u16 index, void *data, u16 len)
struct pending_cmd *cmd;
struct hci_dev *hdev;
int err;
- u8 val;
+ u8 val, enabled;
BT_DBG("request for hci%u", index);
@@ -1280,8 +1280,9 @@ static int set_le(struct sock *sk, u16 index, void *data, u16 len)
}
val = !!cp->val;
+ enabled = !!(hdev->host_features[0] & 0x02);
- if (!hdev_is_powered(hdev)) {
+ if (!hdev_is_powered(hdev) || val == enabled) {
bool changed = false;
if (val != test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
--
1.7.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands 2012-02-27 23:41 [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command johan.hedberg @ 2012-02-27 23:41 ` johan.hedberg 2012-02-28 8:01 ` Andrei Emeltchenko 2012-02-27 23:43 ` [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command Marcel Holtmann 2012-02-28 17:31 ` Andre Guedes 2 siblings, 1 reply; 6+ messages in thread From: johan.hedberg @ 2012-02-27 23:41 UTC (permalink / raw) To: linux-bluetooth From: Johan Hedberg <johan.hedberg@intel.com> Previously the write_le_enable would trigger a read_host_features command but since we have access to the value LE support was set to we can simply just clear or set the bit in hdev->host_features. This also removes a second unnecessary read_host_features command from the device initialization procedure since LE is only enabled after the first read_host_features command completes. Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> --- net/bluetooth/hci_event.c | 59 ++++++++++++++++++++++++++------------------- 1 files changed, 34 insertions(+), 25 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index e920cd5..84a6d2e 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -542,20 +542,6 @@ static void hci_setup_event_mask(struct hci_dev *hdev) hci_send_cmd(hdev, HCI_OP_SET_EVENT_MASK, sizeof(events), events); } -static void hci_set_le_support(struct hci_dev *hdev) -{ - struct hci_cp_write_le_host_supported cp; - - memset(&cp, 0, sizeof(cp)); - - if (enable_le && test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) { - cp.le = 1; - cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR); - } - - hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp), &cp); -} - static void hci_setup(struct hci_dev *hdev) { if (hdev->dev_type != HCI_BREDR) @@ -608,9 +594,6 @@ static void hci_setup(struct hci_dev *hdev) hci_send_cmd(hdev, HCI_OP_WRITE_AUTH_ENABLE, sizeof(enable), &enable); } - - if (hdev->features[4] & LMP_LE) - hci_set_le_support(hdev); } static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb) @@ -730,6 +713,22 @@ static void hci_cc_read_local_features(struct hci_dev *hdev, struct sk_buff *skb hdev->features[6], hdev->features[7]); } +static void hci_set_le_support(struct hci_dev *hdev) +{ + struct hci_cp_write_le_host_supported cp; + + memset(&cp, 0, sizeof(cp)); + + if (enable_le && test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) { + cp.le = 1; + cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR); + } + + if (cp.le != !!(hdev->host_features[0] & 0x02)) + hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, + sizeof(cp), &cp); +} + static void hci_cc_read_local_ext_features(struct hci_dev *hdev, struct sk_buff *skb) { @@ -738,7 +737,7 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev, BT_DBG("%s status 0x%x", hdev->name, rp->status); if (rp->status) - return; + goto done; switch (rp->page) { case 0: @@ -749,6 +748,10 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev, break; } + if (test_bit(HCI_INIT, &hdev->flags) && hdev->features[4] & LMP_LE) + hci_set_le_support(hdev); + +done: hci_req_complete(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, rp->status); } @@ -1149,21 +1152,27 @@ static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb) static inline void hci_cc_write_le_host_supported(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_cp_read_local_ext_features cp; struct hci_cp_write_le_host_supported *sent; __u8 status = *((__u8 *) skb->data); BT_DBG("%s status 0x%x", hdev->name, status); sent = hci_sent_cmd_data(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED); - if (sent && test_bit(HCI_MGMT, &hdev->dev_flags)) - mgmt_le_enable_complete(hdev, sent->le, status); - - if (status) + if (!sent) return; - cp.page = 0x01; - hci_send_cmd(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, sizeof(cp), &cp); + if (!status) { + if (sent->le) + hdev->host_features[0] |= 0x02; + else + hdev->host_features[0] &= !0x02; + } + + if (test_bit(HCI_MGMT, &hdev->dev_flags) && + !test_bit(HCI_INIT, &hdev->flags)) + mgmt_le_enable_complete(hdev, sent->le, status); + + hci_req_complete(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, status); } static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status) -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands 2012-02-27 23:41 ` [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands johan.hedberg @ 2012-02-28 8:01 ` Andrei Emeltchenko 2012-02-28 8:53 ` Johan Hedberg 0 siblings, 1 reply; 6+ messages in thread From: Andrei Emeltchenko @ 2012-02-28 8:01 UTC (permalink / raw) To: johan.hedberg; +Cc: linux-bluetooth Hi Johan, On Tue, Feb 28, 2012 at 01:41:34AM +0200, johan.hedberg@gmail.com wrote: > From: Johan Hedberg <johan.hedberg@intel.com> > > Previously the write_le_enable would trigger a read_host_features > command but since we have access to the value LE support was set to we > can simply just clear or set the bit in hdev->host_features. This also > removes a second unnecessary read_host_features command from the device > initialization procedure since LE is only enabled after the first > read_host_features command completes. > > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > --- > net/bluetooth/hci_event.c | 59 ++++++++++++++++++++++++++------------------- > 1 files changed, 34 insertions(+), 25 deletions(-) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index e920cd5..84a6d2e 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -542,20 +542,6 @@ static void hci_setup_event_mask(struct hci_dev *hdev) > hci_send_cmd(hdev, HCI_OP_SET_EVENT_MASK, sizeof(events), events); > } > > -static void hci_set_le_support(struct hci_dev *hdev) > -{ > - struct hci_cp_write_le_host_supported cp; > - > - memset(&cp, 0, sizeof(cp)); > - > - if (enable_le && test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) { > - cp.le = 1; > - cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR); > - } > - > - hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp), &cp); > -} > - > static void hci_setup(struct hci_dev *hdev) > { > if (hdev->dev_type != HCI_BREDR) > @@ -608,9 +594,6 @@ static void hci_setup(struct hci_dev *hdev) > hci_send_cmd(hdev, HCI_OP_WRITE_AUTH_ENABLE, > sizeof(enable), &enable); > } > - > - if (hdev->features[4] & LMP_LE) > - hci_set_le_support(hdev); > } > > static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb) > @@ -730,6 +713,22 @@ static void hci_cc_read_local_features(struct hci_dev *hdev, struct sk_buff *skb > hdev->features[6], hdev->features[7]); > } > > +static void hci_set_le_support(struct hci_dev *hdev) > +{ > + struct hci_cp_write_le_host_supported cp; > + > + memset(&cp, 0, sizeof(cp)); > + > + if (enable_le && test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) { > + cp.le = 1; Do you mean here "true"? > + cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR); > + } > + > + if (cp.le != !!(hdev->host_features[0] & 0x02)) > + hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, > + sizeof(cp), &cp); > +} > + > static void hci_cc_read_local_ext_features(struct hci_dev *hdev, > struct sk_buff *skb) > { > @@ -738,7 +737,7 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev, > BT_DBG("%s status 0x%x", hdev->name, rp->status); > > if (rp->status) > - return; > + goto done; > > switch (rp->page) { > case 0: > @@ -749,6 +748,10 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev, > break; > } > > + if (test_bit(HCI_INIT, &hdev->flags) && hdev->features[4] & LMP_LE) > + hci_set_le_support(hdev); > + > +done: > hci_req_complete(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, rp->status); > } > > @@ -1149,21 +1152,27 @@ static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb) > static inline void hci_cc_write_le_host_supported(struct hci_dev *hdev, > struct sk_buff *skb) > { > - struct hci_cp_read_local_ext_features cp; > struct hci_cp_write_le_host_supported *sent; > __u8 status = *((__u8 *) skb->data); > > BT_DBG("%s status 0x%x", hdev->name, status); > > sent = hci_sent_cmd_data(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED); > - if (sent && test_bit(HCI_MGMT, &hdev->dev_flags)) > - mgmt_le_enable_complete(hdev, sent->le, status); > - > - if (status) > + if (!sent) > return; > > - cp.page = 0x01; > - hci_send_cmd(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, sizeof(cp), &cp); > + if (!status) { > + if (sent->le) > + hdev->host_features[0] |= 0x02; > + else > + hdev->host_features[0] &= !0x02; BTW: what is 0x02? I saw it also in the second patch. Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands 2012-02-28 8:01 ` Andrei Emeltchenko @ 2012-02-28 8:53 ` Johan Hedberg 0 siblings, 0 replies; 6+ messages in thread From: Johan Hedberg @ 2012-02-28 8:53 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth Hi Andrei, On Tue, Feb 28, 2012, Andrei Emeltchenko wrote: > > + if (enable_le && test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) { > > + cp.le = 1; > > Do you mean here "true"? No, cp.le is a u8 and not a bool. > > - cp.page = 0x01; > > - hci_send_cmd(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, sizeof(cp), &cp); > > + if (!status) { > > + if (sent->le) > > + hdev->host_features[0] |= 0x02; > > + else > > + hdev->host_features[0] &= !0x02; > > BTW: what is 0x02? I saw it also in the second patch. It's LMP_HOST_LE (i.e. the host features bit indicating that LE support is enabled for the host). I fixed this to use the proper define before applying the patch. Johan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command 2012-02-27 23:41 [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command johan.hedberg 2012-02-27 23:41 ` [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands johan.hedberg @ 2012-02-27 23:43 ` Marcel Holtmann 2012-02-28 17:31 ` Andre Guedes 2 siblings, 0 replies; 6+ messages in thread From: Marcel Holtmann @ 2012-02-27 23:43 UTC (permalink / raw) To: johan.hedberg; +Cc: linux-bluetooth Hi Johan, > If the local host features indicate that LE is already in the state that > is desired there's no point in sending the HCI command to try to change > the setting. > > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > --- > net/bluetooth/mgmt.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) Acked-by: Marcel Holtmann <marcel@holtmann.org> Regards Marcel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command 2012-02-27 23:41 [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command johan.hedberg 2012-02-27 23:41 ` [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands johan.hedberg 2012-02-27 23:43 ` [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command Marcel Holtmann @ 2012-02-28 17:31 ` Andre Guedes 2 siblings, 0 replies; 6+ messages in thread From: Andre Guedes @ 2012-02-28 17:31 UTC (permalink / raw) To: johan.hedberg; +Cc: linux-bluetooth Hi Johan, On Mon, Feb 27, 2012 at 8:41 PM, <johan.hedberg@gmail.com> wrote: > From: Johan Hedberg <johan.hedberg@intel.com> > > If the local host features indicate that LE is already in the state that > is desired there's no point in sending the HCI command to try to change > the setting. > > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > --- > net/bluetooth/mgmt.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > index 27830f4..5373bc1 100644 > --- a/net/bluetooth/mgmt.c > +++ b/net/bluetooth/mgmt.c > @@ -1260,7 +1260,7 @@ static int set_le(struct sock *sk, u16 index, void *data, u16 len) > struct pending_cmd *cmd; > struct hci_dev *hdev; > int err; > - u8 val; > + u8 val, enabled; > > BT_DBG("request for hci%u", index); > > @@ -1280,8 +1280,9 @@ static int set_le(struct sock *sk, u16 index, void *data, u16 len) > } > > val = !!cp->val; > + enabled = !!(hdev->host_features[0] & 0x02); You may use lmp_host_le_capable macro here. There are other places in patch 2/2 you may wanna change too. BR, Andre ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-28 17:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-27 23:41 [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command johan.hedberg 2012-02-27 23:41 ` [PATCH 2/2] Bluetooth: Remove redundant read_host_features commands johan.hedberg 2012-02-28 8:01 ` Andrei Emeltchenko 2012-02-28 8:53 ` Johan Hedberg 2012-02-27 23:43 ` [PATCH 1/2] Bluetooth: Don't send unnecessary write_le_enable command Marcel Holtmann 2012-02-28 17:31 ` Andre Guedes
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).