* [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 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 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 ` [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).