Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCHv2 1/5] TODO: remove 'fix MTU exchange' task
From: Johan Hedberg @ 2011-03-18  9:24 UTC (permalink / raw)
  To: Bruna Moreira; +Cc: linux-bluetooth
In-Reply-To: <1300373720-14772-1-git-send-email-bruna.moreira@openbossa.org>

Hi Bruna,

On Thu, Mar 17, 2011, Bruna Moreira wrote:
> ---
>  TODO |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)

All five patches have now been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Add counter for not acked HCI commands
From: Andrzej Kaczmarek @ 2011-03-18  9:22 UTC (permalink / raw)
  To: Brian Gix, linux-bluetooth@vger.kernel.org,
	par-gunnar.p.hjalmdahl@stericsson.com,
	henrik.possung@stericsson.com
In-Reply-To: <20110316232739.GD2339@joana>

Hi Gustavo,

On 17.03.2011 00:27, Gustavo F. Padovan wrote:
>>> That's exactly how linux stack work, and I'm seeing where we could be doing
>>> wrong, so I believe your patch is fixing nothing. A best version of your
>>> patch is:
>>>
>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>> index cebe7588..2d4d441 100644
>>> --- a/net/bluetooth/hci_event.c
>>> +++ b/net/bluetooth/hci_event.c
>>> @@ -1771,7 +1771,7 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
>>>           if (ev->opcode != HCI_OP_NOP)
>>>                   del_timer(&hdev->cmd_timer);
>>>
>>> -       if (ev->ncmd) {
>>> +       if (ev->ncmd&&   ev->opcode != HCI_OP_NOP) {
>>>                   atomic_set(&hdev->cmd_cnt, 1);
>>>                   if (!skb_queue_empty(&hdev->cmd_q))
>>>                           tasklet_schedule(&hdev->cmd_task);
>>
>> I don't think that this is correct.
>
> Yes, this is wrong. It's just a simplified version of Andrzej's patch.

No, my patch does not break other scenarios, i.e. the one mentioned by 
Brian ;-)

I agree with him that we can make proper decision in every case only if 
we have both credit and unacked count separately due to asynchronous 
nature of HCI acks. And it's a good point that we do not have to count 
unacked commands but instead just store information whether last sent 
cmd is not yet acked.

I was going to send v2 patch with suggestions from Brian included but I 
didn't get answer whether cmd_cnt has to be atomic_t for some reason 
other than historical. Both acl_cnt and sco_cnt are simple integers, for 
example.

>> Second: How is cmd_cnt being used?  I see it being decremented when a
>> command is sent in hci_cmd_task, or being set to "1" when one of the two
>> events are received. It is also set to "1" for time-outs and channel
>> open/close/reset activities.
>>
>> It looks to me like the cmd_cnt is being misused.  It should in fact be
>> set to the value indicated by the event, and not just "1".  It also does
>> not address the original problem observed by Andrzej, which is that the
>> Asyncronous link between the Host and Baseband can cause them to be
>> momentarily out of sync with each other.
>
> We limit cmd_cnt to 1 in our stack, to be sure that one command a time will be
> sent, I don't for the reason for that but Marcel certainly has one.

Pretty good reason could be that last command sent is stored in hci_dev 
and then used in cc/cs events. I don't have problem with this kind of 
solution, I just think it's not enough here.

I also don't see point of making it a counter while it behaves like a 
weird flag: set to 1, but decrease instead of set to 0. A bit 
inconsistent in my opinion.

BR,
Andrzej

^ permalink raw reply

* [PATCH] Fix adapter drivers unload order
From: Dmitriy Paliy @ 2011-03-18  8:25 UTC (permalink / raw)
  To: linux-bluetooth, szymon.janc; +Cc: Dmitriy Paliy

Order of unloading drivers shell be preserved as given in
audio_manager_exit. It was reverted in 'c772636 Fix unloading of adapter
drivers', which causes in turn bluetoothd crash on exit.

Removing of media end-point results in removing of A2DP end-point for
the corresponding A2DP server but not vice versa. Therefore, unregistering
and destroying A2DP server, which clears all A2DP end-points, before media
server is incorrect.
---
 src/adapter.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 8c368fe..9fa7c4d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2258,8 +2258,8 @@ static void probe_driver(struct btd_adapter *adapter, gpointer user_data)
 		return;
 	}
 
-	adapter->loaded_drivers = g_slist_prepend(adapter->loaded_drivers,
-									driver);
+	adapter->loaded_drivers = g_slist_append(adapter->loaded_drivers,
+								driver);
 }
 
 static void load_drivers(struct btd_adapter *adapter)
-- 
1.7.1


^ permalink raw reply related

* SMP data within struct l2cap_conn  -vs-  single threading SMP
From: Brian Gix @ 2011-03-17 22:09 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: Claudio Takahasi, BlueZ development


Hi Vinicius,

As you probably know, I am working on adding mgmt.c plumbing into SMP, 
to enable user level input (Confirmation, passkeys, perhaps OOB).

One issue I am running into is matching up the return of user 
confirmation with the (struct l2cap_conn *).  There is nothing within 
the user confirmation aside from the bdaddr that identifies who it is 
intended for, and there is no one-to-one relationship between bdaddrs 
and L2CAP channels.

What would you think about enforcing a "one at a time" SMP process?

The SMP pairing data within the l2cap_conn structure is certainly a 
handy place for it, however it is bulky for the times (most of the time) 
where SMP is *not* taking place, and as in the obvious case I mention 
above, there is not a handy way to track the L2CAP connection back to 
the user input.

I would like to suggest that all of the SMP data be pulled out of the 
l2cap_conn structure, and put into a private structure within smp.c. It 
can be malloc'd when the pairing process starts, free'd when it 
completes, and any traffic (from either the User or the Baseband) that 
takes place when another device is in the midst of pairing gets rejected.

This structure local to smp.c would store both the bdaddr (to match up 
with user input) and the l2cap_conn * to match up with BB traffic, and 
provide the outbound path for the user confirmation which would 
otherwise be difficult to track down.

Your Thoughts?

-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* [PATCH] TODO: set owner of 'Whitelist support' task
From: Andre Guedes @ 2011-03-17 21:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

---
 TODO |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index f5532c2..608096c 100644
--- a/TODO
+++ b/TODO
@@ -245,3 +245,4 @@ Management Interface
 
   Priority: Medium
   Complexity: C2
+  Owner: Andre Guedes <andre.guedes@openbossa.org>
-- 
1.7.1


^ permalink raw reply related

* Re: Switching between SBC and MPEG audio on headsets
From: Peter Dons Tychsen @ 2011-03-17 21:33 UTC (permalink / raw)
  To: Brian Gix
  Cc: Luiz Augusto von Dentz, Arun Raghavan, linux-bluetooth,
	Johan Hedberg
In-Reply-To: <4D823499.3060208@codeaurora.org>

Hi Brian,

On Thu, 2011-03-17 at 09:19 -0700, Brian Gix wrote:
> AVDTP only allows a single signaling channel to exist between any two 
> peers. You are allowed to have separate signaling channels to
> separate 
> remote devices, but even if you are supporting multiple media streams 
> (video/audio, or audio/audio) to the same device, all AVDTP signaling 
> must be done on a single channel. 

Yes, exactly. But i am not sure everyone caught that part back when A2DP
was introduced. :-)

> Actually, I thought I was agreeing with you about *not* having two 
> media 
> channels open simultaneously.

Yes, you were. I just wanted to add an extra warning if someone still
wanted to go that way :-). I agree that compatibility is very important.

Thanks,

/pedro







^ permalink raw reply

* Re: [PATCH 4/4] Bluetooth: mgmt: Add support for setting the local name
From: Gustavo F. Padovan @ 2011-03-17 18:38 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1300278577-23068-4-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-03-16 14:29:37 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> This patch adds a new set_local_name management command as well as a
> local_name_changed management event. With these user space can both
> change the local name as well as monitor changes to it by others.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  include/net/bluetooth/mgmt.h     |   10 +++++
>  net/bluetooth/hci_event.c        |    9 +++--
>  net/bluetooth/mgmt.c             |   75 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 92 insertions(+), 3 deletions(-)

Patch 3 and 4 applied. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/4] Bluetooth: Add define for the maximum name length on HCI level
From: Gustavo F. Padovan @ 2011-03-17 18:30 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1300278577-23068-2-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-03-16 14:29:35 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> This patch adds a clear define for the maximum device name length in HCI
> messages and thereby avoids magic numbers in the code.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    8 +++++---
>  include/net/bluetooth/hci_core.h |    2 +-
>  net/bluetooth/hci_event.c        |    4 ++--
>  net/bluetooth/hci_sysfs.c        |    6 +++---
>  4 files changed, 11 insertions(+), 9 deletions(-)

This one is also applied. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 1/4] Bluetooth: Fix missing hci_dev_lock_bh in user_confirm_reply
From: Gustavo F. Padovan @ 2011-03-17 18:29 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1300278577-23068-1-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-03-16 14:29:34 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> The code was correctly calling _unlock at the end of the function but
> there was no actual _lock call anywhere.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  net/bluetooth/mgmt.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Applied, thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH v2 4/4] Bluetooth: Add add/remove_remote_oob_data management commands
From: Gustavo F. Padovan @ 2011-03-17 18:26 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, henrik.possung
In-Reply-To: <1299000216-6570-5-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2011-03-01 18:23:36 +0100]:

> This patch adds commands to add and remove remote OOB data to the managment
> interface. Remote data is stored in kernel and used by corresponding HCI
> commands and events (also implemented in this patch) when needed.
> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
>  include/net/bluetooth/hci.h      |   19 +++++++++
>  include/net/bluetooth/hci_core.h |   16 ++++++++
>  include/net/bluetooth/mgmt.h     |   12 ++++++
>  net/bluetooth/hci_core.c         |   76 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |   41 ++++++++++++++++++++-
>  net/bluetooth/mgmt.c             |   75 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 238 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 20840dc..b37ed33 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -426,6 +426,20 @@ struct hci_rp_user_confirm_reply {
>  
>  #define HCI_OP_USER_CONFIRM_NEG_REPLY	0x042d
>  
> +
> +#define HCI_OP_REMOTE_OOB_DATA_REPLY	0x0430
> +struct hci_cp_remote_oob_data_reply {
> +	bdaddr_t bdaddr;
> +	__u8     hash[16];
> +	__u8     randomizer[16];
> +} __packed;
> +
> +#define HCI_OP_REMOTE_OOB_DATA_NEG_REPLY	0x0433
> +struct hci_cp_remote_oob_data_neg_reply {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
> +
>  #define HCI_OP_IO_CAPABILITY_NEG_REPLY	0x0434
>  struct hci_cp_io_capability_neg_reply {
>  	bdaddr_t bdaddr;
> @@ -960,6 +974,11 @@ struct hci_ev_user_confirm_req {
>  	__le32		passkey;
>  } __packed;
>  
> +#define HCI_EV_REMOTE_OOB_DATA_REQUEST	0x35
> +struct hci_ev_remote_oob_data_request {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
>  #define HCI_EV_SIMPLE_PAIR_COMPLETE	0x36
>  struct hci_ev_simple_pair_complete {
>  	__u8     status;
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index c8b7ee6..0f1e1d6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -82,6 +82,13 @@ struct link_key {
>  	u8 pin_len;
>  };
>  
> +struct oob_data {
> +	struct list_head list;
> +	bdaddr_t bdaddr;
> +	u8 hash[16];
> +	u8 randomizer[16];
> +};
> +
>  #define NUM_REASSEMBLY 4
>  struct hci_dev {
>  	struct list_head list;
> @@ -169,6 +176,8 @@ struct hci_dev {
>  
>  	struct list_head	link_keys;
>  
> +	struct list_head	remote_oob_data;
> +
>  	struct hci_dev_stats	stat;
>  
>  	struct sk_buff_head	driver_init;
> @@ -505,6 +514,13 @@ int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
>  						u8 *key, u8 type, u8 pin_len);
>  int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
>  
> +int hci_remote_oob_data_clear(struct hci_dev *hdev);
> +struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev,
> +							bdaddr_t *bdaddr);
> +int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 *hash,
> +								u8 *randomizer);
> +int hci_remove_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr);
> +
>  void hci_del_off_timer(struct hci_dev *hdev);
>  
>  void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 916b1c6..ebe288c 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -173,6 +173,18 @@ struct mgmt_rp_read_local_oob_data {
>  	__u8 randomizer[16];
>  } __packed;
>  
> +#define MGMT_OP_ADD_REMOTE_OOB_DATA	0x0018
> +struct mgmt_cp_add_remote_oob_data {
> +	bdaddr_t bdaddr;
> +	__u8 hash[16];
> +	__u8 randomizer[16];
> +} __packed;
> +
> +#define MGMT_OP_REMOVE_REMOTE_OOB_DATA	0x0019
> +struct mgmt_cp_remove_remote_oob_data {
> +	bdaddr_t bdaddr;
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE		0x0001
>  struct mgmt_ev_cmd_complete {
>  	__le16 opcode;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b372fb8..a1d2263 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1077,6 +1077,79 @@ static void hci_cmd_timer(unsigned long arg)
>  	tasklet_schedule(&hdev->cmd_task);
>  }
>  
> +struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev,
> +							bdaddr_t *bdaddr)
> +{
> +	struct list_head *p;
> +
> +	list_for_each(p, &hdev->remote_oob_data) {
> +		struct oob_data *data;
> +
> +		data = list_entry(p, struct oob_data, list);

list_for_each_entry() here.

> +
> +		if (bacmp(bdaddr, &data->bdaddr) == 0)
> +			return data;
> +	}
> +
> +	return NULL;
> +}
> +
> +int hci_remove_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +{
> +	struct oob_data *data;
> +
> +	data = hci_find_remote_oob_data(hdev, bdaddr);
> +	if (!data)
> +		return -ENOENT;
> +
> +	BT_DBG("%s removing %s", hdev->name, batostr(bdaddr));
> +
> +	list_del(&data->list);
> +	kfree(data);
> +
> +	return 0;
> +}
> +
> +int hci_remote_oob_data_clear(struct hci_dev *hdev)
> +{
> +	struct list_head *p, *n;
> +
> +	list_for_each_safe(p, n, &hdev->remote_oob_data) {

list_for_each_entry_safe() here.


-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH v2 3/4] Bluetooth: Add read_local_oob_data management command
From: Gustavo F. Padovan @ 2011-03-17 18:18 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, henrik.possung
In-Reply-To: <1299000216-6570-4-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2011-03-01 18:23:35 +0100]:

> This patch adds a command to read local OOB data to the managment interface.
> The command maps directly to the Read Local OOB Data HCI command.
> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
>  include/net/bluetooth/hci.h      |    7 +++
>  include/net/bluetooth/hci_core.h |    2 +
>  include/net/bluetooth/mgmt.h     |    6 +++
>  net/bluetooth/hci_event.c        |   15 +++++++
>  net/bluetooth/mgmt.c             |   85 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 115 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ec6acf2..20840dc 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -611,6 +611,13 @@ struct hci_cp_write_ssp_mode {
>  	__u8     mode;
>  } __packed;
>  
> +#define HCI_OP_READ_LOCAL_OOB_DATA		0x0c57
> +struct hci_rp_read_local_oob_data {
> +	__u8     status;
> +	__u8     hash[16];
> +	__u8     randomizer[16];
> +} __packed;
> +
>  #define HCI_OP_READ_INQ_RSP_TX_POWER	0x0c58
>  
>  #define HCI_OP_READ_LOCAL_VERSION	0x1001
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 441dadb..c8b7ee6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -767,6 +767,8 @@ int mgmt_user_confirm_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
>  int mgmt_user_confirm_neg_reply_complete(u16 index, bdaddr_t *bdaddr,
>  								u8 status);
>  int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
> +int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
> +								u8 status);
>  
>  /* HCI info for socket */
>  #define hci_pi(sk) ((struct hci_pinfo *) sk)
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 5fabfa8..916b1c6 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -167,6 +167,12 @@ struct mgmt_rp_user_confirm_reply {
>  
>  #define MGMT_OP_USER_CONFIRM_NEG_REPLY	0x0016
>  
> +#define MGMT_OP_READ_LOCAL_OOB_DATA	0x0017
> +struct mgmt_rp_read_local_oob_data {
> +	__u8 hash[16];
> +	__u8 randomizer[16];
> +} __packed;
> +
>  #define MGMT_EV_CMD_COMPLETE		0x0001
>  struct mgmt_ev_cmd_complete {
>  	__le16 opcode;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3fbfa50..4faab87 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -807,6 +807,17 @@ static void hci_cc_user_confirm_reply(struct hci_dev *hdev, struct sk_buff *skb)
>  								rp->status);
>  }
>  
> +static void hci_cc_read_local_oob_data_reply(struct hci_dev *hdev,
> +							struct sk_buff *skb)
> +{
> +	struct hci_rp_read_local_oob_data *rp = (void *) skb->data;
> +
> +	BT_DBG("%s status 0x%x", hdev->name, rp->status);
> +
> +	mgmt_read_local_oob_data_reply_complete(hdev->id, rp->hash,
> +						rp->randomizer, rp->status);
> +}
> +

This should be after hci_cc_pin_code_neg_reply() and not
hci_cc_user_confirm_reply().


-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH v2 1/4] Bluetooth: Rename cmd to cmd_params in pending_cmd
From: Gustavo F. Padovan @ 2011-03-17 18:15 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, henrik.possung
In-Reply-To: <1299000216-6570-2-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2011-03-01 18:23:33 +0100]:

> This field holds not whole command but only command specific
> parameters.
> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
>  net/bluetooth/mgmt.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e6efaae..bde42a5 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -36,7 +36,7 @@ struct pending_cmd {
>  	struct list_head list;
>  	__u16 opcode;
>  	int index;
> -	void *cmd;
> +	void *cmd_params;

I prefer 'param' then.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix HCI_RESET command syncronization
From: Gustavo F. Padovan @ 2011-03-17 16:48 UTC (permalink / raw)
  To: Szymon Janc
  Cc: linux-bluetooth@vger.kernel.org, mmvinni@yahoo.com,
	justinmattock@gmail.com, edt@aei.ca
In-Reply-To: <201103171135.20477.szymon.janc@tieto.com>

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2011-03-17 11:35:20 +0100]:

> > We can't send new commands before a cmd_complete for the HCI_RESET commnnd
> 
> Typo: comm*a*nd
> 
> > shows up.
> > 
> > Reported-by: Mikko Vinni <mmvinni@yahoo.com>
> > Reported-by: Justin P. Mattock <justinmattock@gmail.com>
> > Reported-by: Ed Tomlinson <edt@aei.ca>
> > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> > ---
> >  include/net/bluetooth/hci.h |    2 ++
> >  net/bluetooth/hci_core.c    |    3 +++
> >  net/bluetooth/hci_event.c   |    4 +++-
> >  3 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index ec6acf2..2c0d309 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -84,6 +84,8 @@ enum {
> >  	HCI_SERVICE_CACHE,
> >  	HCI_LINK_KEYS,
> >  	HCI_DEBUG_KEYS,
> > +
> > +	HCI_RESET,
> >  };
> >  
> >  /* HCI ioctl defines */
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index b372fb8..32b82e2 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -534,6 +534,8 @@ int hci_dev_open(__u16 dev)
> >  		set_bit(HCI_INIT, &hdev->flags);
> >  		hdev->init_last_cmd = 0;
> >  
> > +		set_bit(HCI_RESET, &hdev->flags);
> 
> Does this work with HCI_QUIRK_NO_RESET enabled?
> Shouldn't this bit be set only right before sending HCI_OP_RESET
> (in hci_init_req and hci_reset_req) for doing what commit msg says?
> 
> Now it looks more like a workaround for initialization issue..

Ah yes, I fixed this as well now.

> 
> > +
> >  		ret = __hci_request(hdev, hci_init_req, 0,
> >  					msecs_to_jiffies(HCI_INIT_TIMEOUT));
> >  
> > @@ -1074,6 +1076,7 @@ static void hci_cmd_timer(unsigned long arg)
> >  
> >  	BT_ERR("%s command tx timeout", hdev->name);
> >  	atomic_set(&hdev->cmd_cnt, 1);
> > +	clear_bit(HCI_RESET, &hdev->flags);
> >  	tasklet_schedule(&hdev->cmd_task);
> >  }
> >  
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 3fbfa50..cebe7588 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -183,6 +183,8 @@ static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
> >  
> >  	BT_DBG("%s status 0x%x", hdev->name, status);
> >  
> > +	clear_bit(HCI_RESET, &hdev->flags);
> > +
> >  	hci_req_complete(hdev, HCI_OP_RESET, status);
> >  }
> >  
> > @@ -1847,7 +1849,7 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  	if (ev->opcode != HCI_OP_NOP)
> >  		del_timer(&hdev->cmd_timer);
> >  
> > -	if (ev->ncmd) {
> > +	if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) {
> >  		atomic_set(&hdev->cmd_cnt, 1);
> >  		if (!skb_queue_empty(&hdev->cmd_q))
> >  			tasklet_schedule(&hdev->cmd_task);
> 
> Maybe use unlikely() here?

We don't use this in Bluetooth.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: Switching between SBC and MPEG audio on headsets
From: Brian Gix @ 2011-03-17 16:19 UTC (permalink / raw)
  To: donpedro
  Cc: Luiz Augusto von Dentz, Arun Raghavan, linux-bluetooth,
	Johan Hedberg
In-Reply-To: <1300354542.4500.72.camel@donpedro>

Hi Pedro,

On 3/17/2011 2:35 AM, Peter Dons Tychsen wrote:
> Hi Brian,
>
> On Wed, 2011-03-16 at 16:09 -0700, Brian Gix wrote:
>> This is not actually true. You can have multiple media channels open
>> simultaneously (for instance one for Video and one for Audio) which
>> are
>> opened with careful handshaking with the AVDTP_OPEN signaling
>> command.
>> In theory, this could be two audio channels as well.  If someone wants
>> a
>> Journaling channel, it would be opened without the AVDTP_OPEN
>> signaling.
>
> Yes, except protocol-wise the new channel on PSM=0x19 could also be the
> start of a new A2DP signaling connection (however very unlikely). As you
> mentioned, the problem with the Journaling channel can be filtered out
> with correct signaling.

AVDTP only allows a single signaling channel to exist between any two 
peers. You are allowed to have separate signaling channels to separate 
remote devices, but even if you are supporting multiple media streams 
(video/audio, or audio/audio) to the same device, all AVDTP signaling 
must be done on a single channel.

>
> I totally agree with you that your solution is the most elegant. However
> back when A2DP was introduced it did not seem clear to everyone how
> multiple streams would work. Some thought it would work with multiple
> media channels (like you suggest), and some thought it would work with
> multiple signaling channels (with max 1 media channel).
>
> Most of the implementations i have seen (for headsets), unfortunately
> ended up with the last solution. This means that they will not accept a
> secondary media channel. This is unfortunately basically true for all of
> the most popular chipsets and SDKs.

Actually, I thought I was agreeing with you about *not* having two media 
channels open simultaneously. There are too many legacy devices in the 
market that do not support a second media channel for audio. I was just 
pointing out that the limitations are in the implementations, not the 
specification. And I am a big believer in maintaining compatibility with 
legacy devices. It is possible to have two spec compliant devices that 
do not interoperate, which of course is why we have UPFs.

> Your suggestion is clever and follows the standard nicely (and would get
> my vote for a white-paper), but i doubt it will work with many headsets
> out there.
>
> Even worse, many headsets are hardcoded (not nice) to believe that the
> 3rd channel is always the Journaling channel. That is of course, just
> plain wrong due to the reasons you have stated.

I haven't actually even encountered any headsets in the wild that 
support Journaling.

>
> Thanks,
>
> /pedro
>


-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: how to set adapter to master with bluez 4.69?
From: Brad Midgley @ 2011-03-17 15:46 UTC (permalink / raw)
  To: Brian J. Murrell; +Cc: linux-bluetooth, Brad Midgley
In-Reply-To: <4D820B1D.1080903@interlinx.bc.ca>

Brian,

> I wonder why it worked so well on my laptop and doesn't work so well on
> my PC?

I suspect it's because some adapters can deal with switching between
multiple piconets much more gracefully than others, but it could be
something else.

iirc, you should have accept in your link policy.

-- 
Brad Midgley

^ permalink raw reply

* [PATCHv2 5/5] Add Exchange MTU in interactive gatttool
From: Bruna Moreira @ 2011-03-17 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>

---
 attrib/interactive.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index a9157e7..3fafb1e 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -362,6 +362,7 @@ static void cmd_disconnect(int argcp, char **argvp)
 
 	g_attrib_unref(attrib);
 	attrib = NULL;
+	opt_mtu = 0;
 
 	g_io_channel_shutdown(iochannel, FALSE, NULL);
 	g_io_channel_unref(iochannel);
@@ -654,6 +655,65 @@ static void cmd_sec_level(int argcp, char **argvp)
 	}
 }
 
+static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
+							gpointer user_data)
+{
+	uint16_t mtu;
+
+	if (status != 0) {
+		printf("Exchange MTU Request failed: %s\n",
+							att_ecode2str(status));
+		return;
+	}
+
+	if (!dec_mtu_resp(pdu, plen, &mtu)) {
+		printf("Protocol error\n");
+		return;
+	}
+
+	mtu = MIN(mtu, opt_mtu);
+	/* Set new value for MTU in client */
+	if (g_attrib_set_mtu(attrib, mtu))
+		printf("MTU was exchanged successfully: %d\n", mtu);
+	else
+		printf("Error exchanging MTU\n");
+}
+
+static void cmd_mtu(int argcp, char **argvp)
+{
+	if (conn_state != STATE_CONNECTED) {
+		printf("Command failed: not connected.\n");
+		return;
+	}
+
+	if (opt_psm) {
+		printf("Command failed: operation is only available for LE"
+							" transport.\n");
+		return;
+	}
+
+	if (argcp < 2) {
+		printf("Usage: mtu <value>\n");
+		return;
+	}
+
+	if (opt_mtu) {
+		printf("Command failed: MTU exchange can only occur once per"
+							" connection.\n");
+		return;
+	}
+
+	errno = 0;
+	opt_mtu = strtoll(argvp[1], NULL, 0);
+	if (errno != 0 || opt_mtu < ATT_DEFAULT_LE_MTU) {
+		printf("Invalid value. Minimum MTU size is %d\n",
+							ATT_DEFAULT_LE_MTU);
+		return;
+	}
+
+	gatt_exchange_mtu(attrib, opt_mtu, exchange_mtu_cb, NULL);
+}
+
 static struct {
 	const char *cmd;
 	void (*func)(int argcp, char **argvp);
@@ -684,6 +744,8 @@ static struct {
 		"Characteristic Value Write (No response)" },
 	{ "sec-level",		cmd_sec_level,	"[low | medium | high]",
 		"Set security level. Default: low" },
+	{ "mtu",		cmd_mtu,	"<value>",
+		"Exchange MTU for GATT/ATT" },
 	{ NULL, NULL, NULL}
 };
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 4/5] Add Exchange MTU operation in GATT library
From: Bruna Moreira @ 2011-03-17 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>

---
 attrib/gatt.c |   13 +++++++++++++
 attrib/gatt.h |    3 +++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 28654af..0b69daf 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -534,6 +534,19 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 							user_data, NULL);
 }
 
+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+							gpointer user_data)
+{
+	uint8_t *buf;
+	int buflen;
+	guint16 plen;
+
+	buf = g_attrib_get_buffer(attrib, &buflen);
+	plen = enc_mtu_req(mtu, buf, buflen);
+	return g_attrib_send(attrib, 0, ATT_OP_MTU_REQ, buf, plen, func,
+							user_data, NULL);
+}
+
 guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 				GAttribResultFunc func, gpointer user_data)
 {
diff --git a/attrib/gatt.h b/attrib/gatt.h
index c6d3843..221d94d 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -48,3 +48,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
 guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 				bt_uuid_t *uuid, GAttribResultFunc func,
 				gpointer user_data);
+
+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+							gpointer user_data);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 3/5] Use GAttrib buffer for ATT protocol PDUs
From: Bruna Moreira @ 2011-03-17 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>

Prior to this commit, there were local buffers inside GATT functions.
Now, a single buffer is used, to make sure the MTU limit is respected.
---
 attrib/gatt.c |  109 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 64 insertions(+), 45 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 1cd651c..28654af 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -109,9 +109,9 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
 	struct discover_primary *dp = user_data;
 	GSList *ranges, *last;
 	struct att_range *range;
-	uint8_t opdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
 	guint16 oplen;
-	int err = 0;
+	int err = 0, buflen;
 
 	if (status) {
 		err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
@@ -130,13 +130,14 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
 	if (range->end == 0xffff)
 		goto done;
 
+	buf = g_attrib_get_buffer(dp->attrib, &buflen);
 	oplen = encode_discover_primary(range->end + 1, 0xffff, &dp->uuid,
-							opdu, sizeof(opdu));
+								buf, buflen);
 
 	if (oplen == 0)
 		goto done;
 
-	g_attrib_send(dp->attrib, 0, opdu[0], opdu, oplen, primary_by_uuid_cb,
+	g_attrib_send(dp->attrib, 0, buf[0], buf, oplen, primary_by_uuid_cb,
 								dp, NULL);
 	return;
 
@@ -197,12 +198,13 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 	err = 0;
 
 	if (end != 0xffff) {
-		uint8_t opdu[ATT_DEFAULT_LE_MTU];
+		int buflen;
+		uint8_t *buf = g_attrib_get_buffer(dp->attrib, &buflen);
 		guint16 oplen = encode_discover_primary(end + 1, 0xffff, NULL,
-							opdu, sizeof(opdu));
+								buf, buflen);
 
-		g_attrib_send(dp->attrib, 0, opdu[0], opdu, oplen,
-						primary_all_cb, dp, NULL);
+		g_attrib_send(dp->attrib, 0, buf[0], buf, oplen, primary_all_cb,
+								dp, NULL);
 
 		return;
 	}
@@ -216,11 +218,12 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data)
 {
 	struct discover_primary *dp;
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
 	GAttribResultFunc cb;
 	guint16 plen;
 
-	plen = encode_discover_primary(0x0001, 0xffff, uuid, pdu, sizeof(pdu));
+	plen = encode_discover_primary(0x0001, 0xffff, uuid, buf, buflen);
 	if (plen == 0)
 		return 0;
 
@@ -238,7 +241,7 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 	} else
 		cb = primary_all_cb;
 
-	return g_attrib_send(attrib, 0, pdu[0], pdu, plen, cb, dp, NULL);
+	return g_attrib_send(attrib, 0, buf[0], buf, plen, cb, dp, NULL);
 }
 
 static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
@@ -247,7 +250,8 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 	struct discover_char *dc = user_data;
 	struct att_data_list *list;
 	unsigned int i, err;
-	uint8_t opdu[ATT_DEFAULT_LE_MTU];
+	int buflen;
+	uint8_t *buf;
 	guint16 oplen;
 	bt_uuid_t uuid;
 	uint16_t last = 0;
@@ -297,15 +301,17 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 	err = 0;
 
 	if (last != 0) {
+		buf = g_attrib_get_buffer(dc->attrib, &buflen);
+
 		bt_uuid16_create(&uuid, GATT_CHARAC_UUID);
 
-		oplen = enc_read_by_type_req(last + 1, dc->end, &uuid, opdu,
-								sizeof(opdu));
+		oplen = enc_read_by_type_req(last + 1, dc->end, &uuid, buf,
+									buflen);
 
 		if (oplen == 0)
 			return;
 
-		g_attrib_send(dc->attrib, 0, opdu[0], opdu, oplen,
+		g_attrib_send(dc->attrib, 0, buf[0], buf, oplen,
 						char_discovered_cb, dc, NULL);
 
 		return;
@@ -320,14 +326,15 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 						bt_uuid_t *uuid, gatt_cb_t func,
 						gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
 	struct discover_char *dc;
 	bt_uuid_t type_uuid;
 	guint16 plen;
 
 	bt_uuid16_create(&type_uuid, GATT_CHARAC_UUID);
 
-	plen = enc_read_by_type_req(start, end, &type_uuid, pdu, sizeof(pdu));
+	plen = enc_read_by_type_req(start, end, &type_uuid, buf, buflen);
 	if (plen == 0)
 		return 0;
 
@@ -341,7 +348,7 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 	dc->end = end;
 	dc->uuid = g_memdup(uuid, sizeof(bt_uuid_t));
 
-	return g_attrib_send(attrib, 0, pdu[0], pdu, plen, char_discovered_cb,
+	return g_attrib_send(attrib, 0, buf[0], buf, plen, char_discovered_cb,
 								dc, NULL);
 }
 
@@ -349,15 +356,16 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 					bt_uuid_t *uuid, GAttribResultFunc func,
 					gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
 	guint16 plen;
 
-	plen = enc_read_by_type_req(start, end, uuid, pdu, sizeof(pdu));
+	plen = enc_read_by_type_req(start, end, uuid, buf, buflen);
 	if (plen == 0)
 		return 0;
 
 	return g_attrib_send(attrib, 0, ATT_OP_READ_BY_TYPE_REQ,
-					pdu, plen, func, user_data, NULL);
+					buf, plen, func, user_data, NULL);
 }
 
 struct read_long_data {
@@ -388,7 +396,8 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
 							gpointer user_data)
 {
 	struct read_long_data *long_read = user_data;
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
+	int buflen;
 	guint8 *tmp;
 	guint16 plen;
 	guint id;
@@ -409,13 +418,14 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
 	long_read->buffer = tmp;
 	long_read->size += rlen - 1;
 
-	if (rlen < ATT_DEFAULT_LE_MTU)
+	buf = g_attrib_get_buffer(long_read->attrib, &buflen);
+	if (rlen < buflen)
 		goto done;
 
 	plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
-							pdu, sizeof(pdu));
+								buf, buflen);
 	id = g_attrib_send(long_read->attrib, long_read->id,
-				ATT_OP_READ_BLOB_REQ, pdu, plen,
+				ATT_OP_READ_BLOB_REQ, buf, plen,
 				read_blob_helper, long_read, read_long_destroy);
 
 	if (id != 0) {
@@ -434,11 +444,12 @@ static void read_char_helper(guint8 status, const guint8 *rpdu,
 					guint16 rlen, gpointer user_data)
 {
 	struct read_long_data *long_read = user_data;
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(long_read->attrib, &buflen);
 	guint16 plen;
 	guint id;
 
-	if (status != 0 || rlen < ATT_DEFAULT_LE_MTU)
+	if (status != 0 || rlen < buflen)
 		goto done;
 
 	long_read->buffer = g_malloc(rlen);
@@ -449,9 +460,9 @@ static void read_char_helper(guint8 status, const guint8 *rpdu,
 	memcpy(long_read->buffer, rpdu, rlen);
 	long_read->size = rlen;
 
-	plen = enc_read_blob_req(long_read->handle, rlen - 1, pdu, sizeof(pdu));
+	plen = enc_read_blob_req(long_read->handle, rlen - 1, buf, buflen);
 	id = g_attrib_send(long_read->attrib, long_read->id,
-			ATT_OP_READ_BLOB_REQ, pdu, plen, read_blob_helper,
+			ATT_OP_READ_BLOB_REQ, buf, plen, read_blob_helper,
 			long_read, read_long_destroy);
 
 	if (id != 0) {
@@ -468,7 +479,8 @@ done:
 guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
 				GAttribResultFunc func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
+	int buflen;
 	guint16 plen;
 	guint id;
 	struct read_long_data *long_read;
@@ -483,14 +495,15 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
 	long_read->user_data = user_data;
 	long_read->handle = handle;
 
+	buf = g_attrib_get_buffer(attrib, &buflen);
 	if (offset > 0) {
-		plen = enc_read_blob_req(long_read->handle, offset, pdu,
-								sizeof(pdu));
-		id = g_attrib_send(attrib, 0, ATT_OP_READ_BLOB_REQ, pdu, plen,
+		plen = enc_read_blob_req(long_read->handle, offset, buf,
+									buflen);
+		id = g_attrib_send(attrib, 0, ATT_OP_READ_BLOB_REQ, buf, plen,
 				read_blob_helper, long_read, read_long_destroy);
 	} else {
-		plen = enc_read_req(handle, pdu, sizeof(pdu));
-		id = g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen,
+		plen = enc_read_req(handle, buf, buflen);
+		id = g_attrib_send(attrib, 0, ATT_OP_READ_REQ, buf, plen,
 				read_char_helper, long_read, read_long_destroy);
 	}
 
@@ -507,39 +520,45 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
 guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 			int vlen, GAttribResultFunc func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
+	int buflen;
 	guint16 plen;
 
+	buf = g_attrib_get_buffer(attrib, &buflen);
 	if (func)
-		plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
+		plen = enc_write_req(handle, value, vlen, buf, buflen);
 	else
-		plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
+		plen = enc_write_cmd(handle, value, vlen, buf, buflen);
 
-	return g_attrib_send(attrib, 0, pdu[0], pdu, plen, func,
+	return g_attrib_send(attrib, 0, buf[0], buf, plen, func,
 							user_data, NULL);
 }
 
 guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 				GAttribResultFunc func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
+	int buflen;
 	guint16 plen;
 
-	plen = enc_find_info_req(start, end, pdu, sizeof(pdu));
+	buf = g_attrib_get_buffer(attrib, &buflen);
+	plen = enc_find_info_req(start, end, buf, buflen);
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, buf, plen, func,
 							user_data, NULL);
 }
 
 guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
 				GDestroyNotify notify, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_LE_MTU];
+	uint8_t *buf;
+	int buflen;
 	guint16 plen;
 
-	plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, pdu, plen, NULL,
+	buf = g_attrib_get_buffer(attrib, &buflen);
+	plen = enc_write_cmd(handle, value, vlen, buf, buflen);
+	return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, buf, plen, NULL,
 							user_data, notify);
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 2/5] Add internal buffer to GAttrib struct
From: Bruna Moreira @ 2011-03-17 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>

The new buffer is allocated in g_attrib_new() and it will be used to
send/receive PDUs. The buffer size is the MTU read from L2CAP channel
limited to ATT_MAX_MTU. Functions to handle the buffer size were also
created.
---
 attrib/gattrib.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gattrib.h |    3 +++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 07e56de..290cd96 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -40,6 +40,8 @@
 struct _GAttrib {
 	GIOChannel *io;
 	gint refs;
+	uint8_t *buf;
+	int buflen;
 	guint read_watch;
 	guint write_watch;
 	guint timeout_watch;
@@ -193,6 +195,8 @@ static void attrib_destroy(GAttrib *attrib)
 		g_io_channel_unref(attrib->io);
 	}
 
+	g_free(attrib->buf);
+
 	if (attrib->destroy)
 		attrib->destroy(attrib->destroy_user_data);
 
@@ -386,6 +390,7 @@ done:
 GAttrib *g_attrib_new(GIOChannel *io)
 {
 	struct _GAttrib *attrib;
+	uint16_t omtu;
 
 	g_io_channel_set_encoding(io, NULL, NULL);
 	g_io_channel_set_buffered(io, FALSE);
@@ -401,6 +406,17 @@ GAttrib *g_attrib_new(GIOChannel *io)
 			G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
 			received_data, attrib);
 
+	if (bt_io_get(attrib->io, BT_IO_L2CAP, NULL,
+			BT_IO_OPT_OMTU, &omtu,
+			BT_IO_OPT_INVALID)) {
+		if (omtu > ATT_MAX_MTU)
+			omtu = ATT_MAX_MTU;
+	} else
+		omtu = ATT_DEFAULT_LE_MTU;
+
+	attrib->buf = g_malloc0(omtu);
+	attrib->buflen = omtu;
+
 	return g_attrib_ref(attrib);
 }
 
@@ -504,6 +520,36 @@ gboolean g_attrib_set_debug(GAttrib *attrib,
 	return TRUE;
 }
 
+uint8_t *g_attrib_get_buffer(GAttrib *attrib, int *len)
+{
+	if (len == NULL)
+		return NULL;
+
+	*len = attrib->buflen;
+
+	return attrib->buf;
+}
+
+gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
+{
+	if (mtu < ATT_DEFAULT_LE_MTU)
+		mtu = ATT_DEFAULT_LE_MTU;
+
+	if (mtu > ATT_MAX_MTU)
+		mtu = ATT_MAX_MTU;
+
+	if (!bt_io_set(attrib->io, BT_IO_L2CAP, NULL,
+			BT_IO_OPT_OMTU, mtu,
+			BT_IO_OPT_INVALID))
+		return FALSE;
+
+	attrib->buf = g_realloc(attrib->buf, mtu);
+
+	attrib->buflen = mtu;
+
+	return TRUE;
+}
+
 guint g_attrib_register(GAttrib *attrib, guint8 opcode,
 				GAttribNotifyFunc func, gpointer user_data,
 				GDestroyNotify notify)
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index f25208d..4c49879 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -68,6 +68,9 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode,
 
 gboolean g_attrib_is_encrypted(GAttrib *attrib);
 
+uint8_t *g_attrib_get_buffer(GAttrib *attrib, int *len);
+gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu);
+
 gboolean g_attrib_unregister(GAttrib *attrib, guint id);
 gboolean g_attrib_unregister_all(GAttrib *attrib);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 1/5] TODO: remove 'fix MTU exchange' task
From: Bruna Moreira @ 2011-03-17 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>

---
 TODO |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/TODO b/TODO
index f49a9c4..588d9f6 100644
--- a/TODO
+++ b/TODO
@@ -130,11 +130,6 @@ ATT/GATT
   Priority: Medium
   Complexity: C2
 
-- GATT server: fix MTU exchange
-
-  Priority: Medium
-  Complexity: C2
-
 - Implement ATT PDU validation. Malformed PDUs can cause division by zero
   when decoding PDUs. A proper error PDU should be returned for this case.
   See decoding function in att.c file.
-- 
1.7.0.4


^ permalink raw reply related

* Re: how to set adapter to master with bluez 4.69?
From: Brian J. Murrell @ 2011-03-17 13:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Brad Midgley
In-Reply-To: <4D820851.4040809@interlinx.bc.ca>

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

I mean to add before I hit send that I had an opportunity to try all of
this out on my laptop yesterday (so far all of the problems I have been
reporting here have been with my PC which has a:

Bus 002 Device 003: ID 0a5c:4500 Broadcom Corp. BCM2046B1 USB 2.0 Hub
(part of BCM2046 Bluetooth)

Having my mouse and even the LG headset (which has been the most
problematic on my PC) on my laptop worked flawlessly -- without even
having to mess with any of this master/slave stuff.  I was in a
teleconference with the headset for just over an hour and was using the
mouse at the same time without a hiccup.  If only my PC would work as
reliably.

The chipset in my laptop is a:

Bus 001 Device 006: ID 0a5c:217f Broadcom Corp. Bluetooth Controller

What is interesting is that I can't seem to discover much about the
state of the adapter in that machine:

$ hcitool dev
Devices:
	hci0	70:F3:95:3E:92:34
$ hcitool hci0 lm

returns nothing.  When I do connect the mouse using the
bluetooth-applet's Connect menu for the device:

$ hcitool hci0 lm
$ hcitool con
Connections:
	> ACL 00:1F:20:0F:30:6A handle 11 state 1 lm MASTER

After turning on the headset:

$ hcitool hci0 lm
$ hcitool con
Connections:
	> ACL 00:18:6B:E5:4F:7E handle 12 state 1 lm MASTER AUTH ENCRYPT
	> ACL 00:1F:20:0F:30:6A handle 11 state 1 lm MASTER

I wonder why it worked so well on my laptop and doesn't work so well on
my PC?

Cheers,
b.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* [PATCH] Set correct adapter state in cancel_resolve_name
From: Radoslaw Jablonski @ 2011-03-17 13:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Radoslaw Jablonski

Previously resetting STATE_RESOLVNAME for adapter was missing.
This was causing problems with discovering devices when discovery
was quickly turned off/on during resolving name.
---
 src/adapter.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 8c368fe..cc4f43e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -260,6 +260,7 @@ static int pending_remote_name_cancel(struct btd_adapter *adapter)
 	if (!dev) /* no pending request */
 		return -ENODATA;
 
+	adapter->state &= ~STATE_RESOLVNAME;
 	err = adapter_ops->cancel_resolve_name(adapter->dev_id, &dev->bdaddr);
 	if (err < 0)
 		error("Remote name cancel failed: %s(%d)",
-- 
1.7.0.4


^ permalink raw reply related

* Re: how to set adapter to master with bluez 4.69?
From: Brian J. Murrell @ 2011-03-17 13:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Brad Midgley
In-Reply-To: <AANLkTim2oLMjE-9C0G7VCjXafg03xnb7a0Q04zwKh2ns@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

On 11-03-15 11:25 AM, Brad Midgley wrote:
> Brian,

Hi Brad,

> You can have both master and slave connections simultaneously, just
> like you can be slave to more than one master. In both cases it means
> your adapter is in different piconets.

Ahhh.  See.  Shows what I know about the details of B/T.  I thought one
adapter == one piconet.

> There are rules about whether devices can refuse the master/slave
> switch. It's been some time since I read this so I can't remember, but
> maybe it's the initiating device that starts out as slave and the
> remote device has to agree to a role switch.

Hrm.  I guess I will have to play with that a bit.

> So you should be able to
> force the headset to cooperate, even if it's refusing role switches,
> by changing who initiates that connection in addition to a link policy
> that keeps your adapter as master.

OK.  So as far as the link policy that keeps the adapter as master,
after having rebooted this machine today I found the following:

$ hcitool con
Connections:
$ hciconfig hci0 lm
hci0:	Type: BR/EDR  Bus: USB
	BD Address: 00:02:72:1E:E0:12  ACL MTU: 1021:7  SCO MTU: 64:1
	Link mode: SLAVE ACCEPT

So that means that the adapter will want to be the slave in all
connections, is that right?

I should change the above slave to master as such:

$ sudo hciconfig hci0 lm master
$ hciconfig hci0 lm
hci0:	Type: BR/EDR  Bus: USB
	BD Address: 00:02:72:1E:E0:12  ACL MTU: 1021:7  SCO MTU: 64:1
	Link mode: MASTER

Is that correct?  Do I want to enable that "ACCEPT" also?

Once I do that, for my mouse at least, do I want to press the "Connect"
button on the mouse or use the Connect option for the mouse in the
bluetooth-applet?

b.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* Re: [PATCH] TODO: set owner of 'Define attribute server API' task
From: Johan Hedberg @ 2011-03-17 12:59 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1300362901-3920-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Thu, Mar 17, 2011, Anderson Lizardo wrote:
> An initial implementation was sent as RFC.
> ---
>  TODO |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Update TODO regarding bt_uuid_* functions
From: Johan Hedberg @ 2011-03-17 12:59 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1300325814-6247-1-git-send-email-epx@signove.com>

Hi Elvis,

On Wed, Mar 16, 2011, Elvis Pf??tzenreuter wrote:
> ---
>  TODO |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox