From: Andrei Emeltchenko <[email protected]>
Changes:
v1: Changed HCI_<block,packet>_FLOW_CTL_MODE => HCI_FLOW_CTL_MODE_<block,packet>
RFCv1: Initial version
AMP initialization and block flow control code.
Resubmit "remove old code" patch as it seems that there is little
sense to copy it over.
Andrei Emeltchenko (6):
Bluetooth: remove old code
Bluetooth: Split ctrl init to BREDR and AMP parts
Bluetooth: Add HCI Read Flow Control Mode function
Bluetooth: Initialize default flow control mode
Bluetooth: Add HCI Read Data Block Size function
Bluetooth: Recalculate sched for HCI block flow ctrl
include/net/bluetooth/hci.h | 18 +++++++
include/net/bluetooth/hci_core.h | 4 ++
net/bluetooth/hci_core.c | 103 +++++++++++++++++++++++++------------
net/bluetooth/hci_event.c | 53 +++++++++++++++++++
4 files changed, 144 insertions(+), 34 deletions(-)
--
1.7.4.1
Hi Marcel, Andrei,
> Hi Andrei,
> > >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 4a0391e..360b44c 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2303,15 +2305,28 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> >
> > skb = skb_dequeue(&chan->data_q);
> >
> > + if (hdev->flow_ctl_mode ==
> > + HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > + /* Calculate count of blocks used by
> > + * this packet
> > + */
> > + blocks = DIV_ROUND_UP(skb->len -
> > + HCI_ACL_HDR_SIZE, hdev->block_len);
>
> would it not be more efficient to have to functions here? One
> hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> specific details of the flow control mode. And the hci_sched_acl would
> just decide which on to call once.
There has to be a check for which flow control mode is used by the device somewhere, and
this fragment is really the only difference between the two modes, in separate functions
the rest would be duplicate code.
> > +
> > + if (blocks > hdev->acl_cnt)
> > + return;
> > +
> > hci_conn_enter_active_mode(chan->conn,
> > bt_cb(skb)->force_active);
>
> Now an important question that comes up here. The support for the power
> save mode (sniff mainly) and AMP controllers is fully pointless. We need
> to actually make sure we don't try to put an AMP controller into sniff
> mode since that seems like an attempt for failure.
>
> Regards
>
> Marcel
>
>
Regards,
Peter.
--Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Andrei,
> Upstream Code Aurora code with trivial fixes.
> Origin: git://codeaurora.org/kernel/msm.git
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_core.c | 25 ++++++++++++++++++++-----
> 1 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 4a0391e..360b44c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2290,10 +2290,12 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>
> cnt = hdev->acl_cnt;
>
> - while (hdev->acl_cnt &&
> + while (hdev->acl_cnt > 0 &&
> (chan = hci_chan_sent(hdev, ACL_LINK, "e))) {
> u32 priority = (skb_peek(&chan->data_q))->priority;
> - while (quote-- && (skb = skb_peek(&chan->data_q))) {
> + while (quote > 0 && (skb = skb_peek(&chan->data_q))) {
> + int blocks = 1;
> +
> BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
> skb->len, skb->priority);
>
> @@ -2303,15 +2305,28 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>
> skb = skb_dequeue(&chan->data_q);
>
> + if (hdev->flow_ctl_mode ==
> + HCI_FLOW_CTL_MODE_BLOCK_BASED)
> + /* Calculate count of blocks used by
> + * this packet
> + */
> + blocks = DIV_ROUND_UP(skb->len -
> + HCI_ACL_HDR_SIZE, hdev->block_len);
would it not be more efficient to have to functions here? One
hci_sched_acl_block and one hci_sched_acl_pkt which implement the
specific details of the flow control mode. And the hci_sched_acl would
just decide which on to call once.
> +
> + if (blocks > hdev->acl_cnt)
> + return;
> +
> hci_conn_enter_active_mode(chan->conn,
> bt_cb(skb)->force_active);
Now an important question that comes up here. The support for the power
save mode (sniff mainly) and AMP controllers is fully pointless. We need
to actually make sure we don't try to put an AMP controller into sniff
mode since that seems like an attempt for failure.
Regards
Marcel
Hi Andrei,
> Upstream Code Aurora function with minor trivial fixes.
> Origin: git://codeaurora.org/kernel/msm.git
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> include/net/bluetooth/hci.h | 8 ++++++++
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 0290aeb..7d41bd7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -741,6 +741,14 @@ struct hci_rp_read_bd_addr {
> bdaddr_t bdaddr;
> } __packed;
>
> +#define HCI_OP_READ_DATA_BLOCK_SIZE 0x100a
> +struct hci_rp_read_data_block_size {
> + __u8 status;
> + __le16 max_acl_len;
> + __le16 block_len;
> + __le16 num_blocks;
> +} __packed;
> +
> #define HCI_OP_WRITE_PAGE_SCAN_ACTIVITY 0x0c1c
> struct hci_cp_write_page_scan_activity {
> __le16 interval;
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index a7fd63a..869ab72 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -172,6 +172,8 @@ struct hci_dev {
>
> __u8 flow_ctl_mode;
>
> + __u16 block_len;
> +
> unsigned int auto_accept_delay;
>
> unsigned long quirks;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index b743fc3..50482a6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -774,6 +774,33 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
> hci_req_complete(hdev, HCI_OP_READ_BD_ADDR, rp->status);
> }
>
> +static void hci_cc_read_data_block_size(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + struct hci_rp_read_data_block_size *rp = (void *) skb->data;
> +
> + BT_DBG("%s status 0x%x", hdev->name, rp->status);
> +
> + if (rp->status)
> + return;
> +
> + if (hdev->flow_ctl_mode == HCI_FLOW_CTL_MODE_BLOCK_BASED) {
> + hdev->acl_mtu = __le16_to_cpu(rp->max_acl_len);
> + hdev->sco_mtu = 0;
> + hdev->block_len = __le16_to_cpu(rp->block_len);
> + /* acl_pkts indicates the number of blocks */
> + hdev->acl_pkts = __le16_to_cpu(rp->num_blocks);
> + hdev->sco_pkts = 0;
> + hdev->acl_cnt = hdev->acl_pkts;
> + hdev->sco_cnt = 0;
> + }
I am not sure we want to use the same fields that we use for packet
based flow control.
What happens if we switch between the modes? Do we re-read the size
functions or how is that suppose to work out?
We could trigger the reading of the block size or packet size buffer
information based on the success of changing the flow control mode or an
initial reading of the flow control mode.
So the buffer command is triggered from hci_cc_read_flow_control_mode
function and you just need to trigger the reading of flow control mode
from the init functions.
To this extend we might then just change the init handling anyway and
have special HCI_INIT checks in the event callbacks that trigger the
next command to send. Make it true asynchronous and be able to react on
previous read information.
Just some thoughts here. Especially we might actually wanna deal with
AMP controller in packet based mode or BR/EDR controllers in block based
mode. Even if that is just for testing purposes.
Regards
Marcel
Hi Andrei,
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> include/net/bluetooth/hci.h | 4 ++++
> net/bluetooth/hci_core.c | 4 ++++
> 2 files changed, 8 insertions(+), 0 deletions(-)
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
Hi Andrei,
> Remove old code not touched for several years.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/hci_core.c | 12 ------------
> 1 files changed, 0 insertions(+), 12 deletions(-)
since you guys insist on taking this code out ;)
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
From: Andrei Emeltchenko <[email protected]>
Current controller initialization is moved tp bredr_init and new
function added amp_init to handle later AMP init sequence. Current
AMP init sequence include Reset and Read Local Version.
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_core.c | 64 ++++++++++++++++++++++++++++++++------------
net/bluetooth/hci_event.c | 3 ++
2 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc71f96..a0247d3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -192,33 +192,18 @@ static void hci_reset_req(struct hci_dev *hdev, unsigned long opt)
hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
}
-static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
+static void bredr_init(struct hci_dev *hdev)
{
struct hci_cp_delete_stored_link_key cp;
- struct sk_buff *skb;
__le16 param;
__u8 flt_type;
- BT_DBG("%s %ld", hdev->name, opt);
-
- /* Driver initialization */
-
- /* Special commands */
- while ((skb = skb_dequeue(&hdev->driver_init))) {
- bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
- skb->dev = (void *) hdev;
-
- skb_queue_tail(&hdev->cmd_q, skb);
- tasklet_schedule(&hdev->cmd_task);
- }
- skb_queue_purge(&hdev->driver_init);
-
/* Mandatory initialization */
/* Reset */
if (!test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
- set_bit(HCI_RESET, &hdev->flags);
- hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
+ set_bit(HCI_RESET, &hdev->flags);
+ hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
}
/* Read Local Supported Features */
@@ -257,6 +242,49 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
hci_send_cmd(hdev, HCI_OP_DELETE_STORED_LINK_KEY, sizeof(cp), &cp);
}
+static void amp_init(struct hci_dev *hdev)
+{
+ /* Reset */
+ hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
+
+ /* Read Local Version */
+ hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
+}
+
+static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
+{
+ struct sk_buff *skb;
+
+ BT_DBG("%s %ld", hdev->name, opt);
+
+ /* Driver initialization */
+
+ /* Special commands */
+ while ((skb = skb_dequeue(&hdev->driver_init))) {
+ bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
+ skb->dev = (void *) hdev;
+
+ skb_queue_tail(&hdev->cmd_q, skb);
+ tasklet_schedule(&hdev->cmd_task);
+ }
+ skb_queue_purge(&hdev->driver_init);
+
+ switch (hdev->dev_type) {
+ case HCI_BREDR:
+ bredr_init(hdev);
+ break;
+
+ case HCI_AMP:
+ amp_init(hdev);
+ break;
+
+ default:
+ BT_ERR("Unknown device type %d", hdev->dev_type);
+ break;
+ }
+
+}
+
static void hci_le_init_req(struct hci_dev *hdev, unsigned long opt)
{
BT_DBG("%s", hdev->name);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 27b1ede..f287e1c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -560,6 +560,9 @@ static void hci_set_le_support(struct hci_dev *hdev)
static void hci_setup(struct hci_dev *hdev)
{
+ if (hdev->dev_type != HCI_BREDR)
+ return;
+
hci_setup_event_mask(hdev);
if (hdev->hci_ver > BLUETOOTH_VER_1_1)
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/hci.h | 4 ++++
net/bluetooth/hci_core.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ee83c36..0290aeb 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -271,6 +271,10 @@ enum {
#define HCI_ERROR_LOCAL_HOST_TERM 0x16
#define HCI_ERROR_PAIRING_NOT_ALLOWED 0x18
+/* Flow control modes */
+#define HCI_FLOW_CTL_MODE_PACKET_BASED 0x00
+#define HCI_FLOW_CTL_MODE_BLOCK_BASED 0x01
+
/* ----- HCI Commands ---- */
#define HCI_OP_NOP 0x0000
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a0247d3..4a0391e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -198,6 +198,8 @@ static void bredr_init(struct hci_dev *hdev)
__le16 param;
__u8 flt_type;
+ hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_PACKET_BASED;
+
/* Mandatory initialization */
/* Reset */
@@ -244,6 +246,8 @@ static void bredr_init(struct hci_dev *hdev)
static void amp_init(struct hci_dev *hdev)
{
+ hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_BLOCK_BASED;
+
/* Reset */
hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Upstream Code Aurora function with minor trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git
Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/hci.h | 8 ++++++++
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0290aeb..7d41bd7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -741,6 +741,14 @@ struct hci_rp_read_bd_addr {
bdaddr_t bdaddr;
} __packed;
+#define HCI_OP_READ_DATA_BLOCK_SIZE 0x100a
+struct hci_rp_read_data_block_size {
+ __u8 status;
+ __le16 max_acl_len;
+ __le16 block_len;
+ __le16 num_blocks;
+} __packed;
+
#define HCI_OP_WRITE_PAGE_SCAN_ACTIVITY 0x0c1c
struct hci_cp_write_page_scan_activity {
__le16 interval;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a7fd63a..869ab72 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -172,6 +172,8 @@ struct hci_dev {
__u8 flow_ctl_mode;
+ __u16 block_len;
+
unsigned int auto_accept_delay;
unsigned long quirks;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b743fc3..50482a6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -774,6 +774,33 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
hci_req_complete(hdev, HCI_OP_READ_BD_ADDR, rp->status);
}
+static void hci_cc_read_data_block_size(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_read_data_block_size *rp = (void *) skb->data;
+
+ BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ if (hdev->flow_ctl_mode == HCI_FLOW_CTL_MODE_BLOCK_BASED) {
+ hdev->acl_mtu = __le16_to_cpu(rp->max_acl_len);
+ hdev->sco_mtu = 0;
+ hdev->block_len = __le16_to_cpu(rp->block_len);
+ /* acl_pkts indicates the number of blocks */
+ hdev->acl_pkts = __le16_to_cpu(rp->num_blocks);
+ hdev->sco_pkts = 0;
+ hdev->acl_cnt = hdev->acl_pkts;
+ hdev->sco_cnt = 0;
+ }
+
+ BT_DBG("%s acl mtu %d:%d, block len %d", hdev->name, hdev->acl_mtu,
+ hdev->acl_cnt, hdev->block_len);
+
+ hci_req_complete(hdev, HCI_OP_READ_DATA_BLOCK_SIZE, rp->status);
+}
+
static void hci_cc_write_ca_timeout(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
@@ -1981,6 +2008,10 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
hci_cc_read_bd_addr(hdev, skb);
break;
+ case HCI_OP_READ_DATA_BLOCK_SIZE:
+ hci_cc_read_data_block_size(hdev, skb);
+ break;
+
case HCI_OP_WRITE_CA_TIMEOUT:
hci_cc_write_ca_timeout(hdev, skb);
break;
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Upstream Code Aurora code with trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git
Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_core.c | 25 ++++++++++++++++++++-----
1 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4a0391e..360b44c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2290,10 +2290,12 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
cnt = hdev->acl_cnt;
- while (hdev->acl_cnt &&
+ while (hdev->acl_cnt > 0 &&
(chan = hci_chan_sent(hdev, ACL_LINK, "e))) {
u32 priority = (skb_peek(&chan->data_q))->priority;
- while (quote-- && (skb = skb_peek(&chan->data_q))) {
+ while (quote > 0 && (skb = skb_peek(&chan->data_q))) {
+ int blocks = 1;
+
BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
skb->len, skb->priority);
@@ -2303,15 +2305,28 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
skb = skb_dequeue(&chan->data_q);
+ if (hdev->flow_ctl_mode ==
+ HCI_FLOW_CTL_MODE_BLOCK_BASED)
+ /* Calculate count of blocks used by
+ * this packet
+ */
+ blocks = DIV_ROUND_UP(skb->len -
+ HCI_ACL_HDR_SIZE, hdev->block_len);
+
+ if (blocks > hdev->acl_cnt)
+ return;
+
hci_conn_enter_active_mode(chan->conn,
bt_cb(skb)->force_active);
hci_send_frame(skb);
hdev->acl_last_tx = jiffies;
- hdev->acl_cnt--;
- chan->sent++;
- chan->conn->sent++;
+ hdev->acl_cnt -= blocks;
+ quote -= blocks;
+
+ chan->sent += blocks;
+ chan->conn->sent += blocks;
}
}
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Upstream Code Aurora function with minor trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci.h | 6 ++++++
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_event.c | 19 +++++++++++++++++++
3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 376c574..ee83c36 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -677,6 +677,12 @@ struct hci_rp_read_local_oob_data {
#define HCI_OP_READ_INQ_RSP_TX_POWER 0x0c58
+#define HCI_OP_READ_FLOW_CONTROL_MODE 0x0c66
+struct hci_rp_read_flow_control_mode {
+ __u8 status;
+ __u8 mode;
+} __packed;
+
#define HCI_OP_WRITE_LE_HOST_SUPPORTED 0x0c6d
struct hci_cp_write_le_host_supported {
__u8 le;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1795257..a7fd63a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -170,6 +170,8 @@ struct hci_dev {
__u32 amp_max_flush_to;
__u32 amp_be_flush_to;
+ __u8 flow_ctl_mode;
+
unsigned int auto_accept_delay;
unsigned long quirks;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f287e1c..b743fc3 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -720,6 +720,21 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
hci_req_complete(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, rp->status);
}
+static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_read_flow_control_mode *rp = (void *) skb->data;
+
+ BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ hdev->flow_ctl_mode = rp->mode;
+
+ hci_req_complete(hdev, HCI_OP_READ_FLOW_CONTROL_MODE, rp->status);
+}
+
static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_rp_read_buffer_size *rp = (void *) skb->data;
@@ -1970,6 +1985,10 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
hci_cc_write_ca_timeout(hdev, skb);
break;
+ case HCI_OP_READ_FLOW_CONTROL_MODE:
+ hci_cc_read_flow_control_mode(hdev, skb);
+ break;
+
case HCI_OP_READ_LOCAL_AMP_INFO:
hci_cc_read_local_amp_info(hdev, skb);
break;
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Remove old code not touched for several years.
Signed-off-by: Andrei Emeltchenko <[email protected]>
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/hci_core.c | 12 ------------
1 files changed, 0 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ef0423e..bc71f96 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -230,18 +230,6 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
/* Read Buffer Size (ACL mtu, max pkt, etc.) */
hci_send_cmd(hdev, HCI_OP_READ_BUFFER_SIZE, 0, NULL);
-#if 0
- /* Host buffer size */
- {
- struct hci_cp_host_buffer_size cp;
- cp.acl_mtu = cpu_to_le16(HCI_MAX_ACL_SIZE);
- cp.sco_mtu = HCI_MAX_SCO_SIZE;
- cp.acl_max_pkt = cpu_to_le16(0xffff);
- cp.sco_max_pkt = cpu_to_le16(0xffff);
- hci_send_cmd(hdev, HCI_OP_HOST_BUFFER_SIZE, sizeof(cp), &cp);
- }
-#endif
-
/* Read BD Address */
hci_send_cmd(hdev, HCI_OP_READ_BD_ADDR, 0, NULL);
--
1.7.4.1
Hi Marcel,
On Wed, Nov 30, 2011 at 03:22:39PM +0100, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Upstream Code Aurora function with minor trivial fixes.
> > Origin: git://codeaurora.org/kernel/msm.git
> >
> > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > ---
> > include/net/bluetooth/hci.h | 8 ++++++++
> > include/net/bluetooth/hci_core.h | 2 ++
> > net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++++++
> > 3 files changed, 41 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 0290aeb..7d41bd7 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -741,6 +741,14 @@ struct hci_rp_read_bd_addr {
> > bdaddr_t bdaddr;
> > } __packed;
> >
> > +#define HCI_OP_READ_DATA_BLOCK_SIZE 0x100a
> > +struct hci_rp_read_data_block_size {
> > + __u8 status;
> > + __le16 max_acl_len;
> > + __le16 block_len;
> > + __le16 num_blocks;
> > +} __packed;
> > +
> > #define HCI_OP_WRITE_PAGE_SCAN_ACTIVITY 0x0c1c
> > struct hci_cp_write_page_scan_activity {
> > __le16 interval;
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index a7fd63a..869ab72 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -172,6 +172,8 @@ struct hci_dev {
> >
> > __u8 flow_ctl_mode;
> >
> > + __u16 block_len;
> > +
> > unsigned int auto_accept_delay;
> >
> > unsigned long quirks;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index b743fc3..50482a6 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -774,6 +774,33 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
> > hci_req_complete(hdev, HCI_OP_READ_BD_ADDR, rp->status);
> > }
> >
> > +static void hci_cc_read_data_block_size(struct hci_dev *hdev,
> > + struct sk_buff *skb)
> > +{
> > + struct hci_rp_read_data_block_size *rp = (void *) skb->data;
> > +
> > + BT_DBG("%s status 0x%x", hdev->name, rp->status);
> > +
> > + if (rp->status)
> > + return;
> > +
> > + if (hdev->flow_ctl_mode == HCI_FLOW_CTL_MODE_BLOCK_BASED) {
> > + hdev->acl_mtu = __le16_to_cpu(rp->max_acl_len);
> > + hdev->sco_mtu = 0;
> > + hdev->block_len = __le16_to_cpu(rp->block_len);
> > + /* acl_pkts indicates the number of blocks */
> > + hdev->acl_pkts = __le16_to_cpu(rp->num_blocks);
> > + hdev->sco_pkts = 0;
> > + hdev->acl_cnt = hdev->acl_pkts;
> > + hdev->sco_cnt = 0;
> > + }
>
> I am not sure we want to use the same fields that we use for packet
> based flow control.
I see this as a simplest way to reuse existing code.
> What happens if we switch between the modes? Do we re-read the size
> functions or how is that suppose to work out?
>
> We could trigger the reading of the block size or packet size buffer
> information based on the success of changing the flow control mode or an
> initial reading of the flow control mode.
>
> So the buffer command is triggered from hci_cc_read_flow_control_mode
> function and you just need to trigger the reading of flow control mode
> from the init functions.
This can be done in a separate patch.
> To this extend we might then just change the init handling anyway and
> have special HCI_INIT checks in the event callbacks that trigger the
> next command to send. Make it true asynchronous and be able to react on
> previous read information.
Maybe FLOW_CTRL_INIT ?
> Just some thoughts here. Especially we might actually wanna deal with
> AMP controller in packet based mode or BR/EDR controllers in block based
> mode. Even if that is just for testing purposes.
I think this is why we reuse existing fields.
Best regards
Andrei Emeltchenko
Hi Andrei,
> > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > > index 4a0391e..360b44c 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > > @@ -2303,15 +2305,28 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> > > >
> > > > skb = skb_dequeue(&chan->data_q);
> > > >
> > > > + if (hdev->flow_ctl_mode ==
> > > > + HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > > > + /* Calculate count of blocks used by
> > > > + * this packet
> > > > + */
> > > > + blocks = DIV_ROUND_UP(skb->len -
> > > > + HCI_ACL_HDR_SIZE, hdev->block_len);
> > >
> > > would it not be more efficient to have to functions here? One
> > > hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> > > specific details of the flow control mode. And the hci_sched_acl would
> > > just decide which on to call once.
> >
> > There has to be a check for which flow control mode is used by the device somewhere, and
> > this fragment is really the only difference between the two modes, in separate functions
> > the rest would be duplicate code.
>
> I feel the same, we can still make a function like:
are we really sure here. If we have more than one SKB in the queue, I
still feel that the check here is pointless. Especially since we do
certain code changes already.
> blocks = __get_blocks(hdev, skb);
>
> would this be better?
Lets give it a try. Can you whip up a patch for it.
> > > > +
> > > > + if (blocks > hdev->acl_cnt)
> > > > + return;
> > > > +
> > > > hci_conn_enter_active_mode(chan->conn,
> > > > bt_cb(skb)->force_active);
> > >
> > > Now an important question that comes up here. The support for the power
> > > save mode (sniff mainly) and AMP controllers is fully pointless. We need
> > > to actually make sure we don't try to put an AMP controller into sniff
> > > mode since that seems like an attempt for failure.
>
> We can test for BREDR controller in hci_conn_enter_active_mode function.
> This function is already too big.
That is what I am saying, we are running code that we know already is
only required for BR/EDR controllers. And now we have two checks here.
Actually maybe just flipping a bit and entering active before or after
might be a way here.
This is a fundamental hot path in our data processing. We need to be
smarter here.
Regards
Marcel
Hi Marcel, Peter,
On Wed, Nov 30, 2011 at 10:24:33PM -0800, Peter Krystad wrote:
> Hi Marcel, Andrei,
>
> > Hi Andrei,
> > > >
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index 4a0391e..360b44c 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -2303,15 +2305,28 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> > >
> > > skb = skb_dequeue(&chan->data_q);
> > >
> > > + if (hdev->flow_ctl_mode ==
> > > + HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > > + /* Calculate count of blocks used by
> > > + * this packet
> > > + */
> > > + blocks = DIV_ROUND_UP(skb->len -
> > > + HCI_ACL_HDR_SIZE, hdev->block_len);
> >
> > would it not be more efficient to have to functions here? One
> > hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> > specific details of the flow control mode. And the hci_sched_acl would
> > just decide which on to call once.
>
> There has to be a check for which flow control mode is used by the device somewhere, and
> this fragment is really the only difference between the two modes, in separate functions
> the rest would be duplicate code.
I feel the same, we can still make a function like:
blocks = __get_blocks(hdev, skb);
would this be better?
> > > +
> > > + if (blocks > hdev->acl_cnt)
> > > + return;
> > > +
> > > hci_conn_enter_active_mode(chan->conn,
> > > bt_cb(skb)->force_active);
> >
> > Now an important question that comes up here. The support for the power
> > save mode (sniff mainly) and AMP controllers is fully pointless. We need
> > to actually make sure we don't try to put an AMP controller into sniff
> > mode since that seems like an attempt for failure.
We can test for BREDR controller in hci_conn_enter_active_mode function.
This function is already too big.
Best regards
Andrei Emeltchenko