* [PATCH 1/2] Fix not deleting stored keys when a new one is created
From: Luiz Augusto von Dentz @ 2010-08-11 12:44 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
When a new link key is created but it doesn't match any criteria to be
stored any previouly one should be removed from filesystem.
---
src/dbus-hci.c | 12 +++++++-----
src/device.c | 2 +-
src/device.h | 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index 6d27caa..22f378d 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -700,6 +700,13 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
/* Clear any previous debug key */
device_set_debug_key(device, NULL);
+ /* If this is not the first link key set a flag so a subsequent auth
+ * complete event doesn't trigger SDP and remove any stored key */
+ if (old_key_type != 0xff) {
+ device_set_renewed_key(device, TRUE);
+ device_remove_bonding(device);
+ }
+
/* Store the link key only in runtime memory if it's a debug
* key, else store the link key persistently if one of the
* following is true:
@@ -734,11 +741,6 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
} else
temporary = TRUE;
- /* If this is not the first link key set a flag so a subsequent auth
- * complete event doesn't trigger SDP */
- if (old_key_type != 0xff)
- device_set_renewed_key(device, TRUE);
-
if (!device_is_connected(device))
device_set_secmode3_conn(device, TRUE);
else if (!bonding && old_key_type == 0xff)
diff --git a/src/device.c b/src/device.c
index 10ba779..e72f289 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1056,7 +1056,7 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}
-static void device_remove_bonding(struct btd_device *device)
+void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
char srcaddr[18], dstaddr[18];
diff --git a/src/device.h b/src/device.h
index 5f75e61..21f67d0 100644
--- a/src/device.h
+++ b/src/device.h
@@ -66,7 +66,7 @@ void device_set_secmode3_conn(struct btd_device *device, gboolean enable);
DBusMessage *device_create_bonding(struct btd_device *device,
DBusConnection *conn, DBusMessage *msg,
const char *agent_path, uint8_t capability);
-void device_remove_bondind(struct btd_device *device, DBusConnection *connection);
+void device_remove_bonding(struct btd_device *device);
void device_bonding_complete(struct btd_device *device, uint8_t status);
void device_simple_pairing_complete(struct btd_device *device, uint8_t status);
gboolean device_is_creating(struct btd_device *device, const char *sender);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Solve compilation warning usin gcc 4.5.0
From: Jose Antonio Santos Cadenas @ 2010-08-11 9:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
---
audio/control.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/control.c b/audio/control.c
index c8aba53..daa01ef 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -446,7 +446,7 @@ static void avctp_set_state(struct control *control, avctp_state_t new_state)
avctp_disconnected(control->dev);
- if (old_state != AVCTP_STATE_CONNECTED)
+ if (old_state != AVDTP_SESSION_STATE_CONNECTED)
break;
value = FALSE;
--
1.7.2
^ permalink raw reply related
* Re: [RFC 7/7] Bluetooth: Do not limit enhanced L2CAP max PDU size to HCI MTU.
From: Gustavo F. Padovan @ 2010-08-11 5:25 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-8-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:04 -0700]:
> HCI MTU and PDU size have no relationship, since ERTM and streaming PDUs
> can be fragmented over HCI just like other L2CAP frames. The only
> applicable limit for PDU size is the L2CAP MTU, which is now checked
> during L2CAP configuration.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
This one is fine after the changes in the previous patches.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [RFC 6/7] Bluetooth: Reassemble enhanced L2CAP PDUs using skb fragments.
From: Gustavo F. Padovan @ 2010-08-11 5:24 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-7-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:03 -0700]:
> As enhanced L2CAP PDUs arrive, it is not necessary to copy them
> in to a separate skbuff. Instead, the skbuffs can be linked
> together as fragments, only being copied in to a linear buffer
> when the data is copied to userspace. This avoids the need
> to allocate additional buffers for incoming data, and
> eliminates copying of data payloads during SDU reassembly.
> This is of greater concern with high-speed AMP links than
> with BR/EDR.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/l2cap.c | 66 +++++++++++++++++++++-------------------
> 2 files changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2f3222f..9384e87 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -357,6 +357,7 @@ struct l2cap_pinfo {
> __u16 sdu_len;
> __u16 partial_sdu_len;
> struct sk_buff *sdu;
> + struct sk_buff *sdu_last_frag;
>
> __u8 ident;
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index b485c4a..0212035 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -290,6 +290,9 @@ static void l2cap_chan_del(struct sock *sk, int err)
> skb_queue_purge(SREJ_QUEUE(sk));
> skb_queue_purge(BUSY_QUEUE(sk));
>
> + if (l2cap_pi(sk)->sdu)
> + kfree_skb(l2cap_pi(sk)->sdu);
> +
> list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
> list_del(&l->list);
> kfree(l);
> @@ -3635,6 +3638,27 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
> return 0;
> }
>
> +static inline void append_skb_frag(struct sk_buff *skb,
> + struct sk_buff *new_frag, struct sk_buff **last_frag)
Call this l2cap_append_skb_frag()
> +{
> + /* skb->len reflects data in skb as well as all fragments
> + skb->data_len reflects only data in fragments
> + */
> + BT_DBG("skb %p, new_frag %p, *last_frag %p", skb, new_frag, *last_frag);
> +
> + if (!skb_has_frags(skb))
> + skb_shinfo(skb)->frag_list = new_frag;
> +
> + new_frag->next = NULL;
> +
> + (*last_frag)->next = new_frag;
> + *last_frag = new_frag;
> +
> + skb->len += new_frag->len;
> + skb->data_len += new_frag->len;
> + skb->truesize += new_frag->truesize;
> +}
> +
Wondering if it is possible to do that append more simple, but I looked
through the kernel code it's not possible, we have to keep last frag
pointer.
This change should go to l2cap_streaming_reassembly_sdu() as well, then
you can get ride of partial_sdu_len in the struct l2cap_pinfo.
> static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 control)
> {
> struct l2cap_pinfo *pi = l2cap_pi(sk);
> @@ -3643,7 +3667,7 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
>
> switch (control & L2CAP_CTRL_SAR) {
> case L2CAP_SDU_UNSEGMENTED:
> - if (pi->conn_state & L2CAP_CONN_SAR_SDU)
> + if (pi->sdu)
pi->sdu can do the work work of pi->conn_state & L2CAP_CONN_SAR_SDU, so
a separated patch for that change sounds better (you can change that for
the Streaming mode at the same time, it's a similar code, but more simple).
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* [RFC] Bluetooth: Option to enable power management from host
From: suraj Sumangala @ 2010-08-11 4:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jothikumar.Mothilal, suraj Sumangala
This provides a new option to enable advanced power management from
Host. This option by default will be disabled for existing devices.
---
tools/hciattach.8 | 12 +++++++
tools/hciattach.c | 88 ++++++++++++++++++++++++++++++++++++++---------------
2 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/tools/hciattach.8 b/tools/hciattach.8
index 9bd1538..1375905 100644
--- a/tools/hciattach.8
+++ b/tools/hciattach.8
@@ -122,7 +122,19 @@ the serial link (
set by default. To force no flow control use
.B noflow
instead.
+.TP
+.I sleep
+If the keyword
+.I sleep
+is appended to the list of options then power management is enabled
+on the controller. All above mentioned device types have
+.B nosleep
+set by default. To enable this feature use
+.B sleep
+instead.
+Note: This option will only be valid for hardware which support
+this feature.
.TP
.I bdaddr
The
diff --git a/tools/hciattach.c b/tools/hciattach.c
index 81aea7b..8616899 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -63,12 +63,15 @@ struct uart_t {
int init_speed;
int speed;
int flags;
+ int pm;
char *bdaddr;
int (*init) (int fd, struct uart_t *u, struct termios *ti);
int (*post) (int fd, struct uart_t *u, struct termios *ti);
};
#define FLOW_CTL 0x0001
+#define ENABLE_PM 1
+#define DISABLE_PM 0
static volatile sig_atomic_t __io_canceled = 0;
@@ -1006,72 +1009,100 @@ static int bcm2035(int fd, struct uart_t *u, struct termios *ti)
}
struct uart_t uart[] = {
- { "any", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, NULL },
- { "ericsson", 0x0000, 0x0000, HCI_UART_H4, 57600, 115200, FLOW_CTL, NULL, ericsson },
- { "digi", 0x0000, 0x0000, HCI_UART_H4, 9600, 115200, FLOW_CTL, NULL, digi },
+ { "any", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, NULL },
- { "bcsp", 0x0000, 0x0000, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "ericsson", 0x0000, 0x0000, HCI_UART_H4, 57600, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, ericsson },
+
+ { "digi", 0x0000, 0x0000, HCI_UART_H4, 9600, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, digi },
+
+ { "bcsp", 0x0000, 0x0000, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* Xircom PCMCIA cards: Credit Card Adapter and Real Port Adapter */
- { "xircom", 0x0105, 0x080a, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, NULL },
+ { "xircom", 0x0105, 0x080a, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, NULL },
/* CSR Casira serial adapter or BrainBoxes serial dongle (BL642) */
- { "csr", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, csr },
+ { "csr", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, csr },
/* BrainBoxes PCMCIA card (BL620) */
- { "bboxes", 0x0160, 0x0002, HCI_UART_H4, 115200, 460800, FLOW_CTL, NULL, csr },
+ { "bboxes", 0x0160, 0x0002, HCI_UART_H4, 115200, 460800,
+ FLOW_CTL, DISABLE_PM, NULL, csr },
/* Silicon Wave kits */
- { "swave", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, swave },
+ { "swave", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, swave },
/* Texas Instruments Bluelink (BRF) modules */
- { "texas", 0x0000, 0x0000, HCI_UART_LL, 115200, 115200, FLOW_CTL, NULL, texas, texas2 },
- { "texasalt", 0x0000, 0x0000, HCI_UART_LL, 115200, 115200, FLOW_CTL, NULL, texasalt, NULL },
+ { "texas", 0x0000, 0x0000, HCI_UART_LL, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, texas, texas2 },
+
+ { "texasalt", 0x0000, 0x0000, HCI_UART_LL, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, texasalt, NULL },
/* ST Microelectronics minikits based on STLC2410/STLC2415 */
- { "st", 0x0000, 0x0000, HCI_UART_H4, 57600, 115200, FLOW_CTL, NULL, st },
+ { "st", 0x0000, 0x0000, HCI_UART_H4, 57600, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, st },
/* ST Microelectronics minikits based on STLC2500 */
- { "stlc2500", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, "00:80:E1:00:AB:BA", stlc2500 },
+ { "stlc2500", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, "00:80:E1:00:AB:BA", stlc2500 },
/* Philips generic Ericsson IP core based */
- { "philips", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, NULL },
+ { "philips", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, NULL },
/* Philips BGB2xx Module */
- { "bgb2xx", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, "BD:B2:10:00:AB:BA", bgb2xx },
+ { "bgb2xx", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, "BD:B2:10:00:AB:BA", bgb2xx },
/* Sphinx Electronics PICO Card */
- { "picocard", 0x025e, 0x1000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, NULL },
+ { "picocard", 0x025e, 0x1000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, NULL },
/* Inventel BlueBird Module */
- { "inventel", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, NULL },
+ { "inventel", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, NULL },
/* COM One Platinium Bluetooth PC Card */
- { "comone", 0xffff, 0x0101, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "comone", 0xffff, 0x0101, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* TDK Bluetooth PC Card and IBM Bluetooth PC Card II */
- { "tdk", 0x0105, 0x4254, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "tdk", 0x0105, 0x4254, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* Socket Bluetooth CF Card (Rev G) */
- { "socket", 0x0104, 0x0096, HCI_UART_BCSP, 230400, 230400, 0, NULL, bcsp },
+ { "socket", 0x0104, 0x0096, HCI_UART_BCSP, 230400, 230400,
+ 0, DISABLE_PM, NULL, bcsp },
/* 3Com Bluetooth Card (Version 3.0) */
- { "3com", 0x0101, 0x0041, HCI_UART_H4, 115200, 115200, FLOW_CTL, NULL, csr },
+ { "3com", 0x0101, 0x0041, HCI_UART_H4, 115200, 115200,
+ FLOW_CTL, DISABLE_PM, NULL, csr },
/* AmbiCom BT2000C Bluetooth PC/CF Card */
- { "bt2000c", 0x022d, 0x2000, HCI_UART_H4, 57600, 460800, FLOW_CTL, NULL, csr },
+ { "bt2000c", 0x022d, 0x2000, HCI_UART_H4, 57600, 460800,
+ FLOW_CTL, DISABLE_PM, NULL, csr },
/* Zoom Bluetooth PCMCIA Card */
- { "zoom", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "zoom", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* Sitecom CN-504 PCMCIA Card */
- { "sitecom", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "sitecom", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* Billionton PCBTC1 PCMCIA Card */
- { "billionton", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200, 0, NULL, bcsp },
+ { "billionton", 0x0279, 0x950b, HCI_UART_BCSP, 115200, 115200,
+ 0, DISABLE_PM, NULL, bcsp },
/* Broadcom BCM2035 */
- { "bcm2035", 0x0A5C, 0x2035, HCI_UART_H4, 115200, 460800, FLOW_CTL, NULL, bcm2035 },
+ { "bcm2035", 0x0A5C, 0x2035, HCI_UART_H4, 115200, 460800,
+ FLOW_CTL, DISABLE_PM, NULL, bcm2035 },
{ NULL, 0 }
};
@@ -1290,6 +1321,13 @@ int main(int argc, char *argv[])
break;
case 4:
+ if (!strcmp("sleep", argv[optind]))
+ u->pm = ENABLE_PM;
+ else
+ u->pm = DISABLE_PM;
+ break;
+
+ case 5:
u->bdaddr = argv[optind];
break;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [RFC 5/7] Bluetooth: Handle fragmented skbs in bt_sock_stream_recvmsg()
From: Gustavo F. Padovan @ 2010-08-11 4:25 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-6-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:02 -0700]:
> When reading L2CAP skbuffs, add the ability to copy from
> fragmented skbuffs generated during ERTM or streaming mode
> reassembly. This defers extra copying of L2CAP payloads
> until the final, unavoidable copy to userspace.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/af_bluetooth.c | 30 ++++++++++++++++++++++++++++--
> 1 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 77a26fe..5e0375b 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -342,7 +342,7 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
> }
>
> chunk = min_t(unsigned int, skb->len, size);
> - if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
> + if (skb_copy_datagram_iovec(skb, 0, msg->msg_iov, chunk)) {
> skb_queue_head(&sk->sk_receive_queue, skb);
> if (!copied)
> copied = -EFAULT;
> @@ -354,7 +354,33 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
> sock_recv_ts_and_drops(msg, sk, skb);
>
> if (!(flags & MSG_PEEK)) {
> - skb_pull(skb, chunk);
> + int skb_len = skb_headlen(skb);
Why are you using the header length here?
> +
> + if (chunk <= skb_len) {
> + __skb_pull(skb, chunk);
> + } else {
> + struct sk_buff *frag;
> +
> + __skb_pull(skb, skb_len);
> + chunk -= skb_len;
Why do we have this __skb_pull() here? I think skb_walk_frags() can
handle everything.
> +
> + skb_walk_frags(skb, frag) {
> + if (chunk <= frag->len) {
> + /* Pulling partial data */
> + skb->len -= chunk;
> + skb->data_len -= chunk;
> + __skb_pull(frag, chunk);
> + break;
If we break here what will happen whit the rest of the data to be
pulled.
> + } else if (frag->len) {
> + /* Pulling all frag data */
> + chunk -= frag->len;
> + skb->len -= frag->len;
> + skb->data_len -= frag->len;
> + __skb_pull(frag, frag->len);
> + }
> + }
> + }
> +
> if (skb->len) {
> skb_queue_head(&sk->sk_receive_queue, skb);
> break;
> --
> 1.7.1
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [RFC 4/7] Bluetooth: Linearize received L2CAP skbuffs.
From: Gustavo F. Padovan @ 2010-08-11 3:58 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Mat Martineau, linux-bluetooth, rshaffer, linux-arm-msm
In-Reply-To: <1281476313.12579.229.camel@localhost.localdomain>
Hi Mat,
* Marcel Holtmann <marcel@holtmann.org> [2010-08-10 17:38:33 -0400]:
> Hi Mat,
>
> > Fragmented skbs are only encountered by in-kernel protocols and
> > profiles when receiving ERTM or streaming mode L2CAP data. BNEP,
> > CMTP, HIDP, and RFCOMM generally use basic mode, so will not
> > generally need the extra memcpy's necessary to put the L2CAP
> > payload in to a linear buffer. However, it is possible to
> > use enhanced L2CAP in these modes so the possibility needs
> > to be handled.
> >
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> > net/bluetooth/bnep/core.c | 5 ++++-
> > net/bluetooth/cmtp/core.c | 5 ++++-
> > net/bluetooth/hidp/core.c | 10 ++++++++--
> > net/bluetooth/rfcomm/core.c | 5 ++++-
> > 4 files changed, 20 insertions(+), 5 deletions(-)
>
> in general I am fine with this.
Same from me.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Gustavo F. Padovan @ 2010-08-11 3:56 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <20100811033540.GA9151@vigoh>
Hi Mat,
* Gustavo F. Padovan <gustavo@padovan.org> [2010-08-11 00:35:41 -0300]:
> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:00 -0700]:
>
> > In order to not limit ERTM and streaming mode PDUs to the HCI MTU
> > size, L2CAP must be able to split PDUs in to multple HCI fragments.
> > This is done by allocating space for the FCS in the last fragment.
> >
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> > net/bluetooth/l2cap.c | 39 ++++++++++++++++++++++++++++++++++-----
> > 1 files changed, 34 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index aa69c84..b485c4a 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
> > {
> > struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> > struct sk_buff **frag;
> > + struct sk_buff *final;
> > int err, sent = 0;
> >
> > + BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
> > + msg, (int)len, (int)count, skb);
> > +
> > if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
> > return -EFAULT;
> >
> > sent += count;
> > len -= count;
> > + final = skb;
> >
> > /* Continuation fragments (no L2CAP header) */
> > frag = &skb_shinfo(skb)->frag_list;
> > while (len) {
> > + int skblen;
> > count = min_t(unsigned int, conn->mtu, len);
> >
> > - *frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
> > + /* Add room for the FCS if it fits */
> > + if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
> > + len + L2CAP_FCS_SIZE <= conn->mtu)
>
> You don't need to check for (len + L2CAP_FCS_SIZE <= conn=mtu) here.
> Section 5.1 point that:
>
> "Unlike the B-Frame length field, the I-frame length field may be greater
> than the configured MTU because it includes the octet lengths of the
> Control, L2CAP SDU Length (when present), and frame check sequence
> fields as well as the Information octets."
>
> From that I understand "len <=" and not "len + L2CAP_FCS_SIZE <="
So here you might want to the check if we support FCS, and then add 2 to
skblen.
>
>
> > + skblen = count + L2CAP_FCS_SIZE;
> > + else
> > + skblen = count;
> > +
> > + *frag = bt_skb_send_alloc(sk, skblen,
> > + msg->msg_flags & MSG_DONTWAIT, &err);
> > if (!*frag)
> > return -EFAULT;
> > - if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> > +
> > + if (memcpy_fromiovec(skb_put(*frag, count),
> > + msg->msg_iov, count))
> > return -EFAULT;
> >
> > sent += count;
> > len -= count;
> >
> > + final = *frag;
> > +
> > frag = &(*frag)->next;
> > }
> >
> > + if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16) {
> > + if (skb_tailroom(final) < L2CAP_FCS_SIZE) {
> > + *frag = bt_skb_send_alloc(sk, L2CAP_FCS_SIZE,
> > + msg->msg_flags & MSG_DONTWAIT,
> > + &err);
Why do we need to check for FCS again? We already added it required
space to the last fragment.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Gustavo F. Padovan @ 2010-08-11 3:35 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-4-git-send-email-mathewm@codeaurora.org>
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:00 -0700]:
> In order to not limit ERTM and streaming mode PDUs to the HCI MTU
> size, L2CAP must be able to split PDUs in to multple HCI fragments.
> This is done by allocating space for the FCS in the last fragment.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 39 ++++++++++++++++++++++++++++++++++-----
> 1 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index aa69c84..b485c4a 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
> {
> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> struct sk_buff **frag;
> + struct sk_buff *final;
> int err, sent = 0;
>
> + BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
> + msg, (int)len, (int)count, skb);
> +
> if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
> return -EFAULT;
>
> sent += count;
> len -= count;
> + final = skb;
>
> /* Continuation fragments (no L2CAP header) */
> frag = &skb_shinfo(skb)->frag_list;
> while (len) {
> + int skblen;
> count = min_t(unsigned int, conn->mtu, len);
>
> - *frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
> + /* Add room for the FCS if it fits */
> + if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
> + len + L2CAP_FCS_SIZE <= conn->mtu)
You don't need to check for (len + L2CAP_FCS_SIZE <= conn=mtu) here.
Section 5.1 point that:
"Unlike the B-Frame length field, the I-frame length field may be greater
than the configured MTU because it includes the octet lengths of the
Control, L2CAP SDU Length (when present), and frame check sequence
fields as well as the Information octets."
>From that I understand "len <=" and not "len + L2CAP_FCS_SIZE <="
> + skblen = count + L2CAP_FCS_SIZE;
> + else
> + skblen = count;
> +
> + *frag = bt_skb_send_alloc(sk, skblen,
> + msg->msg_flags & MSG_DONTWAIT, &err);
> if (!*frag)
> return -EFAULT;
> - if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> +
> + if (memcpy_fromiovec(skb_put(*frag, count),
> + msg->msg_iov, count))
> return -EFAULT;
>
> sent += count;
> len -= count;
>
> + final = *frag;
> +
> frag = &(*frag)->next;
> }
>
> + if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16) {
> + if (skb_tailroom(final) < L2CAP_FCS_SIZE) {
> + *frag = bt_skb_send_alloc(sk, L2CAP_FCS_SIZE,
> + msg->msg_flags & MSG_DONTWAIT,
> + &err);
> + if (!*frag)
> + return -EFAULT;
> +
> + final = *frag;
> + }
> +
> + skb_put(final, L2CAP_FCS_SIZE);
> + }
> +
> return sent;
> }
>
> @@ -1790,9 +1822,6 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
> return ERR_PTR(err);
> }
>
> - if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
> - put_unaligned_le16(0, skb_put(skb, 2));
> -
> bt_cb(skb)->retries = 0;
> return skb;
> }
> --
> 1.7.1
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [RFC 2/7] Bluetooth: Use enhanced L2CAP header structure and symbolic values.
From: Gustavo F. Padovan @ 2010-08-11 3:23 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <alpine.DEB.2.00.1008101916090.6276@linux-sea-02>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 19:24:20 -0700]:
>
>
> Gustavo -
>
> On Tue, 10 Aug 2010, Gustavo F. Padovan wrote:
>
> >Hi Mat,
> >
> >* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:14:59 -0700]:
> >
> >>
> >>Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>---
> >> net/bluetooth/l2cap.c | 23 +++++++++++++----------
> >> 1 files changed, 13 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>index 5e78c18..aa69c84 100644
> >>--- a/net/bluetooth/l2cap.c
> >>+++ b/net/bluetooth/l2cap.c
> >>@@ -1753,33 +1753,36 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
> >> {
> >> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> >> struct sk_buff *skb;
> >>- int err, count, hlen = L2CAP_HDR_SIZE + 2;
> >>- struct l2cap_hdr *lh;
> >>+ int err, count, hlen = L2CAP_ENHANCED_HDR_SIZE;
> >>+ struct l2cap_enhanced_hdr *lh;
> >
> >This patch shouldn't even compile. Since there no definition for
> >struct l2cap_enhanced_hdr. Anyway we should not have that struct, keep
> >the code to handle this as it is.
> >We'll have to change this code again to add the extended control bit
> >field.
>
> Sorry about that - I did my git format-patch starting one commit too
> late.
>
> I also have an l2cap_extended_hdr struct to add later, but it sounds
> like you don't want that one either.
Get rid of that one too. It's better do not have that extra
complication.
By the way, I think we can hide the handle of extended and enhanced
control fields inside the macros to get control bit fields. So we won't
have to care about that in the code. We still have to talk about that.
>
> >
> >>
> >>- BT_DBG("sk %p len %d", sk, (int)len);
> >>+ BT_DBG("sk %p, msg %p, len %d, control %x, sdulen %d",
> >>+ sk, msg, (int)len, control, (int)sdulen);
> >>
> >> if (!conn)
> >> return ERR_PTR(-ENOTCONN);
> >>
> >> if (sdulen)
> >>- hlen += 2;
> >>+ hlen += L2CAP_SDULEN_SIZE;
> >
> >Separated patch to add L2CAP_SDULEN_SIZE
>
> Ok.
>
> >>
> >> if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
> >>- hlen += 2;
> >>+ hlen += L2CAP_FCS_SIZE;
> >
> >Separated patch to ad L2CAP_FCS_SIZE
>
> Ok. Does this need to be separate from the L2CAP_SDULEN_SIZE patch?
It depends. If we have few occurrence of that macros, they could be in
same patch.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: QuiC AMP development
From: Gustavo F. Padovan @ 2010-08-11 3:15 UTC (permalink / raw)
To: Mat Martineau; +Cc: Ron Shaffer, linux-bluetooth
In-Reply-To: <alpine.DEB.2.00.1008101925000.6276@linux-sea-02>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 19:55:34 -0700]:
>
> Gustavo -
>
> On Tue, 10 Aug 2010, Gustavo F. Padovan wrote:
>
> >Hi all,
> >
> >* Ron Shaffer <rshaffer@codeaurora.org> [2010-08-08 21:43:36 -0500]:
> >
> >>As requested in today's summit discussions, here's a link that can be
> >>used to inspect the code we've done to add AMP support on the latest
> >>next tree.
> >>
> >>https://www.codeaurora.org/gitweb/quic/bluetooth/?p=bluetooth-next-2.6.git;a=summary
> >>branch pk-upstream
> >
> >
> >I'll put here some comments I have about some L2CAP patches:
>
> Thanks for taking the time to look over all these changes!
>
> >
> >
> >+static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16 size)
> >+{
> >+ u16 allocSize;
> >
> >No capital letter here. Do alloc_size. Check the rest of your code for similar
> >stuff.
>
> Yes, someone caught that in our internal code review too. I think
> this was the only one.
>
> >
> >
> > u8 event;
> > struct sk_buff *skb;
> >};
> >+
> >#endif /* __AMP_H */
> >diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> >index f43d7d8..16e74f6 100644
> >--- a/net/bluetooth/amp.c
> >+++ b/net/bluetooth/amp.c
> >@@ -40,6 +40,7 @@ static struct workqueue_struct *amp_workqueue;
> >LIST_HEAD(amp_mgr_list);
> >DEFINE_RWLOCK(amp_mgr_list_lock);
> >
> >+
> >static void remove_amp_mgr(struct amp_mgr *rem)
> >{
> > struct amp_mgr *mgr = NULL;
> >
> >Do not add new lines random places into your patch. Check you code for others
> >things like that. Also check for lines overstepping the column 80.
>
> We definitely know we have work to do to clean up these patches and
> add proper commit messages. Please be assured that we'll have these
> more tidied up before posting patches to linux-bluetooth.
;)
>
> >
> >
> >+struct l2cap_enhanced_hdr {
> >+ __le16 len;
> >+ __le16 cid;
> >+ __le16 control;
> >+} __attribute__ ((packed));
> >+#define L2CAP_ENHANCED_HDR_SIZE 6
> >
> >Get ride off this struct, that actually doesn't help. We tried that before and
> >Marcel and I agreed that it does not help.
>
> If you guys don't like it, I'll take it out.
>
> >
> >- if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> >+
> >+ if (memcpy_fromiovec(skb_put(*frag, count),
> >+ msg->msg_iov, count))
> > return -EFAULT;
> >
> >Avoid changes like that in your patches, you are just breaking a line here. If
> >you want to do that do in a separeted patch.
> >
> >
> >
> >+/* L2CAP ERTM / Streaming extra field lengths */
> >+#define L2CAP_FCS_SIZE 2
> >
> >Separated patch for that, so then you replace 2 for L2CAP_FCS_SIZE in one
> >patch, instead of doing that in many patches like you are doing now. The same
> >goes for L2CAP_SDULEN_SIZE.
>
> Yes, I already split that commit up to send the "RFC" patch set
> earlier today. Unfortunately I missed that commit when generating
> my patches :(
>
>
> >-/* L2CAP Supervisory Function */
> >+/* L2CAP Supervisory Frame Types */
> >+#define L2CAP_SFRAME_RR 0x00
> >+#define L2CAP_SFRAME_REJ 0x01
> >+#define L2CAP_SFRAME_RNR 0x02
> >+#define L2CAP_SFRAME_SREJ 0x03
> >#define L2CAP_SUPER_RCV_READY 0x0000
> >#define L2CAP_SUPER_REJECT 0x0004
> >#define L2CAP_SUPER_RCV_NOT_READY 0x0008
> >#define L2CAP_SUPER_SELECT_REJECT 0x000C
> >
> >/* L2CAP Segmentation and Reassembly */
> >+#define L2CAP_SAR_UNSEGMENTED 0x00
> >+#define L2CAP_SAR_START 0x01
> >+#define L2CAP_SAR_END 0x02
> >+#define L2CAP_SAR_CONTINUE 0x03
> >#define L2CAP_SDU_UNSEGMENTED 0x0000
> >#define L2CAP_SDU_START 0x4000
> >#define L2CAP_SDU_END 0x8000
> >
> >What is that? we already have such defines.
>
> I was trying to not break existing code, while also creating
> constants that were useful for modified code that will work with
> either extended or enhanced headers. The new values are important,
> but I agree that this is not the way to introduce the changes. See
> below for more detail.
>
> >commit 162e6de6d5c11b8ffea91a9fa2d597340afdeb6e
> >Author: Mat Martineau <mathewm@codeaurora.org>
> >Date: Thu Aug 5 16:59:46 2010 -0700
> >
> > Bluetooth: Remove unused L2CAP code.
> >
> >net/bluetooth/l2cap.c | 1104 +------------------------------------------------
> >1 files changed, 15 insertions(+), 1089 deletions(-)
> >
> >That's not acceptable, we have to modify the existent base code and make it
> >work with the new features, instead writing a new one and then switch to it.
>
> When setting up the commits for this git branch, I had to pick
> between two approaches:
>
> * Build up a modified state machine in parallel, so the switchover
> could happen in one patch. The code would compile and run after
> every patch.
>
> * Or, start modifying the state machine piece by piece. Until all
> of the modifications were done, ERTM would not work.
>
> I don't think my approach worked out well (mostly because it doesn't
> preserve history adequately, and doesn't make it clear where code
> has been modified instead of newly written). However, it's what we
> had to share coming in to the Bluetooth summit, and we felt it was
> very important to share it. I want to refine the approach to these
> patches to put them in some acceptable form, so lets talk about the
> best way to do that.
I see. But now that they are public we can talk about and put them in
shape for the merge into the mainline.
I think it's possible to add AMP stuff without break ERTM (and we have
to take care about that, because now ERTM pass all PTS tests). We can
arrange your commits to not break ERTM any time.
>
>
> >commit 09850f68219572288fe62a59235fda3d2629c7b2
> >Author: Peter Krystad <pkrystad@codeaurora.org>
> >Date: Wed Aug 4 16:55:11 2010 -0700
> >
> > Rename l2cap.c to el2cap.c.
> >
> > Necessary before additional files can be added to l2cap module.
> > A module cannot contain a file with the same name as the module.
> >
> >We don't neeed that. We might split l2cap.c into smaller chunks before merge
> >all the AMP stuff into it, so we won't have the module name problem anymore.
>
> There is a technical reason for this. We changed the Makefile to
> create l2cap.ko from two source files - one for L2CAP and one for
> AMP. However, the build system fails if you try to link l2cap.c and
> amp.c in to l2cap.ko. We had to come up with some other name for
> l2cap.c in order to keep the module name the same. There's probably
> a better choice than el2cap.c - any suggestions are welcome.
Yeah, I know, that's why the l2cap.c split will help this. Marcel
told that on the meeting Sunday.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: QuiC AMP development
From: Mat Martineau @ 2010-08-11 2:55 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Ron Shaffer, linux-bluetooth
In-Reply-To: <20100810220238.GB3203@vigoh>
Gustavo -
On Tue, 10 Aug 2010, Gustavo F. Padovan wrote:
> Hi all,
>
> * Ron Shaffer <rshaffer@codeaurora.org> [2010-08-08 21:43:36 -0500]:
>
>> As requested in today's summit discussions, here's a link that can be
>> used to inspect the code we've done to add AMP support on the latest
>> next tree.
>>
>> https://www.codeaurora.org/gitweb/quic/bluetooth/?p=bluetooth-next-2.6.git;a=summary
>> branch pk-upstream
>
>
> I'll put here some comments I have about some L2CAP patches:
Thanks for taking the time to look over all these changes!
>
>
> +static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16 size)
> +{
> + u16 allocSize;
>
> No capital letter here. Do alloc_size. Check the rest of your code for similar
> stuff.
Yes, someone caught that in our internal code review too. I think
this was the only one.
>
>
> u8 event;
> struct sk_buff *skb;
> };
> +
> #endif /* __AMP_H */
> diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> index f43d7d8..16e74f6 100644
> --- a/net/bluetooth/amp.c
> +++ b/net/bluetooth/amp.c
> @@ -40,6 +40,7 @@ static struct workqueue_struct *amp_workqueue;
> LIST_HEAD(amp_mgr_list);
> DEFINE_RWLOCK(amp_mgr_list_lock);
>
> +
> static void remove_amp_mgr(struct amp_mgr *rem)
> {
> struct amp_mgr *mgr = NULL;
>
> Do not add new lines random places into your patch. Check you code for others
> things like that. Also check for lines overstepping the column 80.
We definitely know we have work to do to clean up these patches and
add proper commit messages. Please be assured that we'll have these
more tidied up before posting patches to linux-bluetooth.
>
>
> +struct l2cap_enhanced_hdr {
> + __le16 len;
> + __le16 cid;
> + __le16 control;
> +} __attribute__ ((packed));
> +#define L2CAP_ENHANCED_HDR_SIZE 6
>
> Get ride off this struct, that actually doesn't help. We tried that before and
> Marcel and I agreed that it does not help.
If you guys don't like it, I'll take it out.
>
> - if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> +
> + if (memcpy_fromiovec(skb_put(*frag, count),
> + msg->msg_iov, count))
> return -EFAULT;
>
> Avoid changes like that in your patches, you are just breaking a line here. If
> you want to do that do in a separeted patch.
>
>
>
> +/* L2CAP ERTM / Streaming extra field lengths */
> +#define L2CAP_FCS_SIZE 2
>
> Separated patch for that, so then you replace 2 for L2CAP_FCS_SIZE in one
> patch, instead of doing that in many patches like you are doing now. The same
> goes for L2CAP_SDULEN_SIZE.
Yes, I already split that commit up to send the "RFC" patch set
earlier today. Unfortunately I missed that commit when generating my
patches :(
> -/* L2CAP Supervisory Function */
> +/* L2CAP Supervisory Frame Types */
> +#define L2CAP_SFRAME_RR 0x00
> +#define L2CAP_SFRAME_REJ 0x01
> +#define L2CAP_SFRAME_RNR 0x02
> +#define L2CAP_SFRAME_SREJ 0x03
> #define L2CAP_SUPER_RCV_READY 0x0000
> #define L2CAP_SUPER_REJECT 0x0004
> #define L2CAP_SUPER_RCV_NOT_READY 0x0008
> #define L2CAP_SUPER_SELECT_REJECT 0x000C
>
> /* L2CAP Segmentation and Reassembly */
> +#define L2CAP_SAR_UNSEGMENTED 0x00
> +#define L2CAP_SAR_START 0x01
> +#define L2CAP_SAR_END 0x02
> +#define L2CAP_SAR_CONTINUE 0x03
> #define L2CAP_SDU_UNSEGMENTED 0x0000
> #define L2CAP_SDU_START 0x4000
> #define L2CAP_SDU_END 0x8000
>
> What is that? we already have such defines.
I was trying to not break existing code, while also creating constants
that were useful for modified code that will work with either extended
or enhanced headers. The new values are important, but I agree that
this is not the way to introduce the changes. See below for more
detail.
> commit 162e6de6d5c11b8ffea91a9fa2d597340afdeb6e
> Author: Mat Martineau <mathewm@codeaurora.org>
> Date: Thu Aug 5 16:59:46 2010 -0700
>
> Bluetooth: Remove unused L2CAP code.
>
> net/bluetooth/l2cap.c | 1104 +------------------------------------------------
> 1 files changed, 15 insertions(+), 1089 deletions(-)
>
> That's not acceptable, we have to modify the existent base code and make it
> work with the new features, instead writing a new one and then switch to it.
When setting up the commits for this git branch, I had to pick between
two approaches:
* Build up a modified state machine in parallel, so the switchover
could happen in one patch. The code would compile and run after every
patch.
* Or, start modifying the state machine piece by piece. Until all of
the modifications were done, ERTM would not work.
I don't think my approach worked out well (mostly because it doesn't
preserve history adequately, and doesn't make it clear where code has
been modified instead of newly written). However, it's what we had to
share coming in to the Bluetooth summit, and we felt it was very
important to share it. I want to refine the approach to these patches
to put them in some acceptable form, so lets talk about the best way
to do that.
> commit 09850f68219572288fe62a59235fda3d2629c7b2
> Author: Peter Krystad <pkrystad@codeaurora.org>
> Date: Wed Aug 4 16:55:11 2010 -0700
>
> Rename l2cap.c to el2cap.c.
>
> Necessary before additional files can be added to l2cap module.
> A module cannot contain a file with the same name as the module.
>
> We don't neeed that. We might split l2cap.c into smaller chunks before merge
> all the AMP stuff into it, so we won't have the module name problem anymore.
There is a technical reason for this. We changed the Makefile to
create l2cap.ko from two source files - one for L2CAP and one for AMP.
However, the build system fails if you try to link l2cap.c and amp.c
in to l2cap.ko. We had to come up with some other name for l2cap.c in
order to keep the module name the same. There's probably a better
choice than el2cap.c - any suggestions are welcome.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: patch for firmware download to Qualcomm Bluetooth chip
From: Ron Shaffer @ 2010-08-11 2:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, johan.hedberg
In-Reply-To: <1281474848.12579.218.camel@localhost.localdomain>
On 8/10/2010 5:14 PM, Marcel Holtmann wrote:
> Hi Ron,
>
>> Here's the a ping regarding the patch submitted by Matt back in March? I
>> believe. For you convenience I've included the patch again. Please take
>> a look, and provide your general comments. I'm sure there are changes
>> that need to be made.
>>
>> Fyi. sorry about the column wrap in the patch. Don't feel like fixing it
>> for just a quick review.
>>
>> From 76d0bdd82a0a4e5b3b9544bb864c31888f20cea1 Mon Sep 17 00:00:00 2001
>> From: Wilson, Matt <mtwilson@codeaurora.org>
>> Date: Thu, 11 Feb 2010 11:53:29 -0600
>> Subject: [PATCH] Firmware download for Qualcomm Bluetooth devices
>>
>> ---
>> Makefile.tools | 3 +-
>> tools/hciattach.c | 9 ++
>> tools/hciattach.h | 4 +
>> tools/hciattach_qualcomm.c | 279
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 294 insertions(+), 1 deletions(-)
>> create mode 100644 tools/hciattach_qualcomm.c
>>
>> diff --git a/Makefile.tools b/Makefile.tools
>> index 2735d68..7b92c8f 100644
>> --- a/Makefile.tools
>> +++ b/Makefile.tools
>> @@ -23,7 +23,8 @@ tools_l2ping_LDADD = lib/libbluetooth.la
>> tools_hciattach_SOURCES = tools/hciattach.c tools/hciattach.h \
>> tools/hciattach_st.c \
>> tools/hciattach_ti.c \
>> - tools/hciattach_tialt.c
>> + tools/hciattach_tialt.c \
>> + tools/hciattach_qualcomm.c
>> tools_hciattach_LDADD = lib/libbluetooth.la
>>
>> tools_hciconfig_SOURCES = tools/hciconfig.c tools/csr.h tools/csr.c \
>> diff --git a/tools/hciattach.c b/tools/hciattach.c
>> index 364c5ff..d6aafbe 100644
>> --- a/tools/hciattach.c
>> +++ b/tools/hciattach.c
>> @@ -5,6 +5,7 @@
>> * Copyright (C) 2000-2001 Qualcomm Incorporated
>> * Copyright (C) 2002-2003 Maxim Krasnyansky <maxk@qualcomm.com>
>> * Copyright (C) 2002-2010 Marcel Holtmann <marcel@holtmann.org>
>> + * Copyright (C) 2010, Code Aurora Forum. All rights reserved.
>> *
>> *
>> * This program is free software; you can redistribute it and/or modify
>> @@ -299,6 +300,11 @@ static int texasalt(int fd, struct uart_t *u, struct
>> termios *ti)
>> return texasalt_init(fd, u->speed, ti);
>> }
>>
>> +static int qualcomm(int fd, struct uart_t *u, struct termios *ti)
>> +{
>> + return qualcomm_init(fd, u->speed, ti, u->bdaddr);
>> +}
>> +
>> static int read_check(int fd, void *buf, int count)
>> {
>> int res;
>> @@ -1071,6 +1077,9 @@ struct uart_t uart[] = {
>> /* Broadcom BCM2035 */
>> { "bcm2035", 0x0A5C, 0x2035, HCI_UART_H4, 115200, 460800, FLOW_CTL,
>> NULL, bcm2035 },
>>
>> + /* QUALCOMM BTS */
>> + { "qualcomm", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL,
>> NULL, qualcomm },
>> +
>> { NULL, 0 }
>> };
>>
>> diff --git a/tools/hciattach.h b/tools/hciattach.h
>> index 867563b..5c89013 100644
>> --- a/tools/hciattach.h
>> +++ b/tools/hciattach.h
>> @@ -3,6 +3,7 @@
>> * BlueZ - Bluetooth protocol stack for Linux
>> *
>> * Copyright (C) 2003-2010 Marcel Holtmann <marcel@holtmann.org>
>> + * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
>> *
>> *
>> * This program is free software; you can redistribute it and/or modify
>> @@ -45,3 +46,6 @@ int texas_post(int fd, struct termios *ti);
>> int texasalt_init(int fd, int speed, struct termios *ti);
>> int stlc2500_init(int fd, bdaddr_t *bdaddr);
>> int bgb2xx_init(int dd, bdaddr_t *bdaddr);
>> +int qualcomm_init(int fd, int speed, struct termios *ti, const char
>> *bdaddr);
>> +
>> +
>
> I am actually fine with this. Except strip it from claiming copyright on
> hciattach.c and hciattach.h since these changes are not really anything
> new. They are just wrapping code.
>
> So fix this and submit a new clean patch.
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Yes, will do.
--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [RFC 2/7] Bluetooth: Use enhanced L2CAP header structure and symbolic values.
From: Mat Martineau @ 2010-08-11 2:24 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <20100810220712.GC3203@vigoh>
Gustavo -
On Tue, 10 Aug 2010, Gustavo F. Padovan wrote:
> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:14:59 -0700]:
>
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>> net/bluetooth/l2cap.c | 23 +++++++++++++----------
>> 1 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 5e78c18..aa69c84 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -1753,33 +1753,36 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
>> {
>> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
>> struct sk_buff *skb;
>> - int err, count, hlen = L2CAP_HDR_SIZE + 2;
>> - struct l2cap_hdr *lh;
>> + int err, count, hlen = L2CAP_ENHANCED_HDR_SIZE;
>> + struct l2cap_enhanced_hdr *lh;
>
> This patch shouldn't even compile. Since there no definition for
> struct l2cap_enhanced_hdr. Anyway we should not have that struct, keep
> the code to handle this as it is.
> We'll have to change this code again to add the extended control bit
> field.
Sorry about that - I did my git format-patch starting one commit too
late.
I also have an l2cap_extended_hdr struct to add later, but it sounds
like you don't want that one either.
>
>>
>> - BT_DBG("sk %p len %d", sk, (int)len);
>> + BT_DBG("sk %p, msg %p, len %d, control %x, sdulen %d",
>> + sk, msg, (int)len, control, (int)sdulen);
>>
>> if (!conn)
>> return ERR_PTR(-ENOTCONN);
>>
>> if (sdulen)
>> - hlen += 2;
>> + hlen += L2CAP_SDULEN_SIZE;
>
> Separated patch to add L2CAP_SDULEN_SIZE
Ok.
>>
>> if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
>> - hlen += 2;
>> + hlen += L2CAP_FCS_SIZE;
>
> Separated patch to ad L2CAP_FCS_SIZE
Ok. Does this need to be separate from the L2CAP_SDULEN_SIZE patch?
>
>>
>> count = min_t(unsigned int, (conn->mtu - hlen), len);
>> +
>> skb = bt_skb_send_alloc(sk, count + hlen,
>> msg->msg_flags & MSG_DONTWAIT, &err);
>> if (!skb)
>> - return ERR_PTR(-ENOMEM);
>> + return ERR_PTR(err);
>
> This also should not be in this patch.
No problem.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: why is hciops a plugin ?
From: Johan Hedberg @ 2010-08-11 2:16 UTC (permalink / raw)
To: Pavan Savoy; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTino39jMNvdcV=EiekN33vrfNXLyJVNsHcHtq5wz@mail.gmail.com>
Hi,
On Tue, Aug 10, 2010, Pavan Savoy wrote:
> Any reason behind making hciops a plugin and bluetoothd making use of
> that plugin, wasn't everything straight up in hcid before ?
We're planning on redoing the kernel-userspace interface and in the long
run the traditional raw HCI access wont exist anymore in userspace. In
order to accommodate both old and new kernels in a clean way the
interface towards the kernel needs to be easily interchangable, which is
what hciops is trying to do (it's just a first step in that direction
though). One of the drivers for this change is the need to have the
security logic in one place instead of it being split between kernel and
userspace (this has caused all sorts of trouble with SSP for us).
Originally the plan for the new kernel interface was netlink but now the
idea is to simply extend the stack internal messages of raw HCI sockets
with a more complete two-way protocol between the kernel and userspace.
You should (hopefully) be seeing more patches for this still during this
year.
Johan
^ permalink raw reply
* [PATCH 2/2] HCI events related to AMP functionality
From: Inga Stotland @ 2010-08-11 0:12 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1281485576-32715-1-git-send-email-ingas@codeaurora.org>
Added constructs for the following HCI events:
PHYSICAL_LINK_COMPLETE, EVT_CHANNEL_SELECTED,
EVT_DISCONNECT_PHYSICAL_LINK_COMPLETE,
EVT_PHYSICAL_LINK_LOSS_EARLY_WARNING, EVT_PHYSICAL_LINK_RECOVERY,
EVT_LOGICAL_LINK_COMPLETE, EVT_DISCONNECT_LOGICAL_LINK_COMPLETE,
EVT_FLOW_SPEC_MODIFY_COMPLETE, EVT_NUMBER_COMPLETED_BLOCKS,
EVT_AMP_STATUS_CHANGE
---
lib/hci.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/lib/hci.h b/lib/hci.h
index ae7b6da..8f35422 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -2108,6 +2108,63 @@ typedef struct {
} __attribute__ ((packed)) evt_le_long_term_key_request;
#define EVT_LE_LTK_REQUEST_SIZE 12
+#define EVT_PHYSICAL_LINK_COMPLETE 0x40
+typedef struct {
+ uint8_t status;
+ uint8_t handle;
+} __attribute__ ((packed)) evt_physical_link_complete;
+#define EVT_PHYSICAL_LINK_COMPLETE_SIZE 2
+
+#define EVT_CHANNEL_SELECTED 0x41
+
+#define EVT_DISCONNECT_PHYSICAL_LINK_COMPLETE 0x42
+typedef struct {
+ uint8_t status;
+ uint8_t handle;
+ uint8_t reason;
+} __attribute__ ((packed)) evt_disconn_physical_link_complete;
+#define EVT_DISCONNECT_PHYSICAL_LINK_COMPLETE_SIZE 3
+
+#define EVT_PHYSICAL_LINK_LOSS_EARLY_WARNING 0x43
+typedef struct {
+ uint8_t handle;
+ uint8_t reason;
+} __attribute__ ((packed)) evt_physical_link_loss_warning;
+#define EVT_PHYSICAL_LINK_LOSS_WARNING_SIZE 2
+
+#define EVT_PHYSICAL_LINK_RECOVERY 0x44
+typedef struct {
+ uint8_t handle;
+} __attribute__ ((packed)) evt_physical_link_recovery;
+#define EVT_PHYSICAL_LINK_RECOVERY_SIZE 1
+
+#define EVT_LOGICAL_LINK_COMPLETE 0x45
+typedef struct {
+ uint8_t status;
+ uint16_t log_handle;
+ uint8_t handle;
+ uint8_t tx_flow_id;
+} __attribute__ ((packed)) evt_logical_link_complete;
+#define EVT_LOGICAL_LINK_COMPLETE_SIZE 5
+
+#define EVT_DISCONNECT_LOGICAL_LINK_COMPLETE 0x46
+
+#define EVT_FLOW_SPEC_MODIFY_COMPLETE 0x47
+typedef struct {
+ uint8_t status;
+ uint16_t handle;
+} __attribute__ ((packed)) evt_flow_spec_modify_complete;
+#define EVT_FLOW_SPEC_MODIFY_COMPLETE_SIZE 3
+
+#define EVT_NUMBER_COMPLETED_BLOCKS 0x48
+
+#define EVT_AMP_STATUS_CHANGE 0x4D
+typedef struct {
+ uint8_t status;
+ uint8_t amp_status;
+} __attribute__ ((packed)) evt_amp_status_change;
+#define EVT_AMP_STATUS_CHANGE_SIZE 2
+
#define EVT_TESTING 0xFE
#define EVT_VENDOR 0xFF
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 1/2] HCI commands related to AMP functionality
From: Inga Stotland @ 2010-08-11 0:12 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1281485576-32715-1-git-send-email-ingas@codeaurora.org>
Added constructs for the following HCI commands:
CREATE_PHYSICAL_LINK, ACCEPT_PHYSICAL_LINK, DISCONNECT_PHYSICAL_LINK,
CREATE_LOGICAL_LINK, ACCEPT_LOGICAL_LINK, DISCONNECT_LOGICAL_LINK,
LOGICAL_LINK_CANCEL, FLOW_SPEC_MODIFY,
READ_LOGICAL_LINK_ACCEPT_TIMEOUT, WRITE_LOGICAL_LINK_ACCEPT_TIMEOUT,
SET_EVENT_MASK_PAGE_2, READ_LOCATION_DATA, WRITE_LOCATION_DATA,
READ_FLOW_CONTROL_MODE, WRITE_FLOW_CONTROL_MODE,
READ_ENHANCED_TRANSMIT_POWER_LEVEL, READ_BEST_EFFORT_FLUSH_TIMEOUT,
WRITE_BEST_EFFORT_FLUSH_TIMEOUT, READ_LOCAL_AMP_INFO,
READ_LOCAL_AMP_ASSOC, WRITE_REMOTE_AMP_ASSOC
---
lib/hci.h | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 145 insertions(+), 0 deletions(-)
diff --git a/lib/hci.h b/lib/hci.h
index f31918d..ae7b6da 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -528,6 +528,55 @@ typedef struct {
} __attribute__ ((packed)) io_capability_neg_reply_cp;
#define IO_CAPABILITY_NEG_REPLY_CP_SIZE 7
+#define OCF_CREATE_PHYSICAL_LINK 0x0035
+typedef struct {
+ uint8_t handle;
+ uint8_t key_length;
+ uint8_t key_type;
+ uint8_t key[32];
+} __attribute__ ((packed)) create_physical_link_cp;
+#define CREATE_PHYSICAL_LINK_CP_SIZE 35
+
+#define OCF_ACCEPT_PHYSICAL_LINK 0x0036
+
+#define OCF_DISCONNECT_PHYSICAL_LINK 0x0037
+typedef struct {
+ uint8_t handle;
+ uint8_t reason;
+} __attribute__ ((packed)) disconnect_physical_link_cp;
+#define DISCONNECT_PHYSICAL_LINK_CP_SIZE 2
+
+#define OCF_CREATE_LOGICAL_LINK 0x0038
+typedef struct {
+ uint8_t handle;
+ uint8_t tx_flow[16];
+ uint8_t rx_flow[16];
+} __attribute__ ((packed)) create_logical_link_cp;
+#define CREATE_LOGICAL_LINK_CP_SIZE 33
+
+#define OCF_ACCEPT_LOGICAL_LINK 0x0039
+
+#define OCF_DISCONNECT_LOGICAL_LINK 0x003A
+typedef struct {
+ uint16_t handle;
+} __attribute__ ((packed)) disconnect_logical_link_cp;
+#define DISCONNECT_LOGICAL_LINK_CP_SIZE 2
+
+#define OCF_LOGICAL_LINK_CANCEL 0x003B
+typedef struct {
+ uint8_t handle;
+ uint8_t tx_flow_id;
+} __attribute__ ((packed)) cancel_logical_link_cp;
+#define LOGICAL_LINK_CANCEL_CP_SIZE 2
+typedef struct {
+ uint8_t status;
+ uint8_t handle;
+ uint8_t tx_flow_id;
+} __attribute__ ((packed)) cancel_logical_link_rp;
+#define LOGICAL_LINK_CANCEL_RP_SIZE 3
+
+#define OCF_FLOW_SPEC_MODIFY 0x003C
+
/* Link Policy */
#define OGF_LINK_POLICY 0x02
@@ -1123,6 +1172,57 @@ typedef struct {
} __attribute__ ((packed)) send_keypress_notify_rp;
#define SEND_KEYPRESS_NOTIFY_RP_SIZE 1
+#define OCF_READ_LOGICAL_LINK_ACCEPT_TIMEOUT 0x0061
+typedef struct {
+ uint8_t status;
+ uint16_t timeout;
+} __attribute__ ((packed)) read_log_link_accept_timeout_rp;
+#define READ_LOGICAL_LINK_ACCEPT_TIMEOUT_RP_SIZE 3
+
+#define OCF_WRITE_LOGICAL_LINK_ACCEPT_TIMEOUT 0x0062
+typedef struct {
+ uint16_t timeout;
+} __attribute__ ((packed)) write_log_link_accept_timeout_cp;
+#define WRITE_LOGICAL_LINK_ACCEPT_TIMEOUT_CP_SIZE 2
+
+#define OCF_SET_EVENT_MASK_PAGE_2 0x0063
+
+#define OCF_READ_LOCATION_DATA 0x0064
+
+#define OCF_WRITE_LOCATION_DATA 0x0065
+
+#define OCF_READ_FLOW_CONTROL_MODE 0x0066
+
+#define OCF_WRITE_FLOW_CONTROL_MODE 0x0067
+
+#define OCF_READ_ENHANCED_TRANSMIT_POWER_LEVEL 0x0068
+typedef struct {
+ uint8_t status;
+ uint16_t handle;
+ int8_t level_gfsk;
+ int8_t level_dqpsk;
+ int8_t level_8dpsk;
+} __attribute__ ((packed)) read_enhanced_transmit_power_level_rp;
+#define READ_ENHANCED_TRANSMIT_POWER_LEVEL_RP_SIZE 6
+
+#define OCF_READ_BEST_EFFORT_FLUSH_TIMEOUT 0x0069
+typedef struct {
+ uint8_t status;
+ uint32_t timeout;
+} __attribute__ ((packed)) read_best_effort_flush_timeout_rp;
+#define READ_BEST_EFFORT_FLUSH_TIMEOUT_RP_SIZE 5
+
+#define OCF_WRITE_BEST_EFFORT_FLUSH_TIMEOUT 0x006A
+typedef struct {
+ uint16_t handle;
+ uint32_t timeout;
+} __attribute__ ((packed)) write_best_effort_flush_timeout_cp;
+#define WRITE_BEST_EFFORT_FLUSH_TIMEOUT_CP_SIZE 6
+typedef struct {
+ uint8_t status;
+} __attribute__ ((packed)) write_best_effort_flush_timeout_rp;
+#define WRITE_BEST_EFFORT_FLUSH_TIMEOUT_RP_SIZE 1
+
/* Informational Parameters */
#define OGF_INFO_PARAM 0x04
@@ -1240,6 +1340,51 @@ typedef struct {
} __attribute__ ((packed)) read_clock_rp;
#define READ_CLOCK_RP_SIZE 9
+#define OCF_READ_LOCAL_AMP_INFO 0x0009
+typedef struct {
+ uint8_t status;
+ uint8_t amp_status;
+ uint32_t total_bandwidth;
+ uint32_t max_guaranteed_bandwidth;
+ uint32_t min_latency;
+ uint32_t max_pdu_size;
+ uint8_t controller_type;
+ uint16_t pal_caps;
+ uint16_t max_amp_assoc_length;
+ uint32_t max_flush_timeout;
+ uint32_t best_effort_flush_timeout;
+} __attribute__ ((packed)) read_local_amp_info_rp;
+#define READ_LOCAL_AMP_INFO_RP_SIZE 31
+
+#define OCF_READ_LOCAL_AMP_ASSOC 0x000A
+typedef struct {
+ uint8_t handle;
+ uint16_t length_so_far;
+ uint16_t assoc_length;
+} __attribute__ ((packed)) read_local_amp_assoc_cp;
+#define READ_LOCAL_AMP_ASSOC_CP_SIZE 5
+typedef struct {
+ uint8_t status;
+ uint8_t handle;
+ uint16_t length;
+ uint8_t fragment[248];
+} __attribute__ ((packed)) read_local_amp_assoc_rp;
+#define READ_LOCAL_AMP_ASSOC_RP_SIZE 252
+
+#define OCF_WRITE_REMOTE_AMP_ASSOC 0x000B
+typedef struct {
+ uint8_t handle;
+ uint16_t length_so_far;
+ uint16_t assoc_length;
+ uint8_t fragment[248];
+} __attribute__ ((packed)) write_remote_amp_assoc_cp;
+#define WRITE_REMOTE_AMP_ASSOC_CP_SIZE 253
+typedef struct {
+ uint8_t status;
+ uint8_t handle;
+} __attribute__ ((packed)) write_remote_amp_assoc_rp;
+#define WRITE_REMOTE_AMP_ASSOC_RP_SIZE 2
+
/* Testing commands */
#define OGF_TESTING_CMD 0x3e
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* Support for HCI commands and events related to AMP functionality
From: Inga Stotland @ 2010-08-11 0:12 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel
Added new HCI commands and events constructs for AMP related development.
Will be used in updated testing tools for AMP.
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* why is hciops a plugin ?
From: Pavan Savoy @ 2010-08-10 22:18 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
Any reason behind making hciops a plugin and bluetoothd making use of
that plugin, wasn't everything straight up in hcid before ?
Thanks in advance.
^ permalink raw reply
* Re: [RFC 2/7] Bluetooth: Use enhanced L2CAP header structure and symbolic values.
From: Gustavo F. Padovan @ 2010-08-10 22:07 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-3-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:14:59 -0700]:
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 23 +++++++++++++----------
> 1 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 5e78c18..aa69c84 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1753,33 +1753,36 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
> {
> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> struct sk_buff *skb;
> - int err, count, hlen = L2CAP_HDR_SIZE + 2;
> - struct l2cap_hdr *lh;
> + int err, count, hlen = L2CAP_ENHANCED_HDR_SIZE;
> + struct l2cap_enhanced_hdr *lh;
This patch shouldn't even compile. Since there no definition for
struct l2cap_enhanced_hdr. Anyway we should not have that struct, keep
the code to handle this as it is.
We'll have to change this code again to add the extended control bit
field.
>
> - BT_DBG("sk %p len %d", sk, (int)len);
> + BT_DBG("sk %p, msg %p, len %d, control %x, sdulen %d",
> + sk, msg, (int)len, control, (int)sdulen);
>
> if (!conn)
> return ERR_PTR(-ENOTCONN);
>
> if (sdulen)
> - hlen += 2;
> + hlen += L2CAP_SDULEN_SIZE;
Separated patch to add L2CAP_SDULEN_SIZE
>
> if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
> - hlen += 2;
> + hlen += L2CAP_FCS_SIZE;
Separated patch to ad L2CAP_FCS_SIZE
>
> count = min_t(unsigned int, (conn->mtu - hlen), len);
> +
> skb = bt_skb_send_alloc(sk, count + hlen,
> msg->msg_flags & MSG_DONTWAIT, &err);
> if (!skb)
> - return ERR_PTR(-ENOMEM);
> + return ERR_PTR(err);
This also should not be in this patch.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: QuiC AMP development
From: Gustavo F. Padovan @ 2010-08-10 22:02 UTC (permalink / raw)
To: Ron Shaffer; +Cc: linux-bluetooth
In-Reply-To: <4C5F6B58.6000702@codeaurora.org>
Hi all,
* Ron Shaffer <rshaffer@codeaurora.org> [2010-08-08 21:43:36 -0500]:
> As requested in today's summit discussions, here's a link that can be
> used to inspect the code we've done to add AMP support on the latest
> next tree.
>
> https://www.codeaurora.org/gitweb/quic/bluetooth/?p=bluetooth-next-2.6.git;a=summary
> branch pk-upstream
I'll put here some comments I have about some L2CAP patches:
+static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16 size)
+{
+ u16 allocSize;
No capital letter here. Do alloc_size. Check the rest of your code for similar
stuff.
u8 event;
struct sk_buff *skb;
};
+
#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index f43d7d8..16e74f6 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -40,6 +40,7 @@ static struct workqueue_struct *amp_workqueue;
LIST_HEAD(amp_mgr_list);
DEFINE_RWLOCK(amp_mgr_list_lock);
+
static void remove_amp_mgr(struct amp_mgr *rem)
{
struct amp_mgr *mgr = NULL;
Do not add new lines random places into your patch. Check you code for others
things like that. Also check for lines overstepping the column 80.
+struct l2cap_enhanced_hdr {
+ __le16 len;
+ __le16 cid;
+ __le16 control;
+} __attribute__ ((packed));
+#define L2CAP_ENHANCED_HDR_SIZE 6
Get ride off this struct, that actually doesn't help. We tried that before and
Marcel and I agreed that it does not help.
- if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
+
+ if (memcpy_fromiovec(skb_put(*frag, count),
+ msg->msg_iov, count))
return -EFAULT;
Avoid changes like that in your patches, you are just breaking a line here. If
you want to do that do in a separeted patch.
+/* L2CAP ERTM / Streaming extra field lengths */
+#define L2CAP_FCS_SIZE 2
Separated patch for that, so then you replace 2 for L2CAP_FCS_SIZE in one
patch, instead of doing that in many patches like you are doing now. The same
goes for L2CAP_SDULEN_SIZE.
-/* L2CAP Supervisory Function */
+/* L2CAP Supervisory Frame Types */
+#define L2CAP_SFRAME_RR 0x00
+#define L2CAP_SFRAME_REJ 0x01
+#define L2CAP_SFRAME_RNR 0x02
+#define L2CAP_SFRAME_SREJ 0x03
#define L2CAP_SUPER_RCV_READY 0x0000
#define L2CAP_SUPER_REJECT 0x0004
#define L2CAP_SUPER_RCV_NOT_READY 0x0008
#define L2CAP_SUPER_SELECT_REJECT 0x000C
/* L2CAP Segmentation and Reassembly */
+#define L2CAP_SAR_UNSEGMENTED 0x00
+#define L2CAP_SAR_START 0x01
+#define L2CAP_SAR_END 0x02
+#define L2CAP_SAR_CONTINUE 0x03
#define L2CAP_SDU_UNSEGMENTED 0x0000
#define L2CAP_SDU_START 0x4000
#define L2CAP_SDU_END 0x8000
What is that? we already have such defines.
commit 162e6de6d5c11b8ffea91a9fa2d597340afdeb6e
Author: Mat Martineau <mathewm@codeaurora.org>
Date: Thu Aug 5 16:59:46 2010 -0700
Bluetooth: Remove unused L2CAP code.
net/bluetooth/l2cap.c | 1104 +------------------------------------------------
1 files changed, 15 insertions(+), 1089 deletions(-)
That's not acceptable, we have to modify the existent base code and make it
work with the new features, instead writing a new one and then switch to it.
commit 09850f68219572288fe62a59235fda3d2629c7b2
Author: Peter Krystad <pkrystad@codeaurora.org>
Date: Wed Aug 4 16:55:11 2010 -0700
Rename l2cap.c to el2cap.c.
Necessary before additional files can be added to l2cap module.
A module cannot contain a file with the same name as the module.
We don't neeed that. We might split l2cap.c into smaller chunks before merge
all the AMP stuff into it, so we won't have the module name problem anymore.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply related
* Re: [RFC 1/7] Bluetooth: Calculate L2CAP FCS on fragmented skbuffs.
From: Gustavo F. Padovan @ 2010-08-10 21:57 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-2-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:14:58 -0700]:
> When L2CAP PDUs are longer than the HCI MTU, they are stored in
> fragmented skbuffs. This adds a function to calculate the FCS
> on any skbuff (fragmented or not), and replaces direct calls
> to crc16 with calls to the new apply_fcs() function.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 8d362d7..5e78c18 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -351,6 +351,33 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
> return id;
> }
>
> +static void apply_fcs(struct sk_buff *skb)
I would call this l2cap_apply_fcs().
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH] Bluetooth: Process HCI events in a workqueue instead of a tasklet
From: Marcel Holtmann @ 2010-08-10 21:41 UTC (permalink / raw)
To: David Vrabel; +Cc: linux-bluetooth
In-Reply-To: <4C6161D7.7070507@csr.com>
Hi David,
> >>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >>>> ---
> >>>> include/net/bluetooth/hci_core.h | 2 +
> >>>> net/bluetooth/hci_core.c | 58 ++++++++++++++++++++++++++++++++-----
> >>>> 2 files changed, 52 insertions(+), 8 deletions(-)
> >>> so I stuffed this now into bluetooth-testing tree and would like to see
> >>> some extra testing exposure. So far this has only been tested by myself.
> >>>
> >>> If there are no regression then this should make a lot of HCI and L2CAP
> >>> handling a lot simple.
> >> This may result in packets being processed in a different order to that
> >> which they were received in.
> >>
> >> e.g., what happens to an ACL packet processed before the connection
> >> complete event for that connection?
> >
> > good point. So we would either a) need to disable the RX tasklet when we
> > receive an event and schedule it for processing or b) process the ACL
> > data also in a workqueue.
>
> I think my preferred solution would be for the rx function called by the
> HCI transport drivers to be called in a thread context. There should be
> a helper function that is callable in atomic context that queues the
> packets to be processed in a workqueue.
>
> Our UWB drivers already process packets in a thread so would prefer to
> avoid another thread context switch.
>
> Shall I prepare a patch?
yes please. I need something to make the event handling simpler.
Otherwise every single subcomponent creates "hundreds" of workqueues to
solve exactly the same problem.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 2/7] Bluetooth: Use enhanced L2CAP header structure and symbolic values.
From: Marcel Holtmann @ 2010-08-10 21:39 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-3-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 23 +++++++++++++----------
> 1 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 5e78c18..aa69c84 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1753,33 +1753,36 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
> {
> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> struct sk_buff *skb;
> - int err, count, hlen = L2CAP_HDR_SIZE + 2;
> - struct l2cap_hdr *lh;
> + int err, count, hlen = L2CAP_ENHANCED_HDR_SIZE;
> + struct l2cap_enhanced_hdr *lh;
so where is that patch that actually introduces L2CAP_ENHANCED_HDR_SIZE
etc. I can't see that.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 4/7] Bluetooth: Linearize received L2CAP skbuffs.
From: Marcel Holtmann @ 2010-08-10 21:38 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-5-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Fragmented skbs are only encountered by in-kernel protocols and
> profiles when receiving ERTM or streaming mode L2CAP data. BNEP,
> CMTP, HIDP, and RFCOMM generally use basic mode, so will not
> generally need the extra memcpy's necessary to put the L2CAP
> payload in to a linear buffer. However, it is possible to
> use enhanced L2CAP in these modes so the possibility needs
> to be handled.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/bnep/core.c | 5 ++++-
> net/bluetooth/cmtp/core.c | 5 ++++-
> net/bluetooth/hidp/core.c | 10 ++++++++--
> net/bluetooth/rfcomm/core.c | 5 ++++-
> 4 files changed, 20 insertions(+), 5 deletions(-)
in general I am fine with this.
Regards
Marcel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox