[PATCH] ifx: remove setting to offline during modem enable

[PATCH] ifx: remove setting to offline during modem enable

[Jeevaka Badrappan]

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

---
 plugins/ifx.c |   21 ++-------------------
 1 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/plugins/ifx.c b/plugins/ifx.c
index 411c012..ae53193 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -345,23 +345,6 @@ error:
 	ofono_modem_set_powered(modem, FALSE);
 }
 
-static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
-{
-	struct ofono_modem *modem = user_data;
-	struct ifx_data *data = ofono_modem_get_data(modem);
-
-	DBG("");
-
-	if (!ok) {
-		shutdown_device(data);
-		ofono_modem_set_powered(modem, FALSE);
-		return;
-	}
-
-	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
-					xgendata_query, modem, NULL);
-}
-
 static gboolean dlc_setup(gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -378,8 +361,8 @@ static gboolean dlc_setup(gpointer user_data)
 	g_at_chat_set_slave(data->dlcs[GPRS2_DLC], data->dlcs[NETREG_DLC]);
 	g_at_chat_set_slave(data->dlcs[GPRS3_DLC], data->dlcs[NETREG_DLC]);
 
-	g_at_chat_send(data->dlcs[AUX_DLC], "AT+CFUN=4", NULL,
-					cfun_enable, modem, NULL);
+	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
+					xgendata_query, modem, NULL);
 
 	data->dlc_init_source = 0;
 
-- 
1.7.0.4

Re: [PATCH] ifx: remove setting to offline during modem enable

[Marcel Holtmann]

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

Hi Jeevaka,

>  plugins/ifx.c |   21 ++-------------------
>  1 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/plugins/ifx.c b/plugins/ifx.c
> index 411c012..ae53193 100644
> --- a/plugins/ifx.c
> +++ b/plugins/ifx.c
> @@ -345,23 +345,6 @@ error:
>  	ofono_modem_set_powered(modem, FALSE);
>  }
>  
> -static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> -{
> -	struct ofono_modem *modem = user_data;
> -	struct ifx_data *data = ofono_modem_get_data(modem);
> -
> -	DBG("");
> -
> -	if (!ok) {
> -		shutdown_device(data);
> -		ofono_modem_set_powered(modem, FALSE);
> -		return;
> -	}
> -
> -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> -					xgendata_query, modem, NULL);
> -}
> -
>  static gboolean dlc_setup(gpointer user_data)
>  {
>  	struct ofono_modem *modem = user_data;
> @@ -378,8 +361,8 @@ static gboolean dlc_setup(gpointer user_data)
>  	g_at_chat_set_slave(data->dlcs[GPRS2_DLC], data->dlcs[NETREG_DLC]);
>  	g_at_chat_set_slave(data->dlcs[GPRS3_DLC], data->dlcs[NETREG_DLC]);
>  
> -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+CFUN=4", NULL,
> -					cfun_enable, modem, NULL);
> +	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> +					xgendata_query, modem, NULL);

I toughth we talked about CFUN=4 in ifx_disable on IRC. This one is
needed to bring the modem up in offline mode. I'd rather not remove this
one.

Regards

Marcel

RE: [PATCH] ifx: remove setting to offline during modem enable

[Predon, Frederic]

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

Hi Marcel,


> >  plugins/ifx.c |   21 ++-------------------
> >  1 files changed, 2 insertions(+), 19 deletions(-)
> >
> > diff --git a/plugins/ifx.c b/plugins/ifx.c
> > index 411c012..ae53193 100644
> > --- a/plugins/ifx.c
> > +++ b/plugins/ifx.c
> > @@ -345,23 +345,6 @@ error:
> >  	ofono_modem_set_powered(modem, FALSE);
> >  }
> >
> > -static void cfun_enable(gboolean ok, GAtResult *result, gpointer
> user_data)
> > -{
> > -	struct ofono_modem *modem = user_data;
> > -	struct ifx_data *data = ofono_modem_get_data(modem);
> > -
> > -	DBG("");
> > -
> > -	if (!ok) {
> > -		shutdown_device(data);
> > -		ofono_modem_set_powered(modem, FALSE);
> > -		return;
> > -	}
> > -
> > -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> > -					xgendata_query, modem, NULL);
> > -}
> > -
> >  static gboolean dlc_setup(gpointer user_data)
> >  {
> >  	struct ofono_modem *modem = user_data;
> > @@ -378,8 +361,8 @@ static gboolean dlc_setup(gpointer user_data)
> >  	g_at_chat_set_slave(data->dlcs[GPRS2_DLC], data-
> >dlcs[NETREG_DLC]);
> >  	g_at_chat_set_slave(data->dlcs[GPRS3_DLC], data-
> >dlcs[NETREG_DLC]);
> >
> > -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+CFUN=4", NULL,
> > -					cfun_enable, modem, NULL);
> > +	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> > +					xgendata_query, modem, NULL);
> 
> I toughth we talked about CFUN=4 in ifx_disable on IRC. This one is
> needed to bring the modem up in offline mode. I'd rather not remove
> this
> one.

In a ideal world, the CFUN=4 in the dlc_setup will not be needed because the modem remembers is last state and will remain in offline mode because we have sent CFUN=4 in the ifx_disable function.
But, we could have some error cases where we are not able to send this command anymore because of modem crash or SPI broken, etc..  so we might have to keep the forced CFUN=4 in the dlc_setup

Regards

Fred
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

RE: [PATCH] ifx: remove setting to offline during modem enable

[Marcel Holtmann]

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

Hi Fred,

> > >  plugins/ifx.c |   21 ++-------------------
> > >  1 files changed, 2 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/plugins/ifx.c b/plugins/ifx.c
> > > index 411c012..ae53193 100644
> > > --- a/plugins/ifx.c
> > > +++ b/plugins/ifx.c
> > > @@ -345,23 +345,6 @@ error:
> > >  	ofono_modem_set_powered(modem, FALSE);
> > >  }
> > >
> > > -static void cfun_enable(gboolean ok, GAtResult *result, gpointer
> > user_data)
> > > -{
> > > -	struct ofono_modem *modem = user_data;
> > > -	struct ifx_data *data = ofono_modem_get_data(modem);
> > > -
> > > -	DBG("");
> > > -
> > > -	if (!ok) {
> > > -		shutdown_device(data);
> > > -		ofono_modem_set_powered(modem, FALSE);
> > > -		return;
> > > -	}
> > > -
> > > -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> > > -					xgendata_query, modem, NULL);
> > > -}
> > > -
> > >  static gboolean dlc_setup(gpointer user_data)
> > >  {
> > >  	struct ofono_modem *modem = user_data;
> > > @@ -378,8 +361,8 @@ static gboolean dlc_setup(gpointer user_data)
> > >  	g_at_chat_set_slave(data->dlcs[GPRS2_DLC], data-
> > >dlcs[NETREG_DLC]);
> > >  	g_at_chat_set_slave(data->dlcs[GPRS3_DLC], data-
> > >dlcs[NETREG_DLC]);
> > >
> > > -	g_at_chat_send(data->dlcs[AUX_DLC], "AT+CFUN=4", NULL,
> > > -					cfun_enable, modem, NULL);
> > > +	g_at_chat_send(data->dlcs[AUX_DLC], "AT+XGENDATA", NULL,
> > > +					xgendata_query, modem, NULL);
> > 
> > I toughth we talked about CFUN=4 in ifx_disable on IRC. This one is
> > needed to bring the modem up in offline mode. I'd rather not remove
> > this
> > one.
> 
> In a ideal world, the CFUN=4 in the dlc_setup will not be needed because the modem remembers is last state and will remain in offline mode because we have sent CFUN=4 in the ifx_disable function.
> But, we could have some error cases where we are not able to send this command anymore because of modem crash or SPI broken, etc..  so we might have to keep the forced CFUN=4 in the dlc_setup

I agree. I do wanna keep the CFUN=4 in ifx_enable() to make the sure the
modem is in init state and offline.

Jeevaka and I discussed on IRC, or at least what I thought we
discussed ;), using CFUN=0 in ifx_disable().

Regards

Marcel

RE: [PATCH] ifx: remove setting to offline during modem enable

[Predon, Frederic]

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

Hi Marcel,

> > > I toughth we talked about CFUN=4 in ifx_disable on IRC. This one is
> > > needed to bring the modem up in offline mode. I'd rather not remove
> > > this
> > > one.
> >
> > In a ideal world, the CFUN=4 in the dlc_setup will not be needed
> because the modem remembers is last state and will remain in offline
> mode because we have sent CFUN=4 in the ifx_disable function.
> > But, we could have some error cases where we are not able to send
> this command anymore because of modem crash or SPI broken, etc..  so we
> might have to keep the forced CFUN=4 in the dlc_setup
> 
> I agree. I do wanna keep the CFUN=4 in ifx_enable() to make the sure
> the
> modem is in init state and offline.
> 
> Jeevaka and I discussed on IRC, or at least what I thought we
> discussed ;), using CFUN=0 in ifx_disable().

CFUN=0 is different because it will shutdown the modem. This is something we are willing to add in the ifx_disable but before that, we have to modify the spi driver to be able to start the modem and have a symmetric enable/disable modem functionality.
If disable shutdown the modem then enable should start it.
I'll send you some diagrams in that direction during the week to have your opinion.

Regards

Fred
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.