2010-08-25 22:36:05

by haijun liu

[permalink] [raw]
Subject: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt

>From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
From: haijun.liu <[email protected]>
Date: Mon, 23 Aug 2010 00:00:26 +0800
Subject: [PATCH 13/22] Add three new options for l2cap_options which
used in setsockopt & getsockopt.

---
include/net/bluetooth/l2cap.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d0ae9f5..4f87aec 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -65,6 +65,9 @@ struct l2cap_options {
__u8 fcs;
__u8 max_tx;
__u16 txwin_size;
+ __u8 hschan_req;
+ __u8 guaranteed;
+ __u8 reconfig;
};

#define L2CAP_CONNINFO 0x02
--
1.6.3.3


2010-08-26 10:41:50

by David Vrabel

[permalink] [raw]
Subject: Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt

haijun liu wrote:
> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <[email protected]>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;

This isn't the API that was agreed. See Mat Martineau's "Bluetooth:
Add socket option definitions for AMP" patch.

> + __u8 guaranteed;

Selecting guaranteed without being able to specify flow specs doesn't
seem useful.

I would like to see support for best-effort links merged first before
guaranteed links are considered.

> + __u8 reconfig;

I'd suggest removing this unless there's a profile/application that will
make use of it. I'd have thought that channel reconfiguration was
something the L2CAP stack does automatically when moving channels
between BR/EDR and AMP radios.

> };

David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

2010-08-26 01:10:39

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt

Hi Haijun,

* haijun liu <[email protected]> [2010-08-26 06:36:05 +0800]:

> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <[email protected]>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> };

Here you are extending the API with the userspace, so we need a patch
for each option explaining why you need it and why the userspace should
care about that. It's a good idea here add this option to l2cap_options
in the same patch you implement the handling for it apply such patch hen
it is actually needed. ;)


--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-03 01:19:52

by haijun liu

[permalink] [raw]
Subject: Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt

On Fri, Sep 3, 2010 at 5:41 AM, Mat Martineau <[email protected]> wrote:
>
> On Thu, 26 Aug 2010, haijun liu wrote:
>
>> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
>> From: haijun.liu <[email protected]>
>> Date: Mon, 23 Aug 2010 00:00:26 +0800
>> Subject: [PATCH 13/22] Add three new options for l2cap_options which
>> used in setsockopt & getsockopt.
>>
>> ---
>> include/net/bluetooth/l2cap.h | ? ?3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index d0ae9f5..4f87aec 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -65,6 +65,9 @@ struct l2cap_options {
>> ? ? ? ?__u8 ?fcs;
>> ? ? ? ?__u8 ?max_tx;
>> ? ? ? ?__u16 txwin_size;
>> + ? ? ? __u8 ?hschan_req;
>> + ? ? ? __u8 ?guaranteed;
>> + ? ? ? __u8 ?reconfig;
>> };
>>
>> #define L2CAP_CONNINFO ?0x02
>
> How are each of these new options used?
>
> Are any of the new options changeable after the socket is connected?
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
>

I accept David suggestion, I am waiting your patch merged.

Yes, these are changeable.

--
Haijun Liu

2010-09-02 21:41:26

by Mat Martineau

[permalink] [raw]
Subject: Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt


On Thu, 26 Aug 2010, haijun liu wrote:

> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <[email protected]>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> };
>
> #define L2CAP_CONNINFO 0x02

How are each of these new options used?

Are any of the new options changeable after the socket is connected?

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum