[PATCH_v0 0/3] Unify huawei and huaweicdma plugins

[PATCH_v0 0/3] Unify huawei and huaweicdma plugins

[Guillaume Zajac]

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

Hi,

This set of patch is unifying huawei and huaweicdma plugin.
ATI command is now sent after ATE0 +CMEE=1 to check whether the modem is
CDMA or GSM type. Atoms are now created accordingly.

Concerning CDMA modem with embedded SIM, no sim atom will be created.
We always set online the modem.
Other CDMA modems will have a sim atom that will manage PIN rountine only.
Sim File System cannot be managed for the moment.

Offline command for CDMA modems that support ^RFSWITCH is AT+CFUN=5
unlike GSM modems which is AT+CFUN=7.

Kind regards,
Guillaume

Guillaume Zajac (3):
  udevng: Simplify vendor_list for Huawei constructor
  huawei: Add modem type detection
  huaweicdma: Delete unused plugin

 Makefile.am          |    3 -
 plugins/huawei.c     |   77 ++++++++++++++--
 plugins/huaweicdma.c |  247 --------------------------------------------------
 plugins/udevng.c     |    4 +-
 4 files changed, 70 insertions(+), 261 deletions(-)
 delete mode 100644 plugins/huaweicdma.c

[PATCH_v0 1/3] udevng: Simplify vendor_list for Huawei constructor

[Guillaume Zajac]

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

According to Huawei constructor, the port mapping is identical
for all the modems. We can decide to use GSM or CDMA drivers into
the plugin.
---
 plugins/udevng.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index ef20955..7d81416 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -793,11 +793,9 @@ static struct {
 	{ "hso",	"hso"				},
 	{ "gobi",	"qcserial"			},
 	{ "sierra",	"sierra"			},
-	{ "huawei",	"option",	"201e", "2009"	},
+	{ "huawei",	"option",	"201e"		},
 	{ "huawei",	"cdc_ether",	"12d1"		},
 	{ "huawei",	"option",	"12d1"		},
-	{ "huaweicdma",	"option",	"12d1", "140b"	},
-	{ "huaweicdma", "option",	"201e"		},
 	{ "speedupcdma","option",	"1c9e", "9e00"	},
 	{ "speedup",	"option",	"1c9e"		},
 	{ "speedup",	"option",	"2020"		},
-- 
1.7.1

[PATCH_v0 2/3] huawei: Add modem type detection

[Guillaume Zajac]

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

This plugin is now merged with huaweicdma one.
Atoms will now be created with GSM or CDMA drivers
depending on the result of ATI command.
No SIM atom is created for embedded sim from CDMA
modems.
---
 plugins/huawei.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/plugins/huawei.c b/plugins/huawei.c
index ae15bf9..a3768c8 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -51,6 +51,9 @@
 #include <ofono/message-waiting.h>
 #include <ofono/log.h>
 
+#include <ofono/cdma-netreg.h>
+#include <ofono/cdma-connman.h>
+
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
@@ -66,6 +69,7 @@ enum {
 	SIM_STATE_INVALID_CS =		2,
 	SIM_STATE_INVALID_PS =		3,
 	SIM_STATE_INVALID_PS_AND_CS =	4,
+	SIM_STATE_ROMSIM =		240,
 	SIM_STATE_NOT_EXISTENT =	255,
 };
 
@@ -79,6 +83,7 @@ struct huawei_data {
 	struct cb_data *online_cbd;
 	const char *offline_command;
 	gboolean voice;
+	gboolean gsm;
 };
 
 static int huawei_probe(struct ofono_modem *modem)
@@ -405,7 +410,7 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_modem *modem = user_data;
 	struct huawei_data *data = ofono_modem_get_data(modem);
 
-	if (!ok)
+	if (!ok || !data->gsm)
 		data->offline_command = "AT+CFUN=5";
 	else
 		data->offline_command = "AT+CFUN=7";
@@ -414,6 +419,44 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
 					cfun_enable, modem, NULL);
 }
 
+static gboolean parse_ati_result(GAtResult *result)
+{
+	GAtResultIter iter;
+	const char *line;
+	int num = g_at_result_num_response_lines(result);
+	int i;
+
+	g_at_result_iter_init(&iter, result);
+
+	for (i = 0; i < num; i++) {
+		g_at_result_iter_next(&iter, NULL);
+		line = g_at_result_iter_raw_line(&iter);
+		if (g_str_has_prefix(line, "+GCAP"))
+			return (g_strrstr(line, "+CGSM") != NULL);
+	}
+
+	/* By default we consider modem is GSM type */
+	return TRUE;
+}
+
+static void ati_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct huawei_data *data = ofono_modem_get_data(modem);
+
+	/* By default we consider modem is GSM type */
+	if (!ok)
+		data->gsm = TRUE;
+	else
+		data->gsm = parse_ati_result(result);
+
+	data->sim_state = SIM_STATE_NOT_EXISTENT;
+
+	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
+					rfswitch_support, modem, NULL);
+}
+
+
 static GAtChat *open_device(struct ofono_modem *modem,
 				const char *key, char *debug)
 {
@@ -472,10 +515,7 @@ static int huawei_enable(struct ofono_modem *modem)
 	g_at_chat_send(data->modem, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
 	g_at_chat_send(data->pcui, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
 
-	data->sim_state = SIM_STATE_NOT_EXISTENT;
-
-	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
-					rfswitch_support, modem, NULL);
+	g_at_chat_send(data->pcui, "ATI", NULL, ati_cb, modem, NULL);
 
 	return -EINPROGRESS;
 }
@@ -555,6 +595,7 @@ static void sysinfo_online_cb(gboolean ok, GAtResult *result,
 	case SIM_STATE_INVALID_CS:
 	case SIM_STATE_INVALID_PS:
 	case SIM_STATE_INVALID_PS_AND_CS:
+	case SIM_STATE_ROMSIM:
 		CALLBACK_WITH_SUCCESS(cb, data->online_cbd->data);
 		goto done;
 	}
@@ -650,13 +691,21 @@ static void huawei_set_online(struct ofono_modem *modem, ofono_bool_t online,
 static void huawei_pre_sim(struct ofono_modem *modem)
 {
 	struct huawei_data *data = ofono_modem_get_data(modem);
-	struct ofono_sim *sim;
+	struct ofono_sim *sim = NULL;
 
 	DBG("%p", modem);
 
-	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
-	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
+	if (data->gsm == TRUE) {
+		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
+		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
 					"atmodem", data->pcui);
+	} else {
+		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
+		/* Create sim atom only if sim is not embedded */
+		if (data->sim_state != SIM_STATE_ROMSIM)
+			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
+						"atmodem", data->pcui);
+	}
 
 	if (sim && data->have_sim == TRUE)
 		ofono_sim_inserted_notify(sim, TRUE);
@@ -670,6 +719,9 @@ static void huawei_post_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	if (data->gsm == FALSE)
+		return;
+
 	if (data->voice == TRUE) {
 		ofono_voicecall_create(modem, 0, "huaweimodem", data->pcui);
 		ofono_audio_settings_create(modem, 0,
@@ -695,6 +747,15 @@ static void huawei_post_online(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	if (data->gsm == FALSE) {
+		ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem",
+						data->pcui);
+
+		ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI,
+						"cdmamodem", data->modem);
+		return;
+	}
+
 	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->pcui);
 
 	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
-- 
1.7.1

[PATCH_v0 3/3] huaweicdma: Delete unused plugin

[Guillaume Zajac]

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

---
 Makefile.am          |    3 -
 plugins/huaweicdma.c |  247 --------------------------------------------------
 2 files changed, 0 insertions(+), 250 deletions(-)
 delete mode 100644 plugins/huaweicdma.c

diff --git a/Makefile.am b/Makefile.am
index 9831924..ab99283 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -306,9 +306,6 @@ builtin_sources += plugins/zte.c
 builtin_modules += huawei
 builtin_sources += plugins/huawei.c
 
-builtin_modules += huaweicdma
-builtin_sources += plugins/huaweicdma.c
-
 builtin_modules += sierra
 builtin_sources += plugins/sierra.c
 
diff --git a/plugins/huaweicdma.c b/plugins/huaweicdma.c
deleted file mode 100644
index 6fba0c1..0000000
--- a/plugins/huaweicdma.c
+++ /dev/null
@@ -1,247 +0,0 @@
-/*
- *
- *  oFono - Open Source Telephony
- *
- *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2 as
- *  published by the Free Software Foundation.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <errno.h>
-#include <stdlib.h>
-
-#include <glib.h>
-#include <gatchat.h>
-#include <gattty.h>
-
-#define OFONO_API_SUBJECT_TO_CHANGE
-#include <ofono/plugin.h>
-#include <ofono/modem.h>
-#include <ofono/devinfo.h>
-#include <ofono/cdma-netreg.h>
-#include <ofono/cdma-connman.h>
-#include <ofono/log.h>
-
-#include "drivers/atmodem/vendor.h"
-
-struct huaweicdma_data {
-	GAtChat *modem;
-	GAtChat *pcui;
-};
-
-static void huaweicdma_debug(const char *str, void *data)
-{
-	const char *prefix = data;
-
-	ofono_info("%s%s", prefix, str);
-}
-
-static int huaweicdma_probe(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data;
-
-	DBG("%p", modem);
-
-	data = g_try_new0(struct huaweicdma_data, 1);
-	if (data == NULL)
-		return -ENOMEM;
-
-	ofono_modem_set_data(modem, data);
-
-	return 0;
-}
-
-static void huaweicdma_remove(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	ofono_modem_set_data(modem, NULL);
-
-	/* Cleanup after hot-unplug */
-	g_at_chat_unref(data->pcui);
-
-	g_free(data);
-}
-
-static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
-{
-	struct ofono_modem *modem = user_data;
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("");
-
-	if (!ok) {
-		g_at_chat_unref(data->modem);
-		data->modem = NULL;
-
-		g_at_chat_unref(data->pcui);
-		data->pcui = NULL;
-	}
-
-	ofono_modem_set_powered(modem, ok);
-}
-
-static GAtChat *open_device(struct ofono_modem *modem,
-				const char *key, char *debug)
-{
-	const char *device;
-	GIOChannel *channel;
-	GAtSyntax *syntax;
-	GAtChat *chat;
-
-	device = ofono_modem_get_string(modem, key);
-	if (device == NULL)
-		return NULL;
-
-	DBG("%s %s", key, device);
-
-	channel = g_at_tty_open(device, NULL);
-	if (channel == NULL)
-		return NULL;
-
-	syntax = g_at_syntax_new_gsm_permissive();
-	chat = g_at_chat_new(channel, syntax);
-	g_at_syntax_unref(syntax);
-
-	g_io_channel_unref(channel);
-
-	if (chat == NULL)
-		return NULL;
-
-	if (getenv("OFONO_AT_DEBUG"))
-		g_at_chat_set_debug(chat, huaweicdma_debug, debug);
-
-	return chat;
-}
-
-static int huaweicdma_enable(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("");
-
-	data->modem = open_device(modem, "Modem", "Modem: ");
-	if (data->modem == NULL)
-		return -EINVAL;
-
-	data->pcui = open_device(modem, "Pcui", "PCUI: ");
-	if (data->pcui == NULL) {
-		g_at_chat_unref(data->modem);
-		data->modem = NULL;
-		return -EIO;
-	}
-
-	g_at_chat_set_slave(data->modem, data->pcui);
-
-	g_at_chat_send(data->modem, "ATE0 &C0 +CMEE=1", NULL, NULL, NULL, NULL);
-	g_at_chat_send(data->pcui, "ATE0 &C0 +CMEE=1", NULL, NULL, NULL, NULL);
-
-	g_at_chat_send(data->pcui, "AT+CFUN=1", NULL,
-					cfun_enable, modem, NULL);
-
-	return -EINPROGRESS;
-}
-
-static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
-{
-	struct ofono_modem *modem = user_data;
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("");
-
-	g_at_chat_unref(data->pcui);
-	data->pcui = NULL;
-
-	if (ok)
-		ofono_modem_set_powered(modem, FALSE);
-}
-
-static int huaweicdma_disable(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	g_at_chat_cancel_all(data->modem);
-	g_at_chat_unregister_all(data->modem);
-
-	g_at_chat_unref(data->modem);
-	data->modem = NULL;
-
-	g_at_chat_cancel_all(data->pcui);
-	g_at_chat_unregister_all(data->pcui);
-
-	g_at_chat_send(data->pcui, "AT+CFUN=0", NULL,
-					cfun_disable, modem, NULL);
-
-	return -EINPROGRESS;
-}
-
-static void huaweicdma_pre_sim(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
-}
-
-static void huaweicdma_post_sim(struct ofono_modem *modem)
-{
-	DBG("%p", modem);
-}
-
-static void huaweicdma_post_online(struct ofono_modem *modem)
-{
-	struct huaweicdma_data *data = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem", data->pcui);
-
-	ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI, "cdmamodem",
-					data->modem);
-}
-
-static struct ofono_modem_driver huaweicdma_driver = {
-	.name		= "huaweicdma",
-	.probe		= huaweicdma_probe,
-	.remove		= huaweicdma_remove,
-	.enable		= huaweicdma_enable,
-	.disable	= huaweicdma_disable,
-	.pre_sim	= huaweicdma_pre_sim,
-	.post_sim	= huaweicdma_post_sim,
-	.post_online	= huaweicdma_post_online,
-};
-
-static int huaweicdma_init(void)
-{
-	return ofono_modem_driver_register(&huaweicdma_driver);
-}
-
-static void huaweicdma_exit(void)
-{
-	ofono_modem_driver_unregister(&huaweicdma_driver);
-}
-
-OFONO_PLUGIN_DEFINE(huaweicdma, "Huawei CDMA modem driver", VERSION,
-				OFONO_PLUGIN_PRIORITY_DEFAULT,
-				huaweicdma_init, huaweicdma_exit)
-- 
1.7.1

Re: [PATCH_v0 1/3] udevng: Simplify vendor_list for Huawei constructor

[Marcel Holtmann]

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

Hi Guillaume,

> According to Huawei constructor, the port mapping is identical
> for all the modems. We can decide to use GSM or CDMA drivers into
> the plugin.
> ---
>  plugins/udevng.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

we might should do this last, but I applied the patch anyway.

Regards

Marcel

Re: [PATCH_v0 2/3] huawei: Add modem type detection

[Marcel Holtmann]

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

Hi Guillaume,

> This plugin is now merged with huaweicdma one.
> Atoms will now be created with GSM or CDMA drivers
> depending on the result of ATI command.
> No SIM atom is created for embedded sim from CDMA
> modems.
> ---
>  plugins/huawei.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 69 insertions(+), 8 deletions(-)
> 
> diff --git a/plugins/huawei.c b/plugins/huawei.c
> index ae15bf9..a3768c8 100644
> --- a/plugins/huawei.c
> +++ b/plugins/huawei.c
> @@ -51,6 +51,9 @@
>  #include <ofono/message-waiting.h>
>  #include <ofono/log.h>
>  
> +#include <ofono/cdma-netreg.h>
> +#include <ofono/cdma-connman.h>
> +
>  #include <drivers/atmodem/atutil.h>
>  #include <drivers/atmodem/vendor.h>
>  
> @@ -66,6 +69,7 @@ enum {
>  	SIM_STATE_INVALID_CS =		2,
>  	SIM_STATE_INVALID_PS =		3,
>  	SIM_STATE_INVALID_PS_AND_CS =	4,
> +	SIM_STATE_ROMSIM =		240,
>  	SIM_STATE_NOT_EXISTENT =	255,
>  };

I really don't wanna random intermix of determining modem type and
dealing with CDMA extensions. This should be nicely split into logical
patches.
 
> @@ -79,6 +83,7 @@ struct huawei_data {
>  	struct cb_data *online_cbd;
>  	const char *offline_command;
>  	gboolean voice;
> +	gboolean gsm;

So just calling this gsm is a bit to short. Same goes for voice. That
was a mistake and I fixed that upstream now to rename it to have_voice.

>  };
>  
>  static int huawei_probe(struct ofono_modem *modem)
> @@ -405,7 +410,7 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
>  	struct ofono_modem *modem = user_data;
>  	struct huawei_data *data = ofono_modem_get_data(modem);
>  
> -	if (!ok)
> +	if (!ok || !data->gsm)
>  		data->offline_command = "AT+CFUN=5";
>  	else
>  		data->offline_command = "AT+CFUN=7";

This should be a separate patch as well. I think the important part is
to first enhance the plugin to detect GSM. And then on-top of that
handle the difference between GSM and CDMA.

> @@ -414,6 +419,44 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
>  					cfun_enable, modem, NULL);
>  }
>  
> +static gboolean parse_ati_result(GAtResult *result)
> +{
> +	GAtResultIter iter;
> +	const char *line;
> +	int num = g_at_result_num_response_lines(result);
> +	int i;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	for (i = 0; i < num; i++) {
> +		g_at_result_iter_next(&iter, NULL);
> +		line = g_at_result_iter_raw_line(&iter);
> +		if (g_str_has_prefix(line, "+GCAP"))
> +			return (g_strrstr(line, "+CGSM") != NULL);

This parsing is more complex than it needs to be. It also parses the
string too many times. Especially since GAtChat does that all for you
already anyway.

Instead of posting an example here, I just pushed a patch so you can see
how easily this can be done.

> +	}
> +
> +	/* By default we consider modem is GSM type */
> +	return TRUE;
> +}
> +
> +static void ati_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct huawei_data *data = ofono_modem_get_data(modem);
> +
> +	/* By default we consider modem is GSM type */
> +	if (!ok)
> +		data->gsm = TRUE;
> +	else
> +		data->gsm = parse_ati_result(result);

I don't like this. We should use have_gsm and have_cdma and not randomly
fallback to one of them. If for some reason ATI does not show any of
them, we should also not do anything either.

More important I do wanna get bug reports if ATI does not carry +GCAP or
has some weird capabilities inside.

> +
> +	data->sim_state = SIM_STATE_NOT_EXISTENT;
> +
> +	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
> +					rfswitch_support, modem, NULL);
> +}
> +
> +
>  static GAtChat *open_device(struct ofono_modem *modem,
>  				const char *key, char *debug)
>  {
> @@ -472,10 +515,7 @@ static int huawei_enable(struct ofono_modem *modem)
>  	g_at_chat_send(data->modem, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
>  	g_at_chat_send(data->pcui, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
>  
> -	data->sim_state = SIM_STATE_NOT_EXISTENT;

Leave this sim_state assignment at this location. No point in moving
this higher up.

> -
> -	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
> -					rfswitch_support, modem, NULL);
> +	g_at_chat_send(data->pcui, "ATI", NULL, ati_cb, modem, NULL);
>  
>  	return -EINPROGRESS;
>  }
> @@ -555,6 +595,7 @@ static void sysinfo_online_cb(gboolean ok, GAtResult *result,
>  	case SIM_STATE_INVALID_CS:
>  	case SIM_STATE_INVALID_PS:
>  	case SIM_STATE_INVALID_PS_AND_CS:
> +	case SIM_STATE_ROMSIM:
>  		CALLBACK_WITH_SUCCESS(cb, data->online_cbd->data);

This should be a separate patch together with the state addition.

>  		goto done;
>  	}
> @@ -650,13 +691,21 @@ static void huawei_set_online(struct ofono_modem *modem, ofono_bool_t online,
>  static void huawei_pre_sim(struct ofono_modem *modem)
>  {
>  	struct huawei_data *data = ofono_modem_get_data(modem);
> -	struct ofono_sim *sim;
> +	struct ofono_sim *sim = NULL;

Please don't do that. I prefer not assigning variables with NULL here.
It is better to change the logic around.

>  
>  	DBG("%p", modem);
>  
> -	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
> -	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> +	if (data->gsm == TRUE) {
> +		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
> +		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>  					"atmodem", data->pcui);
> +	} else {
> +		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
> +		/* Create sim atom only if sim is not embedded */
> +		if (data->sim_state != SIM_STATE_ROMSIM)
> +			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> +						"atmodem", data->pcui);

I am really not sure that it is a good idea to just use the GSM SIM atom
here. The EF structure will be different and we might cause more harm
than doing any good in assuming that we get any proper EF fields.

This is clearly the part where we need detailed information from Huawei
on how this is suppose to work. And how this is suppose to be done for
CDMA in the first place anyway.

> +	}
>  
>  	if (sim && data->have_sim == TRUE)
>  		ofono_sim_inserted_notify(sim, TRUE);
> @@ -670,6 +719,9 @@ static void huawei_post_sim(struct ofono_modem *modem)
>  
>  	DBG("%p", modem);
>  
> +	if (data->gsm == FALSE)
> +		return;
> +
>  	if (data->voice == TRUE) {
>  		ofono_voicecall_create(modem, 0, "huaweimodem", data->pcui);
>  		ofono_audio_settings_create(modem, 0,
> @@ -695,6 +747,15 @@ static void huawei_post_online(struct ofono_modem *modem)
>  
>  	DBG("%p", modem);
>  
> +	if (data->gsm == FALSE) {
> +		ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem",
> +						data->pcui);
> +
> +		ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI,
> +						"cdmamodem", data->modem);
> +		return;
> +	}
> +
>  	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->pcui);
>  
>  	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,

Lets do if (data->have_gsm ... and if (data->have_cdma ...) separate
here. I rather no degrade GSM to second class citizen.

Regards

Marcel

Re: [PATCH_v0 3/3] huaweicdma: Delete unused plugin

[Marcel Holtmann]

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

Hi Guillaume,

>  Makefile.am          |    3 -
>  plugins/huaweicdma.c |  247 --------------------------------------------------
>  2 files changed, 0 insertions(+), 250 deletions(-)
>  delete mode 100644 plugins/huaweicdma.c

so I applied this patch as well. Just to get it out of way.

Since we merged huaweicdma into huawei modem plugin, we should also get
rid of drivers/huaweicdmamodem/ as well.

Lets move drivers/huaweicdmamodem/network-registration.c into
drivers/huaweimodem/cdma-netreg.c.

Regards

Marcel

Re: [PATCH_v0 2/3] huawei: Add modem type detection

[Guillaume Zajac]

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

Hi Marcel,

>> This plugin is now merged with huaweicdma one.
>> Atoms will now be created with GSM or CDMA drivers
>> depending on the result of ATI command.
>> No SIM atom is created for embedded sim from CDMA
>> modems.
>> ---
>>   plugins/huawei.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>>   1 files changed, 69 insertions(+), 8 deletions(-)
>>
>> diff --git a/plugins/huawei.c b/plugins/huawei.c
>> index ae15bf9..a3768c8 100644
>> --- a/plugins/huawei.c
>> +++ b/plugins/huawei.c
>> @@ -51,6 +51,9 @@
>>   #include<ofono/message-waiting.h>
>>   #include<ofono/log.h>
>>
>> +#include<ofono/cdma-netreg.h>
>> +#include<ofono/cdma-connman.h>
>> +
>>   #include<drivers/atmodem/atutil.h>
>>   #include<drivers/atmodem/vendor.h>
>>
>> @@ -66,6 +69,7 @@ enum {
>>   	SIM_STATE_INVALID_CS =		2,
>>   	SIM_STATE_INVALID_PS =		3,
>>   	SIM_STATE_INVALID_PS_AND_CS =	4,
>> +	SIM_STATE_ROMSIM =		240,
>>   	SIM_STATE_NOT_EXISTENT =	255,
>>   };
> I really don't wanna random intermix of determining modem type and
> dealing with CDMA extensions. This should be nicely split into logical
> patches.

Yes, that was my intention this was a v0 for review.
Anyway it seems you did it well ;)

>
>> @@ -79,6 +83,7 @@ struct huawei_data {
>>   	struct cb_data *online_cbd;
>>   	const char *offline_command;
>>   	gboolean voice;
>> +	gboolean gsm;
> So just calling this gsm is a bit to short. Same goes for voice. That
> was a mistake and I fixed that upstream now to rename it to have_voice.
>
>>   };
>>
>>   static int huawei_probe(struct ofono_modem *modem)
>> @@ -405,7 +410,7 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
>>   	struct ofono_modem *modem = user_data;
>>   	struct huawei_data *data = ofono_modem_get_data(modem);
>>
>> -	if (!ok)
>> +	if (!ok || !data->gsm)
>>   		data->offline_command = "AT+CFUN=5";
>>   	else
>>   		data->offline_command = "AT+CFUN=7";
> This should be a separate patch as well. I think the important part is
> to first enhance the plugin to detect GSM. And then on-top of that
> handle the difference between GSM and CDMA.
>
>> @@ -414,6 +419,44 @@ static void rfswitch_support(gboolean ok, GAtResult *result, gpointer user_data)
>>   					cfun_enable, modem, NULL);
>>   }
>>
>> +static gboolean parse_ati_result(GAtResult *result)
>> +{
>> +	GAtResultIter iter;
>> +	const char *line;
>> +	int num = g_at_result_num_response_lines(result);
>> +	int i;
>> +
>> +	g_at_result_iter_init(&iter, result);
>> +
>> +	for (i = 0; i<  num; i++) {
>> +		g_at_result_iter_next(&iter, NULL);
>> +		line = g_at_result_iter_raw_line(&iter);
>> +		if (g_str_has_prefix(line, "+GCAP"))
>> +			return (g_strrstr(line, "+CGSM") != NULL);
> This parsing is more complex than it needs to be. It also parses the
> string too many times. Especially since GAtChat does that all for you
> already anyway.
>
> Instead of posting an example here, I just pushed a patch so you can see
> how easily this can be done.

Right, I see this is easier do to how I you submitted it :)
However, I have a dongle EC1261 that is bugged because:
         - when I send ATI, the capabilities line is returned with 
"+GCAP +GCAP:" prefix
         - when I send AT+GCAP, the prefix is "+GCAP:"

Unless the manufacturer fixes this issue, we won't be able to support 
this dongle...

>> +	}
>> +
>> +	/* By default we consider modem is GSM type */
>> +	return TRUE;
>> +}
>> +
>> +static void ati_cb(gboolean ok, GAtResult *result, gpointer user_data)
>> +{
>> +	struct ofono_modem *modem = user_data;
>> +	struct huawei_data *data = ofono_modem_get_data(modem);
>> +
>> +	/* By default we consider modem is GSM type */
>> +	if (!ok)
>> +		data->gsm = TRUE;
>> +	else
>> +		data->gsm = parse_ati_result(result);
> I don't like this. We should use have_gsm and have_cdma and not randomly
> fallback to one of them. If for some reason ATI does not show any of
> them, we should also not do anything either.
>
> More important I do wanna get bug reports if ATI does not carry +GCAP or
> has some weird capabilities inside.
>
>> +
>> +	data->sim_state = SIM_STATE_NOT_EXISTENT;
>> +
>> +	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
>> +					rfswitch_support, modem, NULL);
>> +}
>> +
>> +
>>   static GAtChat *open_device(struct ofono_modem *modem,
>>   				const char *key, char *debug)
>>   {
>> @@ -472,10 +515,7 @@ static int huawei_enable(struct ofono_modem *modem)
>>   	g_at_chat_send(data->modem, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
>>   	g_at_chat_send(data->pcui, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
>>
>> -	data->sim_state = SIM_STATE_NOT_EXISTENT;
> Leave this sim_state assignment at this location. No point in moving
> this higher up.
>
>> -
>> -	g_at_chat_send(data->pcui, "AT^RFSWITCH=?", rfswitch_prefix,
>> -					rfswitch_support, modem, NULL);
>> +	g_at_chat_send(data->pcui, "ATI", NULL, ati_cb, modem, NULL);
>>
>>   	return -EINPROGRESS;
>>   }
>> @@ -555,6 +595,7 @@ static void sysinfo_online_cb(gboolean ok, GAtResult *result,
>>   	case SIM_STATE_INVALID_CS:
>>   	case SIM_STATE_INVALID_PS:
>>   	case SIM_STATE_INVALID_PS_AND_CS:
>> +	case SIM_STATE_ROMSIM:
>>   		CALLBACK_WITH_SUCCESS(cb, data->online_cbd->data);
> This should be a separate patch together with the state addition.
>
>>   		goto done;
>>   	}
>> @@ -650,13 +691,21 @@ static void huawei_set_online(struct ofono_modem *modem, ofono_bool_t online,
>>   static void huawei_pre_sim(struct ofono_modem *modem)
>>   {
>>   	struct huawei_data *data = ofono_modem_get_data(modem);
>> -	struct ofono_sim *sim;
>> +	struct ofono_sim *sim = NULL;
> Please don't do that. I prefer not assigning variables with NULL here.
> It is better to change the logic around.
>
>>
>>   	DBG("%p", modem);
>>
>> -	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>> -	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>> +	if (data->gsm == TRUE) {
>> +		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>> +		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>   					"atmodem", data->pcui);
>> +	} else {
>> +		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
>> +		/* Create sim atom only if sim is not embedded */
>> +		if (data->sim_state != SIM_STATE_ROMSIM)
>> +			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>> +						"atmodem", data->pcui);
> I am really not sure that it is a good idea to just use the GSM SIM atom
> here. The EF structure will be different and we might cause more harm
> than doing any good in assuming that we get any proper EF fields.
>
> This is clearly the part where we need detailed information from Huawei
> on how this is suppose to work. And how this is suppose to be done for
> CDMA in the first place anyway.

I agree, maybe Deng Yin An could ask Huawei how they (plan to) support 
R-UIM in their modem e.g.
- Do they have some commands to read UIM file system
- And what does ROMSIM consist in (differences against R-UIM)?

>> +	}
>>
>>   	if (sim&&  data->have_sim == TRUE)
>>   		ofono_sim_inserted_notify(sim, TRUE);
>> @@ -670,6 +719,9 @@ static void huawei_post_sim(struct ofono_modem *modem)
>>
>>   	DBG("%p", modem);
>>
>> +	if (data->gsm == FALSE)
>> +		return;
>> +
>>   	if (data->voice == TRUE) {
>>   		ofono_voicecall_create(modem, 0, "huaweimodem", data->pcui);
>>   		ofono_audio_settings_create(modem, 0,
>> @@ -695,6 +747,15 @@ static void huawei_post_online(struct ofono_modem *modem)
>>
>>   	DBG("%p", modem);
>>
>> +	if (data->gsm == FALSE) {
>> +		ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem",
>> +						data->pcui);
>> +
>> +		ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI,
>> +						"cdmamodem", data->modem);
>> +		return;
>> +	}
>> +
>>   	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->pcui);
>>
>>   	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
> Lets do if (data->have_gsm ... and if (data->have_cdma ...) separate
> here. I rather no degrade GSM to second class citizen.
>
> Regards
>
> Marce

Kind regards,
Guillaume

Re: [PATCH_v0 2/3] huawei: Add modem type detection

[Marcel Holtmann]

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

Hi Guillaume,

> >> +static gboolean parse_ati_result(GAtResult *result)
> >> +{
> >> +	GAtResultIter iter;
> >> +	const char *line;
> >> +	int num = g_at_result_num_response_lines(result);
> >> +	int i;
> >> +
> >> +	g_at_result_iter_init(&iter, result);
> >> +
> >> +	for (i = 0; i<  num; i++) {
> >> +		g_at_result_iter_next(&iter, NULL);
> >> +		line = g_at_result_iter_raw_line(&iter);
> >> +		if (g_str_has_prefix(line, "+GCAP"))
> >> +			return (g_strrstr(line, "+CGSM") != NULL);
> > This parsing is more complex than it needs to be. It also parses the
> > string too many times. Especially since GAtChat does that all for you
> > already anyway.
> >
> > Instead of posting an example here, I just pushed a patch so you can see
> > how easily this can be done.
> 
> Right, I see this is easier do to how I you submitted it :)
> However, I have a dongle EC1261 that is bugged because:
>          - when I send ATI, the capabilities line is returned with 
> "+GCAP +GCAP:" prefix
>          - when I send AT+GCAP, the prefix is "+GCAP:"
> 
> Unless the manufacturer fixes this issue, we won't be able to support 
> this dongle...

that is because that modem firmware is like super buggy. I bet they also
prefixed the other ATI responses with their counterparts. Whoever owns
that modem should upgrade the firmware to a proper version.

And precisely because of this, I want this to fail if the modem firmware
is acting up again.

Regards

Marcel

RE: [PATCH_v0 2/3] huawei: Add modem type detection

[Deng, Ying An]

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

>>
>>>
>>>   	DBG("%p", modem);
>>>
>>> -	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>>> -	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>> +	if (data->gsm == TRUE) {
>>> +		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>>> +		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>>   					"atmodem", data->pcui);
>>> +	} else {
>>> +		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
>>> +		/* Create sim atom only if sim is not embedded */
>>> +		if (data->sim_state != SIM_STATE_ROMSIM)
>>> +			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>> +						"atmodem", data->pcui);
>> I am really not sure that it is a good idea to just use the GSM SIM atom
>> here. The EF structure will be different and we might cause more harm
>> than doing any good in assuming that we get any proper EF fields.
>>
>> This is clearly the part where we need detailed information from Huawei
>> on how this is suppose to work. And how this is suppose to be done for
>> CDMA in the first place anyway.
>
>I agree, maybe Deng Yin An could ask Huawei how they (plan to) support
>R-UIM in their modem e.g.
>- Do they have some commands to read UIM file system
>- And what does ROMSIM consist in (differences against R-UIM)?

Almost all CDMA modems in Chinese market support UIM/RUIM, including EC1261.
There're commands to check if UIM exists or ROMSIM exists.
The CDMA dongles support AT commands include those to touch SMS, address book,
the values includes field to see if it is from UIM/ROMSIM or NV inside dongle. 
But I didn't see a command to touch raw FS directly.

>
>>> +	}
>>>
>>>   	if (sim&&  data->have_sim == TRUE)
>>>   		ofono_sim_inserted_notify(sim, TRUE);
>>> @@ -670,6 +719,9 @@ static void huawei_post_sim(struct ofono_modem
>*modem)
>>>
>>>   	DBG("%p", modem);
>>>
>>> +	if (data->gsm == FALSE)
>>> +		return;
>>> +
>>>   	if (data->voice == TRUE) {
>>>   		ofono_voicecall_create(modem, 0, "huaweimodem", data->pcui);
>>>   		ofono_audio_settings_create(modem, 0,
>>> @@ -695,6 +747,15 @@ static void huawei_post_online(struct
>ofono_modem *modem)
>>>
>>>   	DBG("%p", modem);
>>>
>>> +	if (data->gsm == FALSE) {
>>> +		ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem",
>>> +						data->pcui);
>>> +
>>> +		ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI,
>>> +						"cdmamodem", data->modem);
>>> +		return;
>>> +	}
>>> +
>>>   	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI,
>"atmodem", data->pcui);
>>>
>>>   	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
>> Lets do if (data->have_gsm ... and if (data->have_cdma ...) separate
>> here. I rather no degrade GSM to second class citizen.
>>
>> Regards
>>
>> Marce
>
>Kind regards,
>Guillaume
>_______________________________________________
>ofono mailing list
>ofono(a)ofono.org
>http://lists.ofono.org/listinfo/ofono

Re: [PATCH_v0 2/3] huawei: Add modem type detection

[Philippe Nunes]

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

Hi Marcel,

On 01/09/2012 02:47 PM, Deng, Ying An wrote:
>>>
>>>>
>>>>    	DBG("%p", modem);
>>>>
>>>> -	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>>>> -	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>>> +	if (data->gsm == TRUE) {
>>>> +		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
>>>> +		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>>>    					"atmodem", data->pcui);
>>>> +	} else {
>>>> +		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
>>>> +		/* Create sim atom only if sim is not embedded */
>>>> +		if (data->sim_state != SIM_STATE_ROMSIM)
>>>> +			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
>>>> +						"atmodem", data->pcui);
>>> I am really not sure that it is a good idea to just use the GSM SIM atom
>>> here. The EF structure will be different and we might cause more harm
>>> than doing any good in assuming that we get any proper EF fields.
>>>
>>> This is clearly the part where we need detailed information from Huawei
>>> on how this is suppose to work. And how this is suppose to be done for
>>> CDMA in the first place anyway.
>>
>> I agree, maybe Deng Yin An could ask Huawei how they (plan to) support
>> R-UIM in their modem e.g.
>> - Do they have some commands to read UIM file system
>> - And what does ROMSIM consist in (differences against R-UIM)?
>
> Almost all CDMA modems in Chinese market support UIM/RUIM, including EC1261.
> There're commands to check if UIM exists or ROMSIM exists.
> The CDMA dongles support AT commands include those to touch SMS, address book,
> the values includes field to see if it is from UIM/ROMSIM or NV inside dongle.
> But I didn't see a command to touch raw FS directly.
>
>>

As previously indicated by Denis, the goal (if possible) is to use a 
unified SIM atom for GSM and CDMA.
So far, this should be possible as the R-UIM can be considered as an 
extension of SIM (R-UIM 3GPP2 specification C.S0023).
In practice, it is a matter of reading different EFs, while CHV/PIN 
handling, BDN, FDN, etc are the same.

Actually, with the R-UIM capable CDMA dongles we have, the PIN 
management appears to be exposed by standard AT commands (+CPIN, +CLCK). 
However, this needs to be confirmed an documented by the manufacturers.
Regarding FS reading/writing, this seems to be rather closed (at least 
through AT commands).

My suggestion is to skip for now all EFs reading in case of R-UIM, and 
support only the pin handling.
We can also think to retrieve the IMSI even if this Id is actually not 
mandatory for ConnMan (as the serial number is also considered).
Are you OK with this first approach?

Regards,

Philippe.

>>>> +	}
>>>>
>>>>    	if (sim&&   data->have_sim == TRUE)
>>>>    		ofono_sim_inserted_notify(sim, TRUE);
>>>> @@ -670,6 +719,9 @@ static void huawei_post_sim(struct ofono_modem
>> *modem)
>>>>
>>>>    	DBG("%p", modem);
>>>>
>>>> +	if (data->gsm == FALSE)
>>>> +		return;
>>>> +
>>>>    	if (data->voice == TRUE) {
>>>>    		ofono_voicecall_create(modem, 0, "huaweimodem", data->pcui);
>>>>    		ofono_audio_settings_create(modem, 0,
>>>> @@ -695,6 +747,15 @@ static void huawei_post_online(struct
>> ofono_modem *modem)
>>>>
>>>>    	DBG("%p", modem);
>>>>
>>>> +	if (data->gsm == FALSE) {
>>>> +		ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem",
>>>> +						data->pcui);
>>>> +
>>>> +		ofono_cdma_connman_create(modem, OFONO_VENDOR_HUAWEI,
>>>> +						"cdmamodem", data->modem);
>>>> +		return;
>>>> +	}
>>>> +
>>>>    	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI,
>> "atmodem", data->pcui);
>>>>
>>>>    	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
>>> Lets do if (data->have_gsm ... and if (data->have_cdma ...) separate
>>> here. I rather no degrade GSM to second class citizen.
>>>
>>> Regards
>>>
>>> Marce
>>
>> Kind regards,
>> Guillaume
>> _______________________________________________
>> ofono mailing list
>> ofono(a)ofono.org
>> http://lists.ofono.org/listinfo/ofono
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>

Re: [PATCH_v0 2/3] huawei: Add modem type detection

[Marcel Holtmann]

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

Hi Philippe,

> >>>
> >>>>
> >>>>    	DBG("%p", modem);
> >>>>
> >>>> -	ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
> >>>> -	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> >>>> +	if (data->gsm == TRUE) {
> >>>> +		ofono_devinfo_create(modem, 0, "atmodem", data->pcui);
> >>>> +		sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> >>>>    					"atmodem", data->pcui);
> >>>> +	} else {
> >>>> +		ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
> >>>> +		/* Create sim atom only if sim is not embedded */
> >>>> +		if (data->sim_state != SIM_STATE_ROMSIM)
> >>>> +			sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> >>>> +						"atmodem", data->pcui);
> >>> I am really not sure that it is a good idea to just use the GSM SIM atom
> >>> here. The EF structure will be different and we might cause more harm
> >>> than doing any good in assuming that we get any proper EF fields.
> >>>
> >>> This is clearly the part where we need detailed information from Huawei
> >>> on how this is suppose to work. And how this is suppose to be done for
> >>> CDMA in the first place anyway.
> >>
> >> I agree, maybe Deng Yin An could ask Huawei how they (plan to) support
> >> R-UIM in their modem e.g.
> >> - Do they have some commands to read UIM file system
> >> - And what does ROMSIM consist in (differences against R-UIM)?
> >
> > Almost all CDMA modems in Chinese market support UIM/RUIM, including EC1261.
> > There're commands to check if UIM exists or ROMSIM exists.
> > The CDMA dongles support AT commands include those to touch SMS, address book,
> > the values includes field to see if it is from UIM/ROMSIM or NV inside dongle.
> > But I didn't see a command to touch raw FS directly.
> >
> >>
> 
> As previously indicated by Denis, the goal (if possible) is to use a 
> unified SIM atom for GSM and CDMA.
> So far, this should be possible as the R-UIM can be considered as an 
> extension of SIM (R-UIM 3GPP2 specification C.S0023).
> In practice, it is a matter of reading different EFs, while CHV/PIN 
> handling, BDN, FDN, etc are the same.
> 
> Actually, with the R-UIM capable CDMA dongles we have, the PIN 
> management appears to be exposed by standard AT commands (+CPIN, +CLCK). 
> However, this needs to be confirmed an documented by the manufacturers.
> Regarding FS reading/writing, this seems to be rather closed (at least 
> through AT commands).
> 
> My suggestion is to skip for now all EFs reading in case of R-UIM, and 
> support only the pin handling.
> We can also think to retrieve the IMSI even if this Id is actually not 
> mandatory for ConnMan (as the serial number is also considered).
> Are you OK with this first approach?

I fully agree with Denis, that a copy&paste of the SIM atom driver from
atmodem driver is not a good solution. So yes, check if the code can be
tweaked to make it work for CDMA as well.

Regards

Marcel

RE: [PATCH_v0 2/3] huawei: Add modem type detection

[Deng, Ying An]

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

>
>Right, I see this is easier do to how I you submitted it :)
>However, I have a dongle EC1261 that is bugged because:
>         - when I send ATI, the capabilities line is returned with
>"+GCAP +GCAP:" prefix
>         - when I send AT+GCAP, the prefix is "+GCAP:"
>
>Unless the manufacturer fixes this issue, we won't be able to support
>this dongle...
>

Hi,
The modem returns following lines after ATI command:

Manufacturer: +GMI: HUAWEI TECHNOLOGIES CO., LTD
Model: EC1261
Revision: +CGMR:11.102.11.00.45
ESN: +GSN:80c55faf
+GCAP: +GCAP: +CIS707-A,CIS-856-A,+MS, +ES, +DS, +FCLASS

So you can see, it is still with prefix "+GCAP:", although there's another "+GCAP:", but it does not impact the string search operation.

RE: [PATCH_v0 2/3] huawei: Add modem type detection

[Marcel Holtmann]

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

Hi Ying An,

> >Right, I see this is easier do to how I you submitted it :)
> >However, I have a dongle EC1261 that is bugged because:
> >         - when I send ATI, the capabilities line is returned with
> >"+GCAP +GCAP:" prefix
> >         - when I send AT+GCAP, the prefix is "+GCAP:"
> >
> >Unless the manufacturer fixes this issue, we won't be able to support
> >this dongle...
> >
>
> The modem returns following lines after ATI command:
> 
> Manufacturer: +GMI: HUAWEI TECHNOLOGIES CO., LTD
> Model: EC1261
> Revision: +CGMR:11.102.11.00.45
> ESN: +GSN:80c55faf
> +GCAP: +GCAP: +CIS707-A,CIS-856-A,+MS, +ES, +DS, +FCLASS
> 
> So you can see, it is still with prefix "+GCAP:", although there's another "+GCAP:", but it does not impact the string search operation.

this is the most hilarious screw-up within a single AT command.

Open your window, take this modem, and throw it out. Nothing can really
help you here. If they did not get this one right, I do not wanna know
what else is broken ;)

Regards

Marcel