[PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[Alex J Lennon]

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

On the basis that tc6x and other Cinterion devices will likely
have similar firmware requirements, provide a generic Cinterion
plugin which is functionally identical to the replaced tc65 plugin.

The udev implementation retains support for "tc65" name for
backwards compatibility, and adds support for the new "cinterion"
name.
---
 Makefile.am         |   4 +-
 plugins/cinterion.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 plugins/tc65.c      | 245 ---------------------------------------------------
 plugins/udev.c      |   9 +-
 4 files changed, 256 insertions(+), 250 deletions(-)
 create mode 100644 plugins/cinterion.c
 delete mode 100644 plugins/tc65.c

diff --git a/Makefile.am b/Makefile.am
index 22249c4..113a2d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -390,8 +390,8 @@ builtin_sources += plugins/stemgr.c
 builtin_modules += caif
 builtin_sources += plugins/caif.c
 
-builtin_modules += tc65
-builtin_sources += plugins/tc65.c
+builtin_modules += cinterion
+builtin_sources += plugins/cinterion.c
 
 builtin_modules += nokia
 builtin_sources += plugins/nokia.c
diff --git a/plugins/cinterion.c b/plugins/cinterion.c
new file mode 100644
index 0000000..7f14fa0
--- /dev/null
+++ b/plugins/cinterion.c
@@ -0,0 +1,248 @@
+/*
+ *
+ *  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/log.h>
+#include <ofono/modem.h>
+#include <ofono/call-barring.h>
+#include <ofono/call-forwarding.h>
+#include <ofono/call-meter.h>
+#include <ofono/call-settings.h>
+#include <ofono/devinfo.h>
+#include <ofono/message-waiting.h>
+#include <ofono/netreg.h>
+#include <ofono/phonebook.h>
+#include <ofono/sim.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/voicecall.h>
+
+#include <drivers/atmodem/atutil.h>
+
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
+
+static int cinterion_probe(struct ofono_modem *modem)
+{
+	return 0;
+}
+
+static void cinterion_remove(struct ofono_modem *modem)
+{
+}
+
+static void cinterion_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static int cinterion_enable(struct ofono_modem *modem)
+{
+	GAtChat *chat;
+	GIOChannel *channel;
+	GAtSyntax *syntax;
+	GHashTable *options;
+	const char *device;
+
+	DBG("%p", modem);
+
+	options = g_hash_table_new(g_str_hash, g_str_equal);
+	if (options == NULL)
+		return -ENOMEM;
+
+	device = ofono_modem_get_string(modem, "Device");
+	if (device == NULL)
+		return -EINVAL;
+
+	g_hash_table_insert(options, "Baud", "115200");
+	g_hash_table_insert(options, "StopBits", "1");
+	g_hash_table_insert(options, "DataBits", "8");
+	g_hash_table_insert(options, "Parity", "none");
+	g_hash_table_insert(options, "XonXoff", "off");
+	g_hash_table_insert(options, "RtsCts", "on");
+	g_hash_table_insert(options, "Local", "on");
+	g_hash_table_insert(options, "Read", "on");
+
+	channel = g_at_tty_open(device, options);
+	g_hash_table_destroy(options);
+
+	if (channel == NULL)
+		return -EIO;
+
+	/*
+         * (Cinterion plugin is based on tc65 plugin. Comment left in but may
+         * not be applicable in the general case)
+         *
+         * TC65 works almost as the 27.007 says. But for example after
+	 * AT+CRSM the modem replies with the data in the queried EF and
+	 * writes three pairs of <CR><LF> after the data and before OK.
+	 */
+	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 -ENOMEM;
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, cinterion_debug, "");
+
+	ofono_modem_set_data(modem, chat);
+
+	return 0;
+}
+
+static int cinterion_disable(struct ofono_modem *modem)
+{
+	GAtChat *chat = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_modem_set_data(modem, NULL);
+
+	g_at_chat_send(chat, "AT+CFUN=7", NULL, NULL, NULL, NULL);
+
+	g_at_chat_unref(chat);
+
+	return 0;
+}
+
+static void set_online_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 void cinterion_set_online(struct ofono_modem *modem, ofono_bool_t online,
+			ofono_modem_online_cb_t cb, void *user_data)
+{
+	GAtChat *chat = ofono_modem_get_data(modem);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=7";
+
+	DBG("modem %p %s", modem, online ? "online" : "offline");
+
+	if (g_at_chat_send(chat, command, NULL, set_online_cb, cbd, g_free))
+		return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+}
+
+static void cinterion_pre_sim(struct ofono_modem *modem)
+{
+	GAtChat *chat = ofono_modem_get_data(modem);
+	struct ofono_sim *sim;
+
+	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "atmodem", chat);
+	sim = ofono_sim_create(modem, 0, "atmodem", chat);
+	ofono_voicecall_create(modem, 0, "atmodem", chat);
+
+	if (sim)
+		ofono_sim_inserted_notify(sim, TRUE);
+}
+
+static void cinterion_post_sim(struct ofono_modem *modem)
+{
+	GAtChat *chat = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_phonebook_create(modem, 0, "atmodem", chat);
+
+	ofono_sms_create(modem, 0, "atmodem", chat);
+}
+
+static void cinterion_post_online(struct ofono_modem *modem)
+{
+	GAtChat *chat = ofono_modem_get_data(modem);
+	struct ofono_message_waiting *mw;
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
+
+	DBG("%p", modem);
+
+	ofono_ussd_create(modem, 0, "atmodem", chat);
+	ofono_call_forwarding_create(modem, 0, "atmodem", chat);
+	ofono_call_settings_create(modem, 0, "atmodem", chat);
+	ofono_netreg_create(modem, 0, "atmodem", chat);
+	ofono_call_meter_create(modem, 0, "atmodem", chat);
+	ofono_call_barring_create(modem, 0, "atmodem", chat);
+
+	gprs = ofono_gprs_create(modem, 0, "atmodem", chat);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", chat);
+
+	if (gprs && gc)
+		ofono_gprs_add_context(gprs, gc);
+
+	mw = ofono_message_waiting_create(modem);
+	if (mw)
+		ofono_message_waiting_register(mw);
+}
+
+static struct ofono_modem_driver cinterion_driver = {
+	.name		= "cinterion",
+	.probe		= cinterion_probe,
+	.remove		= cinterion_remove,
+	.enable		= cinterion_enable,
+	.disable	= cinterion_disable,
+	.set_online	= cinterion_set_online,
+	.pre_sim	= cinterion_pre_sim,
+	.post_sim	= cinterion_post_sim,
+	.post_online	= cinterion_post_online,
+};
+
+static int cinterion_init(void)
+{
+	return ofono_modem_driver_register(&cinterion_driver);
+}
+
+static void cinterion_exit(void)
+{
+	ofono_modem_driver_unregister(&cinterion_driver);
+}
+
+OFONO_PLUGIN_DEFINE(cinterion, "Cinterion driver plugin", VERSION,
+		OFONO_PLUGIN_PRIORITY_DEFAULT, cinterion_init, cinterion_exit)
diff --git a/plugins/tc65.c b/plugins/tc65.c
deleted file mode 100644
index eb64b89..0000000
--- a/plugins/tc65.c
+++ /dev/null
@@ -1,245 +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/log.h>
-#include <ofono/modem.h>
-#include <ofono/call-barring.h>
-#include <ofono/call-forwarding.h>
-#include <ofono/call-meter.h>
-#include <ofono/call-settings.h>
-#include <ofono/devinfo.h>
-#include <ofono/message-waiting.h>
-#include <ofono/netreg.h>
-#include <ofono/phonebook.h>
-#include <ofono/sim.h>
-#include <ofono/sms.h>
-#include <ofono/ussd.h>
-#include <ofono/voicecall.h>
-
-#include <drivers/atmodem/atutil.h>
-
-#include <ofono/gprs.h>
-#include <ofono/gprs-context.h>
-
-static int tc65_probe(struct ofono_modem *modem)
-{
-	return 0;
-}
-
-static void tc65_remove(struct ofono_modem *modem)
-{
-}
-
-static void tc65_debug(const char *str, void *user_data)
-{
-	const char *prefix = user_data;
-
-	ofono_info("%s%s", prefix, str);
-}
-
-static int tc65_enable(struct ofono_modem *modem)
-{
-	GAtChat *chat;
-	GIOChannel *channel;
-	GAtSyntax *syntax;
-	GHashTable *options;
-	const char *device;
-
-	DBG("%p", modem);
-
-	options = g_hash_table_new(g_str_hash, g_str_equal);
-	if (options == NULL)
-		return -ENOMEM;
-
-	device = ofono_modem_get_string(modem, "Device");
-	if (device == NULL)
-		return -EINVAL;
-
-	g_hash_table_insert(options, "Baud", "115200");
-	g_hash_table_insert(options, "StopBits", "1");
-	g_hash_table_insert(options, "DataBits", "8");
-	g_hash_table_insert(options, "Parity", "none");
-	g_hash_table_insert(options, "XonXoff", "off");
-	g_hash_table_insert(options, "RtsCts", "on");
-	g_hash_table_insert(options, "Local", "on");
-	g_hash_table_insert(options, "Read", "on");
-
-	channel = g_at_tty_open(device, options);
-	g_hash_table_destroy(options);
-
-	if (channel == NULL)
-		return -EIO;
-
-	/*
-	 * TC65 works almost as the 27.007 says. But for example after
-	 * AT+CRSM the modem replies with the data in the queried EF and
-	 * writes three pairs of <CR><LF> after the data and before OK.
-	 */
-	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 -ENOMEM;
-
-	if (getenv("OFONO_AT_DEBUG"))
-		g_at_chat_set_debug(chat, tc65_debug, "");
-
-	ofono_modem_set_data(modem, chat);
-
-	return 0;
-}
-
-static int tc65_disable(struct ofono_modem *modem)
-{
-	GAtChat *chat = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	ofono_modem_set_data(modem, NULL);
-
-	g_at_chat_send(chat, "AT+CFUN=7", NULL, NULL, NULL, NULL);
-
-	g_at_chat_unref(chat);
-
-	return 0;
-}
-
-static void set_online_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 void tc65_set_online(struct ofono_modem *modem, ofono_bool_t online,
-			ofono_modem_online_cb_t cb, void *user_data)
-{
-	GAtChat *chat = ofono_modem_get_data(modem);
-	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=7";
-
-	DBG("modem %p %s", modem, online ? "online" : "offline");
-
-	if (g_at_chat_send(chat, command, NULL, set_online_cb, cbd, g_free))
-		return;
-
-	g_free(cbd);
-
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
-}
-
-static void tc65_pre_sim(struct ofono_modem *modem)
-{
-	GAtChat *chat = ofono_modem_get_data(modem);
-	struct ofono_sim *sim;
-
-	DBG("%p", modem);
-
-	ofono_devinfo_create(modem, 0, "atmodem", chat);
-	sim = ofono_sim_create(modem, 0, "atmodem", chat);
-	ofono_voicecall_create(modem, 0, "atmodem", chat);
-
-	if (sim)
-		ofono_sim_inserted_notify(sim, TRUE);
-}
-
-static void tc65_post_sim(struct ofono_modem *modem)
-{
-	GAtChat *chat = ofono_modem_get_data(modem);
-
-	DBG("%p", modem);
-
-	ofono_phonebook_create(modem, 0, "atmodem", chat);
-
-	ofono_sms_create(modem, 0, "atmodem", chat);
-}
-
-static void tc65_post_online(struct ofono_modem *modem)
-{
-	GAtChat *chat = ofono_modem_get_data(modem);
-	struct ofono_message_waiting *mw;
-	struct ofono_gprs *gprs;
-	struct ofono_gprs_context *gc;
-
-	DBG("%p", modem);
-
-	ofono_ussd_create(modem, 0, "atmodem", chat);
-	ofono_call_forwarding_create(modem, 0, "atmodem", chat);
-	ofono_call_settings_create(modem, 0, "atmodem", chat);
-	ofono_netreg_create(modem, 0, "atmodem", chat);
-	ofono_call_meter_create(modem, 0, "atmodem", chat);
-	ofono_call_barring_create(modem, 0, "atmodem", chat);
-
-	gprs = ofono_gprs_create(modem, 0, "atmodem", chat);
-	gc = ofono_gprs_context_create(modem, 0, "atmodem", chat);
-
-	if (gprs && gc)
-		ofono_gprs_add_context(gprs, gc);
-
-	mw = ofono_message_waiting_create(modem);
-	if (mw)
-		ofono_message_waiting_register(mw);
-}
-
-static struct ofono_modem_driver tc65_driver = {
-	.name		= "tc65",
-	.probe		= tc65_probe,
-	.remove		= tc65_remove,
-	.enable		= tc65_enable,
-	.disable	= tc65_disable,
-	.set_online	= tc65_set_online,
-	.pre_sim	= tc65_pre_sim,
-	.post_sim	= tc65_post_sim,
-	.post_online	= tc65_post_online,
-};
-
-static int tc65_init(void)
-{
-	return ofono_modem_driver_register(&tc65_driver);
-}
-
-static void tc65_exit(void)
-{
-	ofono_modem_driver_unregister(&tc65_driver);
-}
-
-OFONO_PLUGIN_DEFINE(tc65, "Cinterion TC65 driver plugin", VERSION,
-		OFONO_PLUGIN_PRIORITY_DEFAULT, tc65_init, tc65_exit)
diff --git a/plugins/udev.c b/plugins/udev.c
index a78cd41..6330228 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -192,7 +192,7 @@ static void add_wavecom(struct ofono_modem *modem,
 	ofono_modem_register(modem);
 }
 
-static void add_tc65(struct ofono_modem *modem,
+static void add_cinterion(struct ofono_modem *modem,
 			struct udev_device *udev_device)
 {
 	const char *devnode;
@@ -243,6 +243,9 @@ static void add_modem(struct udev_device *udev_device)
 		if (devpath == NULL)
 			return;
 
+		if(g_strcmp0(driver, "tc65") == 0)
+			driver = "cinterion";
+
 		modem = ofono_modem_create(NULL, driver);
 		if (modem == NULL)
 			return;
@@ -305,8 +308,8 @@ done:
 		add_isi(modem, udev_device);
 	else if (g_strcmp0(driver, "calypso") == 0)
 		add_calypso(modem, udev_device);
-	else if (g_strcmp0(driver, "tc65") == 0)
-		add_tc65(modem, udev_device);
+	else if (g_strcmp0(driver, "cinterion") == 0)
+		add_cinterion(modem, udev_device);
 	else if (g_strcmp0(driver, "nokiacdma") == 0)
 		add_nokiacdma(modem, udev_device);
 	else if (g_strcmp0(driver, "sim900") == 0)
-- 
1.9.1

[PATCH 2/4] atmodem: Add Cinterion quirk for signal strength

[Alex J Lennon]

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

Implement OFONO_VENDOR_CINTERION specific vendor support to register
textual +CIEV indications for signal strength using AT^SIND command.
---
 drivers/atmodem/network-registration.c | 54 ++++++++++++++++++++++++++++++++++
 drivers/atmodem/vendor.h               |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index a438726..7cfd6b2 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -838,6 +838,39 @@ static void telit_ciev_notify(GAtResult *result, gpointer user_data)
 	ofono_netreg_strength_notify(netreg, strength);
 }
 
+static void cinterion_ciev_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_netreg *netreg = user_data;
+	struct netreg_data *nd = ofono_netreg_get_data(netreg);
+	const char *signal_identifier = "rssi";
+	const char *ind_str;
+	int strength;
+	GAtResultIter iter;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CIEV:"))
+		return;
+
+	if (!g_at_result_iter_next_unquoted_string(&iter, &ind_str))
+		return;
+
+	if (!g_str_equal(signal_identifier, ind_str))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &strength))
+		return;
+
+	DBG("rssi %d", strength);
+
+	if (strength == nd->signal_invalid)
+		strength = -1;
+	else
+		strength = (strength * 100) / (nd->signal_max - nd->signal_min);
+
+	ofono_netreg_strength_notify(netreg, strength);
+}
+
 static void ctzv_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_netreg *netreg = user_data;
@@ -1915,6 +1948,27 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		g_at_chat_send(nd->chat, "AT*TLTS=1", none_prefix,
 						NULL, NULL, NULL);
 		break;
+	case OFONO_VENDOR_CINTERION:
+		/*
+		 * We can't set rssi bounds from Cinterion responses
+		 * so set them up to specified values here
+		 *
+		 * Cinterion rssi signal strength specified as:
+		 * 0      <= -112dBm
+		 * 1 - 4  signal strengh in 15 dB steps
+		 * 5      >= -51 dBm
+		 * 99     not known or undetectable
+		 */
+		nd->signal_min = 0;
+		nd->signal_max = 5;
+		nd->signal_invalid = 99;
+
+		/* Register for specific signal strength reports */
+		g_at_chat_send(nd->chat, "AT^SIND=\"rssi\",1", none_prefix,
+				NULL, NULL, NULL);
+		g_at_chat_register(nd->chat, "+CIEV:",
+				cinterion_ciev_notify, FALSE, netreg, NULL);
+		break;
 	case OFONO_VENDOR_NOKIA:
 	case OFONO_VENDOR_SAMSUNG:
 		/* Signal strength reporting via CIND is not supported */
diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index c132e45..52071c8 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -45,4 +45,5 @@ enum ofono_vendor {
 	OFONO_VENDOR_ALCATEL,
 	OFONO_VENDOR_QUECTEL,
 	OFONO_VENDOR_UBLOX,
+	OFONO_VENDOR_CINTERION,
 };
-- 
1.9.1

[PATCH 3/4] cinterion: Register as OFONO_VENDOR_CINTERION

[Alex J Lennon]

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

This enables us to take advantage of vendor specific quirks
(e.g. signal strength handling specifics for tc65)
---
 plugins/cinterion.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/cinterion.c b/plugins/cinterion.c
index 7f14fa0..6d85189 100644
--- a/plugins/cinterion.c
+++ b/plugins/cinterion.c
@@ -52,6 +52,8 @@
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
 
+#include <drivers/atmodem/vendor.h>
+
 static int cinterion_probe(struct ofono_modem *modem)
 {
 	return 0;
@@ -207,7 +209,7 @@ static void cinterion_post_online(struct ofono_modem *modem)
 	ofono_ussd_create(modem, 0, "atmodem", chat);
 	ofono_call_forwarding_create(modem, 0, "atmodem", chat);
 	ofono_call_settings_create(modem, 0, "atmodem", chat);
-	ofono_netreg_create(modem, 0, "atmodem", chat);
+	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", chat);
 	ofono_call_meter_create(modem, 0, "atmodem", chat);
 	ofono_call_barring_create(modem, 0, "atmodem", chat);
 
-- 
1.9.1

[PATCH 4/4] udev: Add support for ehs6 name

[Alex J Lennon]

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

As with tc65, ehs6 makes use of cinterion plugin
---
 plugins/udev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/udev.c b/plugins/udev.c
index 6330228..3c90e40 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -245,6 +245,8 @@ static void add_modem(struct udev_device *udev_device)
 
 		if(g_strcmp0(driver, "tc65") == 0)
 			driver = "cinterion";
+		if(g_strcmp0(driver, "ehs6") == 0)
+			driver = "cinterion";
 
 		modem = ofono_modem_create(NULL, driver);
 		if (modem == NULL)
-- 
1.9.1

Re: [PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[Denis Kenzior]

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

Hi Alex,

On 05/12/2015 12:22 PM, Alex J Lennon wrote:
> On the basis that tc6x and other Cinterion devices will likely
> have similar firmware requirements, provide a generic Cinterion
> plugin which is functionally identical to the replaced tc65 plugin.
>
> The udev implementation retains support for "tc65" name for
> backwards compatibility, and adds support for the new "cinterion"
> name.
> ---
>   Makefile.am         |   4 +-
>   plugins/cinterion.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   plugins/tc65.c      | 245 ---------------------------------------------------
>   plugins/udev.c      |   9 +-
>   4 files changed, 256 insertions(+), 250 deletions(-)
>   create mode 100644 plugins/cinterion.c
>   delete mode 100644 plugins/tc65.c
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 2/4] atmodem: Add Cinterion quirk for signal strength

[Denis Kenzior]

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

Hi Alex,

On 05/12/2015 12:23 PM, Alex J Lennon wrote:
> Implement OFONO_VENDOR_CINTERION specific vendor support to register
> textual +CIEV indications for signal strength using AT^SIND command.
> ---
>   drivers/atmodem/network-registration.c | 54 ++++++++++++++++++++++++++++++++++
>   drivers/atmodem/vendor.h               |  1 +
>   2 files changed, 55 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 3/4] cinterion: Register as OFONO_VENDOR_CINTERION

[Denis Kenzior]

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

Hi Alex,

On 05/12/2015 12:23 PM, Alex J Lennon wrote:
> This enables us to take advantage of vendor specific quirks
> (e.g. signal strength handling specifics for tc65)
> ---
>   plugins/cinterion.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 4/4] udev: Add support for ehs6 name

[Denis Kenzior]

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

Hi Alex,

On 05/12/2015 12:23 PM, Alex J Lennon wrote:
> As with tc65, ehs6 makes use of cinterion plugin
> ---
>   plugins/udev.c | 2 ++
>   1 file changed, 2 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[Philip Paeps]

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


On 2015-05-12 18:22:59 (+0100), Alex J Lennon <ajlennon@dynamicdevices.co.uk> wrote:
> [...]

Just paging through this driver this afternoon.  Sorry for not reviewing
the earlier patches.  This caught my eye:

> +static void cinterion_set_online(struct ofono_modem *modem, ofono_bool_t online,
> +			ofono_modem_online_cb_t cb, void *user_data)
> +{
> +	GAtChat *chat = ofono_modem_get_data(modem);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=7";
> +
> +	DBG("modem %p %s", modem, online ? "online" : "offline");
> +
> +	if (g_at_chat_send(chat, command, NULL, set_online_cb, cbd, g_free))
> +		return;
> +
> +	g_free(cbd);
> +
> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
> +}

Doesn't this expand into a use after free bug if g_at_chat_send() fails?

Philip

-- 
Philip Paeps
Senior Reality Engineer
Ministry of Information

Re: [PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[Alex J Lennon]

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



On 13/05/2015 17:18, Philip Paeps wrote:
> On 2015-05-12 18:22:59 (+0100), Alex J Lennon <ajlennon@dynamicdevices.co.uk> wrote:
>> [...]
> Just paging through this driver this afternoon.  Sorry for not reviewing
> the earlier patches.  This caught my eye:
>
>> +static void cinterion_set_online(struct ofono_modem *modem, ofono_bool_t online,
>> +			ofono_modem_online_cb_t cb, void *user_data)
>> +{
>> +	GAtChat *chat = ofono_modem_get_data(modem);
>> +	struct cb_data *cbd = cb_data_new(cb, user_data);
>> +	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=7";
>> +
>> +	DBG("modem %p %s", modem, online ? "online" : "offline");
>> +
>> +	if (g_at_chat_send(chat, command, NULL, set_online_cb, cbd, g_free))
>> +		return;
>> +
>> +	g_free(cbd);
>> +
>> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
>> +}
> Doesn't this expand into a use after free bug if g_at_chat_send() fails?
>

Yes I see what you mean. I also see other plugins (e.g. ublox) reverse
the order of the free and callback.

Have provided a patch to bring the cinterion plugin into line.

Regards,

Alex

Re: [PATCH 1/4] tc65: Replace tc65 plugin with cinterion plugin

[Denis Kenzior]

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

Hi Philip,

 >> +static void cinterion_set_online(struct ofono_modem *modem, 
ofono_bool_t online,
>> +			ofono_modem_online_cb_t cb, void *user_data)
>> +{
>> +	GAtChat *chat = ofono_modem_get_data(modem);
>> +	struct cb_data *cbd = cb_data_new(cb, user_data);
>> +	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=7";
>> +
>> +	DBG("modem %p %s", modem, online ? "online" : "offline");
>> +
>> +	if (g_at_chat_send(chat, command, NULL, set_online_cb, cbd, g_free))
>> +		return;
>> +
>> +	g_free(cbd);
>> +
>> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
>> +}
>
> Doesn't this expand into a use after free bug if g_at_chat_send() fails?
>

Yes it does!  I fixed a few more occurrences of this bug.  Thanks for 
pointing it out.

Regards,
-Denis