All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mbm: add quirks for Dell D5530
@ 2010-08-25 16:16 Pekka.Pessi
  2010-08-25 16:46 ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Pekka.Pessi @ 2010-08-25 16:16 UTC (permalink / raw)
  To: ofono

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Dell D5530 is an OEM version of F3507g. It has an annoying habit of
announcing itself to world with its own name. It also crashes upon
processing received cbs messages.
---
 plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/plugins/mbm.c b/plugins/mbm.c
index c9b0ea4..1c2b9a8 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
 static const char *none_prefix[] = { NULL };
 
+enum mbm_variant {
+	MBM_GENERIC,
+	MBM_DELL_D5530,		/* OEM of F3507g */
+};
+
 struct mbm_data {
 	GAtChat *modem_port;
 	GAtChat *data_port;
+	enum mbm_variant variant;
 	guint poll_source;
 	guint poll_count;
 	gboolean have_sim;
@@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
 	return FALSE;
 }
 
+static void d5530_notify(GAtResult *result, gpointer user_data)
+{
+	DBG("D5530");
+}
+
+static void mbm_quirk_d5530(struct ofono_modem *modem)
+{
+	struct mbm_data *data = ofono_modem_get_data(modem);
+
+	data->variant = MBM_DELL_D5530;
+
+	g_at_chat_register(data->modem_port, "D5530", d5530_notify,
+				FALSE, NULL, NULL);
+}
+
+static void mbm_check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	GAtResultIter iter;
+	char const *model = "";
+
+	DBG("");
+
+	if (!ok)
+		goto done;
+
+	g_at_result_iter_init(&iter, result);
+
+	while (g_at_result_iter_next(&iter, NULL)) {
+		if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+			continue;
+
+		if (g_str_equal(model, "D5530"))
+			mbm_quirk_d5530(modem);
+	}
+
+done:
+	init_simpin_check(modem);
+}
+
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
+	struct mbm_data *data = ofono_modem_get_data(modem);
 
 	DBG("");
 
@@ -148,7 +195,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	init_simpin_check(modem);
+	g_at_chat_send(data->modem_port, "AT+CGMM", NULL,
+			mbm_check_model, modem, NULL);
 }
 
 static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
@@ -426,7 +474,10 @@ static void mbm_post_online(struct ofono_modem *modem)
 					"atmodem", data->modem_port);
 
 	ofono_sms_create(modem, 0, "atmodem", data->modem_port);
-	ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
+
+	if (data->variant != MBM_DELL_D5530)
+		ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
+
 	ofono_ussd_create(modem, 0, "atmodem", data->modem_port);
 
 	gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 16:16 Pekka.Pessi
@ 2010-08-25 16:46 ` Denis Kenzior
  2010-08-25 16:54   ` Pekka Pessi
  2010-08-25 18:05   ` Marcel Holtmann
  0 siblings, 2 replies; 10+ messages in thread
From: Denis Kenzior @ 2010-08-25 16:46 UTC (permalink / raw)
  To: ofono

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

Hi Pekka,

On 08/25/2010 11:16 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
> announcing itself to world with its own name. It also crashes upon
> processing received cbs messages.
> ---
>  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/mbm.c b/plugins/mbm.c
> index c9b0ea4..1c2b9a8 100644
> --- a/plugins/mbm.c
> +++ b/plugins/mbm.c
> @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
>  static const char *cpin_prefix[] = { "+CPIN:", NULL };
>  static const char *none_prefix[] = { NULL };
>  
> +enum mbm_variant {
> +	MBM_GENERIC,
> +	MBM_DELL_D5530,		/* OEM of F3507g */
> +};
> +
>  struct mbm_data {
>  	GAtChat *modem_port;
>  	GAtChat *data_port;
> +	enum mbm_variant variant;
>  	guint poll_source;
>  	guint poll_count;
>  	gboolean have_sim;
> @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
>  	return FALSE;
>  }
>  
> +static void d5530_notify(GAtResult *result, gpointer user_data)
> +{
> +	DBG("D5530");
> +}
> +

Should we really bother with this one? Or you're trying to be like
Marcel and waste some processing time with unused unsolicited
notifications? :)

> +static void mbm_quirk_d5530(struct ofono_modem *modem)
> +{
> +	struct mbm_data *data = ofono_modem_get_data(modem);
> +
> +	data->variant = MBM_DELL_D5530;
> +
> +	g_at_chat_register(data->modem_port, "D5530", d5530_notify,
> +				FALSE, NULL, NULL);
> +}
> +
> +static void mbm_check_model(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	GAtResultIter iter;
> +	char const *model = "";
> +
> +	DBG("");
> +
> +	if (!ok)
> +		goto done;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	while (g_at_result_iter_next(&iter, NULL)) {
> +		if (!g_at_result_iter_next_unquoted_string(&iter, &model))
> +			continue;
> +
> +		if (g_str_equal(model, "D5530"))
> +			mbm_quirk_d5530(modem);
> +	}
> +
> +done:
> +	init_simpin_check(modem);
> +}
> +
>  static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>  {
>  	struct ofono_modem *modem = user_data;
> +	struct mbm_data *data = ofono_modem_get_data(modem);
>  
>  	DBG("");
>  
> @@ -148,7 +195,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>  		return;
>  	}
>  
> -	init_simpin_check(modem);
> +	g_at_chat_send(data->modem_port, "AT+CGMM", NULL,
> +			mbm_check_model, modem, NULL);
>  }
>  
>  static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
> @@ -426,7 +474,10 @@ static void mbm_post_online(struct ofono_modem *modem)
>  					"atmodem", data->modem_port);
>  
>  	ofono_sms_create(modem, 0, "atmodem", data->modem_port);
> -	ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
> +
> +	if (data->variant != MBM_DELL_D5530)
> +		ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
> +
>  	ofono_ussd_create(modem, 0, "atmodem", data->modem_port);
>  
>  	gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,


Can you rebase your patch, I get this when applying:

denkenz(a)ubuntu:~/ofono-master$ git am --3way ~/merge/\[PATCH\]\ mbm\:\
add\ quirks\ for\ Dell\ D5530.eml
Applying: mbm: add quirks for Dell D5530
fatal: sha1 information is lacking or useless (plugins/mbm.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 mbm: add quirks for Dell D5530
When you have resolved this problem run "git am -3 --resolved".
If you would prefer to skip this patch, instead run "git am -3 --skip".
To restore the original branch and stop patching run "git am -3 --abort".


Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 16:46 ` Denis Kenzior
@ 2010-08-25 16:54   ` Pekka Pessi
  2010-08-25 18:05   ` Marcel Holtmann
  1 sibling, 0 replies; 10+ messages in thread
From: Pekka Pessi @ 2010-08-25 16:54 UTC (permalink / raw)
  To: ofono

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

2010/8/25 Denis Kenzior <denkenz@gmail.com>:
> Should we really bother with this one? Or you're trying to be like
> Marcel and waste some processing time with unused unsolicited
> notifications? :)

I'm trying to develop with this fine piece of hardware. First time it
was funny when my IMSI was D5530. Or +GCAP: +CGSM, +DS.

> Can you rebase your patch, I get this when applying:
>
> denkenz(a)ubuntu:~/ofono-master$ git am --3way ~/merge/\[PATCH\]\ mbm\:\
> add\ quirks\ for\ Dell\ D5530.eml
> Applying: mbm: add quirks for Dell D5530
> fatal: sha1 information is lacking or useless (plugins/mbm.c).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 mbm: add quirks for Dell D5530
> When you have resolved this problem run "git am -3 --resolved".
> If you would prefer to skip this patch, instead run "git am -3 --skip".
> To restore the original branch and stop patching run "git am -3 --abort".

I'll do.

-- 
Pekka.Pessi mail at nokia.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] mbm: add quirks for Dell D5530
@ 2010-08-25 17:13 Pekka.Pessi
  2010-08-25 17:18 ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Pekka.Pessi @ 2010-08-25 17:13 UTC (permalink / raw)
  To: ofono

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Dell D5530 is an OEM version of F3507g. It has an annoying habit of
announcing itself to world with its own name. Another problem is some weird
+CGAP messages at the same time. It also crashes upon processing received
CBS messages.
---
 plugins/mbm.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/plugins/mbm.c b/plugins/mbm.c
index 8541aaf..cc3a0f5 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -52,12 +52,18 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
 static const char *none_prefix[] = { NULL };
 
+enum mbm_variant {
+	MBM_GENERIC,
+	MBM_DELL_D5530,		/* OEM of F3507g */
+};
+
 struct mbm_data {
 	GAtChat *modem_port;
 	GAtChat *data_port;
 	guint cpin_poll_source;
 	guint cpin_poll_count;
 	gboolean have_sim;
+	enum mbm_variant variant;
 };
 
 static int mbm_probe(struct ofono_modem *modem)
@@ -136,9 +142,54 @@ static gboolean init_simpin_check(gpointer user_data)
 	return FALSE;
 }
 
+static void d5530_notify(GAtResult *result, gpointer user_data)
+{
+	DBG("D5530");
+}
+
+static void mbm_quirk_d5530(struct ofono_modem *modem)
+{
+	struct mbm_data *data = ofono_modem_get_data(modem);
+
+	data->variant = MBM_DELL_D5530;
+
+	/* This Dell modem sends some unsolicated messages when it boots. */
+	/* Try to ignore them. */
+	g_at_chat_register(data->modem_port, "D5530", d5530_notify,
+				FALSE, NULL, NULL);
+	g_at_chat_register(data->modem_port, "+CGAP:", d5530_notify,
+				FALSE, NULL, NULL);
+}
+
+static void mbm_check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	GAtResultIter iter;
+	char const *model = "";
+
+	DBG("");
+
+	if (!ok)
+		goto done;
+
+	g_at_result_iter_init(&iter, result);
+
+	while (g_at_result_iter_next(&iter, NULL)) {
+		if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+			continue;
+
+		if (g_str_equal(model, "D5530"))
+			mbm_quirk_d5530(modem);
+	}
+
+done:
+	init_simpin_check(modem);
+}
+
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
+	struct mbm_data *data = ofono_modem_get_data(modem);
 
 	DBG("");
 
@@ -147,7 +198,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	init_simpin_check(modem);
+	g_at_chat_send(data->modem_port, "AT+CGMM", NULL,
+			mbm_check_model, modem, NULL);
 }
 
 static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
@@ -353,7 +405,15 @@ static void mbm_post_sim(struct ofono_modem *modem)
 					"atmodem", data->modem_port);
 
 	ofono_sms_create(modem, 0, "atmodem", data->modem_port);
-	ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
+
+	switch (data->variant) {
+	case MBM_DELL_D5530:
+		/* DELL D5530 crashes when it processes CBSs */
+		break;
+	default:
+		ofono_cbs_create(modem, 0, "atmodem", data->modem_port);
+	}
+
 	ofono_ussd_create(modem, 0, "atmodem", data->modem_port);
 
 	gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 17:13 [PATCH] mbm: add quirks for Dell D5530 Pekka.Pessi
@ 2010-08-25 17:18 ` Denis Kenzior
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2010-08-25 17:18 UTC (permalink / raw)
  To: ofono

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

Hi Pekka,

On 08/25/2010 12:13 PM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
> announcing itself to world with its own name. Another problem is some weird
> +CGAP messages at the same time. It also crashes upon processing received
> CBS messages.

Applied, thanks.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 16:46 ` Denis Kenzior
  2010-08-25 16:54   ` Pekka Pessi
@ 2010-08-25 18:05   ` Marcel Holtmann
  2010-08-25 18:08     ` Denis Kenzior
  1 sibling, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2010-08-25 18:05 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> > Dell D5530 is an OEM version of F3507g. It has an annoying habit of
> > announcing itself to world with its own name. It also crashes upon
> > processing received cbs messages.
> > ---
> >  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/plugins/mbm.c b/plugins/mbm.c
> > index c9b0ea4..1c2b9a8 100644
> > --- a/plugins/mbm.c
> > +++ b/plugins/mbm.c
> > @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
> >  static const char *cpin_prefix[] = { "+CPIN:", NULL };
> >  static const char *none_prefix[] = { NULL };
> >  
> > +enum mbm_variant {
> > +	MBM_GENERIC,
> > +	MBM_DELL_D5530,		/* OEM of F3507g */
> > +};
> > +
> >  struct mbm_data {
> >  	GAtChat *modem_port;
> >  	GAtChat *data_port;
> > +	enum mbm_variant variant;
> >  	guint poll_source;
> >  	guint poll_count;
> >  	gboolean have_sim;
> > @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
> >  	return FALSE;
> >  }
> >  
> > +static void d5530_notify(GAtResult *result, gpointer user_data)
> > +{
> > +	DBG("D5530");
> > +}
> > +
> 
> Should we really bother with this one? Or you're trying to be like
> Marcel and waste some processing time with unused unsolicited
> notifications? :)

I think we should until we have this all figured out. More debug output
is always a good thing.

And yes, essentially there is some processing wasted, but then again,
this hardware is so damn stupid broken it deserves to be punished ;)

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 18:05   ` Marcel Holtmann
@ 2010-08-25 18:08     ` Denis Kenzior
  2010-08-25 18:23       ` Marcel Holtmann
  0 siblings, 1 reply; 10+ messages in thread
From: Denis Kenzior @ 2010-08-25 18:08 UTC (permalink / raw)
  To: ofono

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

Hi Marcel,

On 08/25/2010 01:05 PM, Marcel Holtmann wrote:
> Hi Denis,
> 
>>> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
>>> announcing itself to world with its own name. It also crashes upon
>>> processing received cbs messages.
>>> ---
>>>  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>  1 files changed, 53 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/plugins/mbm.c b/plugins/mbm.c
>>> index c9b0ea4..1c2b9a8 100644
>>> --- a/plugins/mbm.c
>>> +++ b/plugins/mbm.c
>>> @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
>>>  static const char *cpin_prefix[] = { "+CPIN:", NULL };
>>>  static const char *none_prefix[] = { NULL };
>>>  
>>> +enum mbm_variant {
>>> +	MBM_GENERIC,
>>> +	MBM_DELL_D5530,		/* OEM of F3507g */
>>> +};
>>> +
>>>  struct mbm_data {
>>>  	GAtChat *modem_port;
>>>  	GAtChat *data_port;
>>> +	enum mbm_variant variant;
>>>  	guint poll_source;
>>>  	guint poll_count;
>>>  	gboolean have_sim;
>>> @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
>>>  	return FALSE;
>>>  }
>>>  
>>> +static void d5530_notify(GAtResult *result, gpointer user_data)
>>> +{
>>> +	DBG("D5530");
>>> +}
>>> +
>>
>> Should we really bother with this one? Or you're trying to be like
>> Marcel and waste some processing time with unused unsolicited
>> notifications? :)
> 
> I think we should until we have this all figured out. More debug output
> is always a good thing.
> 
> And yes, essentially there is some processing wasted, but then again,
> this hardware is so damn stupid broken it deserves to be punished ;)
> 

I have to disagree, I can understand if you take the unsolicited
notification and break it down somewhat (like we used to with OCTI,
OWCTI, etc for HSO).  But just printing a debug seems useless.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 18:08     ` Denis Kenzior
@ 2010-08-25 18:23       ` Marcel Holtmann
  2010-08-25 18:29         ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2010-08-25 18:23 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> >>> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
> >>> announcing itself to world with its own name. It also crashes upon
> >>> processing received cbs messages.
> >>> ---
> >>>  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>>  1 files changed, 53 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/plugins/mbm.c b/plugins/mbm.c
> >>> index c9b0ea4..1c2b9a8 100644
> >>> --- a/plugins/mbm.c
> >>> +++ b/plugins/mbm.c
> >>> @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
> >>>  static const char *cpin_prefix[] = { "+CPIN:", NULL };
> >>>  static const char *none_prefix[] = { NULL };
> >>>  
> >>> +enum mbm_variant {
> >>> +	MBM_GENERIC,
> >>> +	MBM_DELL_D5530,		/* OEM of F3507g */
> >>> +};
> >>> +
> >>>  struct mbm_data {
> >>>  	GAtChat *modem_port;
> >>>  	GAtChat *data_port;
> >>> +	enum mbm_variant variant;
> >>>  	guint poll_source;
> >>>  	guint poll_count;
> >>>  	gboolean have_sim;
> >>> @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
> >>>  	return FALSE;
> >>>  }
> >>>  
> >>> +static void d5530_notify(GAtResult *result, gpointer user_data)
> >>> +{
> >>> +	DBG("D5530");
> >>> +}
> >>> +
> >>
> >> Should we really bother with this one? Or you're trying to be like
> >> Marcel and waste some processing time with unused unsolicited
> >> notifications? :)
> > 
> > I think we should until we have this all figured out. More debug output
> > is always a good thing.
> > 
> > And yes, essentially there is some processing wasted, but then again,
> > this hardware is so damn stupid broken it deserves to be punished ;)
> > 
> 
> I have to disagree, I can understand if you take the unsolicited
> notification and break it down somewhat (like we used to with OCTI,
> OWCTI, etc for HSO).  But just printing a debug seems useless.

then lets break it down ;)

	DBG("D");
	DBG("5");
	DBG("5");
	DBG("3");
	DBG("0");

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 18:23       ` Marcel Holtmann
@ 2010-08-25 18:29         ` Denis Kenzior
  2010-08-25 18:40           ` Marcel Holtmann
  0 siblings, 1 reply; 10+ messages in thread
From: Denis Kenzior @ 2010-08-25 18:29 UTC (permalink / raw)
  To: ofono

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

Hi Marcel,

On 08/25/2010 01:23 PM, Marcel Holtmann wrote:
> Hi Denis,
> 
>>>>> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
>>>>> announcing itself to world with its own name. It also crashes upon
>>>>> processing received cbs messages.
>>>>> ---
>>>>>  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>>>  1 files changed, 53 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/plugins/mbm.c b/plugins/mbm.c
>>>>> index c9b0ea4..1c2b9a8 100644
>>>>> --- a/plugins/mbm.c
>>>>> +++ b/plugins/mbm.c
>>>>> @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
>>>>>  static const char *cpin_prefix[] = { "+CPIN:", NULL };
>>>>>  static const char *none_prefix[] = { NULL };
>>>>>  
>>>>> +enum mbm_variant {
>>>>> +	MBM_GENERIC,
>>>>> +	MBM_DELL_D5530,		/* OEM of F3507g */
>>>>> +};
>>>>> +
>>>>>  struct mbm_data {
>>>>>  	GAtChat *modem_port;
>>>>>  	GAtChat *data_port;
>>>>> +	enum mbm_variant variant;
>>>>>  	guint poll_source;
>>>>>  	guint poll_count;
>>>>>  	gboolean have_sim;
>>>>> @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
>>>>>  	return FALSE;
>>>>>  }
>>>>>  
>>>>> +static void d5530_notify(GAtResult *result, gpointer user_data)
>>>>> +{
>>>>> +	DBG("D5530");
>>>>> +}
>>>>> +
>>>>
>>>> Should we really bother with this one? Or you're trying to be like
>>>> Marcel and waste some processing time with unused unsolicited
>>>> notifications? :)
>>>
>>> I think we should until we have this all figured out. More debug output
>>> is always a good thing.
>>>
>>> And yes, essentially there is some processing wasted, but then again,
>>> this hardware is so damn stupid broken it deserves to be punished ;)
>>>
>>
>> I have to disagree, I can understand if you take the unsolicited
>> notification and break it down somewhat (like we used to with OCTI,
>> OWCTI, etc for HSO).  But just printing a debug seems useless.
> 
> then lets break it down ;)
> 
> 	DBG("D");
> 	DBG("5");
> 	DBG("5");
> 	DBG("3");
> 	DBG("0");
> 

Funny ;)

I meant more like, if you don't g_at_result_iter_init() in the callback,
then don't bother registering for it in the first place.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mbm: add quirks for Dell D5530
  2010-08-25 18:29         ` Denis Kenzior
@ 2010-08-25 18:40           ` Marcel Holtmann
  0 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2010-08-25 18:40 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> >>>>> Dell D5530 is an OEM version of F3507g. It has an annoying habit of
> >>>>> announcing itself to world with its own name. It also crashes upon
> >>>>> processing received cbs messages.
> >>>>> ---
> >>>>>  plugins/mbm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>>>>  1 files changed, 53 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/plugins/mbm.c b/plugins/mbm.c
> >>>>> index c9b0ea4..1c2b9a8 100644
> >>>>> --- a/plugins/mbm.c
> >>>>> +++ b/plugins/mbm.c
> >>>>> @@ -53,9 +53,15 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
> >>>>>  static const char *cpin_prefix[] = { "+CPIN:", NULL };
> >>>>>  static const char *none_prefix[] = { NULL };
> >>>>>  
> >>>>> +enum mbm_variant {
> >>>>> +	MBM_GENERIC,
> >>>>> +	MBM_DELL_D5530,		/* OEM of F3507g */
> >>>>> +};
> >>>>> +
> >>>>>  struct mbm_data {
> >>>>>  	GAtChat *modem_port;
> >>>>>  	GAtChat *data_port;
> >>>>> +	enum mbm_variant variant;
> >>>>>  	guint poll_source;
> >>>>>  	guint poll_count;
> >>>>>  	gboolean have_sim;
> >>>>> @@ -137,9 +143,50 @@ static gboolean init_simpin_check(gpointer user_data)
> >>>>>  	return FALSE;
> >>>>>  }
> >>>>>  
> >>>>> +static void d5530_notify(GAtResult *result, gpointer user_data)
> >>>>> +{
> >>>>> +	DBG("D5530");
> >>>>> +}
> >>>>> +
> >>>>
> >>>> Should we really bother with this one? Or you're trying to be like
> >>>> Marcel and waste some processing time with unused unsolicited
> >>>> notifications? :)
> >>>
> >>> I think we should until we have this all figured out. More debug output
> >>> is always a good thing.
> >>>
> >>> And yes, essentially there is some processing wasted, but then again,
> >>> this hardware is so damn stupid broken it deserves to be punished ;)
> >>>
> >>
> >> I have to disagree, I can understand if you take the unsolicited
> >> notification and break it down somewhat (like we used to with OCTI,
> >> OWCTI, etc for HSO).  But just printing a debug seems useless.
> > 
> > then lets break it down ;)
> > 
> > 	DBG("D");
> > 	DBG("5");
> > 	DBG("5");
> > 	DBG("3");
> > 	DBG("0");
> > 
> 
> Funny ;)
> 
> I meant more like, if you don't g_at_result_iter_init() in the callback,
> then don't bother registering for it in the first place.

I figured that and I am fine with your argumentation to not do it, but I
just couldn't resit on this one ;)

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-08-25 18:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-25 17:13 [PATCH] mbm: add quirks for Dell D5530 Pekka.Pessi
2010-08-25 17:18 ` Denis Kenzior
  -- strict thread matches above, loose matches on Subject: below --
2010-08-25 16:16 Pekka.Pessi
2010-08-25 16:46 ` Denis Kenzior
2010-08-25 16:54   ` Pekka Pessi
2010-08-25 18:05   ` Marcel Holtmann
2010-08-25 18:08     ` Denis Kenzior
2010-08-25 18:23       ` Marcel Holtmann
2010-08-25 18:29         ` Denis Kenzior
2010-08-25 18:40           ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.