linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Santiago Carot-Nemesio <sancane@gmail.com>
To: "Elvis Pfützenreuter" <epx@epx.com.br>
Cc: johan.hedberg@gmail.com, linux-bluetooth@vger.kernel.org
Subject: Re: CSP implementation for MCAP
Date: Fri, 03 Sep 2010 10:03:52 +0200	[thread overview]
Message-ID: <4C80ABE8.9080309@gmail.com> (raw)
In-Reply-To: <4A11989E-5448-43B7-887F-217C54977E22@epx.com.br>

Hi Elvis,

On 09/02/10 20:58, Elvis Pfützenreuter wrote:
> This is the repository for the CSP implementation, rebased over the recently accepted MCAP, for your appreciation:
>
> git://gitorious.org/bluez-epx/bluez-epx.git csp
>

I have been divin in CSP code and at first look I found some issues that 
I would like discuss with you.

The call to function mcap_sync_stop(mcl) it's only neccesary in static 
close_mcl function. You can remove the second call in line 799 when user 
closes explicitly the mcl due that first sync_stop will be called from 
watcher set in control channel when its socket is closed.

Second thing is related to code structure. Because standard op codes and 
close synchronization protocol have separate logic, it may better put 
csp parameters away from mcl in a separate structure to avoid mess all 
protocol logic in a big mcl structure, I was thinking in something like 
this:

struct mcap_mcl {
	/* Op code parameters */
	....
	struct	mcap_csp	*csp;
}

struct mcap_csp {
	uint64_t	base_tmstamp;
	struct timespec	base_time;
	guint		local_caps;
	guint		remote_caps;
	guint		rem_req_acc;
	guint		ind_expected;
	MCAPCtrl	csp_req;
	guint		ind_timer;
	guint		set_timer;
	void		*set_data;
	gint		dev_id;
	gint		dev_hci_fd;
	void		*csp_priv_data;
};

Because CSP is optional we can reserve memory for csp only when we will 
use Clock Synchronization protocol.
Of course, I know that it depends on invidivual taste, but I would like 
to comment this issue.

Comments are welcome.

Regards.



  parent reply	other threads:[~2010-09-03  8:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-02 18:58 CSP implementation for MCAP Elvis Pfützenreuter
2010-09-02 20:28 ` Johan Hedberg
2010-09-02 20:36   ` Elvis Pfützenreuter
2010-09-02 20:40     ` Johan Hedberg
2010-09-02 21:49       ` Elvis Pfützenreuter
2010-09-03  8:03 ` Santiago Carot-Nemesio [this message]
2010-09-03 12:53   ` Elvis Pfützenreuter
2010-09-15 22:45 ` Elvis Pfützenreuter
2010-09-16 12:39   ` Johan Hedberg
2010-09-16 12:55     ` José Antonio Santos Cadenas
2010-09-16 14:14     ` Elvis Pfützenreuter
2010-09-16 15:03       ` Johan Hedberg

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=4C80ABE8.9080309@gmail.com \
    --to=sancane@gmail.com \
    --cc=epx@epx.com.br \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.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).