Peter Krystad | 1 Dec 2011 07:19

RE: [RFCv0 0/3] AMP HCI interface from A2MP


Hi Andrei, Marcel

> Hi Marcel,
> > Hi Andrei,
> > > The code adds AMP HCI commands from A2MP protocol. HCI events are handled
> > > similar way as mgmt interface. amp_pending is a kind of copy of mgmt_pending.
> >
> > this is all kernel internal code with no interface to user space. I do
> > not see the need for such a complex infrastructure. Can not just have
> > proper callbacks or event callback table like with L2CAP. Or just
> > something similar.
> 
> I see this as a simple callback. amp_pending is just keeping context of HCI
> command we need to handle. I also included reference counting since we had
> bad experience with l2cap and rfcomm.

There does have to be some way to carry the A2MP message context while performing
local HCI operations to service the message. A2MP Get Remote Assoc requires multiple
HCI commands with data accumulated between the commands before the response can be
sent. 

> > As far as I see it, we get an A2MP command over L2CAP fixed channel, we
> > have to issue a HCI command or do some other task based on this and then
> > respond to it. We only have one user of this first of all. And second of
> > all, I think we can not really have pending A2MP commands anyway. This
> > is pretty much one command at a time (per ACL link).

A2MP commands are serialized by the sender, so A2MP message context could be
associated with the hci_conn for BR-EDR.
(Continue reading)

Peter Krystad | 1 Dec 2011 07:24

RE: [PATCHv1 6/6] Bluetooth: Recalculate sched for HCI block flow ctrl


Hi Marcel, Andrei,

> Hi Andrei,
> > >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 4a0391e..360b44c 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> >  <at>  <at>  -2303,15 +2305,28  <at>  <at>  static inline void hci_sched_acl(struct hci_dev *hdev)
> >
> >  			skb = skb_dequeue(&chan->data_q);
> >
> > +			if (hdev->flow_ctl_mode ==
> > +					HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > +				/* Calculate count of blocks used by
> > +				 * this packet
> > +				 */
> > +				blocks = DIV_ROUND_UP(skb->len -
> > +					HCI_ACL_HDR_SIZE, hdev->block_len);
> 
> would it not be more efficient to have to functions here? One
> hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> specific details of the flow control mode. And the hci_sched_acl would
> just decide which on to call once.

There has to be a check for which flow control mode is used by the device somewhere, and
this fragment is really the only difference between the two modes, in separate functions
the rest would be duplicate code.

(Continue reading)

Luiz Augusto von Dentz | 1 Dec 2011 08:23
Picon

Re: [PATCH 4/4] Bluetooth: use buffer priority to mark URB_ISO_ASAP flag

Hi Vinicius,

On Wed, Nov 30, 2011 at 7:24 PM, Vinicius Costa Gomes
<vinicius.gomes@...> wrote:
> Hi,
>
> On 15:52 Wed 02 Nov, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@...>
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@...>
>> ---
>>  drivers/bluetooth/btusb.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index abfc4ee..9db2476 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>>  <at>  <at>  -727,6 +727,9  <at>  <at>  static int btusb_send_frame(struct sk_buff *skb)
>>               usb_fill_bulk_urb(urb, data->udev, pipe,
>>                               skb->data, skb->len, btusb_tx_complete, skb);
>>
>> +             if (skb->priority >= HCI_PRIO_MAX - 1)
>> +                     urb->transfer_flags  = URB_ISO_ASAP;
>> +
>
> In case someone is having problems:
>
> With CONFIG_USB_DEBUG enabled the check that the URB_ISO_ASAP flag is
> not valid for bulk endpoints is enabled, and that urb is rejected.
(Continue reading)

Luiz Augusto von Dentz | 1 Dec 2011 08:39
Picon

Re: Cracking Sound

Hi,

On Wed, Nov 30, 2011 at 9:27 PM,  <krisztian.kocsis@...> wrote:
> Hi!
>
> I'm trying to get bluetooth audio working on a PowerPC machine, but it has
> some strange effects.
> I'm using a bluetooth headset (HSP profile) via ALSA but it seems that it
> has some problem with various sample rates and other data related stuff.
> The asound.conf contains the appropriate device address, profiles is set to
> voice, device is named as 'btheadset' (the share/alsa/bluetooth.conf is also
> included).
>
> In BlueZ's output it seems that is is playing (statu is PLAY_IN_PROGRESS or
> PLAYING - I don't remember correctly) the stream.
> When BlueZ is connecting to the device I hear a beep in the headset but the
> "stream" is only a cracking, clicking and poping noise, nothing from the
> original sound.
> Also this sound comes if I force aplay to 4KHz or 8KHz, in other settings or
> without forcing, it plays nothing (no sound, only beeps).
>
> Do somebody know which problem can cause this side effect?
>
> I' using ALSA 1.0.24 and BlueZ 4.96 (both are untouched, no patches and no
> config file modifications - except that I had to create an asound.conf).
>
> Is anything else I can provide about the problem?

ALSA plugin is consider deprecated since Media became the default API
for requesting audio transport, in the future it will probably be
(Continue reading)

'Emeltchenko Andrei' | 1 Dec 2011 10:46
Picon

Re: [PATCHv1 6/6] Bluetooth: Recalculate sched for HCI block flow ctrl

Hi Marcel, Peter,

On Wed, Nov 30, 2011 at 10:24:33PM -0800, Peter Krystad wrote:
> Hi Marcel, Andrei,
> 
> > Hi Andrei,
> > > >
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index 4a0391e..360b44c 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > >  <at>  <at>  -2303,15 +2305,28  <at>  <at>  static inline void hci_sched_acl(struct hci_dev *hdev)
> > >
> > >  			skb = skb_dequeue(&chan->data_q);
> > >
> > > +			if (hdev->flow_ctl_mode ==
> > > +					HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > > +				/* Calculate count of blocks used by
> > > +				 * this packet
> > > +				 */
> > > +				blocks = DIV_ROUND_UP(skb->len -
> > > +					HCI_ACL_HDR_SIZE, hdev->block_len);
> > 
> > would it not be more efficient to have to functions here? One
> > hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> > specific details of the flow control mode. And the hci_sched_acl would
> > just decide which on to call once.
> 
> There has to be a check for which flow control mode is used by the device somewhere, and
> this fragment is really the only difference between the two modes, in separate functions
(Continue reading)

krisztian.kocsis | 1 Dec 2011 11:59
Picon

Re: Cracking Sound

Hello!

On Wed, 30 Nov 2011 15:52:19 -0700, Brad Midgley wrote:
> Hi
>
>> When BlueZ is connecting to the device I hear a beep in the headset 
>> but the
>> "stream" is only a cracking, clicking and poping noise, nothing from 
>> the
>> original sound.
>
> the codec has byte order issues. Most everyone runs even embedded
> devices in little endian mode.

Where can I find this codec? Is it part of alsa plugin or bluez or the 
headset itself?

BR,
Krisztian

Marcel Holtmann | 1 Dec 2011 12:15

Re: [PATCHv1 6/6] Bluetooth: Recalculate sched for HCI block flow ctrl

Hi Andrei,

> > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > > index 4a0391e..360b44c 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > >  <at>  <at>  -2303,15 +2305,28  <at>  <at>  static inline void hci_sched_acl(struct hci_dev *hdev)
> > > >
> > > >  			skb = skb_dequeue(&chan->data_q);
> > > >
> > > > +			if (hdev->flow_ctl_mode ==
> > > > +					HCI_FLOW_CTL_MODE_BLOCK_BASED)
> > > > +				/* Calculate count of blocks used by
> > > > +				 * this packet
> > > > +				 */
> > > > +				blocks = DIV_ROUND_UP(skb->len -
> > > > +					HCI_ACL_HDR_SIZE, hdev->block_len);
> > > 
> > > would it not be more efficient to have to functions here? One
> > > hci_sched_acl_block and one hci_sched_acl_pkt which implement the
> > > specific details of the flow control mode. And the hci_sched_acl would
> > > just decide which on to call once.
> > 
> > There has to be a check for which flow control mode is used by the device somewhere, and
> > this fragment is really the only difference between the two modes, in separate functions
> > the rest would be duplicate code.
> 
> I feel the same, we can still make a function like:

are we really sure here. If we have more than one SKB in the queue, I
(Continue reading)

Marcel Holtmann | 1 Dec 2011 12:21

RE: [RFCv0 0/3] AMP HCI interface from A2MP

Hi Peter,

> > > > The code adds AMP HCI commands from A2MP protocol. HCI events are handled
> > > > similar way as mgmt interface. amp_pending is a kind of copy of mgmt_pending.
> > >
> > > this is all kernel internal code with no interface to user space. I do
> > > not see the need for such a complex infrastructure. Can not just have
> > > proper callbacks or event callback table like with L2CAP. Or just
> > > something similar.
> > 
> > I see this as a simple callback. amp_pending is just keeping context of HCI
> > command we need to handle. I also included reference counting since we had
> > bad experience with l2cap and rfcomm.
> 
> There does have to be some way to carry the A2MP message context while performing
> local HCI operations to service the message. A2MP Get Remote Assoc requires multiple
> HCI commands with data accumulated between the commands before the response can be
> sent. 

I agree, but this amp_pending seems to be wrong. I talked about changing
the init handling and at the same time we might can apply this to the
A2MP handling.

Why not just set a bit in hdev->dev_flags or maybe even hcon->dev_flags
or something and based on that we do a nice async state machine that
sends the commands and results in calling back into A2MP core when its
finished.

That seems a bit simpler to me and more aligned in a way I wanna redo
the whole HCI command/event handling anyway. In addition we can just
(Continue reading)

Gustavo Padovan | 1 Dec 2011 12:31

Re: [PATCH] Bluetooth: Consider the type param in start_discovery

Hi André,

* Andre Guedes <andre.guedes@...> [2011-11-22 16:50:25 -0300]:

> The Management Interface API has been changed and now the Start
> Discovery Command has the 'type' parameter. This parameters has
> been define as follows:
> 
> Possible values for the Type parameter are a bit-wise or of the
> following bits:
> 
> 	1       BR/EDR
> 	2       LE Public
> 	3       LE Random
> 
> By combining these e.g. the following values are possible:
> 
> 	1       BR/EDR
> 	6       LE (public & random)
> 	7       BR/EDR/LE (interleaved discovery)
> 
> Further information about Start Discovery Command see mgmt-api.txt
> in BlueZ source.
> 
> Signed-off-by: Andre Guedes <andre.guedes@...>
> ---
>  include/net/bluetooth/hci.h      |    1 +
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/mgmt.c             |   27 ++++++++++++++++++++++++++-
>  3 files changed, 28 insertions(+), 1 deletions(-)
(Continue reading)

Emeltchenko Andrei | 1 Dec 2011 13:25
Picon

Re: [PATCHv1 5/6] Bluetooth: Add HCI Read Data Block Size function

Hi Marcel,

On Wed, Nov 30, 2011 at 03:22:39PM +0100, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Upstream Code Aurora function with minor trivial fixes.
> > Origin: git://codeaurora.org/kernel/msm.git
> > 
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@...>
> > ---
> >  include/net/bluetooth/hci.h      |    8 ++++++++
> >  include/net/bluetooth/hci_core.h |    2 ++
> >  net/bluetooth/hci_event.c        |   31 +++++++++++++++++++++++++++++++
> >  3 files changed, 41 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 0290aeb..7d41bd7 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> >  <at>  <at>  -741,6 +741,14  <at>  <at>  struct hci_rp_read_bd_addr {
> >  	bdaddr_t bdaddr;
> >  } __packed;
> >  
> > +#define HCI_OP_READ_DATA_BLOCK_SIZE	0x100a
> > +struct hci_rp_read_data_block_size {
> > +	__u8     status;
> > +	__le16   max_acl_len;
> > +	__le16   block_len;
> > +	__le16   num_blocks;
> > +} __packed;
(Continue reading)


Gmane