* [PATCH] Fix not unmarking temporary flag when a new key is created
From: Luiz Augusto von Dentz @ 2010-08-11 13:49 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Device may have been marked as temporary so when a new key is generated
that meets the requirements to be stored it should unmark temporary flag
so the device is not removed when disconnected.
---
src/dbus-hci.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index 22f378d..8c1e10c 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -746,8 +746,7 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
else if (!bonding && old_key_type == 0xff)
hcid_dbus_bonding_process_complete(local, peer, 0);
- if (temporary)
- device_set_temporary(device, TRUE);
+ device_set_temporary(device, temporary);
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/2] Fix not unmarking temporary flag when a new key is created
From: Luiz Augusto von Dentz @ 2010-08-11 13:43 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
In-Reply-To: <20100811132757.GC13302@jh-x301>
Hi,
On Wed, Aug 11, 2010 at 4:27 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Luiz,
>
> On Wed, Aug 11, 2010, Luiz Augusto von Dentz wrote:
>> --- a/src/dbus-hci.c
>> +++ b/src/dbus-hci.c
>> @@ -746,8 +746,8 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
>> else if (!bonding && old_key_type == 0xff)
>> hcid_dbus_bonding_process_complete(local, peer, 0);
>>
>> - if (temporary)
>> - device_set_temporary(device, TRUE);
>> + if (device_is_temporary(device) != temporary)
>> + device_set_temporary(device, temporary);
>>
>> return 0;
>
> I'd prefer if you just call device_set_temporary(device, temporary)
> directly. That'd save you the if-check and a call to device_is_temporary.
Fair enough, I will change it and send a new patch in a few minutes.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Solve compilation warning usin gcc 4.5.0
From: Marcel Holtmann @ 2010-08-11 13:40 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <201008111517.07434.santoscadenas@gmail.com>
Hi Jose,
> I have also experienced more warnings, but I don't know what's the best way to
> solve it because they are related to gstreamer and I'm not familiar with it. I
> attach them in case anyone can solve them or help me showing the best way to
> solve do it.
>
> CC audio/audio_libgstbluetooth_la-gstsbcenc.lo
> audio/gstsbcenc.c:138:1: warning: no previous declaration for
> 'gst_sbc_enc_get_type'
> CC audio/audio_libgstbluetooth_la-gstsbcdec.lo
> audio/gstsbcdec.c:37:1: warning: no previous declaration for
> 'gst_sbc_dec_get_type'
> CC audio/audio_libgstbluetooth_la-gstsbcparse.lo
> audio/gstsbcparse.c:37:1: warning: no previous declaration for
> 'gst_sbc_parse_get_type'
> CC audio/audio_libgstbluetooth_la-gstavdtpsink.lo
> audio/gstavdtpsink.c:82:1: warning: no previous declaration for
> 'gst_avdtp_sink_get_type'
> CC audio/audio_libgstbluetooth_la-gsta2dpsink.lo
> audio/gsta2dpsink.c:50:1: warning: no previous declaration for
> 'gst_a2dp_sink_get_type'
> CC audio/audio_libgstbluetooth_la-gstsbcutil.lo
> CC audio/audio_libgstbluetooth_la-gstrtpsbcpay.lo
> audio/gstrtpsbcpay.c:70:1: warning: no previous declaration for
> 'gst_rtp_sbc_pay_get_type'
> CC audio/audio_libgstbluetooth_la-ipc.lo
>
>
> Looking to the code, the problem seems to be in this macro:
> GST_BOILERPLATE
>
> http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-
> GstUtils.html#GST-BOILERPLATE:CAPS
>
> As I said I don't have previous experience with gstreamer and I don't know if
> it is a good idea to declare the "get_type" function previously.
this is GStreamer type generation being utterly stupid. Either way
around it will produce a warning. It added #pragma magic to keep the
warning, but not failing the build. However we might need to turn around
and change that the warning only shows up with old compilers.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] Fix not unmarking temporary flag when a new key is created
From: Johan Hedberg @ 2010-08-11 13:27 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1281530667-25379-2-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Aug 11, 2010, Luiz Augusto von Dentz wrote:
> --- a/src/dbus-hci.c
> +++ b/src/dbus-hci.c
> @@ -746,8 +746,8 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
> else if (!bonding && old_key_type == 0xff)
> hcid_dbus_bonding_process_complete(local, peer, 0);
>
> - if (temporary)
> - device_set_temporary(device, TRUE);
> + if (device_is_temporary(device) != temporary)
> + device_set_temporary(device, temporary);
>
> return 0;
I'd prefer if you just call device_set_temporary(device, temporary)
directly. That'd save you the if-check and a call to device_is_temporary.
Johan
^ permalink raw reply
* Re: [PATCH 1/2] Fix not deleting stored keys when a new one is created
From: Johan Hedberg @ 2010-08-11 13:26 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1281530667-25379-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Aug 11, 2010, Luiz Augusto von Dentz wrote:
> 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(-)
The patch is now upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Solve compilation warning usin gcc 4.5.0
From: Johan Hedberg @ 2010-08-11 13:17 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1281531992-10976-1-git-send-email-santoscadenas@gmail.com>
Hi,
On Wed, Aug 11, 2010, Jose Antonio Santos Cadenas wrote:
> ---
> audio/control.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/control.c b/audio/control.c
> index c8aba53..24db448 100644
> --- a/audio/control.c
> +++ b/audio/control.c
> @@ -437,7 +437,7 @@ static void avctp_set_state(struct control *control, avctp_state_t new_state)
> {
> GSList *l;
> struct audio_device *dev = control->dev;
> - avdtp_session_state_t old_state = control->state;
> + avctp_state_t old_state = control->state;
> gboolean value;
>
> switch (new_state) {
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* Re: [PATCH] Solve compilation warning usin gcc 4.5.0
From: José Antonio Santos Cadenas @ 2010-08-11 13:17 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100811124942.GA12156@jh-x301>
El Wednesday 11 August 2010 14:49:42 Johan Hedberg escribió:
> Hi,
>
> On Wed, Aug 11, 2010, Jose Antonio Santos Cadenas wrote:
> > ---
> >
> > 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;
>
> That's the wrong fix to suppress the warning. The real problem is in the
> beginning of the function:
>
> avdtp_session_state_t old_state = control->state;
>
> Obviously that should be avctp_state_t and not avdtp. Do you
> want to send another patch to fix it?
>
> Johan
I have also experienced more warnings, but I don't know what's the best way to
solve it because they are related to gstreamer and I'm not familiar with it. I
attach them in case anyone can solve them or help me showing the best way to
solve do it.
CC audio/audio_libgstbluetooth_la-gstsbcenc.lo
audio/gstsbcenc.c:138:1: warning: no previous declaration for
'gst_sbc_enc_get_type'
CC audio/audio_libgstbluetooth_la-gstsbcdec.lo
audio/gstsbcdec.c:37:1: warning: no previous declaration for
'gst_sbc_dec_get_type'
CC audio/audio_libgstbluetooth_la-gstsbcparse.lo
audio/gstsbcparse.c:37:1: warning: no previous declaration for
'gst_sbc_parse_get_type'
CC audio/audio_libgstbluetooth_la-gstavdtpsink.lo
audio/gstavdtpsink.c:82:1: warning: no previous declaration for
'gst_avdtp_sink_get_type'
CC audio/audio_libgstbluetooth_la-gsta2dpsink.lo
audio/gsta2dpsink.c:50:1: warning: no previous declaration for
'gst_a2dp_sink_get_type'
CC audio/audio_libgstbluetooth_la-gstsbcutil.lo
CC audio/audio_libgstbluetooth_la-gstrtpsbcpay.lo
audio/gstrtpsbcpay.c:70:1: warning: no previous declaration for
'gst_rtp_sbc_pay_get_type'
CC audio/audio_libgstbluetooth_la-ipc.lo
Looking to the code, the problem seems to be in this macro:
GST_BOILERPLATE
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-
GstUtils.html#GST-BOILERPLATE:CAPS
As I said I don't have previous experience with gstreamer and I don't know if
it is a good idea to declare the "get_type" function previously.
Regards.
^ permalink raw reply
* [PATCH] Solve compilation warning usin gcc 4.5.0
From: Jose Antonio Santos Cadenas @ 2010-08-11 13:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <201008111455.41381.santoscadenas@gmail.com>
---
audio/control.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/control.c b/audio/control.c
index c8aba53..24db448 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -437,7 +437,7 @@ static void avctp_set_state(struct control *control, avctp_state_t new_state)
{
GSList *l;
struct audio_device *dev = control->dev;
- avdtp_session_state_t old_state = control->state;
+ avctp_state_t old_state = control->state;
gboolean value;
switch (new_state) {
--
1.7.2
^ permalink raw reply related
* Re: [PATCH] Solve compilation warning usin gcc 4.5.0
From: José Antonio Santos Cadenas @ 2010-08-11 12:55 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100811124942.GA12156@jh-x301>
El Wednesday 11 August 2010 14:49:42 Johan Hedberg escribió:
> Hi,
>
> On Wed, Aug 11, 2010, Jose Antonio Santos Cadenas wrote:
> > ---
> >
> > 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;
>
> That's the wrong fix to suppress the warning. The real problem is in the
> beginning of the function:
>
> avdtp_session_state_t old_state = control->state;
>
> Obviously that should be avctp_state_t and not avdtp. Do you
> want to send another patch to fix it?
Sure.
>
> Johan
^ permalink raw reply
* Re: [PATCH] Solve compilation warning usin gcc 4.5.0
From: Johan Hedberg @ 2010-08-11 12:49 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1281517766-5612-1-git-send-email-santoscadenas@gmail.com>
Hi,
On Wed, Aug 11, 2010, Jose Antonio Santos Cadenas wrote:
> ---
> 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;
That's the wrong fix to suppress the warning. The real problem is in the
beginning of the function:
avdtp_session_state_t old_state = control->state;
Obviously that should be avctp_state_t and not avdtp. Do you
want to send another patch to fix it?
Johan
^ permalink raw reply
* [PATCH 2/2] Fix not unmarking temporary flag when a new key is created
From: Luiz Augusto von Dentz @ 2010-08-11 12:44 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1281530667-25379-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Device may have been marked as temporary so when a new key is generated
and it meets the requirements to be stored the device should be no longer
temporary anymore.
---
src/dbus-hci.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index 22f378d..5505d00 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -746,8 +746,8 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
else if (!bonding && old_key_type == 0xff)
hcid_dbus_bonding_process_complete(local, peer, 0);
- if (temporary)
- device_set_temporary(device, TRUE);
+ if (device_is_temporary(device) != temporary)
+ device_set_temporary(device, temporary);
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* [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
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