2011-12-08 08:54:36

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

From: Andrei Emeltchenko <[email protected]>

Add processing of Read Local AMP Info in hciemu, the values returned are
used to emulate AMP HCI.
---
test/hciemu.c | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/test/hciemu.c b/test/hciemu.c
index f2879ba..ccd6cc3 100644
--- a/test/hciemu.c
+++ b/test/hciemu.c
@@ -45,6 +45,7 @@
#include <bluetooth/bluetooth.h>
#include <bluetooth/hci.h>
#include <bluetooth/hci_lib.h>
+#include <bluetooth/l2cap.h>

#define VHCI_DEV "/dev/vhci"

@@ -779,6 +780,36 @@ static void hci_info_param(uint16_t ocf, int plen, uint8_t *data)
}
}

+static void hci_status_param(uint16_t ocf, int plen, uint8_t *data)
+{
+ read_local_amp_info_rp ai;
+ uint8_t status;
+
+ const uint16_t ogf = OGF_STATUS_PARAM;
+
+ switch (ocf) {
+ case OCF_READ_LOCAL_AMP_INFO:
+ memset(&ai, 0, sizeof(ai));
+
+ /* BT only */
+ ai.amp_status = 0x01;
+ ai.max_pdu_size = htobl(L2CAP_DEFAULT_MTU);
+ ai.controller_type = HCI_AMP;
+ ai.max_amp_assoc_length = htobl(HCI_MAX_ACL_SIZE);
+ /* No flushing at all */
+ ai.max_flush_timeout = 0xFFFFFFFF;
+ ai.best_effort_flush_timeout = 0xFFFFFFFF;
+
+ command_complete(ogf, ocf, sizeof(ai), &ai);
+ break;
+
+ default:
+ status = 0x01;
+ command_complete(ogf, ocf, 1, &status);
+ break;
+ }
+}
+
static void hci_command(uint8_t *data)
{
hci_command_hdr *ch;
@@ -808,6 +839,10 @@ static void hci_command(uint8_t *data)
case OGF_INFO_PARAM:
hci_info_param(ocf, ch->plen, ptr);
break;
+
+ case OGF_STATUS_PARAM:
+ hci_status_param(ocf, ch->plen, ptr);
+ break;
}
}

--
1.7.4.1



2011-12-10 15:37:48

by Syam Sidhardhan

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hi Frederic,

On Fri, Dec 9, 2011 at 2:18 PM, Johan Hedberg <[email protected]> wrote:
> Hi Frederic,
>
> On Fri, Dec 09, 2011, Frederic Danis wrote:
>> >>fdanis@fdanis-linux:bluez (master %)$ make
>> >>make --no-print-directory all-am
>> >>   CC     test/hciemu.o
>> >>cc1: warnings being treated as errors
>> >>test/hciemu.c: In function ‘hci_host_control’:
>> >>test/hciemu.c:431: error: dereferencing pointer ‘({anonymous})’ does
>> >>break strict-aliasing rules
>> >>test/hciemu.c:431: note: initialized from here
>> >>make[1]: *** [test/hciemu.o] Error 1
>> >>make: *** [all] Error 2
>> >
>> >Cannot reproduce this problem on my Ubuntu 11.04. Are you sure the error is
>> >attributed to my patch?
>>
>> Yes, I replaced line 431 by:
>>       sa.sin_port = ba.b[4];
>> and there is no more build error.
>
> Ok, so the line *is* correct but the function reported by gcc isn't.
> This particular line was last changed way back in 2005 (check with git
> blame) by a commit from Marcel so I don't see how your issue could have
> been caused by recent changes. As I mentioned in the other email I'd
> expect you to provide a proper patch for this with a good commit message
> that explains why it is needed.
>
> Johan

I'm also getting the same error in my ubuntu 11.04 with a gcc version
(Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5.
This can be reproduce with a fresh git clone and build in the above
mentioned ubuntu linux environment.
More logs can be found here: http://pastie.org/2995677.

Syam
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

2011-12-09 08:48:28

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hi Frederic,

On Fri, Dec 09, 2011, Frederic Danis wrote:
> >>fdanis@fdanis-linux:bluez (master %)$ make
> >>make --no-print-directory all-am
> >> CC test/hciemu.o
> >>cc1: warnings being treated as errors
> >>test/hciemu.c: In function ‘hci_host_control’:
> >>test/hciemu.c:431: error: dereferencing pointer ‘({anonymous})’ does
> >>break strict-aliasing rules
> >>test/hciemu.c:431: note: initialized from here
> >>make[1]: *** [test/hciemu.o] Error 1
> >>make: *** [all] Error 2
> >
> >Cannot reproduce this problem on my Ubuntu 11.04. Are you sure the error is
> >attributed to my patch?
>
> Yes, I replaced line 431 by:
> sa.sin_port = ba.b[4];
> and there is no more build error.

Ok, so the line *is* correct but the function reported by gcc isn't.
This particular line was last changed way back in 2005 (check with git
blame) by a commit from Marcel so I don't see how your issue could have
been caused by recent changes. As I mentioned in the other email I'd
expect you to provide a proper patch for this with a good commit message
that explains why it is needed.

Johan

2011-12-09 08:43:38

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hi,

On Fri, Dec 09, 2011, Emeltchenko Andrei wrote:
> > This patch breaks the build on Ubuntu 10.10 32 bits with messages:
> >
> > fdanis@fdanis-linux:bluez (master %)$ make
> > make --no-print-directory all-am
> > CC test/hciemu.o
> > cc1: warnings being treated as errors
> > test/hciemu.c: In function ‘hci_host_control’:
> > test/hciemu.c:431: error: dereferencing pointer ‘({anonymous})’ does
> > break strict-aliasing rules
> > test/hciemu.c:431: note: initialized from here
> > make[1]: *** [test/hciemu.o] Error 1
> > make: *** [all] Error 2
>
> Cannot reproduce this problem on my Ubuntu 11.04. Are you sure the error is
> attributed to my patch?

In the latest git hciemu.c:431 is not inside the hci_host_control
function so it seems Frederic is using some older version of the code.
In any case I'd expect him to provide a patch for this if necessary
since it seems he is the only one who is able to reproduce the issue.

Johan

2011-12-09 08:42:55

by Frederic Danis

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hello Andrei,

Le 09/12/2011 09:16, Emeltchenko Andrei a écrit :
> Hi Frederic,
>
> On Thu, Dec 08, 2011 at 03:58:40PM +0100, Frederic Danis wrote:
>> Hello,
>>
>> Le 08/12/2011 10:40, Johan Hedberg a écrit :
>>> Hi Andrei,
>>>
>>> On Thu, Dec 08, 2011, Emeltchenko Andrei wrote:
>>>> Add processing of Read Local AMP Info in hciemu, the values returned are
>>>> used to emulate AMP HCI.
>>>> ---
>>>> test/hciemu.c | 35 +++++++++++++++++++++++++++++++++++
>>>> 1 files changed, 35 insertions(+), 0 deletions(-)
>>>
>>> Both patches have been applied. Thanks.
>>>
>>> Johan
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>> This patch breaks the build on Ubuntu 10.10 32 bits with messages:
>>
>> fdanis@fdanis-linux:bluez (master %)$ make
>> make --no-print-directory all-am
>> CC test/hciemu.o
>> cc1: warnings being treated as errors
>> test/hciemu.c: In function ‘hci_host_control’:
>> test/hciemu.c:431: error: dereferencing pointer ‘({anonymous})’ does
>> break strict-aliasing rules
>> test/hciemu.c:431: note: initialized from here
>> make[1]: *** [test/hciemu.o] Error 1
>> make: *** [all] Error 2
>
> Cannot reproduce this problem on my Ubuntu 11.04. Are you sure the error is
> attributed to my patch?

Yes, I replaced line 431 by:
sa.sin_port = ba.b[4];
and there is no more build error.

Best regards

Fred


--
Frederic Danis Open Source Technology Centre
[email protected] Intel Corporation


2011-12-09 08:16:39

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hi Frederic,

On Thu, Dec 08, 2011 at 03:58:40PM +0100, Frederic Danis wrote:
> Hello,
>
> Le 08/12/2011 10:40, Johan Hedberg a écrit :
> >Hi Andrei,
> >
> >On Thu, Dec 08, 2011, Emeltchenko Andrei wrote:
> >>Add processing of Read Local AMP Info in hciemu, the values returned are
> >>used to emulate AMP HCI.
> >>---
> >> test/hciemu.c | 35 +++++++++++++++++++++++++++++++++++
> >> 1 files changed, 35 insertions(+), 0 deletions(-)
> >
> >Both patches have been applied. Thanks.
> >
> >Johan
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> >the body of a message to [email protected]
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> This patch breaks the build on Ubuntu 10.10 32 bits with messages:
>
> fdanis@fdanis-linux:bluez (master %)$ make
> make --no-print-directory all-am
> CC test/hciemu.o
> cc1: warnings being treated as errors
> test/hciemu.c: In function ‘hci_host_control’:
> test/hciemu.c:431: error: dereferencing pointer ‘({anonymous})’ does
> break strict-aliasing rules
> test/hciemu.c:431: note: initialized from here
> make[1]: *** [test/hciemu.o] Error 1
> make: *** [all] Error 2

Cannot reproduce this problem on my Ubuntu 11.04. Are you sure the error is
attributed to my patch?

Best regards
Andrei Emeltchenko


2011-12-08 14:58:40

by Frederic Danis

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hello,

Le 08/12/2011 10:40, Johan Hedberg a ?crit :
> Hi Andrei,
>
> On Thu, Dec 08, 2011, Emeltchenko Andrei wrote:
>> Add processing of Read Local AMP Info in hciemu, the values returned are
>> used to emulate AMP HCI.
>> ---
>> test/hciemu.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 files changed, 35 insertions(+), 0 deletions(-)
>
> Both patches have been applied. Thanks.
>
> Johan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

This patch breaks the build on Ubuntu 10.10 32 bits with messages:

fdanis@fdanis-linux:bluez (master %)$ make
make --no-print-directory all-am
CC test/hciemu.o
cc1: warnings being treated as errors
test/hciemu.c: In function ?hci_host_control?:
test/hciemu.c:431: error: dereferencing pointer ?({anonymous})? does
break strict-aliasing rules
test/hciemu.c:431: note: initialized from here
make[1]: *** [test/hciemu.o] Error 1
make: *** [all] Error 2

Regards

Fred

--
Frederic Danis Open Source Technology Centre
[email protected] Intel Corporation


2011-12-08 09:40:32

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH hciemu v2 1/2] Process Read Local AMP Info in hciemu

Hi Andrei,

On Thu, Dec 08, 2011, Emeltchenko Andrei wrote:
> Add processing of Read Local AMP Info in hciemu, the values returned are
> used to emulate AMP HCI.
> ---
> test/hciemu.c | 35 +++++++++++++++++++++++++++++++++++
> 1 files changed, 35 insertions(+), 0 deletions(-)

Both patches have been applied. Thanks.

Johan

2011-12-08 08:54:37

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH hciemu v2 2/2] Print features for AMP devices

From: Andrei Emeltchenko <[email protected]>

Check for HCI device type, the magic shift is due to dev type is packed
to dev_info type.
---
tools/hciconfig.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index 35b80b1..a7249db 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -1870,7 +1870,7 @@ static void print_dev_info(int ctl, struct hci_dev_info *di)
st->byte_tx, st->acl_tx, st->sco_tx, st->cmd_tx, st->err_tx);

if (all && !hci_test_bit(HCI_RAW, &di->flags) &&
- bacmp(&di->bdaddr, BDADDR_ANY)) {
+ (bacmp(&di->bdaddr, BDADDR_ANY) || (di->type >> 4))) {
print_dev_features(di, 0);
print_pkt_type(di);
print_link_policy(di);
--
1.7.4.1