Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1363716255-21332-1-git-send-email-frederic.dalleau@linux.intel.com> <1363716255-21332-3-git-send-email-frederic.dalleau@linux.intel.com> Date: Mon, 8 Apr 2013 10:55:08 +0200 Message-ID: Subject: Re: [PATCH v4 2/6] Bluetooth: Add SCO socket voice_setting option From: "Dalleau, Frederic" To: Marcel Holtmann Cc: =?ISO-8859-1?Q?Fr=E9d=E9ric_Dalleau?= , linux-bluetooth Content-Type: multipart/alternative; boundary=089e01633a8cfeb28a04d9d597b8 List-ID: --089e01633a8cfeb28a04d9d597b8 Content-Type: text/plain; charset=ISO-8859-1 Hi Marcel, On Tue, Mar 26, 2013 at 9:22 PM, Marcel Holtmann wrote: > I find this parameter name a bit long. What about just "setting" or > "settings" or "voice". I am open for suggestions. > > Also should this be part of options or the socket address structure. > > Another option would be to introduce a SCO_SETTINGS or SCO_VOICE socket > option. With just this one parameter. > > Since the default value of voice setting is not 0x0000, it might make > actually more sense to introduce a new socket option. Playing the memset > handling would only work nicely if the default would be 0x0000, but it > isn't. > I agree that having a separate socket option is better. IMHO, having it in the sockaddr is not enough because you don't know in advance what stream is flowing for incoming connections. It could improve the API for outgoing connections. My concern is how do we handle user values. If the user do not set this option, hci_conn->settings contains 0x0000. In this case, it is possible to start the sco connection as we have it today. If the user set 0x0060 (cvsd 16 bits) or 0x0003 (transparent data), then it is possible to start S3, S2, S1 or T2, T1 sequences. For other values, IMHO, returning an error would be the way to go. If the above is fine for you, we can discuss the naming. What about the following ? #define SCO_SETTINGS 0x03 struct sco_settings { __u16 settings; }; Regards, Fred --089e01633a8cfeb28a04d9d597b8 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Hi Marcel,

On Tue, Mar 26, 2013 at 9:22 PM, Marcel Holtmann <mar= cel@holtmann.org> wrote:
I find this parameter nam= e a bit long. What about just "setting" or "settings" o= r "voice". I am open for suggestions.

Also should this be part of options or the socket address structure.

Another option would be to introduce a SCO_SETTINGS or SCO_VOICE socket opt= ion. With just this one parameter.

Since the default value of voice setting is not 0x0000, it might make actua= lly more sense to introduce a new socket option. Playing the memset handlin= g would only work nicely if the default would be 0x0000, but it isn't.<= br>

I agree that having a separate socket option is= better. IMHO, having it in the
sockaddr is not enough because you don&#= 39;t know in advance what stream is
flowing for incoming connections. It= could improve the API for outgoing
connections.

My concern is how do we handle user values.
If the u= ser do not set this option, hci_conn->settings contains 0x0000.
In th= is case, it is possible to start the sco connection as we have it today. If the user set 0x0060 (cvsd 16 bits) or 0x0003 (transparent data), then it= is
possible to start S3, S2, S1 or T2, T1 sequences.

For other v= alues, IMHO, returning an error would be the way to go.

If the above= is fine for you, we can discuss the naming. What about the
following ?

#define SCO_SETTINGS=A0 0x03
struct sco_settings {=A0=A0=A0=A0=A0=A0=A0 __u16=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 settings;
}; =

Regards,
Fred

--089e01633a8cfeb28a04d9d597b8--