All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools/btattach: Add detach option
@ 2017-02-01 17:14 Carlo Caione
  2017-02-01 19:41 ` Marcel Holtmann
  0 siblings, 1 reply; 6+ messages in thread
From: Carlo Caione @ 2017-02-01 17:14 UTC (permalink / raw)
  To: linux-bluetooth, linux; +Cc: Carlo Caione

From: Carlo Caione <carlo@endlessm.com>

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 tools/btattach.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/btattach.c b/tools/btattach.c
index 5adbc8d..f1f115d 100644
--- a/tools/btattach.c
+++ b/tools/btattach.c
@@ -194,6 +194,7 @@ static void usage(void)
 		"\t-P, --protocol <proto> Specify protocol type\n"
 		"\t-S, --speed <baudrate> Specify which baudrate to use\n"
 		"\t-N, --noflowctl        Disable flow control\n"
+		"\t-D, --detach           Open device and then fork\n"
 		"\t-h, --help             Show help options\n");
 }
 
@@ -203,6 +204,7 @@ static const struct option main_options[] = {
 	{ "protocol", required_argument, NULL, 'P' },
 	{ "speed",    required_argument, NULL, 'S' },
 	{ "noflowctl",no_argument,       NULL, 'N' },
+	{ "detach",   no_argument,       NULL, 'D' },
 	{ "version",  no_argument,       NULL, 'v' },
 	{ "help",     no_argument,       NULL, 'h' },
 	{ }
@@ -230,7 +232,7 @@ static const struct {
 int main(int argc, char *argv[])
 {
 	const char *bredr_path = NULL, *amp_path = NULL, *proto = NULL;
-	bool flowctl = true, raw_device = false;
+	bool flowctl = true, raw_device = false, detach = false;
 	sigset_t mask;
 	int exit_status, count = 0, proto_id = HCI_UART_H4;
 	unsigned int speed = B115200;
@@ -263,6 +265,9 @@ int main(int argc, char *argv[])
 		case 'N':
 			flowctl = false;
 			break;
+		case 'D':
+			detach = true;
+			break;
 		case 'R':
 			raw_device = true;
 			break;
@@ -348,6 +353,9 @@ int main(int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
+	if (detach && fork())
+		return 0;
+
 	exit_status = mainloop_run();
 
 	return exit_status;
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/btattach: Add detach option
  2017-02-01 17:14 [PATCH] tools/btattach: Add detach option Carlo Caione
@ 2017-02-01 19:41 ` Marcel Holtmann
  2017-02-01 22:19   ` Carlo Caione
  0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2017-02-01 19:41 UTC (permalink / raw)
  To: Carlo Caione; +Cc: Bluez mailing list, linux, Carlo Caione

Hi Carl,

> Signed-off-by: Carlo Caione <carlo@endlessm.com>

user space does not use signed off by statements.

> ---
> tools/btattach.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/btattach.c b/tools/btattach.c
> index 5adbc8d..f1f115d 100644
> --- a/tools/btattach.c
> +++ b/tools/btattach.c
> @@ -194,6 +194,7 @@ static void usage(void)
> 		"\t-P, --protocol <proto> Specify protocol type\n"
> 		"\t-S, --speed <baudrate> Specify which baudrate to use\n"
> 		"\t-N, --noflowctl        Disable flow control\n"
> +		"\t-D, --detach           Open device and then fork\n"
> 		"\t-h, --help             Show help options\n”);

And is this really a good idea. I think that I had removed all the fork calls from the standard tools. Mainly since we fail to maintain the signals and more important child signals correctly. So we quickly end up with zombies or orphaned processes. My thinking instead was to leave this to systemd to handle.

In addition, we are close to getting serdev bus which means btattach will be something of the past really soon.

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/btattach: Add detach option
  2017-02-01 19:41 ` Marcel Holtmann
@ 2017-02-01 22:19   ` Carlo Caione
  2017-02-02  3:25     ` Marcel Holtmann
  0 siblings, 1 reply; 6+ messages in thread
From: Carlo Caione @ 2017-02-01 22:19 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Carlo Caione, Bluez mailing list, Linux Upstreaming Team

On Wed, Feb 1, 2017 at 8:41 PM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
> Hi Carl,
>
>> Signed-off-by: Carlo Caione <carlo@endlessm.com>
>
> user space does not use signed off by statements.
>
>> ---
>> tools/btattach.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/btattach.c b/tools/btattach.c
>> index 5adbc8d..f1f115d 100644
>> --- a/tools/btattach.c
>> +++ b/tools/btattach.c
>> @@ -194,6 +194,7 @@ static void usage(void)
>>               "\t-P, --protocol <proto> Specify protocol type\n"
>>               "\t-S, --speed <baudrate> Specify which baudrate to use\n"
>>               "\t-N, --noflowctl        Disable flow control\n"
>> +             "\t-D, --detach           Open device and then fork\n"
>>               "\t-h, --help             Show help options\n=E2=80=9D);
>
> And is this really a good idea. I think that I had removed all the fork c=
alls from the standard tools. Mainly since we fail to maintain the signals =
and more important child signals correctly. So we quickly end up with zombi=
es or orphaned processes. My thinking instead was to leave this to systemd =
to handle.

Interesting. Any pointer how to achieve that?
The problem is that I have btattach called by a systemd unit file to
download the firmware to the bluetooth transceiver. I also have
another systemd unit that must be called strictly after btattach has
done uploading the firmware. Since btattach is hanging on the port
AFAIU there is no way I can tell exactly when it is done uploading the
firmware. That's why I was adding this detach option, to mark the unit
as 'Type=3Dforking' and enforcing the ordering with
'Before=3Dnext.service`.
Anything I am missing?


--=20
Carlo Caione  |  +39.340.80.30.096  |  Endless

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/btattach: Add detach option
  2017-02-01 22:19   ` Carlo Caione
@ 2017-02-02  3:25     ` Marcel Holtmann
  2017-02-02  9:40       ` Carlo Caione
  0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2017-02-02  3:25 UTC (permalink / raw)
  To: Carlo Caione; +Cc: Carlo Caione, Bluez mailing list, Linux Upstreaming Team

Hi Carlo,

>>> ---
>>> tools/btattach.c | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/tools/btattach.c b/tools/btattach.c
>>> index 5adbc8d..f1f115d 100644
>>> --- a/tools/btattach.c
>>> +++ b/tools/btattach.c
>>> @@ -194,6 +194,7 @@ static void usage(void)
>>>              "\t-P, --protocol <proto> Specify protocol type\n"
>>>              "\t-S, --speed <baudrate> Specify which baudrate to use\n"
>>>              "\t-N, --noflowctl        Disable flow control\n"
>>> +             "\t-D, --detach           Open device and then fork\n"
>>>              "\t-h, --help             Show help options\n”);
>> 
>> And is this really a good idea. I think that I had removed all the fork calls from the standard tools. Mainly since we fail to maintain the signals and more important child signals correctly. So we quickly end up with zombies or orphaned processes. My thinking instead was to leave this to systemd to handle.
> 
> Interesting. Any pointer how to achieve that?
> The problem is that I have btattach called by a systemd unit file to
> download the firmware to the bluetooth transceiver. I also have
> another systemd unit that must be called strictly after btattach has
> done uploading the firmware. Since btattach is hanging on the port
> AFAIU there is no way I can tell exactly when it is done uploading the
> firmware. That's why I was adding this detach option, to mark the unit
> as 'Type=forking' and enforcing the ordering with
> 'Before=next.service`.
> Anything I am missing?

why do you need to know when the firmware uploading has completed? What hardware is this?

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/btattach: Add detach option
  2017-02-02  3:25     ` Marcel Holtmann
@ 2017-02-02  9:40       ` Carlo Caione
  2017-02-02 10:28         ` Marcel Holtmann
  0 siblings, 1 reply; 6+ messages in thread
From: Carlo Caione @ 2017-02-02  9:40 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Bluez mailing list, Carlo Caione, Linux Upstreaming Team

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

On Thu, 2 Feb 2017 at 04:25, Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Carlo,
>
> >>> ---
> >>> tools/btattach.c | 10 +++++++++-
> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/btattach.c b/tools/btattach.c
> >>> index 5adbc8d..f1f115d 100644
> >>> --- a/tools/btattach.c
> >>> +++ b/tools/btattach.c
> >>> @@ -194,6 +194,7 @@ static void usage(void)
> >>>              "\t-P, --protocol <proto> Specify protocol type\n"
> >>>              "\t-S, --speed <baudrate> Specify which baudrate to use\n"
> >>>              "\t-N, --noflowctl        Disable flow control\n"
> >>> +             "\t-D, --detach           Open device and then fork\n"
> >>>              "\t-h, --help             Show help options\n”);
> >>
> >> And is this really a good idea. I think that I had removed all the fork
> calls from the standard tools. Mainly since we fail to maintain the signals
> and more important child signals correctly. So we quickly end up with
> zombies or orphaned processes. My thinking instead was to leave this to
> systemd to handle.
> >
> > Interesting. Any pointer how to achieve that?
> > The problem is that I have btattach called by a systemd unit file to
> > download the firmware to the bluetooth transceiver. I also have
> > another systemd unit that must be called strictly after btattach has
> > done uploading the firmware. Since btattach is hanging on the port
> > AFAIU there is no way I can tell exactly when it is done uploading the
> > firmware. That's why I was adding this detach option, to mark the unit
> > as 'Type=forking' and enforcing the ordering with
> > 'Before=next.service`.
> > Anything I am missing?
>
> why do you need to know when the firmware uploading has completed? What
> hardware is this?


The hardware is a custom ARM board based on an Amlogic SOC.
More specifically I need btattach systemd unit to run before
systemd-rfkill. This is because the rfkill driver is actually driving the
GPIO enable of the transceiver, so if systemd-rfkill runs before btattach
has completed telling to disable the radio (because we rebooted with the BT
off) this just interrupts the firmware downloading leaving the transceiver
in a non-working state.

> --
Carlo Caione  |  +39.340.80.30.096  |  Endless

[-- Attachment #2: Type: text/html, Size: 3632 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/btattach: Add detach option
  2017-02-02  9:40       ` Carlo Caione
@ 2017-02-02 10:28         ` Marcel Holtmann
  0 siblings, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2017-02-02 10:28 UTC (permalink / raw)
  To: Carlo Caione; +Cc: Bluez mailing list, Carlo Caione, Linux Upstreaming Team

Hi Carlo,

> >>> ---
> >>> tools/btattach.c | 10 +++++++++-
> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/btattach.c b/tools/btattach.c
> >>> index 5adbc8d..f1f115d 100644
> >>> --- a/tools/btattach.c
> >>> +++ b/tools/btattach.c
> >>> @@ -194,6 +194,7 @@ static void usage(void)
> >>>              "\t-P, --protocol <proto> Specify protocol type\n"
> >>>              "\t-S, --speed <baudrate> Specify which baudrate to use\n"
> >>>              "\t-N, --noflowctl        Disable flow control\n"
> >>> +             "\t-D, --detach           Open device and then fork\n"
> >>>              "\t-h, --help             Show help options\n”);
> >>
> >> And is this really a good idea. I think that I had removed all the fork calls from the standard tools. Mainly since we fail to maintain the signals and more important child signals correctly. So we quickly end up with zombies or orphaned processes. My thinking instead was to leave this to systemd to handle.
> >
> > Interesting. Any pointer how to achieve that?
> > The problem is that I have btattach called by a systemd unit file to
> > download the firmware to the bluetooth transceiver. I also have
> > another systemd unit that must be called strictly after btattach has
> > done uploading the firmware. Since btattach is hanging on the port
> > AFAIU there is no way I can tell exactly when it is done uploading the
> > firmware. That's why I was adding this detach option, to mark the unit
> > as 'Type=forking' and enforcing the ordering with
> > 'Before=next.service`.
> > Anything I am missing?
> 
> why do you need to know when the firmware uploading has completed? What hardware is this?
> 
> The hardware is a custom ARM board based on an Amlogic SOC.
> More specifically I need btattach systemd unit to run before systemd-rfkill. This is because the rfkill driver is actually driving the GPIO enable of the transceiver, so if systemd-rfkill runs before btattach has completed telling to disable the radio (because we rebooted with the BT off) this just interrupts the firmware downloading leaving the transceiver in a non-working state.

this whole design mess with RFKILL switches for GPIO reset lines is a problem. We start fixing this for Intel and Broadcom based Bluetooth chips. So I would prefer if we can get this fixed inside the kernel. Also serdev bus is coming that will make btattach obsolete for hardwired Bluetooth UART chips.

And frankly, I would prefer to add RFKILL handling to btattach then trying to introduce a fork() which only existence to is to do some syncing with systemd-rfkill.

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-02 10:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-01 17:14 [PATCH] tools/btattach: Add detach option Carlo Caione
2017-02-01 19:41 ` Marcel Holtmann
2017-02-01 22:19   ` Carlo Caione
2017-02-02  3:25     ` Marcel Holtmann
2017-02-02  9:40       ` Carlo Caione
2017-02-02 10:28         ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.