On Fri, Oct 02, 2020 at 09:50:22AM -0500, Denis Kenzior wrote:
> Hi Lars,
>
> On 10/1/20 6:42 AM, poesc...@lemonage.de wrote:
> > From: Lars Poeschel <poesc...@lemonage.de>
> >
> > Current implementation uses a gpio level of 1 for powering on quectel
> > modems using a gpio and a level of 0 for powering off.
> > This is wrong. Quectel modems use pulses for either power on and power
> > off. They turn on by the first pulse and turn then off by the next
> > pulse. The pulse length varies between different modems.
> > For power on the longest I could in the quectel hardware is "more than
> > 2 seconds" from Quectel M95 Hardware Design Manual.
> > For Quectel EC21 this is ">= 100 ms".
> > For Quectel MC60 this is "recommended to be 100 ms".
> > For Quectel UC15 this is "at least 0.1 s".
> > For power off the four modems in question vary between a minimum pulse
> > length of 600-700ms.
> > This implements a 2100ms pulse for power on and 750ms for power off.
> > ---
> > plugins/quectel.c | 33 ++++++++++++++++++++++++++++++---
> > 1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/plugins/quectel.c b/plugins/quectel.c
> > index 6456775d..61ac906e 100644
> > --- a/plugins/quectel.c
> > +++ b/plugins/quectel.c
> > @@ -234,10 +234,21 @@ static void close_ngsm(struct ofono_modem *modem)
> > ofono_warn("Failed to restore line discipline");
> > }
> > +static gboolean gpio_power_off_cb(gpointer user_data)
> > +{
> > + struct ofono_modem *modem = (struct ofono_modem *)user_data;
> > + struct quectel_data *data = ofono_modem_get_data(modem);
> > + const uint32_t gpio_value = 0;
> > +
> > + l_gpio_writer_set(data->gpio, 1, &gpio_value);
> > + ofono_modem_set_powered(modem, FALSE);
> > + return false;
> > +}
> > +
>
> Ok, this makes sense now...
>
> > static void close_serial(struct ofono_modem *modem)
> > {
> > struct quectel_data *data = ofono_modem_get_data(modem);
> > - uint32_t gpio_value = 0;
> > + uint32_t gpio_value = 1;
> > DBG("%p", modem);
> > @@ -258,8 +269,12 @@ static void close_serial(struct ofono_modem *modem)
> > else
> > close_ngsm(modem);
> > - l_gpio_writer_set(data->gpio, 1, &gpio_value);
> > - ofono_modem_set_powered(modem, FALSE);
> > + if (data->gpio) {
> > + l_gpio_writer_set(data->gpio, 1, &gpio_value);
> > + g_timeout_add(750, gpio_power_off_cb, modem);
>
> Have you considered what happens if the gpio_power_on_cb timeout is still
> running here? For example, if the modem is turned on / off quickly?
>
> Maybe the old timeout should be canceled just in case?
I am in a big luck here! :-)
Setting "Powered" on "org.ofono.Modem" is synchronous. And even if
another process is trying to unset "Powered" in the meantime. This is
blocked:
dbus.exceptions.DBusException: org.ofono.Error.InProgress: Operation already in
progress
And powering on the modem is taking as long as the first replies from
the modem are received. This a lot of time after the timeout happened.
> > + } else
> > + ofono_modem_set_powered(modem, FALSE);
> > +
> > }
> > static void dbus_hw_reply_properties(struct dbus_hw *hw)
> > @@ -1096,6 +1111,15 @@ static void init_timeout_cb(struct l_timeout
> > *timeout, void *user_data)
> > l_timeout_modify_ms(timeout, 500);
> > }
> > +static gboolean gpio_power_on_cb(gpointer user_data)
> > +{
> > + struct quectel_data *data = user_data;
> > + const uint32_t gpio_value = 0;
> > +
> > + l_gpio_writer_set(data->gpio, 1, &gpio_value);
> > + return false;
> > +}
>
> It seems that this timeout is completely independent of
> ofono_modem_set_powered(TRUE), so the core won't prevent the modem from
> being powered off while this is running...
What exactly are you concerned about ? What should not work ?
The ofono_modem_set_powered(TRUE) is done later either in cpin_cb or
qinistat_cb, after communication with the modem and mux are properly
set up.
Powering off during this phase is prevented by ofono:
root@bboxvx:/usr/lib/ofono/test# ./enable-modem & sleep 11; ./disable-modem
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
SystemPath
Connecting modem /quectel_0...
ofonod[507]: ../git/plugins/quectel.c:quectel_enable() 0x583b80
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
Device
ofonod[507]: ../git/plugins/quectel.c:open_serial() 0x583b80
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
RtsCts
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
Device
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: UART: < \r\nR
ofonod[507]: UART: < DY\r\n
ofonod[507]: UART: < \r\n
ofonod[507]: UART: < +CFUN: 1\r\n
ofonod[507]: ../git/plugins/quectel.c:init_timeout_cb() 0x583b80
ofonod[507]: UART: > AT\r
ofonod[507]: UART: < A
ofonod[507]: UART: < T\r\r\nOK\r
ofonod[507]: UART: < \n
ofonod[507]: ../git/plugins/quectel.c:init_cmd_cb() 0x583b80
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
RtsCts
ofonod[507]: UART: > ATE0\r
ofonod[507]: UART: < ATE
ofonod[507]: UART: < 0\r
ofonod[507]: UART: < \r
ofonod[507]: UART: < \nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:ate_cb() 0x583b80
ofonod[507]: UART: > AT+CGMM\r
ofonod[507]: UART: < \r
ofonod[507]: UART: < \nEC21\r\n
ofonod[507]: UART: < \r\nOK\r
ofonod[507]: ../git/plugins/quectel.c:cgmm_cb() 0x583b80 ok 1
ofonod[507]: ../git/plugins/quectel.c:cgmm_cb() 0x583b80 model EC21
ofonod[507]: UART: > AT+CMUX=0,0,5,127,10,3,30,10,2\r
ofonod[507]: UART: < \n
ofonod[507]: UART: < \r
ofonod[507]: UART: < \nOK\r\n
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property Mux
ofonod[507]: ../git/plugins/quectel.c:cmux_cb() 0x583b80
ofonod[507]: ../git/plugins/quectel.c:cmux_ngsm() 0x583b80
ofonod[507]: ../git/src/modem.c:set_modem_property() modem 0x583b80 property
Modem
ofonod[507]: ../git/src/modem.c:unregister_property() property 0x57b088
ofonod[507]: ../git/src/modem.c:set_modem_property() modem 0x583b80 property Aux
ofonod[507]: ../git/src/modem.c:unregister_property() property 0x581070
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/quectel.c:mux_ready_cb() 0x583b80
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
Modem
ofonod[507]: ../git/plugins/quectel.c:open_ttys() 0x583b80
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
Modem
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property Aux
ofonod[507]: ../git/plugins/quectel.c:setup_aux() 0x583b80
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: Aux: > ATE0; &C0; +CMEE=1\r
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: Aux: < \r\nOK\r\n
ofonod[507]: Aux: > AT+QURCCFG="urcport","uart1"\r
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: Aux: < \r\nOK\r\n
ofonod[507]: Aux: > AT+CFUN?\r
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: ../git/plugins/udevng.c:add_serial_device() Device is missing
required OFONO_DRIVER property
ofonod[507]: Aux: < \r\n+CFUN: 1\r\n\r\nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:cfun_query() 0x583b80 ok 1
ofonod[507]: ../git/plugins/quectel.c:cfun_enable() 0x583b80 ok 1
ofonod[507]: Aux: > AT+CFUN=4\r
ofonod[507]: ../git/src/modem.c:get_modem_property() modem 0x583b80 property
SystemPath
==================================
Disconnecting modem /quectel_0...
Traceback (most recent call last):
File "./disable-modem", line 20, in <module>
modem.SetProperty("Powered", dbus.Boolean(0), timeout = 120)
File "/usr/lib/python3.8/site-packages/dbus/proxies.py", line 72, in __call__
return self._proxy_method(*args, **keywords)
File "/usr/lib/python3.8/site-packages/dbus/proxies.py", line 141, in __call__
return self._connection.call_blocking(self._named_service,
File "/usr/lib/python3.8/site-packages/dbus/connection.py", line 652, in
call_blocking
reply_message = self.send_message_with_reply_and_block(
dbus.exceptions.DBusException: org.ofono.Error.InProgress: Operation already in
progress
=================================
root@bboxvx:/usr/lib/ofono/test# ofonod[507]: Aux: < \r\nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:cfun_cb() 0x583b80 ok 1
ofonod[507]: ../git/plugins/quectel.c:dbus_hw_enable() 0x583b80
ofonod[507]: Aux: > AT+CPIN?\r
ofonod[507]: Aux: < \r\n+QIND: PB DONE\r\n
ofonod[507]: ../git/plugins/quectel.c:qind_notify() 0x583b80
ofonod[507]: Aux: < \r\n+CPIN: READY\r\n\r\nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:sim_state_cb() 0x583b80 present 1
ofonod[507]: Aux: > AT+CPIN?\r
ofonod[507]: Aux: < \r\n+CPIN: READY\r\n\r\nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:cpin_cb() 0x583b80
ofonod[507]: ../git/plugins/quectel.c:init_timer_cb() 0x583b80
ofonod[507]: Aux: > AT+QINISTAT\r
ofonod[507]: Aux: < \r\n+QINISTAT: 7\r\n\r\nOK\r\n
ofonod[507]: ../git/plugins/quectel.c:qinistat_cb() 0x583b80
ofonod[507]: ../git/plugins/quectel.c:qinistat_cb() qinistat: 7
ofonod[507]: ../git/src/modem.c:modem_change_state() old state: 0, new state: 1
ofonod[507]: ../git/plugins/quectel.c:quectel_pre_sim() 0x583b80
I highlighted the output of ./disable-modem between the ====
As disable-modem is done before AT+QINISTAT returned in my case, this
does give "Operation already in progress". The same for everything less
than the 10 seconds.
If I do sleep a few seconds longer in between enable-modem and
disable-modem, the disable-modem does fire after AT+QINISTAT and that
then does power down the modem successfully.
> > +
> > static int open_serial(struct ofono_modem *modem)
> > {
> > struct quectel_data *data = ofono_modem_get_data(modem);
> > @@ -1139,6 +1163,9 @@ static int open_serial(struct ofono_modem *modem)
> > return -EIO;
> > }
> > + if (data->gpio)
> > + g_timeout_add(2100, gpio_power_on_cb, data);
> > +
>
> Generally it is a good idea to keep track of any timeout objects you're
> adding. This is especially true on hot-plug/unplug capable hardware since
> the modem object and its data might go away, but the timer would still be
> running, causing a SIGSEGV later.
>
> Granted this is a serial device, so this won't likely happen in this case...
Ok, you mean it is better to use l_timeout_create_ms and keep track of
the returned object manually ?
As a first guess: Either the timeout fired and I destroy it in the cb
or I destroy it in close_serial. close_serial is part of quectel_disable.
I can try that if you want.
Regards,
Lars
_______________________________________________
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
