[PATCH v4 1/4] network-registration.c: add telit support

[PATCH v4 1/4] network-registration.c: add telit support

[Bernhard.Guillon]

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

From: Bernhard Guillon <Bernhard.Guillon@hale.at>

*add support for CIND=? tokens like ("signal",(0-7,99))
*add telit quirk for token encapsulation e.g.
    (("one",(0-7,99)),("two",(0-7,99)))
---
 drivers/atmodem/network-registration.c |   41 ++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index b3aa511..7c86fec 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -56,6 +56,7 @@ struct netreg_data {
 	int signal_index; /* If strength is reported via CIND */
 	int signal_min; /* min strength reported via CIND */
 	int signal_max; /* max strength reported via CIND */
+	int signal_invalid; /* invalid strength reported via CIND */
 	int tech;
 	struct ofono_network_time time;
 	guint nitz_timeout;
@@ -666,7 +667,11 @@ static void ciev_notify(GAtResult *result, gpointer user_data)
 	if (!g_at_result_iter_next_number(&iter, &strength))
 		return;
 
-	strength = (strength * 100) / (nd->signal_max - nd->signal_min);
+	if (strength == nd->signal_invalid)
+		strength = -1;
+	else
+		strength = (strength * 100) / (nd->signal_max - nd->signal_min);
+
 	ofono_netreg_strength_notify(netreg, strength);
 }
 
@@ -798,7 +803,10 @@ static void cind_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	g_at_result_iter_next_number(&iter, &strength);
 
-	strength = (strength * 100) / (nd->signal_max - nd->signal_min);
+	if (strength == nd->signal_invalid)
+		strength = -1;
+	else
+		strength = (strength * 100) / (nd->signal_max - nd->signal_min);
 
 	cb(&error, strength, cbd->data);
 }
@@ -1133,7 +1141,9 @@ static void cind_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	GAtResultIter iter;
 	const char *str;
 	int index;
-	int min, max;
+	int min = 0;
+	int max = 0;
+	int tmp_min, tmp_max, invalid;
 
 	if (!ok)
 		goto error;
@@ -1144,15 +1154,32 @@ static void cind_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	index = 1;
 
+	/*
+	 * Telit encapsulates the CIND=? tokens with braces
+	 * so we need to skip them
+	 */
+	if (nd->vendor == OFONO_VENDOR_TELIT)
+		g_at_result_iter_open_list(&iter);
+
 	while (g_at_result_iter_open_list(&iter)) {
+		/* Reset invalid default value for every token*/
+		invalid = 99;
+
 		if (!g_at_result_iter_next_string(&iter, &str))
 			goto error;
 
 		if (!g_at_result_iter_open_list(&iter))
 			goto error;
 
-		while (g_at_result_iter_next_range(&iter, &min, &max))
-			;
+		while (g_at_result_iter_next_range(&iter, &tmp_min, &tmp_max)) {
+
+			if (tmp_min != tmp_max) {
+				min = tmp_min;
+				max = tmp_max;
+			} else
+				invalid = tmp_min;
+
+		}
 
 		if (!g_at_result_iter_close_list(&iter))
 			goto error;
@@ -1164,11 +1191,15 @@ static void cind_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
 			nd->signal_index = index;
 			nd->signal_min = min;
 			nd->signal_max = max;
+			nd->signal_invalid = invalid;
 		}
 
 		index += 1;
 	}
 
+	if (nd->vendor == OFONO_VENDOR_TELIT)
+		g_at_result_iter_close_list(&iter);
+
 	if (nd->signal_index == 0)
 		goto error;
 
-- 
1.7.0.4



--
Scanned by MailScanner.

Re: [PATCH v4 1/4] network-registration.c: add telit support

[Denis Kenzior]

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

Hi Bernhard,

On 06/08/2011 10:18 AM, Bernhard.Guillon(a)hale.at wrote:
> From: Bernhard Guillon <Bernhard.Guillon@hale.at>
> 
> *add support for CIND=? tokens like ("signal",(0-7,99))
> *add telit quirk for token encapsulation e.g.
>     (("one",(0-7,99)),("two",(0-7,99)))
> ---
>  drivers/atmodem/network-registration.c |   41 ++++++++++++++++++++++++++++----
>  1 files changed, 36 insertions(+), 5 deletions(-)
> 

Patch has been applied (after patch 3) and with a few minor nitpicks
amended.  Thanks.

Regards,
-Denis

[PATCH v4 2/4] Add basic Telit UC864-G support:

[Bernhard.Guillon]

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

From: Bernhard Guillon <Bernhard.Guillon@hale.at>

*add a basic plugin based on different ofono plugins
*use Telit specific QSS for SIM-state
*update Makefile
---
 Makefile.am     |    3 +
 plugins/telit.c |  379 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 382 insertions(+), 0 deletions(-)
 create mode 100644 plugins/telit.c

diff --git a/Makefile.am b/Makefile.am
index 0a7b6d5..dd4fb8f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -326,6 +326,9 @@ builtin_sources += plugins/nokiacdma.c
 builtin_modules += linktop
 builtin_sources += plugins/linktop.c
 
+builtin_modules += telit
+builtin_sources += plugins/telit.c
+
 if BLUETOOTH
 builtin_modules += bluetooth
 builtin_sources += plugins/bluetooth.c plugins/bluetooth.h
diff --git a/plugins/telit.c b/plugins/telit.c
new file mode 100644
index 0000000..8101b8f
--- /dev/null
+++ b/plugins/telit.c
@@ -0,0 +1,379 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  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 <unistd.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/gprs.h>
+#include <ofono/gprs-context.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/voicecall.h>
+
+#include <drivers/atmodem/atutil.h>
+#include <drivers/atmodem/vendor.h>
+
+static const char *none_prefix[] = { NULL };
+static const char *qss_prefix[] = { "#QSS:", NULL };
+
+struct telit_data {
+	GAtChat *chat;
+	struct ofono_sim *sim;
+};
+
+static void telit_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static int telit_probe(struct ofono_modem *modem)
+{
+	struct telit_data *data;
+
+	DBG("%p", modem);
+
+	data = g_try_new0(struct telit_data, 1);
+	if (data == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, data);
+
+	return 0;
+}
+
+static void telit_remove(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_modem_set_data(modem, NULL);
+
+	g_free(data);
+}
+
+static gboolean sim_inserted_timeout_cb(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_sim_inserted_notify(data->sim, TRUE);
+
+	return FALSE;
+}
+
+static void switch_sim_state_status(struct ofono_modem *modem, int status)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	switch (status) {
+	case 0:
+		DBG("SIM not inserted");
+		ofono_sim_inserted_notify(data->sim, FALSE);
+		break;
+	case 1:
+		DBG("SIM inserted");
+		/* We need to sleep a bit */
+		g_timeout_add_seconds(1, sim_inserted_timeout_cb, modem);
+		break;
+	case 2:
+		DBG("SIM inserted and PIN unlocked");
+		break;
+	case 3:
+		DBG("SIM inserted and ready");
+		break;
+	}
+}
+
+static void telit_qss_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	int status;
+	GAtResultIter iter;
+
+	DBG("%p", modem);
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "#QSS:"))
+		return;
+
+	g_at_result_iter_next_number(&iter, &status);
+
+	switch_sim_state_status(modem, status);
+}
+
+static void telit_qss_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	int mode;
+	int status;
+	GAtResultIter iter;
+	g_at_result_iter_init(&iter, result);
+
+	DBG("%p", modem);
+
+	if (!g_at_result_iter_next(&iter, "#QSS:"))
+		return;
+
+	g_at_result_iter_next_number(&iter, &mode);
+	g_at_result_iter_next_number(&iter, &status);
+
+	switch_sim_state_status(modem, status);
+}
+
+static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	if (!ok) {
+		g_at_chat_unref(data->chat);
+		data->chat = NULL;
+		ofono_modem_set_powered(modem, FALSE);
+		return;
+	}
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	/* Enable sim state notification */
+	g_at_chat_send(data->chat, "AT#QSS=1", none_prefix, NULL, NULL, NULL);
+
+	/* Follow sim state */
+	g_at_chat_register(data->chat, "#QSS:", telit_qss_notify,
+				FALSE, modem, NULL);
+
+	/* Query current sim state */
+	g_at_chat_send(data->chat, "AT#QSS?", qss_prefix,
+				telit_qss_cb, modem, NULL);
+}
+
+static void cfun_disable_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	g_at_chat_unref(data->chat);
+	data->chat = NULL;
+
+	if (ok)
+		ofono_modem_set_powered(modem, FALSE);
+}
+
+static GAtChat *open_device(struct ofono_modem *modem,
+				const char *key, char *debug)
+{
+	const char *device;
+	GAtSyntax *syntax;
+	GIOChannel *channel;
+	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_gsmv1();
+	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, telit_debug, debug);
+
+	return chat;
+}
+
+static int telit_enable(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	data->chat = open_device(modem, "Modem", "Modem: ");
+	if (data->chat == NULL)
+		return -EINVAL;
+
+	/*
+	 * Disable command echo and
+	 * enable the Extended Error Result Codes
+	 */
+	g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
+				NULL, NULL, NULL);
+
+	/* Set phone functionality */
+	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
+				cfun_enable_cb, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static int telit_disable(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+	DBG("%p", modem);
+
+	g_at_chat_cancel_all(data->chat);
+	g_at_chat_unregister_all(data->chat);
+
+	/* Power down modem */
+	g_at_chat_send(data->chat, "AT+CFUN=0", none_prefix,
+				cfun_disable_cb, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+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;
+
+	if (ok)
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+	else
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+}
+
+static void telit_set_online(struct ofono_modem *modem, ofono_bool_t online,
+				ofono_modem_online_cb_t cb, void *user_data)
+{
+	struct telit_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");
+
+	g_at_chat_send(data->chat, command, none_prefix, set_online_cb,
+						cbd, g_free);
+}
+
+static void telit_pre_sim(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "atmodem", data->chat);
+	data->sim = ofono_sim_create(modem, 0, "atmodem", data->chat);
+	ofono_voicecall_create(modem, 0, "atmodem", data->chat);
+}
+
+static void telit_post_sim(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_sms_create(modem, 0, "atmodem", data->chat);
+}
+
+static void telit_post_online(struct ofono_modem *modem)
+{
+	struct telit_data *data = ofono_modem_get_data(modem);
+	struct ofono_message_waiting *mw;
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
+
+	DBG("%p", modem);
+
+	ofono_netreg_create(modem, OFONO_VENDOR_TELIT, "atmodem", data->chat);
+	ofono_ussd_create(modem, 0, "atmodem", data->chat);
+	ofono_call_forwarding_create(modem, 0, "atmodem", data->chat);
+	ofono_call_settings_create(modem, 0, "atmodem", data->chat);
+	ofono_call_meter_create(modem, 0, "atmodem", data->chat);
+	ofono_call_barring_create(modem, 0, "atmodem", data->chat);
+
+	gprs = ofono_gprs_create(modem, 0, "atmodem", data->chat);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->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 telit_driver = {
+	.name		= "telit",
+	.probe		= telit_probe,
+	.remove		= telit_remove,
+	.enable		= telit_enable,
+	.disable	= telit_disable,
+	.set_online	= telit_set_online,
+	.pre_sim	= telit_pre_sim,
+	.post_sim	= telit_post_sim,
+	.post_online	= telit_post_online,
+};
+
+static int telit_init(void)
+{
+	return ofono_modem_driver_register(&telit_driver);
+}
+
+static void telit_exit(void)
+{
+	ofono_modem_driver_unregister(&telit_driver);
+}
+
+OFONO_PLUGIN_DEFINE(telit, "telit driver", VERSION,
+		OFONO_PLUGIN_PRIORITY_DEFAULT, telit_init, telit_exit)
-- 
1.7.0.4



--
Scanned by MailScanner.

Re: [PATCH v4 2/4] Add basic Telit UC864-G support:

[Denis Kenzior]

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

Hi Bernhard,

On 06/08/2011 10:18 AM, Bernhard.Guillon(a)hale.at wrote:
> From: Bernhard Guillon <Bernhard.Guillon@hale.at>
> 
> *add a basic plugin based on different ofono plugins
> *use Telit specific QSS for SIM-state
> *update Makefile
> ---
>  Makefile.am     |    3 +
>  plugins/telit.c |  379 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 382 insertions(+), 0 deletions(-)
>  create mode 100644 plugins/telit.c
> 

Patch has been applied, thanks.  I did modify the commit message
somewhat and added 3 simple commits afterwards.  Please check these
starting with commit 025fe0a74d1aa34a3c72054029d23ef4425abc6d and make
sure you're OK with these.

Now we just need to solve the udev integration part.

Regards,
-Denis

Re: [PATCH v4 2/4] Add basic Telit UC864-G support:

[Bernhard Guillon]

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



On Wed, 8 Jun 2011, Denis Kenzior wrote:

>
> Patch has been applied, thanks.  I did modify the commit message
> somewhat and added 3 simple commits afterwards.  Please check these
> starting with commit 025fe0a74d1aa34a3c72054029d23ef4425abc6d and make
> sure you're OK with these.

Hi,
I just tried them (compiled, started, tested modem funcionality, and 
checked the OFONO_AT_DEBUG output) and they are working fine, thanks.

>
> Now we just need to solve the udev integration part.
>

See other mail :)

Best regards,
Bernhard


--
Scanned by MailScanner.

Re: [PATCH v4 2/4] Add basic Telit UC864-G support:

[Marcel Holtmann]

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

Hi Bernhard,

> *add a basic plugin based on different ofono plugins
> *use Telit specific QSS for SIM-state
> *update Makefile
> ---
>  Makefile.am     |    3 +
>  plugins/telit.c |  379 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 382 insertions(+), 0 deletions(-)
>  create mode 100644 plugins/telit.c

I am fine with this patch now, but I leave it up to Denis to apply it.
Not sure when I get to answering your udev question.

Regards

Marcel

[PATCH v4 3/4] atmodem/vendor.h: add OFONO_VENDOR_TELIT

[Bernhard.Guillon]

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

From: Bernhard Guillon <Bernhard.Guillon@hale.at>

---
 drivers/atmodem/vendor.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index 3898fa8..412bc76 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -35,4 +35,5 @@ enum ofono_vendor {
 	OFONO_VENDOR_WAVECOM,
 	OFONO_VENDOR_NOKIA,
 	OFONO_VENDOR_PHONESIM,
+	OFONO_VENDOR_TELIT,
 };
-- 
1.7.0.4



--
Scanned by MailScanner.

Re: [PATCH v4 3/4] atmodem/vendor.h: add OFONO_VENDOR_TELIT

[Denis Kenzior]

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

Hi Bernhard,

On 06/08/2011 10:18 AM, Bernhard.Guillon(a)hale.at wrote:
> From: Bernhard Guillon <Bernhard.Guillon@hale.at>
> 

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Bernhard.Guillon]

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

From: Bernhard Guillon <Bernhard.Guillon@hale.at>

*add Modem and GPS check for telit
*make GPS yes/no configurable with udev rules settings
**this is necessary because telit has different interface
  numbers on different modems e.g. GPS and UART
---
 plugins/ofono.rules |    8 +++++
 plugins/udev.c      |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/plugins/ofono.rules b/plugins/ofono.rules
index 268b327..64fa4b2 100644
--- a/plugins/ofono.rules
+++ b/plugins/ofono.rules
@@ -344,6 +344,11 @@ ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1485", ENV{OFONO_IFACE_NUM}=="02", E
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_HUAWEI_TYPE}="Modem"
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_HUAWEI_TYPE}="Pcui"
 
+#Telit UC864-G
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_TELIT_GPS}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_TELIT_TYPE}="Modem"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_TELIT_TYPE}="GPS"
+
 LABEL="ofono_tty_end"
 
 # ISI/Phonet drivers
@@ -459,4 +464,7 @@ ATTRS{idVendor}=="0421", ATTRS{idProduct}=="00b6", ENV{OFONO_DRIVER}="nokiacdma"
 # Teracom (Linktop/LW27x) 3G Data Card
 ATTRS{idVendor}=="230d", ATTRS{idProduct}=="0001", ENV{OFONO_DRIVER}="linktop"
 
+# Telit
+ATTRS{idVendor}=="1bc7", ENV{OFONO_DRIVER}="telit"
+
 LABEL="ofono_end"
diff --git a/plugins/udev.c b/plugins/udev.c
index 0234fc0..e56d3db 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -587,6 +587,76 @@ static void add_linktop(struct ofono_modem *modem,
 	}
 }
 
+static void add_telit(struct ofono_modem *modem,
+					struct udev_device *udev_device)
+{
+	struct udev_list_entry *entry;
+	const char *devnode, *type;
+	int device, gps_device, has_gps;
+
+	DBG("modem %p", modem);
+
+	/*
+	 * Some Telit modems have a built in GPS device. According to the
+	 * manual the the interface numbers can differ between different
+	 * devices hence we need to use udev rules to set the environment
+	 * accordingly.
+	 */
+	device = ofono_modem_get_integer(modem, "ModemRegistered");
+	gps_device = ofono_modem_get_integer(modem, "GPSRegistered");
+	has_gps = ofono_modem_get_integer(modem, "HasGPS");
+
+	if ((!has_gps && device) || (device && gps_device))
+		return;
+
+	entry = udev_device_get_properties_list_entry(udev_device);
+	while (entry) {
+		const char *name = udev_list_entry_get_name(entry);
+		type = udev_list_entry_get_value(entry);
+
+		if (g_str_equal(name, "OFONO_TELIT_GPS") == TRUE) {
+			int value = g_str_equal(type, "1");
+
+			ofono_modem_set_integer(modem, "HasGPS", value);
+			entry = udev_list_entry_get_next(entry);
+			continue;
+		}
+
+		if (g_str_equal(name, "OFONO_TELIT_TYPE") != TRUE) {
+			entry = udev_list_entry_get_next(entry);
+			continue;
+		}
+
+		if (g_str_equal(type, "Modem") == TRUE) {
+
+			if (device != 0)
+				return;
+
+			devnode = udev_device_get_devnode(udev_device);
+			ofono_modem_set_string(modem, "Modem", devnode);
+			device = 1;
+			ofono_modem_set_integer(modem, "ModemRegistered",
+							device);
+		} else if (g_str_equal(type, "GPS") == TRUE) {
+
+			if (gps_device != 0)
+				return;
+
+			devnode = udev_device_get_devnode(udev_device);
+			ofono_modem_set_string(modem, "GPS", devnode);
+
+			gps_device = 1;
+			ofono_modem_set_integer(modem, "GPSRegistered",
+							gps_device);
+		}
+
+		break;
+	}
+
+	if ((!has_gps && device) || (device && gps_device))
+		ofono_modem_register(modem);
+}
+
 static void add_modem(struct udev_device *udev_device)
 {
 	struct ofono_modem *modem;
@@ -681,6 +751,8 @@ done:
 		add_calypso(modem, udev_device);
 	else if (g_strcmp0(driver, "tc65") == 0)
 		add_tc65(modem, udev_device);
+	else if (g_strcmp0(driver, "telit") == 0)
+		add_telit(modem, udev_device);
 	else if (g_strcmp0(driver, "nokiacdma") == 0)
 		add_nokiacdma(modem, udev_device);
         else if (g_strcmp0(driver, "linktop") == 0)
-- 
1.7.0.4



--
Scanned by MailScanner.

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Bernhard Guillon]

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



On Wed, 8 Jun 2011, Bernhard.Guillon(a)hale.at wrote:

> From: Bernhard Guillon <Bernhard.Guillon@hale.at>
>
> *add Modem and GPS check for telit
> *make GPS yes/no configurable with udev rules settings
> **this is necessary because telit has different interface
>  numbers on different modems e.g. GPS and UART

Hi,
I did not know how to better ask for some help so I sent this patch
without being completely right.

I needed to move the ENV{OFONO_DRIVER}="telit" to the end like the other
modems do. Without the add_modem is called with different modem objects.

The problem about the current patch is:

ofonod[31654]: src/modem.c:ofono_modem_create() name: 000000000002, type: 
telit

I want to ask for some advice on how to best debug this problem. I tried 
it with a short PROGRAM="/tmp/foo.pl" which prints the environment. If I 
add it to the

ATTRS{idVendor}=="1bc7", ENV{OFONO_DRIVER}="telit", PROGRAM="/tmp/foo.pl"

line the environment for OFONO_DRIVER do not exist. If I add the same 
script to one of the lines at the first section I get the proper 
OFONO_DRIVER telit envirionment.

Thanks in advance for any hints :)

I was not able to hard code the interface number as novatel does it
because the interfaces change on different telit modems. I used the huawai 
code as base.

Best regards,
Bernhard

[1]
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
Path
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
Registered
ofonod[31654]: plugins/udev.c:add_modem() 
/devices/pci0000:00/0000:00:1d.2/usb8/8-2/8-2:1.0/ttyUSB1/tty/ttyUSB1 
(telit)
ofonod[31654]: plugins/udev.c:add_telit() modem 0x1f2d890
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
ModemRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
GPSRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
Modem
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
ModemRegistered
ofonod[31654]: plugins/telit.c:telit_probe() 0x1f2d890
ofonod[31654]: plugins/smart-messaging.c:modem_watch() modem: 0x1f2d890, 
added: 1
ofonod[31654]: plugins/push-notification.c:modem_watch() modem: 0x1f2d890, 
added: 1
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
Path
ofonod[31654]: plugins/udev.c:add_modem() 
/devices/pci0000:00/0000:00:1d.2/usb8/8-2/8-2:1.1/ttyUSB2/tty/ttyUSB2 
(telit)
ofonod[31654]: plugins/udev.c:add_telit() modem 0x1f2d890
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
ModemRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
GPSRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:unregister_property() property 0x1f2ce60
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
Path
ofonod[31654]: plugins/udev.c:add_modem() 
/devices/pci0000:00/0000:00:1d.2/usb8/8-2/8-2:1.2/ttyUSB3/tty/ttyUSB3 
(telit)
ofonod[31654]: plugins/udev.c:add_telit() modem 0x1f2d890
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
ModemRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
GPSRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
HasGPS
ofonod[31654]: src/modem.c:unregister_property() property 0x1f2d330
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
GPS
ofonod[31654]: src/modem.c:set_modem_property() modem 0x1f2d890 property 
GPSRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
Path
ofonod[31654]: plugins/udev.c:add_modem() 
/devices/pci0000:00/0000:00:1d.2/usb8/8-2/8-2:1.3/ttyUSB4/tty/ttyUSB4 
(telit)
ofonod[31654]: plugins/udev.c:add_telit() modem 0x1f2d890
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
ModemRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
GPSRegistered
ofonod[31654]: src/modem.c:get_modem_property() modem 0x1f2d890 property 
HasGPS




--
Scanned by MailScanner.

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Denis Kenzior]

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

Hi Bernhard,

> Hi,
> I did not know how to better ask for some help so I sent this patch
> without being completely right.
> 
> I needed to move the ENV{OFONO_DRIVER}="telit" to the end like the other
> modems do. Without the add_modem is called with different modem objects.
> 
> The problem about the current patch is:
> 
> ofonod[31654]: src/modem.c:ofono_modem_create() name: 000000000002,
> type: telit
> 
> I want to ask for some advice on how to best debug this problem. I tried
> it with a short PROGRAM="/tmp/foo.pl" which prints the environment. If I
> add it to the
> 
> ATTRS{idVendor}=="1bc7", ENV{OFONO_DRIVER}="telit", PROGRAM="/tmp/foo.pl"
> 
> line the environment for OFONO_DRIVER do not exist. If I add the same
> script to one of the lines at the first section I get the proper
> OFONO_DRIVER telit envirionment.
> 
> Thanks in advance for any hints :)
> 
> I was not able to hard code the interface number as novatel does it
> because the interfaces change on different telit modems. I used the
> huawai code as base.

So let me understand what you're trying to achieve, you want to set an
environment variable telling the driver that a particular model has a
GPS port?

From what I remember of the Telit modems, all ports are fully
functional, so the GPS port can be on any tty.  If this is the case,
then the way this can be accomplished is by querying the model /
revision of the modem during the enable() stage in the modem driver and
adding the appropriate atom as needed.

Regards,
-Denis

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Bernhard Guillon]

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



On Mon, 6 Jun 2011, Denis Kenzior wrote:

>> I was not able to hard code the interface number as novatel does it
>> because the interfaces change on different telit modems. I used the
>> huawai code as base.
>
> So let me understand what you're trying to achieve, you want to set an
> environment variable telling the driver that a particular model has a
> GPS port?

Yes.

>
> From what I remember of the Telit modems, all ports are fully
> functional, so the GPS port can be on any tty.  If this is the case,
> then the way this can be accomplished is by querying the model /
> revision of the modem during the enable() stage in the modem driver and
> adding the appropriate atom as needed.
>

The GPS port is not fully functional as far is I know it is read only. 
According to the manual it is necessary to enable over the modem line 
with AT$GPSP=1 afterwards it starts reporting NMEA signals on the GPS 
line.

I like the environment setting with udev more. The approach with querying 
the model during enable() requires a update to the telit plugin every time 
a new model with gps is released. The comment about hardcode the interface 
number was an answer to the suggestion to look at the novatel udev code. 
Sorry for mixing up things.

After I dug deeper into the add_modem code I don't know if I have a 
problem at all :)

My current "problem" with the patch I sent is that I get into the

 	if (modem == NULL) {
 		const char *serial = get_serial(parent);

 		modem = ofono_modem_create(serial, driver);

case.

ofonod[16327]: src/modem.c:ofono_modem_create() name: 000000000002, type: 
telit

So my modem dbus modem path is /000000000002 which I thought it was wrong.
Everything else works well. The GPS value is set and both paths are set 
right.

So I think my real question is - is it the intended behavior to have the 
path like this? I don't have any supported modem to test (I really should 
by one :D ) and the phonesim is using /phonesim1 as path

Best regards,
Bernhard


--
Scanned by MailScanner.

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Denis Kenzior]

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

Hi Bernhard,

>>
>> From what I remember of the Telit modems, all ports are fully
>> functional, so the GPS port can be on any tty.  If this is the case,
>> then the way this can be accomplished is by querying the model /
>> revision of the modem during the enable() stage in the modem driver and
>> adding the appropriate atom as needed.
>>
> 
> The GPS port is not fully functional as far is I know it is read only.
> According to the manual it is necessary to enable over the modem line
> with AT$GPSP=1 afterwards it starts reporting NMEA signals on the GPS line.
> 

In theory you can use $GPSNMUN to enable GPS reporting on Ports 1 and 4
which seem to be fully functional.  However, it does look like on my
UC864-G, port #3 is used for the GPS stream.  Port #1 doesn't seem to
respond to anything.

> I like the environment setting with udev more. The approach with
> querying the model during enable() requires a update to the telit plugin
> every time a new model with gps is released. The comment about hardcode
> the interface number was an answer to the suggestion to look at the
> novatel udev code. Sorry for mixing up things.
> 
> After I dug deeper into the add_modem code I don't know if I have a
> problem at all :)
> 
> My current "problem" with the patch I sent is that I get into the
> 
>     if (modem == NULL) {
>         const char *serial = get_serial(parent);
> 
>         modem = ofono_modem_create(serial, driver);
> 
> case.
> 
> ofonod[16327]: src/modem.c:ofono_modem_create() name: 000000000002,
> type: telit
> 
> So my modem dbus modem path is /000000000002 which I thought it was wrong.
> Everything else works well. The GPS value is set and both paths are set
> right.
> 
> So I think my real question is - is it the intended behavior to have the
> path like this? I don't have any supported modem to test (I really
> should by one :D ) and the phonesim is using /phonesim1 as path
> 

Yes and no.  ofono_modem_create has two modes of operation, one where a
known unique id is given.  This can come for example from the serial
number of the USB device.  Some manufacturers provide this information
properly, others do not.  If you cannot trust the USB serial ID, then
giving a NULL parameter to ofono_modem_create will tell oFono to
generate a unique id itself.  Your path will then look like /telit2 or
something like that.

Regards,
-Denis

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Bernhard Guillon]

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



On Wed, 8 Jun 2011, Denis Kenzior wrote:

> Hi Bernhard,
>
>>>
>>> From what I remember of the Telit modems, all ports are fully
>>> functional, so the GPS port can be on any tty.  If this is the case,
>>> then the way this can be accomplished is by querying the model /
>>> revision of the modem during the enable() stage in the modem driver and
>>> adding the appropriate atom as needed.
>>>
>>
>> The GPS port is not fully functional as far is I know it is read only.
>> According to the manual it is necessary to enable over the modem line
>> with AT$GPSP=1 afterwards it starts reporting NMEA signals on the GPS line.
>>
>
> In theory you can use $GPSNMUN to enable GPS reporting on Ports 1 and 4
> which seem to be fully functional.  However, it does look like on my
> UC864-G, port #3 is used for the GPS stream.  Port #1 doesn't seem to
> respond to anything.
>

I just tried the following at my modem:

AT$GPSNMUN=1,1,1,1,1,1,1
AT$GPSP=1

Which enables the GPS reporting on Ports 1 and 4 and adds $GPSNMUN: at the 
beginning of every GPS report e.g.

$GPSNMUN: $GPVTG,,T,,M,,N,,K*4E

Which would be easy to catch from ofono.

In my opinion the problem about this is that the GPS atom needs to be 
loaded for every telit modem, then try AT$GPSNMUN=1,1,1,1,1,1,1 and wait 
for the response. If the response is an error the atom knows that the 
modem does not support GPS. Could this check/enable also be added to a 
part of the telit plugin?

The udev approach has the benifet to be able to do

if (HasGPS)
 	... = ofono_location_reporting_create(...);

or something like that at the telit plugin.

>
> Yes and no.  ofono_modem_create has two modes of operation, one where a
> known unique id is given.  This can come for example from the serial
> number of the USB device.  Some manufacturers provide this information
> properly, others do not.  If you cannot trust the USB serial ID, then
> giving a NULL parameter to ofono_modem_create will tell oFono to
> generate a unique id itself.  Your path will then look like /telit2 or
> something like that.

Ah, ok then the patch is fine I think and I like to get it reviewed :)

But I can alter the patch to only search for the modem device and add the 
GPS check later to the telit plugin or the GPS atom, if you like.

Best regards,
Bernhard


--
Scanned by MailScanner.

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Denis Kenzior]

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

Hi Bernhard,

>> In theory you can use $GPSNMUN to enable GPS reporting on Ports 1 and 4
>> which seem to be fully functional.  However, it does look like on my
>> UC864-G, port #3 is used for the GPS stream.  Port #1 doesn't seem to
>> respond to anything.
>>
> 
> I just tried the following at my modem:
> 
> AT$GPSNMUN=1,1,1,1,1,1,1
> AT$GPSP=1
> 
> Which enables the GPS reporting on Ports 1 and 4 and adds $GPSNMUN: at
> the beginning of every GPS report e.g.
> 
> $GPSNMUN: $GPVTG,,T,,M,,N,,K*4E
> 
> Which would be easy to catch from ofono.
> 
> In my opinion the problem about this is that the GPS atom needs to be
> loaded for every telit modem, then try AT$GPSNMUN=1,1,1,1,1,1,1 and wait
> for the response. If the response is an error the atom knows that the
> modem does not support GPS. Could this check/enable also be added to a
> part of the telit plugin?
> 
> The udev approach has the benifet to be able to do
> 
> if (HasGPS)
>     ... = ofono_location_reporting_create(...);
> 
> or something like that at the telit plugin.
> 

I agree that your proposed approach is the right one.

>>
>> Yes and no.  ofono_modem_create has two modes of operation, one where a
>> known unique id is given.  This can come for example from the serial
>> number of the USB device.  Some manufacturers provide this information
>> properly, others do not.  If you cannot trust the USB serial ID, then
>> giving a NULL parameter to ofono_modem_create will tell oFono to
>> generate a unique id itself.  Your path will then look like /telit2 or
>> something like that.
> 
> Ah, ok then the patch is fine I think and I like to get it reviewed :)
> 
> But I can alter the patch to only search for the modem device and add
> the GPS check later to the telit plugin or the GPS atom, if you like.
> 

Patch has been reviewed, however we might have to examine whether we
need to tweak the udev rules for this serial number / modem path stuff
to work properly.  Marcel will have to take a look as I'm lost in that
part of the udev plugin.

Regards,
-Denis

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Bernhard Guillon]

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



On Mon, 13 Jun 2011, Denis Kenzior wrote:

> Patch has been reviewed, however we might have to examine whether we
> need to tweak the udev rules for this serial number / modem path stuff
> to work properly.  Marcel will have to take a look as I'm lost in that
> part of the udev plugin.

Thanks,
I do not have access to the modem for two weeks from now but 
afterwards I will work on it again :)

Best regards,
Bernhard


--
Scanned by MailScanner.

Re: [PATCH v4 4/4] udev: add Telit UC864-G and update udev rules

[Denis Kenzior]

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

Hi Bernhard,

On 06/08/2011 10:18 AM, Bernhard.Guillon(a)hale.at wrote:
> From: Bernhard Guillon <Bernhard.Guillon@hale.at>
> 
> *add Modem and GPS check for telit
> *make GPS yes/no configurable with udev rules settings
> **this is necessary because telit has different interface
>   numbers on different modems e.g. GPS and UART
> ---
>  plugins/ofono.rules |    8 +++++
>  plugins/udev.c      |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/ofono.rules b/plugins/ofono.rules
> index 268b327..64fa4b2 100644
> --- a/plugins/ofono.rules
> +++ b/plugins/ofono.rules
> @@ -344,6 +344,11 @@ ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1485", ENV{OFONO_IFACE_NUM}=="02", E
>  ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_HUAWEI_TYPE}="Modem"
>  ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_HUAWEI_TYPE}="Pcui"
>  
> +#Telit UC864-G
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_TELIT_GPS}="1"
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_TELIT_TYPE}="Modem"
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_TELIT_TYPE}="GPS"
> +

What I would do here is something like:
ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004",
ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_TELIT_TYPE}="Modem"

ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004",
ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_TELIT_TYPE}="GPS"

ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004",
ENV{OFONO_IFACE_NUM}=="03", ENV{OFONO_TELIT_TYPE}="Data"

>  LABEL="ofono_tty_end"
>  
>  # ISI/Phonet drivers
> @@ -459,4 +464,7 @@ ATTRS{idVendor}=="0421", ATTRS{idProduct}=="00b6", ENV{OFONO_DRIVER}="nokiacdma"
>  # Teracom (Linktop/LW27x) 3G Data Card
>  ATTRS{idVendor}=="230d", ATTRS{idProduct}=="0001", ENV{OFONO_DRIVER}="linktop"
>  
> +# Telit
> +ATTRS{idVendor}=="1bc7", ENV{OFONO_DRIVER}="telit"
> +
>  LABEL="ofono_end"
> diff --git a/plugins/udev.c b/plugins/udev.c
> index 0234fc0..e56d3db 100644
> --- a/plugins/udev.c
> +++ b/plugins/udev.c
> @@ -587,6 +587,76 @@ static void add_linktop(struct ofono_modem *modem,
>  	}
>  }
>  
> +static void add_telit(struct ofono_modem *modem,
> +					struct udev_device *udev_device)
> +{
> +	struct udev_list_entry *entry;
> +	const char *devnode, *type;
> +	int device, gps_device, has_gps;
> +
> +	DBG("modem %p", modem);
> +
> +	/*
> +	 * Some Telit modems have a built in GPS device. According to the
> +	 * manual the the interface numbers can differ between different
> +	 * devices hence we need to use udev rules to set the environment
> +	 * accordingly.
> +	 */
> +	device = ofono_modem_get_integer(modem, "ModemRegistered");
> +	gps_device = ofono_modem_get_integer(modem, "GPSRegistered");
> +	has_gps = ofono_modem_get_integer(modem, "HasGPS");
> +
> +	if ((!has_gps && device) || (device && gps_device))
> +		return;

You can then skip the HasGPS variable.  I would then model the detection
logic after add mbm. e.g. something like:

registered = ofono_modem_get_integer(modem, "Registered");

> +
> +	entry = udev_device_get_properties_list_entry(udev_device);
> +	while (entry) {
> +		const char *name = udev_list_entry_get_name(entry);
> +		type = udev_list_entry_get_value(entry);
> +
> +		if (g_str_equal(name, "OFONO_TELIT_GPS") == TRUE) {
> +			int value = g_str_equal(type, "1");
> +
> +			ofono_modem_set_integer(modem, "HasGPS", value);
> +			entry = udev_list_entry_get_next(entry);
> +			continue;
> +		}

drop this if statement

> +
> +		if (g_str_equal(name, "OFONO_TELIT_TYPE") != TRUE) {
> +			entry = udev_list_entry_get_next(entry);
> +			continue;
> +		}
> +
> +		if (g_str_equal(type, "Modem") == TRUE) {

if (registered == 0 && g_str_equal(type, "Modem") == TRUE) {
	...

> +

We prefer nested if statements like this to be written like:

if (expression) {
	if ()
		...
}

e.g. no spaces before the nested if.

> +			if (device != 0)
> +				return;
> +
> +			devnode = udev_device_get_devnode(udev_device);
> +			ofono_modem_set_string(modem, "Modem", devnode);
> +			device = 1;
> +			ofono_modem_set_integer(modem, "ModemRegistered",
> +							device);

Drop the device=1 and ofono_modem_set_integer calls..

> +		} else if (g_str_equal(type, "GPS") == TRUE) {
> +
> +			if (gps_device != 0)
> +				return;
> +
> +			devnode = udev_device_get_devnode(udev_device);
> +			ofono_modem_set_string(modem, "GPS", devnode);
> +
> +			gps_device = 1;
> +			ofono_modem_set_integer(modem, "GPSRegistered",
> +							gps_device);
> +		}

Same comments as above for this statement

Then add another if statement detecting the Data port.

> +
> +		break;
> +	}
> +
> +	if ((!has_gps && device) || (device && gps_device))
> +		ofono_modem_register(modem);

Then here do something like:

if (registered == 1)
	return;

if (ofono_modem_get_string(modem, "Modem") != NULL &&
		ofono_modem_get_string(modem, "Data") != NULL) {
	ofono_modem_set_integer(modem, "Registered", 1");
	ofono_modem_register(modem);
}

> +}
> +
>  static void add_modem(struct udev_device *udev_device)
>  {
>  	struct ofono_modem *modem;
> @@ -681,6 +751,8 @@ done:
>  		add_calypso(modem, udev_device);
>  	else if (g_strcmp0(driver, "tc65") == 0)
>  		add_tc65(modem, udev_device);
> +	else if (g_strcmp0(driver, "telit") == 0)
> +		add_telit(modem, udev_device);
>  	else if (g_strcmp0(driver, "nokiacdma") == 0)
>  		add_nokiacdma(modem, udev_device);
>          else if (g_strcmp0(driver, "linktop") == 0)

Regards,
-Denis