linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gustavo Padovan <padovan@profusion.mobi>
To: Mat Martineau <mathewm@codeaurora.org>
Cc: Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com>,
	linux-bluetooth@vger.kernel.org, pkrystad@codeaurora.org
Subject: Re: [PATCH 7/9] Bluetooth: Use symbolic values for the fixed channel map
Date: Fri, 14 Oct 2011 20:51:35 -0300	[thread overview]
Message-ID: <20111014235135.GF30989@joana> (raw)
In-Reply-To: <alpine.DEB.2.02.1110141559060.9993@mathewm-linux>

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [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 <mathewm@codeaurora.org>
> >>---
> >> 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

  reply	other threads:[~2011-10-14 23:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-13 22:00 [PATCH 0/9] AMP interface and signal framework Mat Martineau
2011-10-13 22:00 ` [PATCH 1/9] Bluetooth: Add BT_AMP_POLICY socket option Mat Martineau
2011-10-13 23:38   ` Marcel Holtmann
2011-10-14 22:38     ` Mat Martineau
2011-10-15 18:43       ` Marcel Holtmann
2011-10-17 10:25       ` Luiz Augusto von Dentz
2011-10-17 15:56         ` Marcel Holtmann
2011-10-13 22:00 ` [PATCH 2/9] Bluetooth: Add AMP policy information to l2cap_chan Mat Martineau
2011-10-14 11:20   ` Andrei Emeltchenko
2011-10-14 22:39     ` Mat Martineau
2011-10-13 22:00 ` [PATCH 3/9] Bluetooth: Get/set AMP policy socket option Mat Martineau
2011-10-13 22:35   ` Marcel Holtmann
2011-10-13 23:05     ` Mat Martineau
2011-10-13 23:27       ` Marcel Holtmann
2011-10-13 22:00 ` [PATCH 4/9] Bluetooth: Add AMP-related data and structures for channel signals Mat Martineau
2011-10-14 12:19   ` Andrei Emeltchenko
2011-10-14 22:58     ` Mat Martineau
2011-10-17  7:23       ` Andrei Emeltchenko
2011-10-13 22:00 ` [PATCH 5/9] Bluetooth: Add signal handlers for channel creation Mat Martineau
2011-10-13 22:00 ` [PATCH 6/9] Bluetooth: Add definitions for L2CAP fixed channels Mat Martineau
2011-10-13 22:00 ` [PATCH 7/9] Bluetooth: Use symbolic values for the fixed channel map Mat Martineau
2011-10-14 12:36   ` Andrei Emeltchenko
2011-10-14 23:04     ` Mat Martineau
2011-10-14 23:51       ` Gustavo Padovan [this message]
2011-10-13 22:00 ` [PATCH 8/9] Bluetooth: Add AMP header file Mat Martineau
2011-10-14 12:38   ` Andrei Emeltchenko
2011-10-14 23:07     ` Mat Martineau
2011-10-13 22:00 ` [PATCH 9/9] Bluetooth: Add signal handlers for channel moves Mat Martineau
2011-10-14 18:46   ` Gustavo Padovan
2011-10-14 23:19     ` Mat Martineau
2011-10-17  7:31       ` Andrei Emeltchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111014235135.GF30989@joana \
    --to=padovan@profusion.mobi \
    --cc=andrei.emeltchenko.news@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=mathewm@codeaurora.org \
    --cc=pkrystad@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).