[PATCH] plugins/ril: complete modem power off process

[PATCH] plugins/ril: complete modem power off process

[caiwen.zhang]

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

From: caiwen Zhang <caiwen.zhang@intel.com>

---
 plugins/ril.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/plugins/ril.c b/plugins/ril.c
index 2298bb6..2ea3fb5 100644
--- a/plugins/ril.c
+++ b/plugins/ril.c
@@ -409,15 +409,30 @@ int ril_enable(struct ofono_modem *modem)
 	return -EINPROGRESS;
 }
 
+static void power_off_cb(struct ril_msg *message, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	struct ril_data *rd = cbd->user;
+	struct ofono_modem *modem = cbd->data;
+
+	if (rd) {
+		g_ril_unref(rd->ril);
+		rd->ril = NULL;
+	}
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
 int ril_disable(struct ofono_modem *modem)
 {
 	struct ril_data *rd = ofono_modem_get_data(modem);
+	struct cb_data *cbd = cb_data_new(NULL, modem, rd);
 
 	DBG("%p", modem);
 
-	ril_send_power(rd, FALSE, NULL, NULL);
+	ril_send_power(rd, FALSE, power_off_cb, cbd);
 
-	return 0;
+	return -EINPROGRESS;
 }
 
 static struct ofono_modem_driver ril_driver = {
-- 
2.7.4

Re: [PATCH] plugins/ril: complete modem power off process

[Denis Kenzior]

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

Hi Caiwen,

On 05/12/2016 12:51 AM, caiwen.zhang(a)intel.com wrote:
> From: caiwen Zhang <caiwen.zhang@intel.com>
>
> ---
>   plugins/ril.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>

I went ahead and applied this patch, however, see below:

> diff --git a/plugins/ril.c b/plugins/ril.c
> index 2298bb6..2ea3fb5 100644
> --- a/plugins/ril.c
> +++ b/plugins/ril.c
> @@ -409,15 +409,30 @@ int ril_enable(struct ofono_modem *modem)
>   	return -EINPROGRESS;
>   }
>
> +static void power_off_cb(struct ril_msg *message, gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	struct ril_data *rd = cbd->user;
> +	struct ofono_modem *modem = cbd->data;
> +
> +	if (rd) {
> +		g_ril_unref(rd->ril);

It seems GRil doesn't gracefully handle g_ril_unrefs from within a 
result callback.  Have you tried running this with valgrind?

We might need to come up with a fix for GRil...

> +		rd->ril = NULL;
> +	}
> +
> +	ofono_modem_set_powered(modem, FALSE);
> +}
> +
>   int ril_disable(struct ofono_modem *modem)
>   {
>   	struct ril_data *rd = ofono_modem_get_data(modem);
> +	struct cb_data *cbd = cb_data_new(NULL, modem, rd);
>
>   	DBG("%p", modem);
>
> -	ril_send_power(rd, FALSE, NULL, NULL);
> +	ril_send_power(rd, FALSE, power_off_cb, cbd);
>
> -	return 0;
> +	return -EINPROGRESS;
>   }
>
>   static struct ofono_modem_driver ril_driver = {
>

Regards,
-Denis

RE: [PATCH] plugins/ril: complete modem power off process

[Zhang, Caiwen]

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


Hi Denis,

> > ---
> >   plugins/ril.c | 19 +++++++++++++++++--
> >   1 file changed, 17 insertions(+), 2 deletions(-)
> >
> 
> I went ahead and applied this patch, however, see below:
> 
> > diff --git a/plugins/ril.c b/plugins/ril.c index 2298bb6..2ea3fb5
> > 100644
> > --- a/plugins/ril.c
> > +++ b/plugins/ril.c
> > @@ -409,15 +409,30 @@ int ril_enable(struct ofono_modem *modem)
> >   	return -EINPROGRESS;
> >   }
> >
> > +static void power_off_cb(struct ril_msg *message, gpointer user_data)
> > +{
> > +	struct cb_data *cbd = user_data;
> > +	struct ril_data *rd = cbd->user;
> > +	struct ofono_modem *modem = cbd->data;
> > +
> > +	if (rd) {
> > +		g_ril_unref(rd->ril);
> 
> It seems GRil doesn't gracefully handle g_ril_unrefs from within a result
> callback.  Have you tried running this with valgrind?
> 
> We might need to come up with a fix for GRil...
> 

The g_ril_unref should be fine. There are two glib runtime warning messages, it is because:
In handle_response/gril.c: handle_response, after " req->callback" is called, "gril" has been cleaned up,
both 'out_queue' and 'command_queue' are NULL.

It seems it's harmless. I will submit a patch to avoid the warnings.


> > +		rd->ril = NULL;
> > +	}
> > +
> > +	ofono_modem_set_powered(modem, FALSE); }
> > +
> >   int ril_disable(struct ofono_modem *modem)
> >   {
> >   	struct ril_data *rd = ofono_modem_get_data(modem);
> > +	struct cb_data *cbd = cb_data_new(NULL, modem, rd);
> >
> >   	DBG("%p", modem);
> >
> > -	ril_send_power(rd, FALSE, NULL, NULL);
> > +	ril_send_power(rd, FALSE, power_off_cb, cbd);

Regards,
Caiwen