This patch set includes the AMP policy socket option that was
previously discussed and accepted in to bluetooth-testing, and the
signaling framework for AMP channel creation and AMP channel moves.
There is overlap with Andrei Emeltchenko's RFC ("L2CAP and HCI
preparation code for A2MP") earlier this week. We (and Andrei) aren't
trying to have dueling patch sets - we very much want to cooperate and
get the AMP functionality upstreamed efficiently. We were in touch
with him earlier in the week to see how far he was with the porting
effort.
Next steps:
* L2CAP lockstep configuration
* HCI layer changes for AMP HCI commands and physical/logical links
* The rest of the create channel and move channel code
* Move channel coordination with ERTM
* AMP manager
* ERTM optimizations
Mat Martineau (9):
Bluetooth: Add BT_AMP_POLICY socket option
Bluetooth: Add AMP policy information to l2cap_chan
Bluetooth: Get/set AMP policy socket option
Bluetooth: Add AMP-related data and structures for channel signals
Bluetooth: Add signal handlers for channel creation
Bluetooth: Add definitions for L2CAP fixed channels
Bluetooth: Use symbolic values for the fixed channel map
Bluetooth: Add AMP header file
Bluetooth: Add signal handlers for channel moves
include/net/bluetooth/amp.h | 20 +++++
include/net/bluetooth/bluetooth.h | 27 ++++++
include/net/bluetooth/l2cap.h | 65 ++++++++++++++
net/bluetooth/l2cap_core.c | 172 ++++++++++++++++++++++++++++++++++++-
net/bluetooth/l2cap_sock.c | 25 ++++++
5 files changed, 308 insertions(+), 1 deletions(-)
create mode 100644 include/net/bluetooth/amp.h
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Luiz,
> >> Not sure if applications actually do care, but for OBEX it might be
> >> interesting to know if you are on an AMP controller right now or not.
> >
> > Yes, that would be a nice feature. OBEX seems to work well even without it,
> > though. The OBEX connection is made over BREDR, and the socket option is
> > changed after the OBEX connection is made but before any gets or puts. What
> > you see in hcidump is that the move starts before the first get or put goes
> > out over BREDR - those OBEX packets sit in the ERTM tx queue while the
> > channel move happens. When the move completes, the transfer immediatly
> > proceeds on the AMP controller.
>
> I guess the is implementation dependent, for obexd we might want to
> check if the transfer will take a considerable amount of time to
> decide whether or not to use AMP, in some case it might not worth to
> switch because it will take more time than transferring over BDEDR.
there has been discussions that you should be waiting for the first
packet on BR/EDR and hope that it indicates the length of the data to
follow. And then make a decision to allow switching or not. All three
options would support this behavior right now.
Until we are collecting real data for the switching time with BlueZ, we
will have no idea anyway and everything will be a guess.
Regards
Marcel
Hi Mat,
On Sat, Oct 15, 2011 at 1:38 AM, Mat Martineau <[email protected]> wro=
te:
>>
>> Not sure if applications actually do care, but for OBEX it might be
>> interesting to know if you are on an AMP controller right now or not.
>
> Yes, that would be a nice feature. =A0OBEX seems to work well even withou=
t it,
> though. =A0The OBEX connection is made over BREDR, and the socket option =
is
> changed after the OBEX connection is made but before any gets or puts. =
=A0What
> you see in hcidump is that the move starts before the first get or put go=
es
> out over BREDR - those OBEX packets sit in the ERTM tx queue while the
> channel move happens. =A0When the move completes, the transfer immediatly
> proceeds on the AMP controller.
I guess the is implementation dependent, for obexd we might want to
check if the transfer will take a considerable amount of time to
decide whether or not to use AMP, in some case it might not worth to
switch because it will take more time than transferring over BDEDR.
--=20
Luiz Augusto von Dentz
Hi Mat,
On Fri, Oct 14, 2011 at 04:19:53PM -0700, Mat Martineau wrote:
> >>AMP channels can be moved between BR/EDR and AMP controllers using a
> >>sequence of signals. Every attempted channel move involves a series of
> >>four signals:
> >>
> >> Move Initiator Move Responder
> >> | |
> >> | Move Channel Request |
> >> | ----------------------------> |
> >> | |
> >> | Move Channel Response |
> >> | <---------------------------- |
> >> | |
> >> | Move Channel Confirm |
> >> | ----------------------------> |
> >> | |
> >> | Move Channel Confirm Response |
> >> | <---------------------------- |
> >>
> >>All four signals are sent even if the move fails.
> >>
> >>Signed-off-by: Mat Martineau <[email protected]>
> >>---
> >> net/bluetooth/l2cap_core.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
> >> 1 files changed, 123 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>index e38258b..ba64bab 100644
> >>--- a/net/bluetooth/l2cap_core.c
> >>+++ b/net/bluetooth/l2cap_core.c
> >>@@ -2957,6 +2957,112 @@ static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
> >> return l2cap_connect_rsp(conn, cmd, data);
> >> }
> >>
> >>+static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
> >>+ u16 icid, u16 result)
> >>+{
> >>+ struct l2cap_move_chan_rsp rsp;
> >>+
> >>+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
BTW: why do you need type conversion here and below?
Best regards
Andrei Emeltchenko
> >>+
> >>+ rsp.icid = cpu_to_le16(icid);
> >>+ rsp.result = cpu_to_le16(result);
> >>+
> >>+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
> >>+}
> >>+
> >>+static void l2cap_send_move_chan_cfm(struct l2cap_conn *conn,
> >>+ struct l2cap_chan *chan, u16 icid, u16 result)
> >>+{
> >>+ struct l2cap_move_chan_cfm cfm;
> >>+ u8 ident;
> >>+
> >>+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
> >>+
> >>+ ident = l2cap_get_ident(conn);
> >>+ if (chan)
> >>+ chan->ident = ident;
> >>+
> >>+ cfm.icid = cpu_to_le16(icid);
> >>+ cfm.result = cpu_to_le16(result);
> >>+
> >>+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM, sizeof(cfm), &cfm);
> >>+}
> >>+
> >>+static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
> >>+ u16 icid)
> >>+{
> >>+ struct l2cap_move_chan_cfm_rsp rsp;
> >>+
> >>+ BT_DBG("icid %d", (int) icid);
> >>+
> >>+ rsp.icid = cpu_to_le16(icid);
> >>+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
> >>+}
> >>+
> >>+static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> >>+ struct l2cap_cmd_hdr *cmd, u8 *data)
> >>+{
> >>+ struct l2cap_move_chan_req *req = (struct l2cap_move_chan_req *) data;
> >>+ u16 icid = 0;
> >>+ u16 result = L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED;
> >>+
> >>+ icid = le16_to_cpu(req->icid);
> >>+
> >>+ BT_DBG("icid %d, dest_amp_id %d", (int) icid, (int) req->dest_amp_id);
> >>+
> >>+ /* Placeholder: Always refuse */
> >>+ l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
> >>+
> >>+ return 0;
> >>+}
> >
> >It's a good idea protect the move channel request with enable_hs.
>
> Yes, I'll require enable_hs to have the move channel do anything.
> When enable_hs is false, it seems like it should return a
> L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED response like it does now.
Hi Mat,
On Fri, Oct 14, 2011 at 03:58:27PM -0700, Mat Martineau wrote:
> >>+#define L2CAP_CREATE_CHAN_STATUS_NONE 0x0000
> >>+#define L2CAP_CREATE_CHAN_STATUS_AUTHENTICATION 0x0001
> >>+#define L2CAP_CREATE_CHAN_STATUS_AUTHORIZATION 0x0002
> >
> >Isn't it a little bit log? why not CR/CS as it is already done?
>
> I was staying close to the spec, but I admit it's verbose by Linux
> kernel standards. I'll adopt your shorter names.
>
>
> >BTW: I've done it different way. Since STATUS and RESULT are almost the same for
> >connect/create my patch is:
> >
> ><------8<-----------------------------------------
> >| @@ -214,14 +216,15 @@ struct l2cap_conn_rsp {
> >| #define L2CAP_CID_DYN_START 0x0040
> >| #define L2CAP_CID_DYN_END 0xffff
> >|
> >| -/* connect result */
> >| +/* connect/create channel results */
> >| #define L2CAP_CR_SUCCESS 0x0000
> >| #define L2CAP_CR_PEND 0x0001
> >| #define L2CAP_CR_BAD_PSM 0x0002
> >| #define L2CAP_CR_SEC_BLOCK 0x0003
> >| #define L2CAP_CR_NO_MEM 0x0004
> >| +#define L2CAP_CR_BAD_AMP 0x0005
> >|
> >| -/* connect status */
> >| +/* connect/create channel status */
> >| #define L2CAP_CS_NO_INFO 0x0000
> >| #define L2CAP_CS_AUTHEN_PEND 0x0001
> >| #define L2CAP_CS_AUTHOR_PEND 0x0002
> >|
> ><------8<-----------------------------------------
> >
> >>+
> >>+struct l2cap_move_chan_req {
> >>+ __le16 icid;
> >>+ __u8 dest_amp_id;
> >>+} __attribute__ ((packed));
> >>+
> >>+struct l2cap_move_chan_rsp {
> >>+ __le16 icid;
> >>+ __le16 result;
> >>+} __attribute__ ((packed));
> >>+
> >>+#define L2CAP_MOVE_CHAN_SUCCESS 0x0000
> >>+#define L2CAP_MOVE_CHAN_PENDING 0x0001
> >>+#define L2CAP_MOVE_CHAN_REFUSED_CONTROLLER 0x0002
> >>+#define L2CAP_MOVE_CHAN_REFUSED_SAME_ID 0x0003
> >>+#define L2CAP_MOVE_CHAN_REFUSED_CONFIG 0x0004
> >>+#define L2CAP_MOVE_CHAN_REFUSED_COLLISION 0x0005
> >>+#define L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED 0x0006
> >
> >Same here: use CR and shorter names
>
> How about L2CAP_MC_SUCCESS, L2CAP_MC_PEND, etc.? While create
> channel is very similar to the connect request, moving a channel is
> a whole different idea.
Maybe MR (Move Result, similar to CR) is better.
Best regards
Andrei Emeltchenko
Hi Mat,
> >> Allow control of AMP functionality on L2CAP sockets. By default,
> >> connections will be restricted to BR/EDR. Manipulating the
> >> BT_AMP_POLICY option allows for channels to be moved to or created
> >> on AMP controllers.
> >>
> >> Signed-off-by: Mat Martineau <[email protected]>
> >> ---
> >> include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++
> >> 1 files changed, 27 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> >> index e727555..14ae418 100644
> >> --- a/include/net/bluetooth/bluetooth.h
> >> +++ b/include/net/bluetooth/bluetooth.h
> >> @@ -77,6 +77,33 @@ struct bt_power {
> >> #define BT_POWER_FORCE_ACTIVE_OFF 0
> >> #define BT_POWER_FORCE_ACTIVE_ON 1
> >>
> >> +#define BT_AMP_POLICY 10
> >> +
> >> +/* Require BR/EDR (default policy)
> >> + * AMP controllers cannot be used.
> >> + * Channel move requests from the remote device are denied.
> >> + * If the L2CAP channel is currently using AMP, move the channel to BR/EDR.
> >> + */
> >> +#define BT_AMP_POLICY_REQUIRE_BR_EDR 0
> >
> > so lets brainstorm a little bit with the names here. Do we really wanna
> > use the term AMP_POLICY? Is it really AMP specific? Or do we better make
> > ourselves a bit more future proof?
> >
> > I am currently thinking a bit more generic. Maybe this is not the best
> > choice, but lets entertain this for a bit. So what about calling this
> > BT_CHANNEL_POLICY?
> >
> > And instead of REQUIRE_BR_EDR I would call it BREDR_ONLY.
>
> It's no problem to fine-tune the names.
>
> So, if BT_CHANNEL_POLICY is a good option name, how about these option
> values:
>
> BT_CHANNEL_POLICY_BREDR_ONLY
> BT_CHANNEL_POLICY_AMP_PREFERRED
> BT_CHANNEL_POLICY_BREDR_PREFERRED
lets resort the list as
BT_CHANNEL_POLICY_BREDR_ONLY
BT_CHANNEL_POLICY_BREDR_PREFERRED
BT_CHANNEL_POLICY_AMP_PREFERRED
and I would think we are good. However any other opinion on the naming
would be welcome here as well. I was just brainstorming here.
> >> +/* Prefer AMP
> >> + * Allow use of AMP controllers
> >> + * If the L2CAP channel is currently on BR/EDR and AMP controller
> >> + * resources are available, initiate a channel move to AMP.
> >> + * Channel move requests from the remote device are allowed.
> >> + * If the L2CAP socket has not been connected yet, try to create
> >> + * and configure the channel directly on an AMP controller rather
> >> + * than BR/EDR.
> >> + */
> >> +#define BT_AMP_POLICY_PREFER_AMP 1
> >> +
> >> +/* Prefer BR/EDR
> >> + * Allow use of AMP controllers.
> >> + * If the L2CAP channel is currently on AMP, move it to BR/EDR.
> >> + * Channel move requests from the remote device are allowed.
> >> + */
> >> +#define BT_AMP_POLICY_PREFER_BR_EDR 2
> >
> > We might also wanna look into the option of having a SCM message in the
> > socket that tells us the current type. Or at least tells us when we
> > successfully switched to AMP or back to BR/EDR. Since obviously we are
> > not blocking on the setsockopt call.
> >
> > Not sure if applications actually do care, but for OBEX it might be
> > interesting to know if you are on an AMP controller right now or not.
>
> Yes, that would be a nice feature. OBEX seems to work well even
> without it, though. The OBEX connection is made over BREDR, and the
> socket option is changed after the OBEX connection is made but before
> any gets or puts. What you see in hcidump is that the move starts
> before the first get or put goes out over BREDR - those OBEX packets
> sit in the ERTM tx queue while the channel move happens. When the
> move completes, the transfer immediatly proceeds on the AMP
> controller.
If it works for fine for you guys right now, then we could do that later
and don't have to worry right now. However before we make enable_hs on
by default I like to have this figured out.
Regards
Marcel
Hi Mat,
* Mat Martineau <[email protected]> [2011-10-14 16:04:01 -0700]:
>
>
> On Fri, 14 Oct 2011, Andrei Emeltchenko wrote:
>
> >Hi Mat,
> >
> >On Thu, Oct 13, 2011 at 03:00:45PM -0700, Mat Martineau wrote:
> >>The A2MP fixed channel bit is only set when high-speed mode is enabled.
> >
> >It might make sense to merge previous patch with definitions of
> >L2CAP_FC_L2CAP and L2CAP_FC_A2MP, otherwise these 2 patches are a bit
> >small.
> >
> >>
> >>Signed-off-by: Mat Martineau <[email protected]>
> >>---
> >> net/bluetooth/l2cap_core.c | 6 +++++-
> >> 1 files changed, 5 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>index d023156..e38258b 100644
> >>--- a/net/bluetooth/l2cap_core.c
> >>+++ b/net/bluetooth/l2cap_core.c
> >>@@ -60,7 +60,7 @@ int disable_ertm;
> >> int enable_hs;
> >>
> >> static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
> >>-static u8 l2cap_fixed_chan[8] = { 0x02, };
> >>+static u8 l2cap_fixed_chan[8] = { L2CAP_FC_L2CAP, };
> >
> >I personally do not like present approach with allocating 8-octet array
> >when we need only one octet _FOR_NOW_. (Also assigning is not clear enough).
> >
> >Why not:
> >
> ><------8<-----------------------------------------------
> >| @@ -60,7 +60,7 @@ int disable_ertm;
> >| int enable_hs;
> >|
> >| static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
> >| -static u8 l2cap_fixed_chan[8] = { 0x02, };
> >| +static u8 l2cap_fixed_chan = L2CAP_FC_L2CAP;
> >|
> >| static LIST_HEAD(chan_list);
> >| static DEFINE_RWLOCK(chan_list_lock);
> >|
> ><------8<-----------------------------------------------
>
> Since the fixed channel map is 8 bytes, it makes some sense to have
> this data structure match what is used in the info request. The 8th
> byte contains a bit that's defined for the AMP test manager too.
Yes, let's keep the 8 octet array. It make more sense as we use it on the
information response. Btw, I fine with this patch.
Gustavo
On Fri, 14 Oct 2011, Gustavo Padovan wrote:
> Hi Mat,
>
> * Mat Martineau <[email protected]> [2011-10-13 15:00:47 -0700]:
>
>> AMP channels can be moved between BR/EDR and AMP controllers using a
>> sequence of signals. Every attempted channel move involves a series of
>> four signals:
>>
>> Move Initiator Move Responder
>> | |
>> | Move Channel Request |
>> | ----------------------------> |
>> | |
>> | Move Channel Response |
>> | <---------------------------- |
>> | |
>> | Move Channel Confirm |
>> | ----------------------------> |
>> | |
>> | Move Channel Confirm Response |
>> | <---------------------------- |
>>
>> All four signals are sent even if the move fails.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> net/bluetooth/l2cap_core.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 123 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index e38258b..ba64bab 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2957,6 +2957,112 @@ static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
>> return l2cap_connect_rsp(conn, cmd, data);
>> }
>>
>> +static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
>> + u16 icid, u16 result)
>> +{
>> + struct l2cap_move_chan_rsp rsp;
>> +
>> + BT_DBG("icid %d, result %d", (int) icid, (int) result);
>> +
>> + rsp.icid = cpu_to_le16(icid);
>> + rsp.result = cpu_to_le16(result);
>> +
>> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
>> +}
>> +
>> +static void l2cap_send_move_chan_cfm(struct l2cap_conn *conn,
>> + struct l2cap_chan *chan, u16 icid, u16 result)
>> +{
>> + struct l2cap_move_chan_cfm cfm;
>> + u8 ident;
>> +
>> + BT_DBG("icid %d, result %d", (int) icid, (int) result);
>> +
>> + ident = l2cap_get_ident(conn);
>> + if (chan)
>> + chan->ident = ident;
>> +
>> + cfm.icid = cpu_to_le16(icid);
>> + cfm.result = cpu_to_le16(result);
>> +
>> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM, sizeof(cfm), &cfm);
>> +}
>> +
>> +static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>> + u16 icid)
>> +{
>> + struct l2cap_move_chan_cfm_rsp rsp;
>> +
>> + BT_DBG("icid %d", (int) icid);
>> +
>> + rsp.icid = cpu_to_le16(icid);
>> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>> +}
>> +
>> +static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>> + struct l2cap_cmd_hdr *cmd, u8 *data)
>> +{
>> + struct l2cap_move_chan_req *req = (struct l2cap_move_chan_req *) data;
>> + u16 icid = 0;
>> + u16 result = L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED;
>> +
>> + icid = le16_to_cpu(req->icid);
>> +
>> + BT_DBG("icid %d, dest_amp_id %d", (int) icid, (int) req->dest_amp_id);
>> +
>> + /* Placeholder: Always refuse */
>> + l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>> +
>> + return 0;
>> +}
>
> It's a good idea protect the move channel request with enable_hs.
Yes, I'll require enable_hs to have the move channel do anything.
When enable_hs is false, it seems like it should return a
L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED response like it does now.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Fri, 14 Oct 2011, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Thu, Oct 13, 2011 at 03:00:46PM -0700, Mat Martineau wrote:
>> This file will be home to all AMP- and A2MP-related declarations. The
>> first macros map between HCI device indexes (for AMP controllers) and
>> AMP controller ids that are passed over the air.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> include/net/bluetooth/amp.h | 20 ++++++++++++++++++++
>> 1 files changed, 20 insertions(+), 0 deletions(-)
>> create mode 100644 include/net/bluetooth/amp.h
>>
>> diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
>> new file mode 100644
>> index 0000000..b84ce53
>> --- /dev/null
>> +++ b/include/net/bluetooth/amp.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + Copyright (c) 2010-2011 Code Aurora Forum. All rights reserved.
>> +
>> + This program is free software; you can redistribute it and/or modify
>> + it under the terms of the GNU General Public License version 2 and
>> + only version 2 as published by the Free Software Foundation.
>> +
>> + This program is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + GNU General Public License for more details.
>> +*/
>> +
>> +#ifndef __AMP_H
>> +#define __AMP_H
>> +
>> +#define HCI_A2MP_ID(id) ((id)+0x10) /* convert HCI dev index to AMP ID */
>> +#define A2MP_HCI_ID(id) ((id)-0x10) /* convert AMP ID to HCI dev index */
>
> Missing spaces? Can this be in hci.h ?
After looking at this some more, I'm going to eliminate these macros
altogether. "0" is not a valid AMP id, so if AMP controllers are not
registered as hci0, these macros go away and the code gets a lot
cleaner.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Fri, 14 Oct 2011, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Thu, Oct 13, 2011 at 03:00:45PM -0700, Mat Martineau wrote:
>> The A2MP fixed channel bit is only set when high-speed mode is enabled.
>
> It might make sense to merge previous patch with definitions of
> L2CAP_FC_L2CAP and L2CAP_FC_A2MP, otherwise these 2 patches are a bit
> small.
>
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> net/bluetooth/l2cap_core.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index d023156..e38258b 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -60,7 +60,7 @@ int disable_ertm;
>> int enable_hs;
>>
>> static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
>> -static u8 l2cap_fixed_chan[8] = { 0x02, };
>> +static u8 l2cap_fixed_chan[8] = { L2CAP_FC_L2CAP, };
>
> I personally do not like present approach with allocating 8-octet array
> when we need only one octet _FOR_NOW_. (Also assigning is not clear enough).
>
> Why not:
>
> <------8<-----------------------------------------------
> | @@ -60,7 +60,7 @@ int disable_ertm;
> | int enable_hs;
> |
> | static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
> | -static u8 l2cap_fixed_chan[8] = { 0x02, };
> | +static u8 l2cap_fixed_chan = L2CAP_FC_L2CAP;
> |
> | static LIST_HEAD(chan_list);
> | static DEFINE_RWLOCK(chan_list_lock);
> |
> <------8<-----------------------------------------------
Since the fixed channel map is 8 bytes, it makes some sense to have
this data structure match what is used in the info request. The 8th
byte contains a bit that's defined for the AMP test manager too.
Gustavo, what do you think?
>>
>> static LIST_HEAD(chan_list);
>> static DEFINE_RWLOCK(chan_list_lock);
>> @@ -2849,6 +2849,10 @@ static inline int l2cap_information_req(struct l2cap_conn *conn, struct l2cap_cm
>> } else if (type == L2CAP_IT_FIXED_CHAN) {
>> u8 buf[12];
>> struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
>> +
>> + if (enable_hs)
>> + l2cap_fixed_chan[0] |= L2CAP_FC_A2MP;
>> +
>> rsp->type = cpu_to_le16(L2CAP_IT_FIXED_CHAN);
>> rsp->result = cpu_to_le16(L2CAP_IR_SUCCESS);
>> memcpy(buf + 4, l2cap_fixed_chan, 8);
>
> and then here:
>
> <------8<--------------------------------------------------------------------------------------------------
> | @@ -2895,9 +2895,16 @@ static inline int l2cap_information_req(struct l2cap_conn *conn, struct l2cap_cm
> | } else if (type == L2CAP_IT_FIXED_CHAN) {
> | u8 buf[12];
> | struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
> | +
> | + memset(buf, 0, sizeof(buf));
> | rsp->type = cpu_to_le16(L2CAP_IT_FIXED_CHAN);
> | rsp->result = cpu_to_le16(L2CAP_IR_SUCCESS);
> | - memcpy(buf + 4, l2cap_fixed_chan, 8);
> | +
> | + if (enable_hs)
> | + l2cap_fixed_chan |= L2CAP_FC_A2MP;
> | +
> | + rsp->data[0] = l2cap_fixed_chan;
> | +
> | l2cap_send_cmd(conn, cmd->ident,
> | L2CAP_INFO_RSP, sizeof(buf), buf);
> | } else {
> |
> <------8<--------------------------------------------------------------------------------------------------
>
> See my patch here:
>
> http://www.spinics.net/lists/linux-bluetooth/msg17023.html
Thanks,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Fri, 14 Oct 2011, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Thu, Oct 13, 2011 at 03:00:42PM -0700, Mat Martineau wrote:
>> AMP channel creation and channel moves are coordinated using the L2CAP
>> signaling channel. These definitions cover the "create channel",
>> "move channel", and "move channel confirm" signals.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> include/net/bluetooth/l2cap.h | 60 +++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 60 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index e77d39f..2ea0d15 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -89,6 +89,12 @@ struct l2cap_conninfo {
>> #define L2CAP_ECHO_RSP 0x09
>> #define L2CAP_INFO_REQ 0x0a
>> #define L2CAP_INFO_RSP 0x0b
>> +#define L2CAP_CREATE_CHAN_REQ 0x0c
>> +#define L2CAP_CREATE_CHAN_RSP 0x0d
>> +#define L2CAP_MOVE_CHAN_REQ 0x0e
>> +#define L2CAP_MOVE_CHAN_RSP 0x0f
>> +#define L2CAP_MOVE_CHAN_CFM 0x10
>
> more tabs
Ok, I can line up the whole list.
>
>> +#define L2CAP_MOVE_CHAN_CFM_RSP 0x11
>> #define L2CAP_CONN_PARAM_UPDATE_REQ 0x12
>> #define L2CAP_CONN_PARAM_UPDATE_RSP 0x13
>>
>> @@ -296,6 +302,60 @@ struct l2cap_info_rsp {
>> __u8 data[0];
>> } __packed;
>>
>> +struct l2cap_create_chan_req {
>> + __le16 psm;
>> + __le16 scid;
>> + __u8 amp_id;
>> +} __attribute__ ((packed));
>
> I prefer tabs instead of spaces here and below. I will send patch cleaning
> remaining spaces in l2cap.h
Ok, it will be good to have them all the same style.
I also need to replace "__attribute__ ((packed))" with "__packed".
>
>> +
>> +struct l2cap_create_chan_rsp {
>> + __le16 dcid;
>> + __le16 scid;
>> + __le16 result;
>> + __le16 status;
>> +} __attribute__ ((packed));
>> +
>> +#define L2CAP_CREATE_CHAN_SUCCESS 0x0000
>> +#define L2CAP_CREATE_CHAN_PENDING 0x0001
>> +#define L2CAP_CREATE_CHAN_REFUSED_PSM 0x0002
>> +#define L2CAP_CREATE_CHAN_REFUSED_SECURITY 0x0003
>> +#define L2CAP_CREATE_CHAN_REFUSED_RESOURCES 0x0004
>> +#define L2CAP_CREATE_CHAN_REFUSED_CONTROLLER 0x0005
>> +
>> +#define L2CAP_CREATE_CHAN_STATUS_NONE 0x0000
>> +#define L2CAP_CREATE_CHAN_STATUS_AUTHENTICATION 0x0001
>> +#define L2CAP_CREATE_CHAN_STATUS_AUTHORIZATION 0x0002
>
> Isn't it a little bit log? why not CR/CS as it is already done?
I was staying close to the spec, but I admit it's verbose by Linux
kernel standards. I'll adopt your shorter names.
> BTW: I've done it different way. Since STATUS and RESULT are almost the same for
> connect/create my patch is:
>
> <------8<-----------------------------------------
> | @@ -214,14 +216,15 @@ struct l2cap_conn_rsp {
> | #define L2CAP_CID_DYN_START 0x0040
> | #define L2CAP_CID_DYN_END 0xffff
> |
> | -/* connect result */
> | +/* connect/create channel results */
> | #define L2CAP_CR_SUCCESS 0x0000
> | #define L2CAP_CR_PEND 0x0001
> | #define L2CAP_CR_BAD_PSM 0x0002
> | #define L2CAP_CR_SEC_BLOCK 0x0003
> | #define L2CAP_CR_NO_MEM 0x0004
> | +#define L2CAP_CR_BAD_AMP 0x0005
> |
> | -/* connect status */
> | +/* connect/create channel status */
> | #define L2CAP_CS_NO_INFO 0x0000
> | #define L2CAP_CS_AUTHEN_PEND 0x0001
> | #define L2CAP_CS_AUTHOR_PEND 0x0002
> |
> <------8<-----------------------------------------
>
>> +
>> +struct l2cap_move_chan_req {
>> + __le16 icid;
>> + __u8 dest_amp_id;
>> +} __attribute__ ((packed));
>> +
>> +struct l2cap_move_chan_rsp {
>> + __le16 icid;
>> + __le16 result;
>> +} __attribute__ ((packed));
>> +
>> +#define L2CAP_MOVE_CHAN_SUCCESS 0x0000
>> +#define L2CAP_MOVE_CHAN_PENDING 0x0001
>> +#define L2CAP_MOVE_CHAN_REFUSED_CONTROLLER 0x0002
>> +#define L2CAP_MOVE_CHAN_REFUSED_SAME_ID 0x0003
>> +#define L2CAP_MOVE_CHAN_REFUSED_CONFIG 0x0004
>> +#define L2CAP_MOVE_CHAN_REFUSED_COLLISION 0x0005
>> +#define L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED 0x0006
>
> Same here: use CR and shorter names
How about L2CAP_MC_SUCCESS, L2CAP_MC_PEND, etc.? While create channel
is very similar to the connect request, moving a channel is a whole
different idea.
>> +
>> +struct l2cap_move_chan_cfm {
>> + __le16 icid;
>> + __le16 result;
>> +} __attribute__ ((packed));
>> +
>> +#define L2CAP_MOVE_CHAN_CONFIRMED 0x0000
>> +#define L2CAP_MOVE_CHAN_UNCONFIRMED 0x0001
>> +
>> +struct l2cap_move_chan_cfm_rsp {
>> + __le16 icid;
>> +} __attribute__ ((packed));
>> +
>> /* info type */
>> #define L2CAP_IT_CL_MTU 0x0001
>> #define L2CAP_IT_FEAT_MASK 0x0002
>> --
>> 1.7.7
>>
Thanks for the careful review!
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Fri, 14 Oct 2011, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Thu, Oct 13, 2011 at 03:00:40PM -0700, Mat Martineau wrote:
>> Each channel has an active AMP policy to require BR/EDR (the default),
>> prefer AMP, or prefer BR/EDR.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> include/net/bluetooth/l2cap.h | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 806b950..e77d39f 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -357,6 +357,7 @@ struct l2cap_chan {
>> __u8 num_conf_rsp;
>>
>> __u8 fcs;
>> + __u8 amp_policy;
>
> I would merge this patch with following one since this is only one line.
Sure. I've seen a lot of requests to split up header changes in to
separate commits, but I agree that doing it for one line is a little
extreme.
>>
>> __u16 tx_win;
>> __u8 max_tx;
>> --
>> 1.7.7
>>
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Thu, 13 Oct 2011, Marcel Holtmann wrote:
> Hi Mat,
>
>> Allow control of AMP functionality on L2CAP sockets. By default,
>> connections will be restricted to BR/EDR. Manipulating the
>> BT_AMP_POLICY option allows for channels to be moved to or created
>> on AMP controllers.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++
>> 1 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>> index e727555..14ae418 100644
>> --- a/include/net/bluetooth/bluetooth.h
>> +++ b/include/net/bluetooth/bluetooth.h
>> @@ -77,6 +77,33 @@ struct bt_power {
>> #define BT_POWER_FORCE_ACTIVE_OFF 0
>> #define BT_POWER_FORCE_ACTIVE_ON 1
>>
>> +#define BT_AMP_POLICY 10
>> +
>> +/* Require BR/EDR (default policy)
>> + * AMP controllers cannot be used.
>> + * Channel move requests from the remote device are denied.
>> + * If the L2CAP channel is currently using AMP, move the channel to BR/EDR.
>> + */
>> +#define BT_AMP_POLICY_REQUIRE_BR_EDR 0
>
> so lets brainstorm a little bit with the names here. Do we really wanna
> use the term AMP_POLICY? Is it really AMP specific? Or do we better make
> ourselves a bit more future proof?
>
> I am currently thinking a bit more generic. Maybe this is not the best
> choice, but lets entertain this for a bit. So what about calling this
> BT_CHANNEL_POLICY?
>
> And instead of REQUIRE_BR_EDR I would call it BREDR_ONLY.
It's no problem to fine-tune the names.
So, if BT_CHANNEL_POLICY is a good option name, how about these option
values:
BT_CHANNEL_POLICY_BREDR_ONLY
BT_CHANNEL_POLICY_AMP_PREFERRED
BT_CHANNEL_POLICY_BREDR_PREFERRED
>> +/* Prefer AMP
>> + * Allow use of AMP controllers
>> + * If the L2CAP channel is currently on BR/EDR and AMP controller
>> + * resources are available, initiate a channel move to AMP.
>> + * Channel move requests from the remote device are allowed.
>> + * If the L2CAP socket has not been connected yet, try to create
>> + * and configure the channel directly on an AMP controller rather
>> + * than BR/EDR.
>> + */
>> +#define BT_AMP_POLICY_PREFER_AMP 1
>> +
>> +/* Prefer BR/EDR
>> + * Allow use of AMP controllers.
>> + * If the L2CAP channel is currently on AMP, move it to BR/EDR.
>> + * Channel move requests from the remote device are allowed.
>> + */
>> +#define BT_AMP_POLICY_PREFER_BR_EDR 2
>
> We might also wanna look into the option of having a SCM message in the
> socket that tells us the current type. Or at least tells us when we
> successfully switched to AMP or back to BR/EDR. Since obviously we are
> not blocking on the setsockopt call.
>
> Not sure if applications actually do care, but for OBEX it might be
> interesting to know if you are on an AMP controller right now or not.
Yes, that would be a nice feature. OBEX seems to work well even
without it, though. The OBEX connection is made over BREDR, and the
socket option is changed after the OBEX connection is made but before
any gets or puts. What you see in hcidump is that the move starts
before the first get or put goes out over BREDR - those OBEX packets
sit in the ERTM tx queue while the channel move happens. When the
move completes, the transfer immediatly proceeds on the AMP
controller.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Mat,
* Mat Martineau <[email protected]> [2011-10-13 15:00:47 -0700]:
> AMP channels can be moved between BR/EDR and AMP controllers using a
> sequence of signals. Every attempted channel move involves a series of
> four signals:
>
> Move Initiator Move Responder
> | |
> | Move Channel Request |
> | ----------------------------> |
> | |
> | Move Channel Response |
> | <---------------------------- |
> | |
> | Move Channel Confirm |
> | ----------------------------> |
> | |
> | Move Channel Confirm Response |
> | <---------------------------- |
>
> All four signals are sent even if the move fails.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 123 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index e38258b..ba64bab 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2957,6 +2957,112 @@ static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
> return l2cap_connect_rsp(conn, cmd, data);
> }
>
> +static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
> + u16 icid, u16 result)
> +{
> + struct l2cap_move_chan_rsp rsp;
> +
> + BT_DBG("icid %d, result %d", (int) icid, (int) result);
> +
> + rsp.icid = cpu_to_le16(icid);
> + rsp.result = cpu_to_le16(result);
> +
> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
> +}
> +
> +static void l2cap_send_move_chan_cfm(struct l2cap_conn *conn,
> + struct l2cap_chan *chan, u16 icid, u16 result)
> +{
> + struct l2cap_move_chan_cfm cfm;
> + u8 ident;
> +
> + BT_DBG("icid %d, result %d", (int) icid, (int) result);
> +
> + ident = l2cap_get_ident(conn);
> + if (chan)
> + chan->ident = ident;
> +
> + cfm.icid = cpu_to_le16(icid);
> + cfm.result = cpu_to_le16(result);
> +
> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM, sizeof(cfm), &cfm);
> +}
> +
> +static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
> + u16 icid)
> +{
> + struct l2cap_move_chan_cfm_rsp rsp;
> +
> + BT_DBG("icid %d", (int) icid);
> +
> + rsp.icid = cpu_to_le16(icid);
> + l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
> +}
> +
> +static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> + struct l2cap_cmd_hdr *cmd, u8 *data)
> +{
> + struct l2cap_move_chan_req *req = (struct l2cap_move_chan_req *) data;
> + u16 icid = 0;
> + u16 result = L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED;
> +
> + icid = le16_to_cpu(req->icid);
> +
> + BT_DBG("icid %d, dest_amp_id %d", (int) icid, (int) req->dest_amp_id);
> +
> + /* Placeholder: Always refuse */
> + l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
> +
> + return 0;
> +}
It's a good idea protect the move channel request with enable_hs.
Gustavo
Hi Mat,
On Thu, Oct 13, 2011 at 03:00:46PM -0700, Mat Martineau wrote:
> This file will be home to all AMP- and A2MP-related declarations. The
> first macros map between HCI device indexes (for AMP controllers) and
> AMP controller ids that are passed over the air.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> include/net/bluetooth/amp.h | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
> create mode 100644 include/net/bluetooth/amp.h
>
> diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
> new file mode 100644
> index 0000000..b84ce53
> --- /dev/null
> +++ b/include/net/bluetooth/amp.h
> @@ -0,0 +1,20 @@
> +/*
> + Copyright (c) 2010-2011 Code Aurora Forum. All rights reserved.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License version 2 and
> + only version 2 as published by the Free Software Foundation.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +*/
> +
> +#ifndef __AMP_H
> +#define __AMP_H
> +
> +#define HCI_A2MP_ID(id) ((id)+0x10) /* convert HCI dev index to AMP ID */
> +#define A2MP_HCI_ID(id) ((id)-0x10) /* convert AMP ID to HCI dev index */
Missing spaces? Can this be in hci.h ?
Best regards
Andrei Emeltchenko
Hi Mat,
On Thu, Oct 13, 2011 at 03:00:45PM -0700, Mat Martineau wrote:
> The A2MP fixed channel bit is only set when high-speed mode is enabled.
It might make sense to merge previous patch with definitions of
L2CAP_FC_L2CAP and L2CAP_FC_A2MP, otherwise these 2 patches are a bit
small.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d023156..e38258b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -60,7 +60,7 @@ int disable_ertm;
> int enable_hs;
>
> static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
> -static u8 l2cap_fixed_chan[8] = { 0x02, };
> +static u8 l2cap_fixed_chan[8] = { L2CAP_FC_L2CAP, };
I personally do not like present approach with allocating 8-octet array
when we need only one octet _FOR_NOW_. (Also assigning is not clear enough).
Why not:
<------8<-----------------------------------------------
| @@ -60,7 +60,7 @@ int disable_ertm;
| int enable_hs;
|
| static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
| -static u8 l2cap_fixed_chan[8] = { 0x02, };
| +static u8 l2cap_fixed_chan = L2CAP_FC_L2CAP;
|
| static LIST_HEAD(chan_list);
| static DEFINE_RWLOCK(chan_list_lock);
|
<------8<-----------------------------------------------
>
> static LIST_HEAD(chan_list);
> static DEFINE_RWLOCK(chan_list_lock);
> @@ -2849,6 +2849,10 @@ static inline int l2cap_information_req(struct l2cap_conn *conn, struct l2cap_cm
> } else if (type == L2CAP_IT_FIXED_CHAN) {
> u8 buf[12];
> struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
> +
> + if (enable_hs)
> + l2cap_fixed_chan[0] |= L2CAP_FC_A2MP;
> +
> rsp->type = cpu_to_le16(L2CAP_IT_FIXED_CHAN);
> rsp->result = cpu_to_le16(L2CAP_IR_SUCCESS);
> memcpy(buf + 4, l2cap_fixed_chan, 8);
and then here:
<------8<--------------------------------------------------------------------------------------------------
| @@ -2895,9 +2895,16 @@ static inline int l2cap_information_req(struct l2cap_conn *conn, struct l2cap_cm
| } else if (type == L2CAP_IT_FIXED_CHAN) {
| u8 buf[12];
| struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
| +
| + memset(buf, 0, sizeof(buf));
| rsp->type = cpu_to_le16(L2CAP_IT_FIXED_CHAN);
| rsp->result = cpu_to_le16(L2CAP_IR_SUCCESS);
| - memcpy(buf + 4, l2cap_fixed_chan, 8);
| +
| + if (enable_hs)
| + l2cap_fixed_chan |= L2CAP_FC_A2MP;
| +
| + rsp->data[0] = l2cap_fixed_chan;
| +
| l2cap_send_cmd(conn, cmd->ident,
| L2CAP_INFO_RSP, sizeof(buf), buf);
| } else {
|
<------8<--------------------------------------------------------------------------------------------------
See my patch here:
http://www.spinics.net/lists/linux-bluetooth/msg17023.html
Best regards
Andrei Emeltchenko
Hi Mat,
On Thu, Oct 13, 2011 at 03:00:42PM -0700, Mat Martineau wrote:
> AMP channel creation and channel moves are coordinated using the L2CAP
> signaling channel. These definitions cover the "create channel",
> "move channel", and "move channel confirm" signals.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 60 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index e77d39f..2ea0d15 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -89,6 +89,12 @@ struct l2cap_conninfo {
> #define L2CAP_ECHO_RSP 0x09
> #define L2CAP_INFO_REQ 0x0a
> #define L2CAP_INFO_RSP 0x0b
> +#define L2CAP_CREATE_CHAN_REQ 0x0c
> +#define L2CAP_CREATE_CHAN_RSP 0x0d
> +#define L2CAP_MOVE_CHAN_REQ 0x0e
> +#define L2CAP_MOVE_CHAN_RSP 0x0f
> +#define L2CAP_MOVE_CHAN_CFM 0x10
more tabs
> +#define L2CAP_MOVE_CHAN_CFM_RSP 0x11
> #define L2CAP_CONN_PARAM_UPDATE_REQ 0x12
> #define L2CAP_CONN_PARAM_UPDATE_RSP 0x13
>
> @@ -296,6 +302,60 @@ struct l2cap_info_rsp {
> __u8 data[0];
> } __packed;
>
> +struct l2cap_create_chan_req {
> + __le16 psm;
> + __le16 scid;
> + __u8 amp_id;
> +} __attribute__ ((packed));
I prefer tabs instead of spaces here and below. I will send patch cleaning
remaining spaces in l2cap.h
> +
> +struct l2cap_create_chan_rsp {
> + __le16 dcid;
> + __le16 scid;
> + __le16 result;
> + __le16 status;
> +} __attribute__ ((packed));
> +
> +#define L2CAP_CREATE_CHAN_SUCCESS 0x0000
> +#define L2CAP_CREATE_CHAN_PENDING 0x0001
> +#define L2CAP_CREATE_CHAN_REFUSED_PSM 0x0002
> +#define L2CAP_CREATE_CHAN_REFUSED_SECURITY 0x0003
> +#define L2CAP_CREATE_CHAN_REFUSED_RESOURCES 0x0004
> +#define L2CAP_CREATE_CHAN_REFUSED_CONTROLLER 0x0005
> +
> +#define L2CAP_CREATE_CHAN_STATUS_NONE 0x0000
> +#define L2CAP_CREATE_CHAN_STATUS_AUTHENTICATION 0x0001
> +#define L2CAP_CREATE_CHAN_STATUS_AUTHORIZATION 0x0002
Isn't it a little bit log? why not CR/CS as it is already done?
BTW: I've done it different way. Since STATUS and RESULT are almost the same for
connect/create my patch is:
<------8<-----------------------------------------
| @@ -214,14 +216,15 @@ struct l2cap_conn_rsp {
| #define L2CAP_CID_DYN_START 0x0040
| #define L2CAP_CID_DYN_END 0xffff
|
| -/* connect result */
| +/* connect/create channel results */
| #define L2CAP_CR_SUCCESS 0x0000
| #define L2CAP_CR_PEND 0x0001
| #define L2CAP_CR_BAD_PSM 0x0002
| #define L2CAP_CR_SEC_BLOCK 0x0003
| #define L2CAP_CR_NO_MEM 0x0004
| +#define L2CAP_CR_BAD_AMP 0x0005
|
| -/* connect status */
| +/* connect/create channel status */
| #define L2CAP_CS_NO_INFO 0x0000
| #define L2CAP_CS_AUTHEN_PEND 0x0001
| #define L2CAP_CS_AUTHOR_PEND 0x0002
|
<------8<-----------------------------------------
> +
> +struct l2cap_move_chan_req {
> + __le16 icid;
> + __u8 dest_amp_id;
> +} __attribute__ ((packed));
> +
> +struct l2cap_move_chan_rsp {
> + __le16 icid;
> + __le16 result;
> +} __attribute__ ((packed));
> +
> +#define L2CAP_MOVE_CHAN_SUCCESS 0x0000
> +#define L2CAP_MOVE_CHAN_PENDING 0x0001
> +#define L2CAP_MOVE_CHAN_REFUSED_CONTROLLER 0x0002
> +#define L2CAP_MOVE_CHAN_REFUSED_SAME_ID 0x0003
> +#define L2CAP_MOVE_CHAN_REFUSED_CONFIG 0x0004
> +#define L2CAP_MOVE_CHAN_REFUSED_COLLISION 0x0005
> +#define L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED 0x0006
Same here: use CR and shorter names
Best regards
Andrei Emeltchenko
> +
> +struct l2cap_move_chan_cfm {
> + __le16 icid;
> + __le16 result;
> +} __attribute__ ((packed));
> +
> +#define L2CAP_MOVE_CHAN_CONFIRMED 0x0000
> +#define L2CAP_MOVE_CHAN_UNCONFIRMED 0x0001
> +
> +struct l2cap_move_chan_cfm_rsp {
> + __le16 icid;
> +} __attribute__ ((packed));
> +
> /* info type */
> #define L2CAP_IT_CL_MTU 0x0001
> #define L2CAP_IT_FEAT_MASK 0x0002
> --
> 1.7.7
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mat,
On Thu, Oct 13, 2011 at 03:00:40PM -0700, Mat Martineau wrote:
> Each channel has an active AMP policy to require BR/EDR (the default),
> prefer AMP, or prefer BR/EDR.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 806b950..e77d39f 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -357,6 +357,7 @@ struct l2cap_chan {
> __u8 num_conf_rsp;
>
> __u8 fcs;
> + __u8 amp_policy;
I would merge this patch with following one since this is only one line.
>
> __u16 tx_win;
> __u8 max_tx;
> --
> 1.7.7
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Hi Mat,
> Allow control of AMP functionality on L2CAP sockets. By default,
> connections will be restricted to BR/EDR. Manipulating the
> BT_AMP_POLICY option allows for channels to be moved to or created
> on AMP controllers.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index e727555..14ae418 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -77,6 +77,33 @@ struct bt_power {
> #define BT_POWER_FORCE_ACTIVE_OFF 0
> #define BT_POWER_FORCE_ACTIVE_ON 1
>
> +#define BT_AMP_POLICY 10
> +
> +/* Require BR/EDR (default policy)
> + * AMP controllers cannot be used.
> + * Channel move requests from the remote device are denied.
> + * If the L2CAP channel is currently using AMP, move the channel to BR/EDR.
> + */
> +#define BT_AMP_POLICY_REQUIRE_BR_EDR 0
so lets brainstorm a little bit with the names here. Do we really wanna
use the term AMP_POLICY? Is it really AMP specific? Or do we better make
ourselves a bit more future proof?
I am currently thinking a bit more generic. Maybe this is not the best
choice, but lets entertain this for a bit. So what about calling this
BT_CHANNEL_POLICY?
And instead of REQUIRE_BR_EDR I would call it BREDR_ONLY.
> +/* Prefer AMP
> + * Allow use of AMP controllers
> + * If the L2CAP channel is currently on BR/EDR and AMP controller
> + * resources are available, initiate a channel move to AMP.
> + * Channel move requests from the remote device are allowed.
> + * If the L2CAP socket has not been connected yet, try to create
> + * and configure the channel directly on an AMP controller rather
> + * than BR/EDR.
> + */
> +#define BT_AMP_POLICY_PREFER_AMP 1
> +
> +/* Prefer BR/EDR
> + * Allow use of AMP controllers.
> + * If the L2CAP channel is currently on AMP, move it to BR/EDR.
> + * Channel move requests from the remote device are allowed.
> + */
> +#define BT_AMP_POLICY_PREFER_BR_EDR 2
We might also wanna look into the option of having a SCM message in the
socket that tells us the current type. Or at least tells us when we
successfully switched to AMP or back to BR/EDR. Since obviously we are
not blocking on the setsockopt call.
Not sure if applications actually do care, but for OBEX it might be
interesting to know if you are on an AMP controller right now or not.
Regards
Marcel
Hi Mat,
> >> Checks for valid policy value and L2CAP mode.
> >>
> >> Signed-off-by: Mat Martineau <[email protected]>
> >> ---
> >> net/bluetooth/l2cap_sock.c | 25 +++++++++++++++++++++++++
> >> 1 files changed, 25 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> >> index 836d12e..9431e38 100644
> >> --- a/net/bluetooth/l2cap_sock.c
> >> +++ b/net/bluetooth/l2cap_sock.c
> >> @@ -467,6 +467,11 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
> >>
> >> break;
> >>
> >> + case BT_AMP_POLICY:
> >> + if (put_user(chan->amp_policy, (u32 __user *) optval))
> >> + err = -EFAULT;
> >
> > I prefer to not just add such an option just yet. We only want to add
> > socket option once they are functional. Otherwise we have problems to
> > detect if such an option is working or not. So enabling this option
> > should be the last patch after we have AMP implemented.
>
> My plan was to add checks for enable_hs before adding any code that
> takes action based on amp_policy. Would it be acceptable to add those
> enable_hs checks now so the code is in place, but disabled by default?
> It would be helpful for development and testing.
if it is protected enable_hs, then this is fine with me.
Regards
Marcel
On Thu, 13 Oct 2011, Marcel Holtmann wrote:
> Hi Mat,
>
>> Checks for valid policy value and L2CAP mode.
>>
>> Signed-off-by: Mat Martineau <[email protected]>
>> ---
>> net/bluetooth/l2cap_sock.c | 25 +++++++++++++++++++++++++
>> 1 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> index 836d12e..9431e38 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -467,6 +467,11 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
>>
>> break;
>>
>> + case BT_AMP_POLICY:
>> + if (put_user(chan->amp_policy, (u32 __user *) optval))
>> + err = -EFAULT;
>
> I prefer to not just add such an option just yet. We only want to add
> socket option once they are functional. Otherwise we have problems to
> detect if such an option is working or not. So enabling this option
> should be the last patch after we have AMP implemented.
Hi Marcel -
My plan was to add checks for enable_hs before adding any code that
takes action based on amp_policy. Would it be acceptable to add those
enable_hs checks now so the code is in place, but disabled by default?
It would be helpful for development and testing.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Mat,
> Checks for valid policy value and L2CAP mode.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> net/bluetooth/l2cap_sock.c | 25 +++++++++++++++++++++++++
> 1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 836d12e..9431e38 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -467,6 +467,11 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
>
> break;
>
> + case BT_AMP_POLICY:
> + if (put_user(chan->amp_policy, (u32 __user *) optval))
> + err = -EFAULT;
I prefer to not just add such an option just yet. We only want to add
socket option once they are functional. Otherwise we have problems to
detect if such an option is working or not. So enabling this option
should be the last patch after we have AMP implemented.
Regards
Marcel
Handle both "create channel request" and "create channel response".
Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/l2cap_core.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 439e715..d023156 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2918,6 +2918,41 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
return 0;
}
+static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ struct l2cap_create_chan_req *req =
+ (struct l2cap_create_chan_req *) data;
+ struct l2cap_create_chan_rsp rsp;
+ u16 psm, scid;
+
+ psm = le16_to_cpu(req->psm);
+ scid = le16_to_cpu(req->scid);
+
+ BT_DBG("psm %d, scid %d, amp_id %d", (int) psm, (int) scid,
+ (int) req->amp_id);
+
+ /* Placeholder: Always reject */
+
+ rsp.dcid = 0;
+ rsp.scid = cpu_to_le16(scid);
+ rsp.result = L2CAP_CREATE_CHAN_REFUSED_CONTROLLER;
+ rsp.status = L2CAP_CREATE_CHAN_STATUS_NONE;
+
+ l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
+ sizeof(rsp), &rsp);
+
+ return 0;
+}
+
+static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ BT_DBG("conn %p", conn);
+
+ return l2cap_connect_rsp(conn, cmd, data);
+}
+
static inline int l2cap_check_conn_param(u16 min, u16 max, u16 latency,
u16 to_multiplier)
{
@@ -3030,6 +3065,14 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
err = l2cap_information_rsp(conn, cmd, data);
break;
+ case L2CAP_CREATE_CHAN_REQ:
+ err = l2cap_create_channel_req(conn, cmd, data);
+ break;
+
+ case L2CAP_CREATE_CHAN_RSP:
+ err = l2cap_create_channel_rsp(conn, cmd, data);
+ break;
+
default:
BT_ERR("Unknown BR/EDR signaling command 0x%2.2x", cmd->code);
err = -EINVAL;
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Each channel has an active AMP policy to require BR/EDR (the default),
prefer AMP, or prefer BR/EDR.
Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/l2cap.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 806b950..e77d39f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -357,6 +357,7 @@ struct l2cap_chan {
__u8 num_conf_rsp;
__u8 fcs;
+ __u8 amp_policy;
__u16 tx_win;
__u8 max_tx;
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Checks for valid policy value and L2CAP mode.
Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/l2cap_sock.c | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 836d12e..9431e38 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -467,6 +467,11 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
break;
+ case BT_AMP_POLICY:
+ if (put_user(chan->amp_policy, (u32 __user *) optval))
+ err = -EFAULT;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -690,6 +695,26 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
clear_bit(FLAG_FORCE_ACTIVE, &chan->flags);
break;
+ case BT_AMP_POLICY:
+ if (get_user(opt, (u32 __user *) optval)) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opt > BT_AMP_POLICY_PREFER_BR_EDR) {
+ err = -EINVAL;
+ break;
+ }
+
+ if (chan->mode != L2CAP_MODE_ERTM &&
+ chan->mode != L2CAP_MODE_STREAMING) {
+ err = -EINVAL;
+ break;
+ }
+
+ chan->amp_policy = (u8) opt;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
The A2MP fixed channel bit is only set when high-speed mode is enabled.
Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/l2cap_core.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d023156..e38258b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -60,7 +60,7 @@ int disable_ertm;
int enable_hs;
static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
-static u8 l2cap_fixed_chan[8] = { 0x02, };
+static u8 l2cap_fixed_chan[8] = { L2CAP_FC_L2CAP, };
static LIST_HEAD(chan_list);
static DEFINE_RWLOCK(chan_list_lock);
@@ -2849,6 +2849,10 @@ static inline int l2cap_information_req(struct l2cap_conn *conn, struct l2cap_cm
} else if (type == L2CAP_IT_FIXED_CHAN) {
u8 buf[12];
struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
+
+ if (enable_hs)
+ l2cap_fixed_chan[0] |= L2CAP_FC_A2MP;
+
rsp->type = cpu_to_le16(L2CAP_IT_FIXED_CHAN);
rsp->result = cpu_to_le16(L2CAP_IR_SUCCESS);
memcpy(buf + 4, l2cap_fixed_chan, 8);
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
AMP channels can be moved between BR/EDR and AMP controllers using a
sequence of signals. Every attempted channel move involves a series of
four signals:
Move Initiator Move Responder
| |
| Move Channel Request |
| ----------------------------> |
| |
| Move Channel Response |
| <---------------------------- |
| |
| Move Channel Confirm |
| ----------------------------> |
| |
| Move Channel Confirm Response |
| <---------------------------- |
All four signals are sent even if the move fails.
Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/l2cap_core.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 123 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e38258b..ba64bab 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2957,6 +2957,112 @@ static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
return l2cap_connect_rsp(conn, cmd, data);
}
+static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
+ u16 icid, u16 result)
+{
+ struct l2cap_move_chan_rsp rsp;
+
+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
+
+ rsp.icid = cpu_to_le16(icid);
+ rsp.result = cpu_to_le16(result);
+
+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
+}
+
+static void l2cap_send_move_chan_cfm(struct l2cap_conn *conn,
+ struct l2cap_chan *chan, u16 icid, u16 result)
+{
+ struct l2cap_move_chan_cfm cfm;
+ u8 ident;
+
+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
+
+ ident = l2cap_get_ident(conn);
+ if (chan)
+ chan->ident = ident;
+
+ cfm.icid = cpu_to_le16(icid);
+ cfm.result = cpu_to_le16(result);
+
+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM, sizeof(cfm), &cfm);
+}
+
+static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
+ u16 icid)
+{
+ struct l2cap_move_chan_cfm_rsp rsp;
+
+ BT_DBG("icid %d", (int) icid);
+
+ rsp.icid = cpu_to_le16(icid);
+ l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
+}
+
+static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ struct l2cap_move_chan_req *req = (struct l2cap_move_chan_req *) data;
+ u16 icid = 0;
+ u16 result = L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED;
+
+ icid = le16_to_cpu(req->icid);
+
+ BT_DBG("icid %d, dest_amp_id %d", (int) icid, (int) req->dest_amp_id);
+
+ /* Placeholder: Always refuse */
+ l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
+
+ return 0;
+}
+
+static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ struct l2cap_move_chan_rsp *rsp = (struct l2cap_move_chan_rsp *) data;
+ u16 icid, result;
+
+ icid = le16_to_cpu(rsp->icid);
+ result = le16_to_cpu(rsp->result);
+
+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
+
+ /* Placeholder: Always unconfirmed */
+ l2cap_send_move_chan_cfm(conn, NULL, icid, L2CAP_MOVE_CHAN_UNCONFIRMED);
+
+ return 0;
+}
+
+static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ struct l2cap_move_chan_cfm *cfm = (struct l2cap_move_chan_cfm *) data;
+ u16 icid, result;
+
+ icid = le16_to_cpu(cfm->icid);
+ result = le16_to_cpu(cfm->result);
+
+ BT_DBG("icid %d, result %d", (int) icid, (int) result);
+
+ l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
+
+ return 0;
+}
+
+static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ struct l2cap_move_chan_cfm_rsp *rsp =
+ (struct l2cap_move_chan_cfm_rsp *) data;
+ u16 icid;
+
+ icid = le16_to_cpu(rsp->icid);
+
+ BT_DBG("icid %d", (int) icid);
+
+ return 0;
+}
+
static inline int l2cap_check_conn_param(u16 min, u16 max, u16 latency,
u16 to_multiplier)
{
@@ -3077,6 +3183,23 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
err = l2cap_create_channel_rsp(conn, cmd, data);
break;
+ case L2CAP_MOVE_CHAN_REQ:
+ err = l2cap_move_channel_req(conn, cmd, data);
+ break;
+
+ case L2CAP_MOVE_CHAN_RSP:
+ err = l2cap_move_channel_rsp(conn, cmd, data);
+ break;
+
+ case L2CAP_MOVE_CHAN_CFM:
+ err = l2cap_move_channel_confirm(conn, cmd, data);
+ break;
+
+ case L2CAP_MOVE_CHAN_CFM_RSP:
+ err = l2cap_move_channel_confirm_rsp(conn, cmd, data);
+ break;
+
+
default:
BT_ERR("Unknown BR/EDR signaling command 0x%2.2x", cmd->code);
err = -EINVAL;
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Symbolic fixed channel IDs will be used instead of magic numbers.
Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/l2cap.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 2ea0d15..e5d860e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -114,6 +114,10 @@ struct l2cap_conninfo {
#define L2CAP_FCS_NONE 0x00
#define L2CAP_FCS_CRC16 0x01
+/* L2CAP fixed channels */
+#define L2CAP_FC_L2CAP 0x02
+#define L2CAP_FC_A2MP 0x08
+
/* L2CAP Control Field bit masks */
#define L2CAP_CTRL_SAR 0xC000
#define L2CAP_CTRL_REQSEQ 0x3F00
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
This file will be home to all AMP- and A2MP-related declarations. The
first macros map between HCI device indexes (for AMP controllers) and
AMP controller ids that are passed over the air.
Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/amp.h | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
create mode 100644 include/net/bluetooth/amp.h
diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
new file mode 100644
index 0000000..b84ce53
--- /dev/null
+++ b/include/net/bluetooth/amp.h
@@ -0,0 +1,20 @@
+/*
+ Copyright (c) 2010-2011 Code Aurora Forum. All rights reserved.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License version 2 and
+ only version 2 as published by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+*/
+
+#ifndef __AMP_H
+#define __AMP_H
+
+#define HCI_A2MP_ID(id) ((id)+0x10) /* convert HCI dev index to AMP ID */
+#define A2MP_HCI_ID(id) ((id)-0x10) /* convert AMP ID to HCI dev index */
+
+#endif /* __AMP_H */
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
AMP channel creation and channel moves are coordinated using the L2CAP
signaling channel. These definitions cover the "create channel",
"move channel", and "move channel confirm" signals.
Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/l2cap.h | 60 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e77d39f..2ea0d15 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -89,6 +89,12 @@ struct l2cap_conninfo {
#define L2CAP_ECHO_RSP 0x09
#define L2CAP_INFO_REQ 0x0a
#define L2CAP_INFO_RSP 0x0b
+#define L2CAP_CREATE_CHAN_REQ 0x0c
+#define L2CAP_CREATE_CHAN_RSP 0x0d
+#define L2CAP_MOVE_CHAN_REQ 0x0e
+#define L2CAP_MOVE_CHAN_RSP 0x0f
+#define L2CAP_MOVE_CHAN_CFM 0x10
+#define L2CAP_MOVE_CHAN_CFM_RSP 0x11
#define L2CAP_CONN_PARAM_UPDATE_REQ 0x12
#define L2CAP_CONN_PARAM_UPDATE_RSP 0x13
@@ -296,6 +302,60 @@ struct l2cap_info_rsp {
__u8 data[0];
} __packed;
+struct l2cap_create_chan_req {
+ __le16 psm;
+ __le16 scid;
+ __u8 amp_id;
+} __attribute__ ((packed));
+
+struct l2cap_create_chan_rsp {
+ __le16 dcid;
+ __le16 scid;
+ __le16 result;
+ __le16 status;
+} __attribute__ ((packed));
+
+#define L2CAP_CREATE_CHAN_SUCCESS 0x0000
+#define L2CAP_CREATE_CHAN_PENDING 0x0001
+#define L2CAP_CREATE_CHAN_REFUSED_PSM 0x0002
+#define L2CAP_CREATE_CHAN_REFUSED_SECURITY 0x0003
+#define L2CAP_CREATE_CHAN_REFUSED_RESOURCES 0x0004
+#define L2CAP_CREATE_CHAN_REFUSED_CONTROLLER 0x0005
+
+#define L2CAP_CREATE_CHAN_STATUS_NONE 0x0000
+#define L2CAP_CREATE_CHAN_STATUS_AUTHENTICATION 0x0001
+#define L2CAP_CREATE_CHAN_STATUS_AUTHORIZATION 0x0002
+
+struct l2cap_move_chan_req {
+ __le16 icid;
+ __u8 dest_amp_id;
+} __attribute__ ((packed));
+
+struct l2cap_move_chan_rsp {
+ __le16 icid;
+ __le16 result;
+} __attribute__ ((packed));
+
+#define L2CAP_MOVE_CHAN_SUCCESS 0x0000
+#define L2CAP_MOVE_CHAN_PENDING 0x0001
+#define L2CAP_MOVE_CHAN_REFUSED_CONTROLLER 0x0002
+#define L2CAP_MOVE_CHAN_REFUSED_SAME_ID 0x0003
+#define L2CAP_MOVE_CHAN_REFUSED_CONFIG 0x0004
+#define L2CAP_MOVE_CHAN_REFUSED_COLLISION 0x0005
+#define L2CAP_MOVE_CHAN_REFUSED_NOT_ALLOWED 0x0006
+
+struct l2cap_move_chan_cfm {
+ __le16 icid;
+ __le16 result;
+} __attribute__ ((packed));
+
+#define L2CAP_MOVE_CHAN_CONFIRMED 0x0000
+#define L2CAP_MOVE_CHAN_UNCONFIRMED 0x0001
+
+struct l2cap_move_chan_cfm_rsp {
+ __le16 icid;
+} __attribute__ ((packed));
+
/* info type */
#define L2CAP_IT_CL_MTU 0x0001
#define L2CAP_IT_FEAT_MASK 0x0002
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Allow control of AMP functionality on L2CAP sockets. By default,
connections will be restricted to BR/EDR. Manipulating the
BT_AMP_POLICY option allows for channels to be moved to or created
on AMP controllers.
Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index e727555..14ae418 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -77,6 +77,33 @@ struct bt_power {
#define BT_POWER_FORCE_ACTIVE_OFF 0
#define BT_POWER_FORCE_ACTIVE_ON 1
+#define BT_AMP_POLICY 10
+
+/* Require BR/EDR (default policy)
+ * AMP controllers cannot be used.
+ * Channel move requests from the remote device are denied.
+ * If the L2CAP channel is currently using AMP, move the channel to BR/EDR.
+ */
+#define BT_AMP_POLICY_REQUIRE_BR_EDR 0
+
+/* Prefer AMP
+ * Allow use of AMP controllers
+ * If the L2CAP channel is currently on BR/EDR and AMP controller
+ * resources are available, initiate a channel move to AMP.
+ * Channel move requests from the remote device are allowed.
+ * If the L2CAP socket has not been connected yet, try to create
+ * and configure the channel directly on an AMP controller rather
+ * than BR/EDR.
+ */
+#define BT_AMP_POLICY_PREFER_AMP 1
+
+/* Prefer BR/EDR
+ * Allow use of AMP controllers.
+ * If the L2CAP channel is currently on AMP, move it to BR/EDR.
+ * Channel move requests from the remote device are allowed.
+ */
+#define BT_AMP_POLICY_PREFER_BR_EDR 2
+
__attribute__((format (printf, 2, 3)))
int bt_printk(const char *level, const char *fmt, ...);
--
1.7.7
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum