Re: [PATCH 7/9] Bluetooth: Use symbolic values for the fixed channel map

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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 <mathewm@xxxxxxxxxxxxxx>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux