8:36 a.m.
Some modems can screw up everything and then we will need to do a silent
reset of the modem. This patch take the modem back to the OFFLINE state.
---
include/modem.h | 2 ++
src/modem.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/include/modem.h b/include/modem.h
index 7b13ee0..a92eb88 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -46,6 +46,8 @@ int ofono_modem_register(struct ofono_modem *modem);
ofono_bool_t ofono_modem_is_registered(struct ofono_modem *modem);
void ofono_modem_remove(struct ofono_modem *modem);
+void ofono_modem_reset(struct ofono_modem *modem);
+
void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
diff --git a/src/modem.c b/src/modem.c
index 6d346c3..e57f8fc 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -70,6 +70,7 @@ struct ofono_modem {
guint interface_update;
ofono_bool_t powered;
ofono_bool_t powered_pending;
+ ofono_bool_t reset;
guint timeout;
ofono_bool_t online;
GHashTable *properties;
@@ -433,6 +434,8 @@ static void sim_state_watch(enum ofono_sim_state new_state, void
*user)
if (modem->driver->set_online == NULL)
modem_change_state(modem, MODEM_STATE_ONLINE);
+ modem->reset = FALSE;
+
break;
}
}
@@ -784,7 +787,8 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t
powered)
return;
}
- ofono_dbus_signal_property_changed(conn, modem->path,
+ if (!modem->reset)
+ ofono_dbus_signal_property_changed(conn, modem->path,
OFONO_MODEM_INTERFACE,
"Powered", DBUS_TYPE_BOOLEAN,
&dbus_powered);
@@ -799,6 +803,17 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t
powered)
} else
modem_change_state(modem, MODEM_STATE_POWER_OFF);
+ if (modem->reset && !powering_down) {
+ if (!modem->powered) {
+ int err = set_powered(modem, TRUE);
+
+ if (err == -EINPROGRESS)
+ return;
+
+ modem_change_state(modem, MODEM_STATE_PRE_SIM);
+ }
+ }
+
out:
if (powering_down && powered == FALSE) {
modems_remaining -= 1;
@@ -806,6 +821,7 @@ out:
if (modems_remaining == 0)
__ofono_exit();
}
+
}
ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem)
@@ -1565,6 +1581,32 @@ void ofono_modem_remove(struct ofono_modem *modem)
g_free(modem);
}
+static gboolean __reset_modem(void *data)
+{
+ struct ofono_modem *modem = data;
+ int err;
+
+ modem->reset = TRUE;
+
+ err = set_powered(modem, FALSE);
+ if (err == -EINPROGRESS)
+ return FALSE;
+
+ err = set_powered(modem, TRUE);
+ if (err == -EINPROGRESS)
+ return FALSE;
+
+ modem_change_state(modem, MODEM_STATE_PRE_SIM);
+ return FALSE;
+}
+
+void ofono_modem_reset(struct ofono_modem *modem)
+{
+ DBG("%p", modem);
+
+ g_idle_add(__reset_modem, modem);
+}
+
int ofono_modem_driver_register(const struct ofono_modem_driver *d)
{
DBG("driver: %p, name: %s", d, d->name);
--
1.7.3.1
8:36 a.m.
New subject: [PATCH -v3 2/3] phonesim: Add modem reset trigger
---
plugins/phonesim.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index d2faf42..7426da6 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -237,6 +237,13 @@ static void cfun_set_on_cb(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_set_powered(modem, ok);
}
+static void crst_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+
+ ofono_modem_reset(modem);
+}
+
static void phonesim_disconnected(gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -389,6 +396,9 @@ static int phonesim_enable(struct ofono_modem *modem)
g_at_chat_send(data->chat, "AT+CSCS=\"GSM\"", none_prefix,
NULL, NULL, NULL);
+ g_at_chat_register(data->chat, "+CRST:",
+ crst_notify, FALSE, modem, NULL);
+
return 0;
}
--
1.7.3.1
8:36 a.m.
New subject: [PATCH -v3 3/3] modem: add support to restore state when resetting the modem
---
src/modem.c | 74 ++++++++++++++++++++++++++++++++--------------------------
1 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/src/modem.c b/src/modem.c
index e57f8fc..704de29 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -61,6 +61,7 @@ enum modem_state {
struct ofono_modem {
char *path;
enum modem_state modem_state;
+ enum modem_state old_state;
GSList *atoms;
struct ofono_watchlist *atom_watches;
GSList *interface_list;
@@ -375,7 +376,7 @@ static void modem_change_state(struct ofono_modem *modem,
if (old_state == new_state)
return;
- if (new_online != modem->online) {
+ if (new_online != modem->online && !modem->reset) {
DBusConnection *conn = ofono_dbus_get_connection();
modem->online = new_online;
ofono_dbus_signal_property_changed(conn, modem->path,
@@ -414,48 +415,26 @@ static void modem_change_state(struct ofono_modem *modem,
}
}
-static void sim_state_watch(enum ofono_sim_state new_state, void *user)
-{
- struct ofono_modem *modem = user;
-
- switch (new_state) {
- case OFONO_SIM_STATE_NOT_PRESENT:
- modem_change_state(modem, MODEM_STATE_PRE_SIM);
- break;
- case OFONO_SIM_STATE_INSERTED:
- break;
- case OFONO_SIM_STATE_READY:
- modem_change_state(modem, MODEM_STATE_OFFLINE);
-
- /*
- * If we don't have the set_online method, also proceed
- * straight to the online state
- */
- if (modem->driver->set_online == NULL)
- modem_change_state(modem, MODEM_STATE_ONLINE);
-
- modem->reset = FALSE;
-
- break;
- }
-}
-
static void online_cb(const struct ofono_error *error, void *data)
{
struct ofono_modem *modem = data;
DBusMessage *reply;
if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
- modem->modem_state == MODEM_STATE_OFFLINE)
+ modem->modem_state == MODEM_STATE_OFFLINE) {
+ modem_change_state(modem, MODEM_STATE_ONLINE);
+
+ if (modem->reset) {
+ modem->reset = FALSE;
+ return;
+ }
+
reply = dbus_message_new_method_return(modem->pending);
- else
+ } else {
reply = __ofono_error_failed(modem->pending);
+ }
__ofono_dbus_pending_reply(&modem->pending, reply);
-
- if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
- modem->modem_state == MODEM_STATE_OFFLINE)
- modem_change_state(modem, MODEM_STATE_ONLINE);
}
static void offline_cb(const struct ofono_error *error, void *data)
@@ -475,6 +454,34 @@ static void offline_cb(const struct ofono_error *error, void *data)
modem_change_state(modem, MODEM_STATE_OFFLINE);
}
+static void sim_state_watch(enum ofono_sim_state new_state, void *user)
+{
+ struct ofono_modem *modem = user;
+
+ switch (new_state) {
+ case OFONO_SIM_STATE_NOT_PRESENT:
+ modem_change_state(modem, MODEM_STATE_PRE_SIM);
+ break;
+ case OFONO_SIM_STATE_INSERTED:
+ break;
+ case OFONO_SIM_STATE_READY:
+ modem_change_state(modem, MODEM_STATE_OFFLINE);
+
+ /*
+ * If we don't have the set_online method, also proceed
+ * straight to the online state
+ */
+ if (modem->driver->set_online == NULL)
+ modem_change_state(modem, MODEM_STATE_ONLINE);
+ else if (modem->old_state > MODEM_STATE_OFFLINE)
+ modem->driver->set_online(modem, 1, online_cb, modem);
+ else
+ modem->reset = FALSE;
+
+ break;
+ }
+}
+
static DBusMessage *set_property_online(struct ofono_modem *modem,
DBusMessage *msg,
DBusMessageIter *var)
@@ -1586,6 +1593,7 @@ static gboolean __reset_modem(void *data)
struct ofono_modem *modem = data;
int err;
+ modem->old_state = modem->modem_state;
modem->reset = TRUE;
err = set_powered(modem, FALSE);
--
1.7.3.1
10:04 p.m.
New subject: [PATCH -v3 3/3] modem: add support to restore state when resetting the modem
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
---
src/modem.c | 74 ++++++++++++++++++++++++++++++++--------------------------
1 files changed, 41 insertions(+), 33 deletions(-)
Ah ok, you implement the getting back to online process in this patch.
Ignore my previous comment.
diff --git a/src/modem.c b/src/modem.c
index e57f8fc..704de29 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -61,6 +61,7 @@ enum modem_state {
struct ofono_modem {
char *path;
enum modem_state modem_state;
+ enum modem_state old_state;
Can we simply use a boolean like 'get_online' that is FALSE by default,
but gets set to TRUE (assuming we're online) during a silent reset?
This can then get re-used for implementing of the Lockdown property as well.
It also seems that the 'reset' value is not really needed.
Regards,
-Denis
4:50 a.m.
New subject: [PATCH -v3 3/3] modem: add support to restore state when resetting the modem
Hi Denis,
* Denis Kenzior <denkenz(a)gmail.com> [2010-11-23 05:04:40 -0600]:
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
> ---
> src/modem.c | 74 ++++++++++++++++++++++++++++++++--------------------------
> 1 files changed, 41 insertions(+), 33 deletions(-)
>
Ah ok, you implement the getting back to online process in this patch.
Ignore my previous comment.
> diff --git a/src/modem.c b/src/modem.c
> index e57f8fc..704de29 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -61,6 +61,7 @@ enum modem_state {
> struct ofono_modem {
> char *path;
> enum modem_state modem_state;
> + enum modem_state old_state;
Can we simply use a boolean like 'get_online' that is FALSE by default,
but gets set to TRUE (assuming we're online) during a silent reset?
This can then get re-used for implementing of the Lockdown property as well.
Yeah, I'm reusing it there.
It also seems that the 'reset' value is not really needed.
I'm using that to avoid emit DBus signal during the silent reset, Not
seeing how to do that without modem->reset, maybe we can rely on
modem->pending, not sure yet.
--
Gustavo F. Padovan
http://profusion.mobi
8:33 a.m.
New subject: [PATCH -v3 3/3] modem: add support to restore state when resetting the modem
* Gustavo F. Padovan <padovan(a)profusion.mobi> [2010-11-23 15:50:09 -0200]:
Hi Denis,
* Denis Kenzior <denkenz(a)gmail.com> [2010-11-23 05:04:40 -0600]:
> Hi Gustavo,
>
> On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
> > ---
> > src/modem.c | 74 ++++++++++++++++++++++++++++++++--------------------------
> > 1 files changed, 41 insertions(+), 33 deletions(-)
> >
>
> Ah ok, you implement the getting back to online process in this patch.
> Ignore my previous comment.
>
> > diff --git a/src/modem.c b/src/modem.c
> > index e57f8fc..704de29 100644
> > --- a/src/modem.c
> > +++ b/src/modem.c
> > @@ -61,6 +61,7 @@ enum modem_state {
> > struct ofono_modem {
> > char *path;
> > enum modem_state modem_state;
> > + enum modem_state old_state;
>
> Can we simply use a boolean like 'get_online' that is FALSE by default,
> but gets set to TRUE (assuming we're online) during a silent reset?
> This can then get re-used for implementing of the Lockdown property as well.
Yeah, I'm reusing it there.
>
> It also seems that the 'reset' value is not really needed.
I'm using that to avoid emit DBus signal during the silent reset, Not
seeing how to do that without modem->reset, maybe we can rely on
modem->pending, not sure yet.
Or do you mean that have DBus signaling about Powered and Online during
silent reset is just fine? That is starting to make sense to me. We may
want to notify users that the modem is not there.
--
Gustavo F. Padovan
http://profusion.mobi
9:07 a.m.
New subject: [PATCH -v3 3/3] modem: add support to restore state when resetting the modem
Hi Gustavo,
>>
>> It also seems that the 'reset' value is not really needed.
>
> I'm using that to avoid emit DBus signal during the silent reset, Not
> seeing how to do that without modem->reset, maybe we can rely on
> modem->pending, not sure yet.
Or do you mean that have DBus signaling about Powered and Online during
silent reset is just fine? That is starting to make sense to me. We may
want to notify users that the modem is not there.
I actually think that emitting powered / online going on and off is
okay, there's no real way to hide this since all the atoms have to be
re-initialized. So all the interfaces go away and magically re-appear,
all properties are re-emitted, etc. Not emitting Powered or Online is
the least of our worries.
Silent resets are really just about avoiding having the user re-enter
the PIN when the modem crashes.
Regards,
-Denis
9:59 p.m.
New subject: [PATCH -v3 2/3] phonesim: Add modem reset trigger
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
---
plugins/phonesim.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index d2faf42..7426da6 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -237,6 +237,13 @@ static void cfun_set_on_cb(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_set_powered(modem, ok);
}
+static void crst_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+
+ ofono_modem_reset(modem);
I suggest you simply clean up the phonesim connection (e.g. closing the
tcp socket) when you receive this notification, and then call
modem_reset at the end.
+}
+
static void phonesim_disconnected(gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -389,6 +396,9 @@ static int phonesim_enable(struct ofono_modem *modem)
g_at_chat_send(data->chat, "AT+CSCS=\"GSM\"", none_prefix,
NULL, NULL, NULL);
+ g_at_chat_register(data->chat, "+CRST:",
+ crst_notify, FALSE, modem, NULL);
+
return 0;
}
Regards,
-Denis
7:01 a.m.
New subject: [PATCH -v3 2/3] phonesim: Add modem reset trigger
* Denis Kenzior <denkenz(a)gmail.com> [2010-11-23 04:59:33 -0600]:
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
> ---
> plugins/phonesim.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/plugins/phonesim.c b/plugins/phonesim.c
> index d2faf42..7426da6 100644
> --- a/plugins/phonesim.c
> +++ b/plugins/phonesim.c
> @@ -237,6 +237,13 @@ static void cfun_set_on_cb(gboolean ok, GAtResult *result,
gpointer user_data)
> ofono_modem_set_powered(modem, ok);
> }
>
> +static void crst_notify(GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> +
> + ofono_modem_reset(modem);
I suggest you simply clean up the phonesim connection (e.g. closing the
tcp socket) when you receive this notification, and then call
modem_reset at the end.
So here do you mean unref phonesim data->chat and also call
ofono_modem_set_powered(modem, FALSE) before call ofono_modem_reset()?
--
Gustavo F. Padovan
http://profusion.mobi
8:52 a.m.
New subject: [PATCH -v3 2/3] phonesim: Add modem reset trigger
Hi Gustavo,
>> +static void crst_notify(GAtResult *result, gpointer
user_data)
>> +{
>> + struct ofono_modem *modem = user_data;
>> +
>> + ofono_modem_reset(modem);
>
> I suggest you simply clean up the phonesim connection (e.g. closing the
> tcp socket) when you receive this notification, and then call
> modem_reset at the end.
So here do you mean unref phonesim data->chat and also call
ofono_modem_set_powered(modem, FALSE) before call ofono_modem_reset()?
So I really mean performing the same steps as disable. Calling
ofono_modem_set_powered is pretty much optional. The core should simply
assume that once ofono_modem_reset has been called, the modem is off and
needs to be re-initialized from the beginning.
Regards,
-Denis
9:57 p.m.
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
Some modems can screw up everything and then we will need to do a
silent
reset of the modem. This patch take the modem back to the OFFLINE state.
---
include/modem.h | 2 ++
src/modem.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/include/modem.h b/include/modem.h
index 7b13ee0..a92eb88 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -46,6 +46,8 @@ int ofono_modem_register(struct ofono_modem *modem);
ofono_bool_t ofono_modem_is_registered(struct ofono_modem *modem);
void ofono_modem_remove(struct ofono_modem *modem);
+void ofono_modem_reset(struct ofono_modem *modem);
+
void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
diff --git a/src/modem.c b/src/modem.c
index 6d346c3..e57f8fc 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -70,6 +70,7 @@ struct ofono_modem {
guint interface_update;
ofono_bool_t powered;
ofono_bool_t powered_pending;
+ ofono_bool_t reset;
guint timeout;
ofono_bool_t online;
GHashTable *properties;
@@ -433,6 +434,8 @@ static void sim_state_watch(enum ofono_sim_state new_state, void
*user)
if (modem->driver->set_online == NULL)
modem_change_state(modem, MODEM_STATE_ONLINE);
+ modem->reset = FALSE;
+
break;
}
}
@@ -784,7 +787,8 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t
powered)
return;
}
- ofono_dbus_signal_property_changed(conn, modem->path,
+ if (!modem->reset)
+ ofono_dbus_signal_property_changed(conn, modem->path,
OFONO_MODEM_INTERFACE,
"Powered", DBUS_TYPE_BOOLEAN,
&dbus_powered);
@@ -799,6 +803,17 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t
powered)
} else
modem_change_state(modem, MODEM_STATE_POWER_OFF);
+ if (modem->reset && !powering_down) {
+ if (!modem->powered) {
+ int err = set_powered(modem, TRUE);
+
+ if (err == -EINPROGRESS)
+ return;
+
+ modem_change_state(modem, MODEM_STATE_PRE_SIM);
+ }
+ }
+
out:
if (powering_down && powered == FALSE) {
modems_remaining -= 1;
@@ -806,6 +821,7 @@ out:
if (modems_remaining == 0)
__ofono_exit();
}
+
}
ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem)
@@ -1565,6 +1581,32 @@ void ofono_modem_remove(struct ofono_modem *modem)
g_free(modem);
}
+static gboolean __reset_modem(void *data)
There's no need to use '__' prefix for static functions. That prefix is
reserved for private API functions declared in ofono.h
+{
+ struct ofono_modem *modem = data;
+ int err;
+
+ modem->reset = TRUE;
+
+ err = set_powered(modem, FALSE);
+ if (err == -EINPROGRESS)
+ return FALSE;
So we really don't need to power down the modem for the silent reset.
Simply assume that the modem driver has performed the necessary steps of
cleaning up the modem to a 'powered off state'. We can also assume that
silent resets only happen when the modem is at least powered.
One thing I'm not seeing in this patch is the functionality for getting
the modem back into online state after a silent reset.
+
+ err = set_powered(modem, TRUE);
+ if (err == -EINPROGRESS)
+ return FALSE;
+
+ modem_change_state(modem, MODEM_STATE_PRE_SIM);
+ return FALSE;
+}
+
+void ofono_modem_reset(struct ofono_modem *modem)
+{
+ DBG("%p", modem);
+
+ g_idle_add(__reset_modem, modem);
In general I don't like using g_idle_add in the core. What is the
purpose of this one? If you really have to do it, please track the
gsource and make sure to remove it when the modem is removed.
+}
+
int ofono_modem_driver_register(const struct ofono_modem_driver *d)
{
DBG("driver: %p, name: %s", d, d->name);
Regards,
-Denis
4:41 a.m.
Hi Denis,
* Denis Kenzior <denkenz(a)gmail.com> [2010-11-23 04:57:35 -0600]:
Hi Gustavo,
On 11/19/2010 03:36 PM, Gustavo F. Padovan wrote:
> Some modems can screw up everything and then we will need to do a silent
> reset of the modem. This patch take the modem back to the OFFLINE state.
> ---
> include/modem.h | 2 ++
> src/modem.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 1 deletions(-)
>
> diff --git a/include/modem.h b/include/modem.h
> index 7b13ee0..a92eb88 100644
> --- a/include/modem.h
> +++ b/include/modem.h
> @@ -46,6 +46,8 @@ int ofono_modem_register(struct ofono_modem *modem);
> ofono_bool_t ofono_modem_is_registered(struct ofono_modem *modem);
> void ofono_modem_remove(struct ofono_modem *modem);
>
> +void ofono_modem_reset(struct ofono_modem *modem);
> +
> void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
> ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
>
> diff --git a/src/modem.c b/src/modem.c
> index 6d346c3..e57f8fc 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -70,6 +70,7 @@ struct ofono_modem {
> guint interface_update;
> ofono_bool_t powered;
> ofono_bool_t powered_pending;
> + ofono_bool_t reset;
> guint timeout;
> ofono_bool_t online;
> GHashTable *properties;
> @@ -433,6 +434,8 @@ static void sim_state_watch(enum ofono_sim_state new_state, void
*user)
> if (modem->driver->set_online == NULL)
> modem_change_state(modem, MODEM_STATE_ONLINE);
>
> + modem->reset = FALSE;
> +
> break;
> }
> }
> @@ -784,7 +787,8 @@ void ofono_modem_set_powered(struct ofono_modem *modem,
ofono_bool_t powered)
> return;
> }
>
> - ofono_dbus_signal_property_changed(conn, modem->path,
> + if (!modem->reset)
> + ofono_dbus_signal_property_changed(conn, modem->path,
> OFONO_MODEM_INTERFACE,
> "Powered", DBUS_TYPE_BOOLEAN,
> &dbus_powered);
> @@ -799,6 +803,17 @@ void ofono_modem_set_powered(struct ofono_modem *modem,
ofono_bool_t powered)
> } else
> modem_change_state(modem, MODEM_STATE_POWER_OFF);
>
> + if (modem->reset && !powering_down) {
> + if (!modem->powered) {
> + int err = set_powered(modem, TRUE);
> +
> + if (err == -EINPROGRESS)
> + return;
> +
> + modem_change_state(modem, MODEM_STATE_PRE_SIM);
> + }
> + }
> +
> out:
> if (powering_down && powered == FALSE) {
> modems_remaining -= 1;
> @@ -806,6 +821,7 @@ out:
> if (modems_remaining == 0)
> __ofono_exit();
> }
> +
> }
>
> ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem)
> @@ -1565,6 +1581,32 @@ void ofono_modem_remove(struct ofono_modem *modem)
> g_free(modem);
> }
>
> +static gboolean __reset_modem(void *data)
There's no need to use '__' prefix for static functions. That prefix is
reserved for private API functions declared in ofono.h
Ok.
> +{
> + struct ofono_modem *modem = data;
> + int err;
> +
> + modem->reset = TRUE;
> +
> + err = set_powered(modem, FALSE);
> + if (err == -EINPROGRESS)
> + return FALSE;
So we really don't need to power down the modem for the silent reset.
Simply assume that the modem driver has performed the necessary steps of
cleaning up the modem to a 'powered off state'. We can also assume that
silent resets only happen when the modem is at least powered.
Ok.
One thing I'm not seeing in this patch is the functionality for getting
the modem back into online state after a silent reset.
> +
> + err = set_powered(modem, TRUE);
> + if (err == -EINPROGRESS)
> + return FALSE;
> +
> + modem_change_state(modem, MODEM_STATE_PRE_SIM);
> + return FALSE;
> +}
> +
> +void ofono_modem_reset(struct ofono_modem *modem)
> +{
> + DBG("%p", modem);
> +
> + g_idle_add(__reset_modem, modem);
In general I don't like using g_idle_add in the core. What is the
purpose of this one? If you really have to do it, please track the
gsource and make sure to remove it when the modem is removed.
I was facing a segfault due to caling driver->disable here. It destroys
the data->chat in phonesim, but we need it there because we are in the
middle of callback. I'll ckeck if with the new changes you proposed we
will still need this.
--
Gustavo F. Padovan
http://profusion.mobi
4317
days inactive
4321
days old
11 comments
2 participants
participants (2)
-
Denis Kenzior -
Gustavo F. Padovan