Re: [PATCH 3/6] Bluetooth: Use LE buffers for LE traffic

[Claudio Takahasi <claudio.takahasi@xxxxxxxxxxxxx>]

Hi Ville,

On Mon, Oct 25, 2010 at 10:21 AM, Ville Tervo <ville.tervo@xxxxxxxxx> wrote:
> BLuetooth chips may have separate buffers for
> LE traffic. This patch add support to use LE
> buffers provided by the chip.
>
> Signed-off-by: Ville Tervo <ville.tervo@xxxxxxxxx>
> ---
> Âinclude/net/bluetooth/hci.h   Â|  Â1 +
> Âinclude/net/bluetooth/hci_core.h | Â Â5 +++
> Ânet/bluetooth/hci_conn.c     |  Â5 +++
> Ânet/bluetooth/hci_core.c     |  74 +++++++++++++++++++++++++++++++++++--
> Ânet/bluetooth/hci_event.c    Â|  40 +++++++++++++++++++-
> Â5 files changed, 119 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 02055b9..2103731 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -189,6 +189,7 @@ enum {
>
> Â#define LMP_EV4 Â Â Â Â Â Â Â Â0x01
> Â#define LMP_EV5 Â Â Â Â Â Â Â Â0x02
> +#define LMP_LE Â Â Â Â 0x40
>
> Â#define LMP_SNIFF_SUBR 0x02
> Â#define LMP_EDR_ESCO_2M Â Â Â Â0x20
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 2b7f94a..e2d857a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -103,15 +103,19 @@ struct hci_dev {
>    Âatomic_t    Âcmd_cnt;
>    Âunsigned int  Âacl_cnt;
>    Âunsigned int  Âsco_cnt;
> +    unsigned int  Âle_cnt;
>
>    Âunsigned int  Âacl_mtu;
>    Âunsigned int  Âsco_mtu;
> +    unsigned int  Âle_mtu;
>    Âunsigned int  Âacl_pkts;
>    Âunsigned int  Âsco_pkts;
> +    unsigned int  Âle_pkts;
>
>    Âunsigned long  cmd_last_tx;
>    Âunsigned long  acl_last_tx;
>    Âunsigned long  sco_last_tx;
> +    unsigned long  le_last_tx;
>
> Â Â Â Âstruct workqueue_struct *workqueue;
>
> @@ -473,6 +477,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> Â#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
> Â#define lmp_esco_capable(dev) Â Â Â((dev)->features[3] & LMP_ESCO)
> Â#define lmp_ssp_capable(dev) Â Â Â ((dev)->features[6] & LMP_SIMPLE_PAIR)
> +#define lmp_le_capable(dev) Â Â Â Â((dev)->features[4] & LMP_LE)
>
> Â/* ----- HCI protocols ----- */
> Âstruct hci_proto {
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0944c0c..ddc2e5e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -324,6 +324,11 @@ int hci_conn_del(struct hci_conn *conn)
>
> Â Â Â Â Â Â Â Â/* Unacked frames */
> Â Â Â Â Â Â Â Âhdev->acl_cnt += conn->sent;
> + Â Â Â } else if (conn->type == LE_LINK) {
> + Â Â Â Â Â Â Â if (hdev->le_pkts)
> + Â Â Â Â Â Â Â Â Â Â Â hdev->le_cnt += conn->sent;
> + Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â hdev->acl_cnt += conn->sent;
> Â Â Â Â} else {
> Â Â Â Â Â Â Â Âstruct hci_conn *acl = conn->link;
> Â Â Â Â Â Â Â Âif (acl) {
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index bc2a052..45c78c2 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -254,6 +254,14 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
> Â Â Â Âhci_send_cmd(hdev, HCI_OP_WRITE_CA_TIMEOUT, 2, &param);
> Â}
>
> +static void hci_le_init_req(struct hci_dev *hdev, unsigned long opt)
> +{
> + Â Â Â BT_DBG("%s", hdev->name);
> +
> + Â Â Â /* Read LE buffer size */
> + Â Â Â hci_send_cmd(hdev, HCI_OP_LE_READ_BUFFER_SIZE, 0, NULL);
> +}
> +
> Âstatic void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
> Â{
> Â Â Â Â__u8 scan = opt;
> @@ -509,6 +517,10 @@ int hci_dev_open(__u16 dev)
> Â Â Â Â Â Â Â Âret = __hci_request(hdev, hci_init_req, 0,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âmsecs_to_jiffies(HCI_INIT_TIMEOUT));
>
> + Â Â Â Â Â Â Â if (lmp_le_capable(hdev))
> + Â Â Â Â Â Â Â Â Â Â Â ret = __hci_request(hdev, hci_le_init_req, 0,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â msecs_to_jiffies(HCI_INIT_TIMEOUT));
> +
> Â Â Â Â Â Â Â Âclear_bit(HCI_INIT, &hdev->flags);
> Â Â Â Â}
>
> @@ -645,7 +657,7 @@ int hci_dev_reset(__u16 dev)
> Â Â Â Â Â Â Â Âhdev->flush(hdev);
>
> Â Â Â Âatomic_set(&hdev->cmd_cnt, 1);
> - Â Â Â hdev->acl_cnt = 0; hdev->sco_cnt = 0;
> + Â Â Â hdev->acl_cnt = 0; hdev->sco_cnt = 0; hdev->le_cnt = 0;
>
> Â Â Â Âif (!test_bit(HCI_RAW, &hdev->flags))
> Â Â Â Â Â Â Â Âret = __hci_request(hdev, hci_reset_req, 0,
> @@ -1456,8 +1468,25 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
> Â Â Â Â}
>
> Â Â Â Âif (conn) {
> - Â Â Â Â Â Â Â int cnt = (type == ACL_LINK ? hdev->acl_cnt : hdev->sco_cnt);
> - Â Â Â Â Â Â Â int q = cnt / num;
> + Â Â Â Â Â Â Â int cnt, q;
> +
> + Â Â Â Â Â Â Â switch (conn->type) {
> + Â Â Â Â Â Â Â case ACL_LINK:
> + Â Â Â Â Â Â Â Â Â Â Â cnt = hdev->acl_cnt;
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case SCO_LINK:
> + Â Â Â Â Â Â Â case ESCO_LINK:
> + Â Â Â Â Â Â Â Â Â Â Â cnt = hdev->sco_cnt;
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case LE_LINK:
> + Â Â Â Â Â Â Â Â Â Â Â cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â default:
> + Â Â Â Â Â Â Â Â Â Â Â cnt = 0;
> + Â Â Â Â Â Â Â Â Â Â Â BT_ERR("Unknown link type");
> + Â Â Â Â Â Â Â }
> +
> + Â Â Â Â Â Â Â q = cnt / num;
> Â Â Â Â Â Â Â Â*quote = q ? q : 1;
> Â Â Â Â} else
> Â Â Â Â Â Â Â Â*quote = 0;
> @@ -1556,6 +1585,40 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
> Â Â Â Â}
> Â}
>
> +static inline void hci_sched_le(struct hci_dev *hdev)
> +{
> + Â Â Â struct hci_conn *conn;
> + Â Â Â struct sk_buff *skb;
> + Â Â Â int quote, cnt;
> +
> + Â Â Â BT_DBG("%s", hdev->name);
> +
> + Â Â Â if (!test_bit(HCI_RAW, &hdev->flags)) {
> + Â Â Â Â Â Â Â /* ACL tx timeout must be longer than maximum
> + Â Â Â Â Â Â Â Â* link supervision timeout (40.9 seconds) */
> + Â Â Â Â Â Â Â if (!hdev->le_cnt &&
> + Â Â Â Â Â Â Â Â Â time_after(jiffies, hdev->le_last_tx + HZ * 45))
> + Â Â Â Â Â Â Â Â Â Â Â hci_acl_tx_to(hdev);
> + Â Â Â }

It seems that the ACL tx timeout is causing some problems: BR/EDR and
LE connections are not working properly on macbooks! Don't ask me why
on macbooks only! But I double checked. I tested your branch and also
bluetooth-next + LE patches and both are not working as expected. I
didn't have time to investigate it, do you have any clue?

For BR/EDR I am receiving "killing stalled ACL connection" messages
for all connections.


Regards,
Claudio


> +
> + Â Â Â cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
> + Â Â Â while (cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
> + Â Â Â Â Â Â Â while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> + Â Â Â Â Â Â Â Â Â Â Â BT_DBG("skb %p len %d", skb, skb->len);
> +
> + Â Â Â Â Â Â Â Â Â Â Â hci_send_frame(skb);
> + Â Â Â Â Â Â Â Â Â Â Â hdev->le_last_tx = jiffies;
> +
> + Â Â Â Â Â Â Â Â Â Â Â cnt--;
> + Â Â Â Â Â Â Â Â Â Â Â conn->sent++;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> + Â Â Â if (hdev->le_pkts)
> + Â Â Â Â Â Â Â hdev->le_cnt = cnt;
> + Â Â Â else
> + Â Â Â Â Â Â Â hdev->acl_cnt = cnt;
> +}
> +
> Âstatic void hci_tx_task(unsigned long arg)
> Â{
> Â Â Â Âstruct hci_dev *hdev = (struct hci_dev *) arg;
> @@ -1563,7 +1626,8 @@ static void hci_tx_task(unsigned long arg)
>
> Â Â Â Âread_lock(&hci_task_lock);
>
> - Â Â Â BT_DBG("%s acl %d sco %d", hdev->name, hdev->acl_cnt, hdev->sco_cnt);
> + Â Â Â BT_DBG("%s acl %d sco %d le %d", hdev->name, hdev->acl_cnt,
> + Â Â Â Â Â Â Â hdev->sco_cnt, hdev->le_cnt);
>
> Â Â Â Â/* Schedule queues and send stuff to HCI driver */
>
> @@ -1573,6 +1637,8 @@ static void hci_tx_task(unsigned long arg)
>
> Â Â Â Âhci_sched_esco(hdev);
>
> + Â Â Â hci_sched_le(hdev);
> +
> Â Â Â Â/* Send next queued raw (unknown type) packet */
> Â Â Â Âwhile ((skb = skb_dequeue(&hdev->raw_q)))
> Â Â Â Â Â Â Â Âhci_send_frame(skb);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 92c8621..da23502 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -539,6 +539,26 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
> Â Â Â Âhci_req_complete(hdev, rp->status);
> Â}
>
> +static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct sk_buff *skb)
> +{
> + Â Â Â struct hci_rp_le_read_buffer_size *rp = (void *) skb->data;
> +
> + Â Â Â BT_DBG("%s status 0x%x", hdev->name, rp->status);
> +
> + Â Â Â if (rp->status)
> + Â Â Â Â Â Â Â return;
> +
> + Â Â Â hdev->le_mtu = __le16_to_cpu(rp->le_mtu);
> + Â Â Â hdev->le_pkts = rp->le_max_pkt;
> +
> + Â Â Â hdev->le_cnt = hdev->le_pkts;
> +
> + Â Â Â BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts);
> +
> + Â Â Â hci_req_complete(hdev, rp->status);
> +}
> +
> Âstatic inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
> Â{
> Â Â Â ÂBT_DBG("%s status 0x%x", hdev->name, status);
> @@ -1353,6 +1373,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_LE_READ_BUFFER_SIZE:
> + Â Â Â Â Â Â Â hci_cc_le_read_buffer_size(hdev, skb);
> + Â Â Â Â Â Â Â break;
> +
> Â Â Â Âdefault:
> Â Â Â Â Â Â Â ÂBT_DBG("%s opcode 0x%x", hdev->name, opcode);
> Â Â Â Â Â Â Â Âbreak;
> @@ -1490,10 +1514,22 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
> Â Â Â Â Â Â Â Â Â Â Â Âconn->sent -= count;
>
> Â Â Â Â Â Â Â Â Â Â Â Âif (conn->type == ACL_LINK) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if ((hdev->acl_cnt += count) > hdev->acl_pkts)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->acl_cnt += count;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (hdev->acl_cnt > hdev->acl_pkts)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âhdev->acl_cnt = hdev->acl_pkts;
> + Â Â Â Â Â Â Â Â Â Â Â } else if (conn->type == LE_LINK) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (hdev->le_pkts) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->le_cnt += count;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (hdev->le_cnt > hdev->le_pkts)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->le_cnt = hdev->le_pkts;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â } else {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->acl_cnt += count;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (hdev->acl_cnt > hdev->acl_pkts)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->acl_cnt = hdev->acl_pkts;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â} else {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if ((hdev->sco_cnt += count) > hdev->sco_pkts)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hdev->sco_cnt += count;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (hdev->sco_cnt > hdev->sco_pkts)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âhdev->sco_cnt = hdev->sco_pkts;
> Â Â Â Â Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â}
> --
> 1.7.1
>
ÿô.nlj·Ÿ®‰­†+%ŠË±é¥Šwÿº{.nlj·¥Š{±ý¶â^n‡r¡öë¨è&£ûz¹Þúzf£¢·hšˆ§~†­†Ûÿÿïÿ‘ê_èæ+v‰¨þ)ßø