[PATCHv2 0/8] AMP/BREDR initialization patches

[PATCHv2 0/8] AMP/BREDR initialization patches

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Changes:
	v2: added helper function for block calculation, added patches for
	checking BREDR when going to Sniff mode and complete Read Local Version
	HCI command.
	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 (8):
  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
  Bluetooth: Use SNIFF mode only for BREDR controller
  Bluetooth: Complete Read Local Version HCI cmd

 include/net/bluetooth/hci.h      |   18 ++++++
 include/net/bluetooth/hci_core.h |    4 +
 net/bluetooth/hci_conn.c         |    6 ++
 net/bluetooth/hci_core.c         |  116 +++++++++++++++++++++++++++-----------
 net/bluetooth/hci_event.c        |   58 +++++++++++++++++++-
 5 files changed, 167 insertions(+), 35 deletions(-)

-- 
1.7.4.1

[PATCHv2 1/8] Bluetooth: remove old code

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Remove old code not touched for several years.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 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

Re: [PATCHv2 1/8] Bluetooth: remove old code

[Gustavo Padovan]

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-12-01 14:42:08 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Remove old code not touched for several years.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/hci_core.c |   12 ------------
>  1 files changed, 0 insertions(+), 12 deletions(-)

Applied, thanks.

	Gustavo

[PATCHv2 2/8] Bluetooth: Split ctrl init to BREDR and AMP parts

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

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 <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 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 957cdd5..f81b47b 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

[PATCHv2 3/8] Bluetooth: Add HCI Read Flow Control Mode function

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Upstream Code Aurora function with minor trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 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 b8dfdb0..453e7fc 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 f81b47b..efad012 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;
@@ -1978,6 +1993,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

[PATCHv2 4/8] Bluetooth: Initialize default flow control mode

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 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 453e7fc..a6a2509 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

[PATCHv2 5/8] Bluetooth: Add HCI Read Data Block Size function

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Upstream Code Aurora function with minor trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 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 a6a2509..c5de256 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 efad012..a646310 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);
@@ -1989,6 +2016,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

Re: [PATCHv2 5/8] Bluetooth: Add HCI Read Data Block Size function

[Marcel Holtmann]

Hi Andrei,

> Upstream Code Aurora function with minor trivial fixes.
> Origin: git://codeaurora.org/kernel/msm.git
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  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 a6a2509..c5de256 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 efad012..a646310 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 still not convinced that overloading the variables is a good idea.
And so far nobody has me convinced that this is the best way.

Regards

Marcel

Re: [PATCHv2 5/8] Bluetooth: Add HCI Read Data Block Size function

[Emeltchenko Andrei]

Hi Marcel,

On Thu, Dec 01, 2011 at 02:13:43PM +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 <andrei.emeltchenko@intel.com>
> > ---
> >  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 a6a2509..c5de256 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 efad012..a646310 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 still not convinced that overloading the variables is a good idea.
> And so far nobody has me convinced that this is the best way.

I think that we can use different variables like block_cnt and acl_blocks,
then function split as you mentioned in other comment does make sense.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv2 5/8] Bluetooth: Add HCI Read Data Block Size function

[Emeltchenko Andrei]

Hi Marcel,

On Thu, Dec 01, 2011 at 03:37:33PM +0200, Emeltchenko Andrei wrote:
> > > 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;
> > > +

What do you think about following:

<------8<----------------------------------------
|  diff --git a/include/net/bluetooth/hci_core.h
|  b/include/net/bluetooth/hci_core.h
|  index 44f130f..5a4e85c 100644
|  @@ -188,6 +188,11 @@ struct hci_dev {
|          unsigned int    sco_pkts;
|          unsigned int    le_pkts;
|
|  +       __u16           block_len;
|  +       __u16           block_mtu;
|  +       __u16           num_blocks;
|  +       __u16           block_cnt;
|  +
|          unsigned long   acl_last_tx;
|          unsigned long   sco_last_tx;
|          unsigned long   le_last_tx;
|
<------8<----------------------------------------

> > >  	unsigned int	auto_accept_delay;
> > >  
> > >  	unsigned long	quirks;
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index efad012..a646310 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 still not convinced that overloading the variables is a good idea.
> > And so far nobody has me convinced that this is the best way.
> 
> I think that we can use different variables like block_cnt and acl_blocks,
> then function split as you mentioned in other comment does make sense.

then here comes:

<------8<--------------------------------------------------------------
|  +       if (hdev->flow_ctl_mode == HCI_FLOW_CTL_MODE_BLOCK_BASED) {
|  +               hdev->block_mtu = __le16_to_cpu(rp->max_acl_len);
|  +               hdev->block_len = __le16_to_cpu(rp->block_len);
|  +               hdev->num_blocks = __le16_to_cpu(rp->num_blocks);
|  +
|  +               hdev->block_cnt = hdev->num_blocks;
|  +       }
<------8<--------------------------------------------------------------

Best regards 
Andrei Emeltchenko 

Re: [PATCHv2 5/8] Bluetooth: Add HCI Read Data Block Size function

[Marcel Holtmann]

Hi Andrei,

> > > > 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;
> > > > +
> 
> What do you think about following:
> 
> <------8<----------------------------------------
> |  diff --git a/include/net/bluetooth/hci_core.h
> |  b/include/net/bluetooth/hci_core.h
> |  index 44f130f..5a4e85c 100644
> |  @@ -188,6 +188,11 @@ struct hci_dev {
> |          unsigned int    sco_pkts;
> |          unsigned int    le_pkts;
> |
> |  +       __u16           block_len;
> |  +       __u16           block_mtu;
> |  +       __u16           num_blocks;
> |  +       __u16           block_cnt;
> |  +
> |          unsigned long   acl_last_tx;
> |          unsigned long   sco_last_tx;
> |          unsigned long   le_last_tx;
> |
> <------8<----------------------------------------

lets see how this works out.

> > > >  	unsigned int	auto_accept_delay;
> > > >  
> > > >  	unsigned long	quirks;
> > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > index efad012..a646310 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 still not convinced that overloading the variables is a good idea.
> > > And so far nobody has me convinced that this is the best way.
> > 
> > I think that we can use different variables like block_cnt and acl_blocks,
> > then function split as you mentioned in other comment does make sense.
> 
> then here comes:
> 
> <------8<--------------------------------------------------------------
> |  +       if (hdev->flow_ctl_mode == HCI_FLOW_CTL_MODE_BLOCK_BASED) {
> |  +               hdev->block_mtu = __le16_to_cpu(rp->max_acl_len);
> |  +               hdev->block_len = __le16_to_cpu(rp->block_len);
> |  +               hdev->num_blocks = __le16_to_cpu(rp->num_blocks);
> |  +
> |  +               hdev->block_cnt = hdev->num_blocks;
> |  +       }
> <------8<--------------------------------------------------------------

Except that you do not need the if flow_ctl_mode check. Just set the
variables. They are not overlapping anymore.

Regards

Marcel

[PATCHv2 6/8] Bluetooth: Recalculate sched for HCI block flow ctrl

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Upstream Code Aurora code with trivial fixes.
Origin: git://codeaurora.org/kernel/msm.git
---
 net/bluetooth/hci_core.c |   38 +++++++++++++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4a0391e..6769eb5 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2269,6 +2269,26 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type)
 	}
 }
 
+static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	int blocks;
+
+	switch (hdev->flow_ctl_mode) {
+	case 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);
+		break;
+
+	case HCI_FLOW_CTL_MODE_PACKET_BASED:
+	default:
+		blocks = 1;
+		break;
+	}
+
+	return blocks;
+}
+
 static inline void hci_sched_acl(struct hci_dev *hdev)
 {
 	struct hci_chan *chan;
@@ -2290,10 +2310,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, &quote))) {
 		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 +2325,21 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
 
 			skb = skb_dequeue(&chan->data_q);
 
+			blocks = __get_blocks(hdev, skb);
+			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

[PATCHv2 7/8] Bluetooth: Use SNIFF mode only for BREDR controller

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 net/bluetooth/hci_conn.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b328ac6..beb280e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -749,6 +749,9 @@ void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active)
 	if (test_bit(HCI_RAW, &hdev->flags))
 		return;
 
+	if (hdev->dev_type != HCI_BREDR)
+		return;
+
 	if (conn->mode != HCI_CM_SNIFF)
 		goto timer;
 
@@ -783,6 +786,9 @@ void hci_conn_enter_sniff_mode(struct hci_conn *conn)
 	if (conn->mode != HCI_CM_ACTIVE || !(conn->link_policy & HCI_LP_SNIFF))
 		return;
 
+	if (hdev->dev_type != HCI_BREDR)
+		return;
+
 	if (lmp_sniffsubr_capable(hdev) && lmp_sniffsubr_capable(conn)) {
 		struct hci_cp_sniff_subrate cp;
 		cp.handle             = cpu_to_le16(conn->handle);
-- 
1.7.4.1

Re: [PATCHv2 7/8] Bluetooth: Use SNIFF mode only for BREDR controller

[Marcel Holtmann]

Hi Andrei,

>  net/bluetooth/hci_conn.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index b328ac6..beb280e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -749,6 +749,9 @@ void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active)
>  	if (test_bit(HCI_RAW, &hdev->flags))
>  		return;
>  
> +	if (hdev->dev_type != HCI_BREDR)
> +		return;
> +
>  	if (conn->mode != HCI_CM_SNIFF)
>  		goto timer;
>  
> @@ -783,6 +786,9 @@ void hci_conn_enter_sniff_mode(struct hci_conn *conn)
>  	if (conn->mode != HCI_CM_ACTIVE || !(conn->link_policy & HCI_LP_SNIFF))
>  		return;
>  
> +	if (hdev->dev_type != HCI_BREDR)
> +		return;
> +
>  	if (lmp_sniffsubr_capable(hdev) && lmp_sniffsubr_capable(conn)) {
>  		struct hci_cp_sniff_subrate cp;
>  		cp.handle             = cpu_to_le16(conn->handle);

I am really not sure that this is the best solution. It sounds like way
to much overhead. Essentially I read this is being ignorant to what kind
of controller we have and only last last minute we bother checking.

Regards

Marcel

Re: [PATCHv2 7/8] Bluetooth: Use SNIFF mode only for BREDR controller

[Emeltchenko Andrei]

Hi Marcel,

On Thu, Dec 01, 2011 at 02:11:56PM +0100, Marcel Holtmann wrote:
> Hi Andrei,
> 
> >  net/bluetooth/hci_conn.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index b328ac6..beb280e 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -749,6 +749,9 @@ void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active)
> >  	if (test_bit(HCI_RAW, &hdev->flags))
> >  		return;
> >  
> > +	if (hdev->dev_type != HCI_BREDR)
> > +		return;
> > +
> >  	if (conn->mode != HCI_CM_SNIFF)
> >  		goto timer;
> >  
> > @@ -783,6 +786,9 @@ void hci_conn_enter_sniff_mode(struct hci_conn *conn)
> >  	if (conn->mode != HCI_CM_ACTIVE || !(conn->link_policy & HCI_LP_SNIFF))
> >  		return;
> >  
> > +	if (hdev->dev_type != HCI_BREDR)
> > +		return;
> > +
> >  	if (lmp_sniffsubr_capable(hdev) && lmp_sniffsubr_capable(conn)) {
> >  		struct hci_cp_sniff_subrate cp;
> >  		cp.handle             = cpu_to_le16(conn->handle);
> 
> I am really not sure that this is the best solution. It sounds like way
> to much overhead. Essentially I read this is being ignorant to what kind
> of controller we have and only last last minute we bother checking.

Do you think that the check shall be the first in
enter_[sniff|active]_mode commands or it shall be somewhere before those functions?

BTW: Can compiler do something for us if we use "unlikely" before those
checks?

Best regards 
Andrei Emeltchenko 

[PATCHv2 8/8] Bluetooth: Complete Read Local Version HCI cmd

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Mark request status as done for Read Local Version HCI command.
---
 net/bluetooth/hci_event.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a646310..e064aa1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -598,7 +598,7 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
 	BT_DBG("%s status 0x%x", hdev->name, rp->status);
 
 	if (rp->status)
-		return;
+		goto done;
 
 	hdev->hci_ver = rp->hci_ver;
 	hdev->hci_rev = __le16_to_cpu(rp->hci_rev);
@@ -612,6 +612,9 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (test_bit(HCI_INIT, &hdev->flags))
 		hci_setup(hdev);
+
+done:
+	hci_req_complete(hdev, HCI_OP_READ_LOCAL_VERSION, rp->status);
 }
 
 static void hci_setup_link_policy(struct hci_dev *hdev)
-- 
1.7.4.1

Re: [PATCHv2 8/8] Bluetooth: Complete Read Local Version HCI cmd

[Marcel Holtmann]

Hi Andrei,

> Mark request status as done for Read Local Version HCI command.
> ---
>  net/bluetooth/hci_event.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a646310..e064aa1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -598,7 +598,7 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
>  	BT_DBG("%s status 0x%x", hdev->name, rp->status);
>  
>  	if (rp->status)
> -		return;
> +		goto done;
>  
>  	hdev->hci_ver = rp->hci_ver;
>  	hdev->hci_rev = __le16_to_cpu(rp->hci_rev);
> @@ -612,6 +612,9 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
>  
>  	if (test_bit(HCI_INIT, &hdev->flags))
>  		hci_setup(hdev);
> +
> +done:
> +	hci_req_complete(hdev, HCI_OP_READ_LOCAL_VERSION, rp->status);
>  }

this is all getting out of control. We call hci_req_complete for random
functions now. And that is because our init handling sucks.

Regards

Marcel

Re: [PATCHv2 8/8] Bluetooth: Complete Read Local Version HCI cmd

[Emeltchenko Andrei]

Hi Marcel,

On Thu, Dec 01, 2011 at 02:12:57PM +0100, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Mark request status as done for Read Local Version HCI command.
> > ---
> >  net/bluetooth/hci_event.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index a646310..e064aa1 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -598,7 +598,7 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
> >  	BT_DBG("%s status 0x%x", hdev->name, rp->status);
> >  
> >  	if (rp->status)
> > -		return;
> > +		goto done;
> >  
> >  	hdev->hci_ver = rp->hci_ver;
> >  	hdev->hci_rev = __le16_to_cpu(rp->hci_rev);
> > @@ -612,6 +612,9 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
> >  
> >  	if (test_bit(HCI_INIT, &hdev->flags))
> >  		hci_setup(hdev);
> > +
> > +done:
> > +	hci_req_complete(hdev, HCI_OP_READ_LOCAL_VERSION, rp->status);
> >  }
> 
> this is all getting out of control. We call hci_req_complete for random
> functions now. And that is because our init handling sucks.

You are right here. The reason for this patch is AMP initialization where
this HCI command is last and does not "complete itself".

Maybe we need to "complete" each HCI Request?

Best regards 
Andrei Emeltchenko