From: Tzu-Jung Lee <[email protected]>
---
tools/hciattach.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hciattach.c b/tools/hciattach.c
index 1904ac5..542b5b3 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -1276,7 +1276,7 @@ static void usage(void)
{
printf("hciattach - HCI UART driver initialization utility\n");
printf("Usage:\n");
- printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [bdaddr]\n");
+ printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]\n");
printf("\thciattach -l\n");
}
--
2.0.4
Hi Luiz,
Any comment for the patch 2/3 ?
Thanks.
Roy
On Mon, Sep 15, 2014 at 11:23 AM, Tzu-Jung Lee <[email protected]> wrote:
> Hi Luiz,
>
> On Mon, Sep 15, 2014 at 12:00 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi,
>>
>> On Tue, Sep 9, 2014 at 12:21 AM, <[email protected]> wrote:
>>> From: Tzu-Jung Lee <[email protected]>
>>>
>>> ---
>>> tools/hciattach.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/hciattach.c b/tools/hciattach.c
>>> index 1904ac5..542b5b3 100644
>>> --- a/tools/hciattach.c
>>> +++ b/tools/hciattach.c
>>> @@ -1276,7 +1276,7 @@ static void usage(void)
>>> {
>>> printf("hciattach - HCI UART driver initialization utility\n");
>>> printf("Usage:\n");
>>> - printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [bdaddr]\n");
>>> + printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]\n");
>>> printf("\thciattach -l\n");
>>> }
>>>
>>> --
>>> 2.0.4
>>
>> It doesn't look like you are doing anything with sleep|nosleep here,
>> also you should probably a add more information regarding what it is
>> fixing.
>>
>
> I probably should have separated these 3 patches to 2 different sets
> though they are all trivial fixes.
>
>
> The first one attempts to reveal the missing [speed | nospeed] to the user:
>
> Currently, the argument parsing code in the hciattach.c expect very
> specific order of the arguments:
>
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed]
> <tty> <type | id> [speed] [flow|noflow] [bdaddr]
>
> For example, if an user would like to specify the BDADDR, none of the
> following works:
>
> hciattach TTY any BDADDR
> hciattach TTY any SPEED BDADDR
> hciattach TTY any SPEED NOFLOW BDADDR
>
> The user has to put the BDADDR exactly in the 4th argument after <type
> | id>, ex:
>
> hciattach TTY any SPEED NOFLOW NOSLEEP BDADDR
>
> And this is impossible to figure out without looking at the source code.
> The patch only helps a little (hope so) by revealing the missing
> argument to the user.
>
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed]
> <tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]
>
> Though I do agree the information is still too little, and we probably
> should rewrite the argument parsing code to remove the enforcement of
> order.
> Otherwise, the precise usage syntax for the current logic should be:
>
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
> <type | id> [speed]
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
> <type | id> [speed flow|noflow]
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
> <type | id> [speed flow|noflow] sleep|nosleep]
> hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
> <type | id> [speed flow|noflow sleep|nosleep bdaddr]
>
> Even though, user still need to figure out what suppose to be put in
> the flow/sleep if he only wants to change the bdaddr without changing
> other default settings.
>
>
> The second patch speed up the firmware loading speed by raising the
> UART baudrate before (and after) loading firmware.
> This helps our project reduce the hciattach from 7+ seconds, to 750 ms.
> The tricky part for the controller is that it drops the UART back to
> 115200 after firmware loading.
> So we still need to update the baudrate again after firmware is loaded.
>
> Thanks.
> Roy
>
>> --
>> Luiz Augusto von Dentz
Hi Luiz,
On Mon, Sep 15, 2014 at 12:00 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Tue, Sep 9, 2014 at 12:21 AM, <[email protected]> wrote:
>> From: Tzu-Jung Lee <[email protected]>
>>
>> ---
>> tools/hciattach.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/hciattach.c b/tools/hciattach.c
>> index 1904ac5..542b5b3 100644
>> --- a/tools/hciattach.c
>> +++ b/tools/hciattach.c
>> @@ -1276,7 +1276,7 @@ static void usage(void)
>> {
>> printf("hciattach - HCI UART driver initialization utility\n");
>> printf("Usage:\n");
>> - printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [bdaddr]\n");
>> + printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]\n");
>> printf("\thciattach -l\n");
>> }
>>
>> --
>> 2.0.4
>
> It doesn't look like you are doing anything with sleep|nosleep here,
> also you should probably a add more information regarding what it is
> fixing.
>
I probably should have separated these 3 patches to 2 different sets
though they are all trivial fixes.
The first one attempts to reveal the missing [speed | nospeed] to the user:
Currently, the argument parsing code in the hciattach.c expect very
specific order of the arguments:
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed]
<tty> <type | id> [speed] [flow|noflow] [bdaddr]
For example, if an user would like to specify the BDADDR, none of the
following works:
hciattach TTY any BDADDR
hciattach TTY any SPEED BDADDR
hciattach TTY any SPEED NOFLOW BDADDR
The user has to put the BDADDR exactly in the 4th argument after <type
| id>, ex:
hciattach TTY any SPEED NOFLOW NOSLEEP BDADDR
And this is impossible to figure out without looking at the source code.
The patch only helps a little (hope so) by revealing the missing
argument to the user.
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed]
<tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]
Though I do agree the information is still too little, and we probably
should rewrite the argument parsing code to remove the enforcement of
order.
Otherwise, the precise usage syntax for the current logic should be:
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
<type | id> [speed]
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
<type | id> [speed flow|noflow]
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
<type | id> [speed flow|noflow] sleep|nosleep]
hciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty>
<type | id> [speed flow|noflow sleep|nosleep bdaddr]
Even though, user still need to figure out what suppose to be put in
the flow/sleep if he only wants to change the bdaddr without changing
other default settings.
The second patch speed up the firmware loading speed by raising the
UART baudrate before (and after) loading firmware.
This helps our project reduce the hciattach from 7+ seconds, to 750 ms.
The tricky part for the controller is that it drops the UART back to
115200 after firmware loading.
So we still need to update the baudrate again after firmware is loaded.
Thanks.
Roy
> --
> Luiz Augusto von Dentz
Hi,
On Tue, Sep 9, 2014 at 12:21 AM, <[email protected]> wrote:
> From: Tzu-Jung Lee <[email protected]>
>
> ---
> tools/hciattach.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/hciattach.c b/tools/hciattach.c
> index 1904ac5..542b5b3 100644
> --- a/tools/hciattach.c
> +++ b/tools/hciattach.c
> @@ -1276,7 +1276,7 @@ static void usage(void)
> {
> printf("hciattach - HCI UART driver initialization utility\n");
> printf("Usage:\n");
> - printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [bdaddr]\n");
> + printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [sleep|nosleep] [bdaddr]\n");
> printf("\thciattach -l\n");
> }
>
> --
> 2.0.4
It doesn't look like you are doing anything with sleep|nosleep here,
also you should probably a add more information regarding what it is
fixing.
--
Luiz Augusto von Dentz
From: Tzu-Jung Lee <[email protected]>
---
tools/hciattach_bcm43xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hciattach_bcm43xx.c b/tools/hciattach_bcm43xx.c
index 630220e..53dbce5 100644
--- a/tools/hciattach_bcm43xx.c
+++ b/tools/hciattach_bcm43xx.c
@@ -220,7 +220,7 @@ static int bcm43xx_set_clock(int fd, unsigned char clock)
static int bcm43xx_load_firmware(int fd, const char *fw)
{
unsigned char cmd[] = { HCI_COMMAND_PKT, 0x2e, 0xfc, 0x00 };
- struct timespec tm_mode = { 0, 50000 };
+ struct timespec tm_mode = { 0, 50000000 };
struct timespec tm_ready = { 0, 2000000 };
unsigned char resp[CC_MIN_SIZE];
unsigned char tx_buf[1024];
--
2.0.4
From: Tzu-Jung Lee <[email protected]>
---
tools/hciattach_bcm43xx.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tools/hciattach_bcm43xx.c b/tools/hciattach_bcm43xx.c
index cb4bfb9..630220e 100644
--- a/tools/hciattach_bcm43xx.c
+++ b/tools/hciattach_bcm43xx.c
@@ -359,9 +359,27 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
if (bcm43xx_locate_patch(FIRMWARE_DIR, chip_name, fw_path)) {
fprintf(stderr, "Patch not found, continue anyway\n");
} else {
+ /* Speed up firmware loading */
+ if (speed > 3000000 && bcm43xx_set_clock(fd, BCM43XX_CLOCK_48))
+ return -1;
+
+ if (bcm43xx_set_speed(fd, speed))
+ return -1;
+
+ if (set_speed(fd, ti, speed))
+ return -1;
+
if (bcm43xx_load_firmware(fd, fw_path))
return -1;
+ /*
+ * The controller reset the uart speed to 115200 after
+ * firmware loading, so we have to reset the terminal
+ * correspondingly
+ */
+ if (set_speed(fd, ti, 115200))
+ return -1;
+
if (bcm43xx_reset(fd))
return -1;
}
@@ -375,5 +393,9 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
if (bcm43xx_set_speed(fd, speed))
return -1;
+ if (set_speed(fd, ti, speed))
+ return -1;
+
+
return 0;
}
--
2.0.4