[PATCH 1/6] Add SIMCOM support.

[PATCH 1/6] Add SIMCOM support.

[Anthony Viallard]

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

Implemented using the SIM5216E module:

  Manufacturer: SIMCOM INCORPORATED
  Model: SIMCOM_SIM5216E
  Revision: 1575B11SIM5216E
            SIM5216E_1575_111104_V1.21

Also tested with SIM5216A (which is the america version of SIM5216) and it works too.
It should work with SIM5215 but I didn't have this module for testing.

 - based on SIM5215_SIM5216_ATC_V1.18.pdf documentation ;
 - SMS and GPRS work (in the same time) ;
 - SIM card presence check ;
 - use GSM charset ;
 - use 115200bps, 8 bit data, no parity, 1 bit stop, no data stream control
   for tty configuration ;
 - flight mode support ;
 - no voice part (because I could't test it).
---
 Makefile.am      |   3 +
 plugins/simcom.c | 369 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 372 insertions(+)
 create mode 100644 plugins/simcom.c

diff --git a/Makefile.am b/Makefile.am
index cc763b8..12c5b1c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -408,6 +408,9 @@ builtin_sources += plugins/samsung.c
 builtin_modules += sim900
 builtin_sources += plugins/sim900.c
 
+builtin_modules += simcom
+builtin_sources += plugins/simcom.c
+
 if BLUETOOTH
 if BLUEZ4
 builtin_modules += bluez4
diff --git a/plugins/simcom.c b/plugins/simcom.c
new file mode 100644
index 0000000..9cd984d
--- /dev/null
+++ b/plugins/simcom.c
@@ -0,0 +1,369 @@
+/*
+ *
+ *  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 <stdio.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/netreg.h>
+#include <ofono/sim.h>
+#include <ofono/cbs.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
+#include <ofono/radio-settings.h>
+#include <ofono/phonebook.h>
+#include <ofono/log.h>
+
+#include <drivers/atmodem/atutil.h>
+#include <drivers/atmodem/vendor.h>
+
+static const char *none_prefix[] = { NULL };
+
+struct simcom_data {
+	GAtChat *modem;
+	GAtChat *chat;
+	ofono_bool_t have_sim;
+	struct at_util_sim_state_query *sim_state_query;
+};
+
+/* Callback and helpers functions */
+static void simcom_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static void sim_state_cb(gboolean present, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	at_util_sim_state_query_free(data->sim_state_query);
+	data->sim_state_query = NULL;
+
+	data->have_sim = present;
+
+	/* DCD (Data Carrier Detect) always enabled */
+	g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->chat, "AT&C0", NULL, NULL, NULL, NULL);
+
+	ofono_modem_set_powered(modem, TRUE);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	if (!ok) {
+		g_at_chat_unref(data->modem);
+		data->modem = NULL;
+
+		g_at_chat_unref(data->chat);
+		data->chat = NULL;
+
+		ofono_modem_set_powered(modem, FALSE);
+		return;
+	}
+
+	data->sim_state_query = at_util_sim_state_query_new(data->chat,
+						2, 20, sim_state_cb, modem,
+						NULL);
+}
+
+static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->chat);
+	data->chat = NULL;
+
+	if (ok)
+		ofono_modem_set_powered(modem, FALSE);
+}
+
+static void flightmode_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_modem_online_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
+}
+
+static GAtChat *open_device(struct ofono_modem *modem,
+			    const char *key,
+			    char *debug)
+{
+	const char *device;
+	GIOChannel *channel;
+	GAtSyntax *syntax;
+	GAtChat *chat;
+	GHashTable *options;
+
+	device = ofono_modem_get_string(modem, key);
+	if (device == NULL) {
+		ofono_error("Failed to get modem '%s'", key);
+		return NULL;
+	}
+
+	DBG("%s %s", key, device);
+
+	options = g_hash_table_new(g_str_hash, g_str_equal);
+	if (options == NULL)
+		return NULL;
+
+	g_hash_table_insert(options, "Baud", "115200");
+	g_hash_table_insert(options, "Parity", "none");
+	g_hash_table_insert(options, "StopBits", "1");
+	g_hash_table_insert(options, "DataBits", "8");
+	g_hash_table_insert(options, "XonXoff", "off");
+
+	channel = g_at_tty_open(device, options);
+
+	g_hash_table_destroy(options);
+
+	if (channel == NULL) {
+		ofono_error("Failed to get tty for '%s'", key);
+		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) {
+		ofono_error("Failed to get chat for '%s'", key);
+		return NULL;
+	}
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, simcom_debug, debug);
+
+	return chat;
+}
+
+/* Modem interface function */
+static int simcom_probe(struct ofono_modem *modem)
+{
+	struct simcom_data *data;
+
+	DBG("%p", modem);
+
+	data = g_try_new0(struct simcom_data, 1);
+	if (data == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, data);
+
+	return 0;
+}
+
+static void simcom_remove(struct ofono_modem *modem)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_modem_set_data(modem, NULL);
+
+	/* Cleanup potential SIM state polling */
+	at_util_sim_state_query_free(data->sim_state_query);
+
+	/* Cleanup after hot-unplug */
+	g_at_chat_unref(data->chat);
+
+	g_free(data);
+}
+
+static int simcom_enable(struct ofono_modem *modem)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	data->modem = open_device(modem, "Modem", "Modem: ");
+	if (data->modem == NULL)
+		return -EINVAL;
+
+	data->chat = open_device(modem, "Data", "Chat: ");
+	if (data->chat == NULL) {
+		g_at_chat_unref(data->modem);
+		data->modem = NULL;
+		return -EIO;
+	}
+
+	g_at_chat_set_slave(data->modem, data->chat);
+
+	/* Configure AT commands behavior */
+	g_at_chat_send(data->modem, "ATZ E0 +CMEE=1", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->chat, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
+
+	/* Ensure that the modem is using GSM character set and not IRA */
+	g_at_chat_send(data->modem, "AT+CSCS=\"GSM\"", none_prefix,
+			NULL, NULL, NULL);
+	g_at_chat_send(data->chat, "AT+CSCS=\"GSM\"", none_prefix,
+			NULL, NULL, NULL);
+
+	/* Make it up */
+	g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
+		       cfun_enable, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static int simcom_disable(struct ofono_modem *modem)
+{
+	struct simcom_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->chat);
+	g_at_chat_unregister_all(data->chat);
+
+	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
+		       cfun_disable, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static void simcom_set_online(struct ofono_modem *modem, ofono_bool_t online,
+			      ofono_modem_online_cb_t cb, void *user_data)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
+
+	DBG("modem %p %s", modem, online ? "online" : "offline");
+
+	if (g_at_chat_send(data->chat, command, none_prefix,
+			   flightmode_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
+static void simcom_pre_sim(struct ofono_modem *modem)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+	struct ofono_sim *sim;
+
+	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "atmodem", data->chat);
+	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+				data->chat);
+
+	if (sim)
+		ofono_sim_inserted_notify(sim, data->have_sim);
+}
+
+static void simcom_post_sim(struct ofono_modem *modem)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
+
+	DBG("%p", modem);
+
+	ofono_phonebook_create(modem, 0, "atmodem", data->chat);
+
+	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+				data->chat);
+
+	gprs = ofono_gprs_create(modem, 0, "atmodem", data->chat);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->modem);
+
+	if (gprs && gc)
+		ofono_gprs_add_context(gprs, gc);
+}
+
+static void simcom_post_online(struct ofono_modem *modem)
+{
+	struct simcom_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->chat);
+	/* disable CBS module because it causes a deadlock issue */
+	/* ofono_cbs_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", */
+	/*		    data->chat); */
+	g_at_chat_send(data->chat, "AT+CSCB=0", none_prefix,
+		       NULL, NULL, NULL);
+	ofono_ussd_create(modem, 0, "atmodem", data->chat);
+}
+
+static struct ofono_modem_driver simcom_driver = {
+	.name		= "simcom",
+	.probe		= simcom_probe,
+	.remove		= simcom_remove,
+	.enable		= simcom_enable,
+	.disable	= simcom_disable,
+	.set_online     = simcom_set_online,
+	.pre_sim	= simcom_pre_sim,
+	.post_sim	= simcom_post_sim,
+	.post_online	= simcom_post_online,
+};
+
+static int simcom_init(void)
+{
+	return ofono_modem_driver_register(&simcom_driver);
+}
+
+static void simcom_exit(void)
+{
+	ofono_modem_driver_unregister(&simcom_driver);
+}
+
+OFONO_PLUGIN_DEFINE(simcom, "SIMCOM modem driver", VERSION,
+		    OFONO_PLUGIN_PRIORITY_DEFAULT,
+		    simcom_init, simcom_exit)
-- 
1.8.3.1

[PATCH 2/6] SIMCOM: add a quirk for AT+CNMI command building.

[Anthony Viallard]

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

Use mode=1 otherwise it fails (ref. "SIM5215_SIM5216_ATC_V1.18.pdf - §6.9")
---
 drivers/atmodem/sms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c
index fde90ba..f93dd23 100644
--- a/drivers/atmodem/sms.c
+++ b/drivers/atmodem/sms.c
@@ -805,6 +805,7 @@ static gboolean build_cnmi_string(char *buf, int *cnmi_opts,
 	case OFONO_VENDOR_NOVATEL:
 	case OFONO_VENDOR_HUAWEI:
 	case OFONO_VENDOR_ZTE:
+	case OFONO_VENDOR_SIMCOM:
 		/* MSM devices advertise support for mode 2, but return an
 		 * error if we attempt to actually use it. */
 		mode = "1";
-- 
1.8.3.1

[PATCH 3/6] SIMCOM: add a quirk for signal strength reporting.

[Anthony Viallard]

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

We must tell we want the signal strength reporting using
AT+AUTOCSQ command (ref. "SIM5215_SIM5216_ATC_V1.18.pdf - §10.7").
---
 drivers/atmodem/network-registration.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 24b71a3..8cc04b7 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -1753,6 +1753,14 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	}
 
 	switch (nd->vendor) {
+	case OFONO_VENDOR_SIMCOM:
+		/* Register for CSQ changes */
+		g_at_chat_send(nd->chat, "AT+AUTOCSQ=1,1", none_prefix,
+				NULL, NULL, NULL);
+
+		g_at_chat_register(nd->chat, "+CSQ:",
+				   csq_notify, FALSE, netreg, NULL);
+		break;
 	case OFONO_VENDOR_PHONESIM:
 		g_at_chat_register(nd->chat, "+CSQ:",
 					csq_notify, FALSE, netreg, NULL);
@@ -1898,7 +1906,6 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		break;
 	case OFONO_VENDOR_NOKIA:
 	case OFONO_VENDOR_SAMSUNG:
-	case OFONO_VENDOR_SIMCOM:
 		/* Signal strength reporting via CIND is not supported */
 		break;
 	default:
-- 
1.8.3.1

[PATCH 4/6] SIMCOM: add a quirk to fix crsm request

[Anthony Viallard]

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

---
 drivers/atmodem/sim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index f97f5b8..f8e0472 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -167,6 +167,7 @@ static void at_sim_read_info(struct ofono_sim *sim, int fileid,
 	case OFONO_VENDOR_SIERRA:
 	case OFONO_VENDOR_SPEEDUP:
 	case OFONO_VENDOR_QUALCOMM_MSM:
+	case OFONO_VENDOR_SIMCOM:
 		/* Maximum possible length */
 		len += sprintf(buf + len, ",0,0,255");
 		break;
-- 
1.8.3.1

[PATCH 5/6] Add a function to be able to report network technology change

[Anthony Viallard]

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

When we detect a change of the network technology, ofono don't have
a function to report this change at higher level (dbus signal, ...)
---
 include/netreg.h | 1 +
 src/network.c    | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/netreg.h b/include/netreg.h
index 4338c14..94c8e9f 100644
--- a/include/netreg.h
+++ b/include/netreg.h
@@ -91,6 +91,7 @@ struct ofono_netreg_driver {
 void ofono_netreg_strength_notify(struct ofono_netreg *netreg, int strength);
 void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status,
 					int lac, int ci, int tech);
+void ofono_netreg_tech_notify(struct ofono_netreg *netreg, int tech);
 void ofono_netreg_time_notify(struct ofono_netreg *netreg,
 				struct ofono_network_time *info);
 
diff --git a/src/network.c b/src/network.c
index d1bfca6..b4cd62f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1393,6 +1393,12 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status,
 	notify_status_watches(netreg);
 }
 
+void ofono_netreg_tech_notify(struct ofono_netreg *netreg, int tech)
+{
+	if (netreg->technology != tech)
+		set_registration_technology(netreg, tech);
+}
+
 void ofono_netreg_time_notify(struct ofono_netreg *netreg,
 				struct ofono_network_time *info)
 {
-- 
1.8.3.1

[PATCH 6/6] SIMCOM: add a quirk to retrieve network technology used

[Anthony Viallard]

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

Use CNSMOD command to get network technology and monitor
changes.
---
 drivers/atmodem/network-registration.c | 76 ++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 8cc04b7..d634dbb 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -239,6 +239,14 @@ static void at_registration_status(struct ofono_netreg *netreg,
 	cbd->user = nd;
 
 	switch (nd->vendor) {
+	case OFONO_VENDOR_SIMCOM:
+		/*
+		 * Send +CNSMOD? to find out the current tech, it will be
+		 * intercepted in simcom_cnsmod_notify
+		 */
+		g_at_chat_send(nd->chat, "AT+CNSMOD?", none_prefix,
+				NULL, NULL, NULL);
+		break;
 	case OFONO_VENDOR_MBM:
 		/*
 		 * Send *ERINFO to find out the current tech, it will be
@@ -1329,6 +1337,66 @@ static int cnti_to_tech(const char *cnti)
 	return -1;
 }
 
+static void simcom_cnsmod_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_netreg *netreg = user_data;
+	struct netreg_data *nd = ofono_netreg_get_data(netreg);
+	GAtResultIter iter;
+	int n1,
+		n2,
+		mode;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (g_at_result_iter_next(&iter, "+CNSMOD:") == FALSE)
+		return;
+
+	if (g_at_result_iter_next_number(&iter, &n1) == FALSE)
+		return;
+
+	if (g_at_result_iter_next_number(&iter, &n2) == FALSE)
+		n2 = -1;
+
+	if (n2 != -1)
+		mode = n2;
+	else
+		mode = n1;
+
+	switch (mode) {
+	case 1:
+	case 2:
+		/* GSM, GPRS */
+		nd->tech = ACCESS_TECHNOLOGY_GSM;
+		break;
+	case 3:
+		/* EDGE */
+		nd->tech = ACCESS_TECHNOLOGY_GSM_EGPRS;
+		break;
+	case 4:
+		/* WCDMA */
+		nd->tech = ACCESS_TECHNOLOGY_UTRAN;
+		break;
+	case 5:
+		/* HSDPA */
+		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSDPA;
+		break;
+	case 6:
+		/* HSUPA */
+		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSUPA;
+		break;
+	case 7:	/* HSUPA and HSDPA */
+		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA;
+		break;
+	default:
+	case 0:
+		/* no service */
+		nd->tech = -1;
+		break;
+	}
+
+	ofono_netreg_tech_notify(netreg, nd->tech);
+}
+
 static void gobi_cnti_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_netreg *netreg = user_data;
@@ -1760,6 +1828,14 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 		g_at_chat_register(nd->chat, "+CSQ:",
 				   csq_notify, FALSE, netreg, NULL);
+
+		/* Register for network technology change */
+		g_at_chat_register(nd->chat, "+CNSMOD:",
+				   simcom_cnsmod_notify, FALSE, netreg, NULL);
+
+		g_at_chat_send(nd->chat, "AT+CNSMOD=1", none_prefix,
+			       NULL, NULL, NULL);
+
 		break;
 	case OFONO_VENDOR_PHONESIM:
 		g_at_chat_register(nd->chat, "+CSQ:",
-- 
1.8.3.1

Re: [PATCH 1/6] Add SIMCOM support.

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> Implemented using the SIM5216E module:
>
>    Manufacturer: SIMCOM INCORPORATED
>    Model: SIMCOM_SIM5216E
>    Revision: 1575B11SIM5216E
>              SIM5216E_1575_111104_V1.21
>
> Also tested with SIM5216A (which is the america version of SIM5216) and it works too.

I guess you mean 'American' version.  Might want to be more specific 
here, e.g. 'North American'.  Anyway, just being picky :)

> It should work with SIM5215 but I didn't have this module for testing.
>
>   - based on SIM5215_SIM5216_ATC_V1.18.pdf documentation ;
>   - SMS and GPRS work (in the same time) ;
>   - SIM card presence check ;
>   - use GSM charset ;
>   - use 115200bps, 8 bit data, no parity, 1 bit stop, no data stream control
>     for tty configuration ;
>   - flight mode support ;
>   - no voice part (because I could't test it).
> ---
>   Makefile.am      |   3 +
>   plugins/simcom.c | 369 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 372 insertions(+)
>   create mode 100644 plugins/simcom.c
>
> diff --git a/Makefile.am b/Makefile.am
> index cc763b8..12c5b1c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -408,6 +408,9 @@ builtin_sources += plugins/samsung.c
>   builtin_modules += sim900
>   builtin_sources += plugins/sim900.c
>
> +builtin_modules += simcom
> +builtin_sources += plugins/simcom.c
> +
>   if BLUETOOTH
>   if BLUEZ4
>   builtin_modules += bluez4
> diff --git a/plugins/simcom.c b/plugins/simcom.c
> new file mode 100644
> index 0000000..9cd984d
> --- /dev/null
> +++ b/plugins/simcom.c
> @@ -0,0 +1,369 @@
> +/*
> + *
> + *  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 <stdio.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/netreg.h>
> +#include <ofono/sim.h>
> +#include <ofono/cbs.h>
> +#include <ofono/sms.h>
> +#include <ofono/ussd.h>
> +#include <ofono/gprs.h>
> +#include <ofono/gprs-context.h>
> +#include <ofono/radio-settings.h>
> +#include <ofono/phonebook.h>
> +#include <ofono/log.h>
> +
> +#include <drivers/atmodem/atutil.h>
> +#include <drivers/atmodem/vendor.h>
> +
> +static const char *none_prefix[] = { NULL };
> +
> +struct simcom_data {
> +	GAtChat *modem;
> +	GAtChat *chat;
> +	ofono_bool_t have_sim;
> +	struct at_util_sim_state_query *sim_state_query;
> +};
> +
> +/* Callback and helpers functions */
> +static void simcom_debug(const char *str, void *user_data)
> +{
> +	const char *prefix = user_data;
> +
> +	ofono_info("%s%s", prefix, str);
> +}
> +
> +static void sim_state_cb(gboolean present, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	at_util_sim_state_query_free(data->sim_state_query);
> +	data->sim_state_query = NULL;
> +
> +	data->have_sim = present;
> +
> +	/* DCD (Data Carrier Detect) always enabled */
> +	g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
> +	g_at_chat_send(data->chat, "AT&C0", NULL, NULL, NULL, NULL);
> +
> +	ofono_modem_set_powered(modem, TRUE);
> +}
> +
> +static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	if (!ok) {
> +		g_at_chat_unref(data->modem);
> +		data->modem = NULL;
> +
> +		g_at_chat_unref(data->chat);
> +		data->chat = NULL;
> +

So here you unref both channels...

> +		ofono_modem_set_powered(modem, FALSE);
> +		return;
> +	}
> +
> +	data->sim_state_query = at_util_sim_state_query_new(data->chat,
> +						2, 20, sim_state_cb, modem,
> +						NULL);
> +}
> +
> +static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	g_at_chat_unref(data->chat);
> +	data->chat = NULL;

Here you unref just the chat.  You might want to be consistent.

> +
> +	if (ok)
> +		ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static void flightmode_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_modem_online_cb_t cb = cbd->cb;
> +	struct ofono_error error;
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +	cb(&error, cbd->data);
> +}
> +
> +static GAtChat *open_device(struct ofono_modem *modem,
> +			    const char *key,
> +			    char *debug)
> +{
> +	const char *device;
> +	GIOChannel *channel;
> +	GAtSyntax *syntax;
> +	GAtChat *chat;
> +	GHashTable *options;
> +
> +	device = ofono_modem_get_string(modem, key);
> +	if (device == NULL) {
> +		ofono_error("Failed to get modem '%s'", key);
> +		return NULL;
> +	}
> +
> +	DBG("%s %s", key, device);
> +
> +	options = g_hash_table_new(g_str_hash, g_str_equal);
> +	if (options == NULL)
> +		return NULL;
> +
> +	g_hash_table_insert(options, "Baud", "115200");
> +	g_hash_table_insert(options, "Parity", "none");
> +	g_hash_table_insert(options, "StopBits", "1");
> +	g_hash_table_insert(options, "DataBits", "8");
> +	g_hash_table_insert(options, "XonXoff", "off");
> +
> +	channel = g_at_tty_open(device, options);
> +
> +	g_hash_table_destroy(options);
> +
> +	if (channel == NULL) {
> +		ofono_error("Failed to get tty for '%s'", key);
> +		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) {
> +		ofono_error("Failed to get chat for '%s'", key);
> +		return NULL;
> +	}
> +
> +	if (getenv("OFONO_AT_DEBUG"))
> +		g_at_chat_set_debug(chat, simcom_debug, debug);
> +
> +	return chat;
> +}
> +
> +/* Modem interface function */

This comment is useless

> +static int simcom_probe(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data;
> +
> +	DBG("%p", modem);
> +
> +	data = g_try_new0(struct simcom_data, 1);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	ofono_modem_set_data(modem, data);
> +
> +	return 0;
> +}
> +
> +static void simcom_remove(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	ofono_modem_set_data(modem, NULL);
> +
> +	/* Cleanup potential SIM state polling */

As is this comment

> +	at_util_sim_state_query_free(data->sim_state_query);
> +
> +	/* Cleanup after hot-unplug */

And this comment

> +	g_at_chat_unref(data->chat);

Why do we not clean up the modem chat here?

> +
> +	g_free(data);
> +}
> +
> +static int simcom_enable(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	data->modem = open_device(modem, "Modem", "Modem: ");
> +	if (data->modem == NULL)
> +		return -EINVAL;
> +
> +	data->chat = open_device(modem, "Data", "Chat: ");
> +	if (data->chat == NULL) {
> +		g_at_chat_unref(data->modem);
> +		data->modem = NULL;
> +		return -EIO;
> +	}
> +
> +	g_at_chat_set_slave(data->modem, data->chat);
> +

Just FYI, g_at_chat_set_slave just refs the passed in argument, so it is 
not taking ownership of the other chat object.

> +	/* Configure AT commands behavior */
> +	g_at_chat_send(data->modem, "ATZ E0 +CMEE=1", NULL, NULL, NULL, NULL);
> +	g_at_chat_send(data->chat, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
> +
> +	/* Ensure that the modem is using GSM character set and not IRA */
> +	g_at_chat_send(data->modem, "AT+CSCS=\"GSM\"", none_prefix,
> +			NULL, NULL, NULL);
> +	g_at_chat_send(data->chat, "AT+CSCS=\"GSM\"", none_prefix,
> +			NULL, NULL, NULL);
> +
> +	/* Make it up */

Might want to change that to 'Power it up'

> +	g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
> +		       cfun_enable, modem, NULL);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static int simcom_disable(struct ofono_modem *modem)
> +{
> +	struct simcom_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->chat);
> +	g_at_chat_unregister_all(data->chat);
> +
> +	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
> +		       cfun_disable, modem, NULL);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static void simcom_set_online(struct ofono_modem *modem, ofono_bool_t online,
> +			      ofono_modem_online_cb_t cb, void *user_data)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
> +
> +	DBG("modem %p %s", modem, online ? "online" : "offline");
> +
> +	if (g_at_chat_send(data->chat, command, none_prefix,
> +			   flightmode_cb, cbd, g_free) > 0)
> +		return;
> +
> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void simcom_pre_sim(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +	struct ofono_sim *sim;
> +
> +	DBG("%p", modem);
> +
> +	ofono_devinfo_create(modem, 0, "atmodem", data->chat);
> +	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> +				data->chat);
> +
> +	if (sim)
> +		ofono_sim_inserted_notify(sim, data->have_sim);
> +}
> +
> +static void simcom_post_sim(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +	struct ofono_gprs *gprs;
> +	struct ofono_gprs_context *gc;
> +
> +	DBG("%p", modem);
> +
> +	ofono_phonebook_create(modem, 0, "atmodem", data->chat);
> +
> +	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> +				data->chat);
> +
> +	gprs = ofono_gprs_create(modem, 0, "atmodem", data->chat);
> +	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->modem);
> +
> +	if (gprs && gc)
> +		ofono_gprs_add_context(gprs, gc);
> +}
> +
> +static void simcom_post_online(struct ofono_modem *modem)
> +{
> +	struct simcom_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->chat);
> +	/* disable CBS module because it causes a deadlock issue */
> +	/* ofono_cbs_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", */
> +	/*		    data->chat); */

Please don't submit commented out code upstream.  You can always add it 
later.  That is what git is good for.

> +	g_at_chat_send(data->chat, "AT+CSCB=0", none_prefix,
> +		       NULL, NULL, NULL);

This is Cell Broadcast related, why is this here?

> +	ofono_ussd_create(modem, 0, "atmodem", data->chat);
> +}
> +
> +static struct ofono_modem_driver simcom_driver = {
> +	.name		= "simcom",
> +	.probe		= simcom_probe,
> +	.remove		= simcom_remove,
> +	.enable		= simcom_enable,
> +	.disable	= simcom_disable,
> +	.set_online     = simcom_set_online,
> +	.pre_sim	= simcom_pre_sim,
> +	.post_sim	= simcom_post_sim,
> +	.post_online	= simcom_post_online,
> +};
> +
> +static int simcom_init(void)
> +{
> +	return ofono_modem_driver_register(&simcom_driver);
> +}
> +
> +static void simcom_exit(void)
> +{
> +	ofono_modem_driver_unregister(&simcom_driver);
> +}
> +
> +OFONO_PLUGIN_DEFINE(simcom, "SIMCOM modem driver", VERSION,
> +		    OFONO_PLUGIN_PRIORITY_DEFAULT,
> +		    simcom_init, simcom_exit)
>

Regards,
-Denis

Re: [PATCH 2/6] SIMCOM: add a quirk for AT+CNMI command building.

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> Use mode=1 otherwise it fails (ref. "SIM5215_SIM5216_ATC_V1.18.pdf - §6.9")
> ---
>   drivers/atmodem/sms.c | 1 +
>   1 file changed, 1 insertion(+)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 3/6] SIMCOM: add a quirk for signal strength reporting.

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> We must tell we want the signal strength reporting using
> AT+AUTOCSQ command (ref. "SIM5215_SIM5216_ATC_V1.18.pdf - §10.7").
> ---
>   drivers/atmodem/network-registration.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 4/6] SIMCOM: add a quirk to fix crsm request

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> ---
>   drivers/atmodem/sim.c | 1 +
>   1 file changed, 1 insertion(+)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 5/6] Add a function to be able to report network technology change

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> When we detect a change of the network technology, ofono don't have
> a function to report this change at higher level (dbus signal, ...)
> ---
>   include/netreg.h | 1 +
>   src/network.c    | 6 ++++++
>   2 files changed, 7 insertions(+)
>
> diff --git a/include/netreg.h b/include/netreg.h
> index 4338c14..94c8e9f 100644
> --- a/include/netreg.h
> +++ b/include/netreg.h
> @@ -91,6 +91,7 @@ struct ofono_netreg_driver {
>   void ofono_netreg_strength_notify(struct ofono_netreg *netreg, int strength);
>   void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status,
>   					int lac, int ci, int tech);
> +void ofono_netreg_tech_notify(struct ofono_netreg *netreg, int tech);

Please don't do this, the tech is already covered by netreg_status_notify

>   void ofono_netreg_time_notify(struct ofono_netreg *netreg,
>   				struct ofono_network_time *info);
>
> diff --git a/src/network.c b/src/network.c
> index d1bfca6..b4cd62f 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1393,6 +1393,12 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status,
>   	notify_status_watches(netreg);
>   }
>
> +void ofono_netreg_tech_notify(struct ofono_netreg *netreg, int tech)
> +{
> +	if (netreg->technology != tech)
> +		set_registration_technology(netreg, tech);
> +}
> +
>   void ofono_netreg_time_notify(struct ofono_netreg *netreg,
>   				struct ofono_network_time *info)
>   {
>

Regards,
-Denis

Re: [PATCH 6/6] SIMCOM: add a quirk to retrieve network technology used

[Denis Kenzior]

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

Hi Anthony,

On 07/19/2013 08:35 AM, Anthony Viallard wrote:
> Use CNSMOD command to get network technology and monitor
> changes.
> ---
>   drivers/atmodem/network-registration.c | 76 ++++++++++++++++++++++++++++++++++
>   1 file changed, 76 insertions(+)
>
> diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
> index 8cc04b7..d634dbb 100644
> --- a/drivers/atmodem/network-registration.c
> +++ b/drivers/atmodem/network-registration.c
> @@ -239,6 +239,14 @@ static void at_registration_status(struct ofono_netreg *netreg,
>   	cbd->user = nd;
>
>   	switch (nd->vendor) {
> +	case OFONO_VENDOR_SIMCOM:
> +		/*
> +		 * Send +CNSMOD? to find out the current tech, it will be
> +		 * intercepted in simcom_cnsmod_notify
> +		 */
> +		g_at_chat_send(nd->chat, "AT+CNSMOD?", none_prefix,
> +				NULL, NULL, NULL);
> +		break;
>   	case OFONO_VENDOR_MBM:
>   		/*
>   		 * Send *ERINFO to find out the current tech, it will be
> @@ -1329,6 +1337,66 @@ static int cnti_to_tech(const char *cnti)
>   	return -1;
>   }
>
> +static void simcom_cnsmod_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_netreg *netreg = user_data;
> +	struct netreg_data *nd = ofono_netreg_get_data(netreg);
> +	GAtResultIter iter;
> +	int n1,
> +		n2,
> +		mode;

This is not according to our coding style ;)

> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (g_at_result_iter_next(&iter, "+CNSMOD:") == FALSE)
> +		return;
> +
> +	if (g_at_result_iter_next_number(&iter, &n1) == FALSE)
> +		return;
> +
> +	if (g_at_result_iter_next_number(&iter, &n2) == FALSE)
> +		n2 = -1;
> +
> +	if (n2 != -1)
> +		mode = n2;
> +	else
> +		mode = n1;
> +
> +	switch (mode) {
> +	case 1:
> +	case 2:
> +		/* GSM, GPRS */
> +		nd->tech = ACCESS_TECHNOLOGY_GSM;
> +		break;
> +	case 3:
> +		/* EDGE */
> +		nd->tech = ACCESS_TECHNOLOGY_GSM_EGPRS;
> +		break;
> +	case 4:
> +		/* WCDMA */
> +		nd->tech = ACCESS_TECHNOLOGY_UTRAN;
> +		break;
> +	case 5:
> +		/* HSDPA */
> +		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSDPA;
> +		break;
> +	case 6:
> +		/* HSUPA */
> +		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSUPA;
> +		break;
> +	case 7:	/* HSUPA and HSDPA */
> +		nd->tech = ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA;
> +		break;
> +	default:
> +	case 0:
> +		/* no service */
> +		nd->tech = -1;
> +		break;
> +	}
> +
> +	ofono_netreg_tech_notify(netreg, nd->tech);
> +}
> +
>   static void gobi_cnti_notify(GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_netreg *netreg = user_data;
> @@ -1760,6 +1828,14 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
>
>   		g_at_chat_register(nd->chat, "+CSQ:",
>   				   csq_notify, FALSE, netreg, NULL);
> +
> +		/* Register for network technology change */
> +		g_at_chat_register(nd->chat, "+CNSMOD:",
> +				   simcom_cnsmod_notify, FALSE, netreg, NULL);
> +
> +		g_at_chat_send(nd->chat, "AT+CNSMOD=1", none_prefix,
> +			       NULL, NULL, NULL);
> +

Does CNSMOD refer to the current Bearer or the current supported 
technology of the cell?  If it is the latter, then generally any change 
in technology would also involve the change in the cell id as well. 
Look at how we handle similar hardware behavior in creg_notify.

Do you have any AT logs showing how +CNSMOD works?  e.g. do you see it 
changing without a corresponding CREG change?  Does it matter whether 
you have an active GPRS context or not?

>   		break;
>   	case OFONO_VENDOR_PHONESIM:
>   		g_at_chat_register(nd->chat, "+CSQ:",
>

Regards,
-Denis