Hi Kalle,
> Ofono either crashed or busy looped with my Huawei E1552 3G modem when I
> tried to deactivate GPRS context. The reason was that gcd->chat was
> unreferenced already in setup_ppp() but the chat was still accessed
> later in at_gprs_deactivate_primary().
>
> To fix the problem, change the logic instead to suspend chat session
> for PPP and resume when PPP has disconnected. Now it doesn't crash
> anymore.
>
> Also remove the CGACT=0 command during deactivation, it's enough to shutdown
> ppp.
>
> Deactivation still doesn't work properly with Huawei E1552, and most
> probably with other Huawei modems, because the modem hangs up the chat
> line after PPP deactivation. This needs to be fixed separately. The
> workaround is to reboot the modem, for example physically unplug and plug
> it in again.
> ---
>
> drivers/atmodem/gprs-context.c | 56
> +++++++++++++---------------------------
> 1 files changed, 18 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
> index ba5f0c0..794db8a 100644
> --- a/drivers/atmodem/gprs-context.c
> +++ b/drivers/atmodem/gprs-context.c
> @@ -65,22 +65,6 @@ struct gprs_context_data {
> void *cb_data; /* Callback data */
> };
>
> -static void at_cgact_down_cb(gboolean ok, GAtResult *result, gpointer
> user_data)
> -{
> - struct cb_data *cbd = user_data;
> - ofono_gprs_context_cb_t cb = cbd->cb;
> - struct ofono_gprs_context *gc = cbd->user;
> - struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
> - struct ofono_error error;
> -
> - if (ok)
> - gcd->active_context = 0;
> -
> - decode_at_error(&error, g_at_result_final_response(result));
> -
> - cb(&error, cbd->data);
> -}
> -
> static void ppp_connect(const char *interface, const char *ip,
> const char *dns1, const char *dns2,
> gpointer user_data)
> @@ -104,13 +88,19 @@ static void ppp_disconnect(GAtPPPDisconnectReason
> reason, gpointer user_data)
> struct ofono_gprs_context *gc = user_data;
> struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
>
> + DBG("");
> +
> + g_at_chat_resume(gcd->chat);
> +
> if (gcd->state == STATE_ENABLING) {
> CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, FALSE, NULL,
> NULL, NULL, NULL, gcd->cb_data);
> - return;
> + } else if (gcd->state == STATE_DISABLING) {
> + CALLBACK_WITH_SUCCESS(gcd->down_cb, gcd->cb_data);
> + } else {
> + ofono_gprs_context_deactivated(gc, gcd->active_context);
> }
the easier to read coding style here is noew clearly a switch statement.
> - ofono_gprs_context_deactivated(gc, gcd->active_context);
> gcd->active_context = 0;
> gcd->state = STATE_IDLE;
> }
> @@ -118,13 +108,14 @@ static void ppp_disconnect(GAtPPPDisconnectReason
> reason, gpointer user_data)
> static gboolean setup_ppp(struct ofono_gprs_context *gc)
> {
> struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
> - GIOChannel *channel;
> + GAtIO *io;
> +
> + io = g_at_chat_get_io(gcd->chat);
>
> - channel = g_at_chat_get_channel(gcd->chat);
> - g_at_chat_unref(gcd->chat);
> + g_at_chat_suspend(gcd->chat);
>
> /* open ppp */
> - gcd->ppp = g_at_ppp_new(channel);
> + gcd->ppp = g_at_ppp_new_from_io(io);
>
> if (gcd->ppp == NULL)
> return FALSE;
> @@ -227,25 +218,14 @@ static void at_gprs_deactivate_primary(struct
> ofono_gprs_context *gc,
> ofono_gprs_context_cb_t cb, void *data)
> {
> struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
> - struct cb_data *cbd = cb_data_new(cb, data);
> - char buf[64];
> -
> - if (!cbd)
> - goto error;
> -
> - cbd->user = gc;
>
> - snprintf(buf, sizeof(buf), "AT+CGACT=0,%u", id);
> + DBG("");
>
> - if (g_at_chat_send(gcd->chat, buf, none_prefix,
> - at_cgact_down_cb, cbd, g_free) > 0)
> - return;
> -
> -error:
> - if (cbd)
> - g_free(cbd);
> + gcd->state = STATE_DISABLING;
> + gcd->down_cb = cb;
> + gcd->cb_data = data;
>
> - CALLBACK_WITH_FAILURE(cb, data);
> + g_at_ppp_shutdown(gcd->ppp);
> }
>
> static int at_gprs_context_probe(struct ofono_gprs_context *gc,
The rest looks fine to me. However maybe splitting this into one patch
that removes the CGACT and another that adds suspend/resume calls seems
a bit cleaner to me.
Regards
Marcel
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
