[PATCH 0/10] Unregister AT notifiers when removing drivers

[PATCH 0/10] Unregister AT notifiers when removing drivers

[Zhenhua Zhang]

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

Hi,

This series unregister AT notifiers in removing various AT modem drivers when modem goes to offline mode. Please review them.

We should fix other modem drivers other than atmodem as well. I will send seperate patches for review.

To Emmanuel, could you try these patches to see wheter it fixes your problems? Thanks.

Regards,
Zhenhua

[PATCH 01/10] gatchat: Check notify id in g_at_chat_unregister

[Zhenhua Zhang]

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

Return earlier if notify id is zero.
---
 gatchat/gatchat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatchat.c b/gatchat/gatchat.c
index f192a90..a177e10 100644
--- a/gatchat/gatchat.c
+++ b/gatchat/gatchat.c
@@ -1169,7 +1169,7 @@ gboolean g_at_chat_unregister(GAtChat *chat, guint id)
 	gpointer key, value;
 	GSList *l;
 
-	if (chat == NULL || chat->notify_list == NULL)
+	if (chat == NULL || chat->notify_list == NULL || id == 0)
 		return FALSE;
 
 	g_hash_table_iter_init(&iter, chat->notify_list);
-- 
1.7.0.4

[PATCH 02/10] gprs: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifiers when removing gprs driver.
---
 drivers/atmodem/gprs.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index bf82d06..dcf9759 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -48,6 +48,8 @@ static const char *none_prefix[] = { NULL };
 struct gprs_data {
 	GAtChat *chat;
 	unsigned int vendor;
+	guint cgev_id;
+	guint cgreg_id;
 };
 
 static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -181,9 +183,10 @@ static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_gprs *gprs = user_data;
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 
-	g_at_chat_register(gd->chat, "+CGEV:", cgev_notify, FALSE, gprs, NULL);
-	g_at_chat_register(gd->chat, "+CGREG:", cgreg_notify,
-				FALSE, gprs, NULL);
+	gd->cgev_id = g_at_chat_register(gd->chat, "+CGEV:", cgev_notify,
+						FALSE, gprs, NULL);
+	gd->cgreg_id = g_at_chat_register(gd->chat, "+CGREG:", cgreg_notify,
+						FALSE, gprs, NULL);
 
 	ofono_gprs_register(gprs);
 }
@@ -321,6 +324,9 @@ static void at_gprs_remove(struct ofono_gprs *gprs)
 {
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 
+	g_at_chat_unregister(gd->chat, gd->cgev_id);
+	g_at_chat_unregister(gd->chat, gd->cgreg_id);
+
 	ofono_gprs_set_data(gprs, NULL);
 	g_free(gd);
 }
-- 
1.7.0.4

[PATCH 03/10] netreg: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifications when removing network registeration driver.
---
 drivers/atmodem/network-registration.c |   49 ++++++++++++++++++++-----------
 1 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index b8ec012..8b52bbe 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -57,6 +57,9 @@ struct netreg_data {
 	int signal_max; /* max strength reported via CIND */
 	int tech;
 	unsigned int vendor;
+	guint ciev_id;
+	guint creg_id;
+	guint vendor_id;
 };
 
 struct tech_query {
@@ -945,10 +948,10 @@ static void cind_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	g_at_chat_send(nd->chat, "AT+CMER=3,0,0,1", NULL,
 			NULL, NULL, NULL);
-	g_at_chat_register(nd->chat, "+CIEV:",
-				ciev_notify, FALSE, netreg, NULL);
-	g_at_chat_register(nd->chat, "+CREG:",
-				creg_notify, FALSE, netreg, NULL);
+	nd->ciev_id = g_at_chat_register(nd->chat, "+CIEV:", ciev_notify,
+						FALSE, netreg, NULL);
+	nd->creg_id = g_at_chat_register(nd->chat, "+CREG:", creg_notify,
+						FALSE, netreg, NULL);
 
 	ofono_netreg_register(netreg);
 	return;
@@ -975,22 +978,25 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	switch (nd->vendor) {
 	case OFONO_VENDOR_PHONESIM:
-		g_at_chat_register(nd->chat, "+CSQ:",
-					csq_notify, FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "+CSQ:",
+							csq_notify,
+							FALSE, netreg, NULL);
 		break;
 	case OFONO_VENDOR_CALYPSO:
 		g_at_chat_send(nd->chat, "AT%CSQ=1", none_prefix,
 				NULL, NULL, NULL);
-		g_at_chat_register(nd->chat, "%CSQ:", calypso_csq_notify,
-					FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "%CSQ:",
+							calypso_csq_notify,
+							FALSE, netreg, NULL);
 		break;
 	case OFONO_VENDOR_OPTION_HSO:
 		g_at_chat_send(nd->chat, "AT_OSSYS=1", none_prefix,
 				NULL, NULL, NULL);
 		g_at_chat_send(nd->chat, "AT_OSQI=1", none_prefix,
 				NULL, NULL, NULL);
-		g_at_chat_register(nd->chat, "_OSIGQ:", option_osigq_notify,
-					FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "_OSIGQ:",
+							option_osigq_notify,
+							FALSE, netreg, NULL);
 
 		g_at_chat_send(nd->chat, "AT_OSSYS?", none_prefix,
 				NULL, NULL, NULL);
@@ -1000,8 +1006,9 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	case OFONO_VENDOR_MBM:
 		g_at_chat_send(nd->chat, "AT*ERINFO=1", none_prefix,
 				NULL, NULL, NULL);
-		g_at_chat_register(nd->chat, "*ERINFO:", mbm_erinfo_notify,
-					FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "*ERINFO:",
+							mbm_erinfo_notify,
+							FALSE, netreg, NULL);
 		g_at_chat_send(nd->chat, "AT+CIND=?", cind_prefix,
 				cind_support_cb, netreg, NULL);
 		return;
@@ -1011,12 +1018,14 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		 * of technology changes, but register a handle for
 		 * CNTI so we get notified by any query.
 		 */
-		g_at_chat_register(nd->chat, "$CNTI:", nw_cnti_notify,
-					FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "$CNTI:",
+							nw_cnti_notify,
+							FALSE, netreg, NULL);
 		break;
 	case OFONO_VENDOR_HUAWEI:
-		g_at_chat_register(nd->chat, "^RSSI:", huawei_rssi_notify,
-					FALSE, netreg, NULL);
+		nd->vendor_id = g_at_chat_register(nd->chat, "^RSSI:",
+							huawei_rssi_notify,
+							FALSE, netreg, NULL);
 		break;
 	default:
 		g_at_chat_send(nd->chat, "AT+CIND=?", cind_prefix,
@@ -1024,8 +1033,8 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	g_at_chat_register(nd->chat, "+CREG:",
-				creg_notify, FALSE, netreg, NULL);
+	nd->creg_id = g_at_chat_register(nd->chat, "+CREG:", creg_notify,
+						FALSE, netreg, NULL);
 	ofono_netreg_register(netreg);
 }
 
@@ -1098,6 +1107,10 @@ static void at_netreg_remove(struct ofono_netreg *netreg)
 {
 	struct netreg_data *nd = ofono_netreg_get_data(netreg);
 
+	g_at_chat_unregister(nd->chat, nd->ciev_id);
+	g_at_chat_unregister(nd->chat, nd->creg_id);
+	g_at_chat_unregister(nd->chat, nd->vendor_id);
+
 	ofono_netreg_set_data(netreg, NULL);
 
 	g_free(nd);
-- 
1.7.0.4

[PATCH 04/10] cbs: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifications when removing cbs driver.
---
 drivers/atmodem/cbs.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/cbs.c b/drivers/atmodem/cbs.c
index a1c4037..ef312e9 100644
--- a/drivers/atmodem/cbs.c
+++ b/drivers/atmodem/cbs.c
@@ -47,6 +47,7 @@ struct cbs_data {
 	GAtChat *chat;
 	gboolean cscb_mode_1;
 	unsigned int vendor;
+	guint cbm_id;
 };
 
 static void at_cbm_notify(GAtResult *result, gpointer user_data)
@@ -182,7 +183,8 @@ static void at_cbs_register(gboolean ok, GAtResult *result, gpointer user)
 	 * The default SMS driver will setup the CNMI for +CBM delivery
 	 * appropriately for us
 	 */
-	g_at_chat_register(data->chat, "+CBM:", at_cbm_notify, TRUE, cbs, NULL);
+	data->cbm_id = g_at_chat_register(data->chat, "+CBM:",
+						at_cbm_notify, TRUE, cbs, NULL);
 
 	ofono_cbs_register(cbs);
 }
@@ -252,6 +254,8 @@ static void at_cbs_remove(struct ofono_cbs *cbs)
 {
 	struct cbs_data *data = ofono_cbs_get_data(cbs);
 
+	g_at_chat_unregister(data->chat, data->cbm_id);
+
 	ofono_cbs_set_data(cbs, NULL);
 
 	g_free(data);
-- 
1.7.0.4

[PATCH 05/10] ussd: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Add struct ussd_data to hold gatchat and cusd_id. So that we could
unregister AT notifications in driver removing phase.
---
 drivers/atmodem/ussd.c |   33 ++++++++++++++++++++++++++-------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index 555ce13..0def9b1 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -47,6 +47,11 @@ struct cusd_req {
 	struct ofono_ussd *ussd;
 };
 
+struct ussd_data {
+	GAtChat *chat;
+	guint cusd_id;
+};
+
 static const char *cusd_prefix[] = { "+CUSD:", NULL };
 static const char *none_prefix[] = { NULL };
 
@@ -119,7 +124,7 @@ static void cusd_request_cb(gboolean ok, GAtResult *result, gpointer user_data)
 static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 				ofono_ussd_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_ussd_get_data(ussd);
+	struct ussd_data *ud = ofono_ussd_get_data(ussd);
 	struct cusd_req *cbd = g_try_new0(struct cusd_req, 1);
 	unsigned char *converted = NULL;
 	int dcs;
@@ -152,7 +157,7 @@ static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 	g_free(converted);
 	converted = NULL;
 
-	if (g_at_chat_send(chat, buf, cusd_prefix,
+	if (g_at_chat_send(ud->chat, buf, cusd_prefix,
 				cusd_request_cb, cbd, g_free) > 0)
 		return;
 
@@ -180,13 +185,13 @@ static void cusd_cancel_cb(gboolean ok, GAtResult *result, gpointer user_data)
 static void at_ussd_cancel(struct ofono_ussd *ussd,
 				ofono_ussd_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_ussd_get_data(ussd);
+	struct ussd_data *ud = ofono_ussd_get_data(ussd);
 	struct cb_data *cbd = cb_data_new(cb, data);
 
 	if (!cbd)
 		goto error;
 
-	if (g_at_chat_send(chat, "AT+CUSD=2", none_prefix,
+	if (g_at_chat_send(ud->chat, "AT+CUSD=2", none_prefix,
 				cusd_cancel_cb, cbd, g_free) > 0)
 		return;
 
@@ -207,14 +212,16 @@ static void cusd_notify(GAtResult *result, gpointer user_data)
 static void at_ussd_register(gboolean ok, GAtResult *result, gpointer user)
 {
 	struct ofono_ussd *ussd = user;
-	GAtChat *chat = ofono_ussd_get_data(ussd);
+	struct ussd_data *data = ofono_ussd_get_data(ussd);
 
 	if (!ok) {
 		ofono_error("Could not enable CUSD notifications");
 		return;
 	}
 
-	g_at_chat_register(chat, "+CUSD:", cusd_notify, FALSE, ussd, NULL);
+	data->cusd_id = g_at_chat_register(data->chat, "+CUSD:",
+						cusd_notify, FALSE,
+						ussd, NULL);
 
 	ofono_ussd_register(ussd);
 }
@@ -223,8 +230,13 @@ static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
 				void *data)
 {
 	GAtChat *chat = data;
+	struct ussd_data *ud;
+
+	ud = g_new0(struct ussd_data, 1);
+
+	ud->chat = chat;
 
-	ofono_ussd_set_data(ussd, chat);
+	ofono_ussd_set_data(ussd, ud);
 
 	g_at_chat_send(chat, "AT+CUSD=1", NULL, at_ussd_register, ussd, NULL);
 
@@ -233,6 +245,13 @@ static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
 
 static void at_ussd_remove(struct ofono_ussd *ussd)
 {
+	struct ussd_data *ud = ofono_ussd_get_data(ussd);
+
+	g_at_chat_unregister(ud->chat, ud->cusd_id);
+
+	ofono_ussd_set_data(ussd, NULL);
+
+	g_free(ud);
 }
 
 static struct ofono_ussd_driver driver = {
-- 
1.7.0.4

[PATCH 06/10] ssn: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifications when removing ssn driver.
---
 drivers/atmodem/ssn.c |   30 ++++++++++++++++++++++++++----
 1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/ssn.c b/drivers/atmodem/ssn.c
index f219cde..ce8f1e0 100644
--- a/drivers/atmodem/ssn.c
+++ b/drivers/atmodem/ssn.c
@@ -39,6 +39,12 @@
 
 static const char *none_prefix[] = { NULL };
 
+struct ssn_data {
+	GAtChat *chat;
+	guint cssi_id;
+	guint cssu_id;
+};
+
 static void cssi_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_ssn *ssn = user_data;
@@ -100,10 +106,12 @@ static void at_ssn_initialized(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
 	struct ofono_ssn *ssn = user_data;
-	GAtChat *chat = ofono_ssn_get_data(ssn);
+	struct ssn_data *sd = ofono_ssn_get_data(ssn);
 
-	g_at_chat_register(chat, "+CSSI:", cssi_notify, FALSE, ssn, NULL);
-	g_at_chat_register(chat, "+CSSU:", cssu_notify, FALSE, ssn, NULL);
+	sd->cssi_id = g_at_chat_register(sd->chat, "+CSSI:", cssi_notify,
+						FALSE, ssn, NULL);
+	sd->cssu_id = g_at_chat_register(sd->chat, "+CSSU:", cssu_notify,
+						FALSE, ssn, NULL);
 
 	ofono_ssn_register(ssn);
 }
@@ -112,8 +120,14 @@ static int at_ssn_probe(struct ofono_ssn *ssn, unsigned int vendor,
 				void *data)
 {
 	GAtChat *chat = data;
+	struct ssn_data *sd;
+
+	sd = g_new0(struct ssn_data, 1);
+
+	sd->chat = chat;
+
+	ofono_ssn_set_data(ssn, sd);
 
-	ofono_ssn_set_data(ssn, chat);
 	g_at_chat_send(chat, "AT+CSSN=1,1", none_prefix,
 			at_ssn_initialized, ssn, NULL);
 
@@ -122,6 +136,14 @@ static int at_ssn_probe(struct ofono_ssn *ssn, unsigned int vendor,
 
 static void at_ssn_remove(struct ofono_ssn *ssn)
 {
+	struct ssn_data *sd = ofono_ssn_get_data(ssn);
+
+	g_at_chat_unregister(sd->chat, sd->cssi_id);
+	g_at_chat_unregister(sd->chat, sd->cssu_id);
+
+	ofono_ssn_set_data(ssn, NULL);
+
+	g_free(sd);
 }
 
 static struct ofono_ssn_driver driver = {
-- 
1.7.0.4

[PATCH 07/10] stk: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifications when removing stk driver.
---
 drivers/atmodem/stk.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index d6c19da..de687f8 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -44,6 +44,8 @@
 struct stk_data {
 	GAtChat *chat;
 	unsigned int vendor;
+	guint tcmd_id;
+	guint tend_id;
 };
 
 static const char *csim_prefix[] = { "+CSIM:", NULL };
@@ -294,10 +296,12 @@ static gboolean at_stk_register(gpointer user)
 	struct stk_data *sd = ofono_stk_get_data(stk);
 
 	if (sd->vendor == OFONO_VENDOR_PHONESIM) {
-		g_at_chat_register(sd->chat, "*TCMD:", phonesim_tcmd_notify,
+		sd->tcmd_id = g_at_chat_register(sd->chat, "*TCMD:",
+							phonesim_tcmd_notify,
 							FALSE, stk, NULL);
 
-		g_at_chat_register(sd->chat, "*TEND", phonesim_tend_notify,
+		sd->tend_id = g_at_chat_register(sd->chat, "*TEND",
+							phonesim_tend_notify,
 							FALSE, stk, NULL);
 	}
 
@@ -325,6 +329,9 @@ static void at_stk_remove(struct ofono_stk *stk)
 {
 	struct stk_data *sd = ofono_stk_get_data(stk);
 
+	g_at_chat_unregister(sd->chat, sd->tcmd_id);
+	g_at_chat_unregister(sd->chat, sd->tend_id);
+
 	ofono_stk_set_data(stk, NULL);
 
 	g_free(sd);
-- 
1.7.0.4

[PATCH 08/10] sms: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifiers when removing sms driver.
---
 drivers/atmodem/sms.c |   31 +++++++++++++++++++++----------
 1 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c
index ba98c12..335f456 100644
--- a/drivers/atmodem/sms.c
+++ b/drivers/atmodem/sms.c
@@ -78,6 +78,11 @@ struct sms_data {
 	guint timeout_source;
 	GAtChat *chat;
 	unsigned int vendor;
+	guint cmti_id;
+	guint cmt_id;
+	guint cds_id;
+	guint cdsi_id;
+	guint cmgr_id;
 };
 
 struct cpms_request {
@@ -599,18 +604,18 @@ static void at_cmgl_done(struct ofono_sms *sms)
 		return;
 	}
 
-	g_at_chat_register(data->chat, "+CMTI:", at_cmti_notify, FALSE,
-				sms, NULL);
-	g_at_chat_register(data->chat, "+CMT:", at_cmt_notify, TRUE,
-				sms, NULL);
-	g_at_chat_register(data->chat, "+CDS:", at_cds_notify, TRUE,
-				sms, NULL);
-	g_at_chat_register(data->chat, "+CDSI:", at_cdsi_notify, FALSE,
-				sms, NULL);
+	data->cmti_id = g_at_chat_register(data->chat, "+CMTI:", at_cmti_notify,
+						FALSE, sms, NULL);
+	data->cmt_id = g_at_chat_register(data->chat, "+CMT:", at_cmt_notify,
+						TRUE, sms, NULL);
+	data->cds_id = g_at_chat_register(data->chat, "+CDS:", at_cds_notify,
+						TRUE, sms, NULL);
+	data->cdsi_id = g_at_chat_register(data->chat, "+CDSI:", at_cdsi_notify,
+						FALSE, sms, NULL);
 
 	/* We treat CMGR just like a notification */
-	g_at_chat_register(data->chat, "+CMGR:", at_cmgr_notify, TRUE,
-				sms, NULL);
+	data->cmgr_id = g_at_chat_register(data->chat, "+CMGR:", at_cmgr_notify,
+						TRUE, sms, NULL);
 }
 
 static void at_cmgl_notify(GAtResult *result, gpointer user_data)
@@ -1233,6 +1238,12 @@ static void at_sms_remove(struct ofono_sms *sms)
 	if (data->timeout_source > 0)
 		g_source_remove(data->timeout_source);
 
+	g_at_chat_unregister(data->chat, data->cmti_id);
+	g_at_chat_unregister(data->chat, data->cmt_id);
+	g_at_chat_unregister(data->chat, data->cds_id);
+	g_at_chat_unregister(data->chat, data->cdsi_id);
+	g_at_chat_unregister(data->chat, data->cmgr_id);
+
 	g_free(data);
 }
 
-- 
1.7.0.4

[PATCH 09/10] call-meter: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifiers when removing call-meter at modem driver.
---
 drivers/atmodem/call-meter.c |   55 ++++++++++++++++++++++++++++-------------
 1 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/atmodem/call-meter.c b/drivers/atmodem/call-meter.c
index 38774d4..f36ff3c 100644
--- a/drivers/atmodem/call-meter.c
+++ b/drivers/atmodem/call-meter.c
@@ -45,6 +45,12 @@ static const char *cacm_prefix[] = { "+CACM:", NULL };
 static const char *camm_prefix[] = { "+CAMM:", NULL };
 static const char *cpuc_prefix[] = { "+CPUC:", NULL };
 
+struct cm_data {
+	GAtChat *chat;
+	guint cccm_id;
+	guint ccwv_id;
+};
+
 static void caoc_cacm_camm_query_cb(gboolean ok,
 		GAtResult *result, gpointer user_data)
 {
@@ -113,14 +119,14 @@ static void at_caoc_query(struct ofono_call_meter *cm,
 				ofono_call_meter_query_cb_t cb,
 				void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 
 	if (!cbd)
 		goto error;
 
 	cbd->user = "+CAOC:";
-	if (g_at_chat_send(chat, "AT+CAOC=0", caoc_prefix,
+	if (g_at_chat_send(cd->chat, "AT+CAOC=0", caoc_prefix,
 				caoc_cacm_camm_query_cb, cbd, g_free) > 0)
 		return;
 
@@ -135,14 +141,14 @@ static void at_cacm_query(struct ofono_call_meter *cm,
 				ofono_call_meter_query_cb_t cb,
 				void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 
 	if (!cbd)
 		goto error;
 
 	cbd->user = "+CACM:";
-	if (g_at_chat_send(chat, "AT+CACM?", cacm_prefix,
+	if (g_at_chat_send(cd->chat, "AT+CACM?", cacm_prefix,
 				caoc_cacm_camm_query_cb, cbd, g_free) > 0)
 		return;
 
@@ -167,7 +173,7 @@ static void generic_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 static void at_cacm_set(struct ofono_call_meter *cm, const char *passwd,
 			ofono_call_meter_set_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 
@@ -176,7 +182,7 @@ static void at_cacm_set(struct ofono_call_meter *cm, const char *passwd,
 
 	snprintf(buf, sizeof(buf), "AT+CACM=\"%s\"", passwd);
 
-	if (g_at_chat_send(chat, buf, none_prefix,
+	if (g_at_chat_send(cd->chat, buf, none_prefix,
 				generic_set_cb, cbd, g_free) > 0)
 		return;
 
@@ -191,14 +197,14 @@ static void at_camm_query(struct ofono_call_meter *cm,
 				ofono_call_meter_query_cb_t cb,
 				void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 
 	if (!cbd)
 		goto error;
 
 	cbd->user = "+CAMM:";
-	if (g_at_chat_send(chat, "AT+CAMM?", camm_prefix,
+	if (g_at_chat_send(cd->chat, "AT+CAMM?", camm_prefix,
 				caoc_cacm_camm_query_cb, cbd, g_free) > 0)
 		return;
 
@@ -213,7 +219,7 @@ static void at_camm_set(struct ofono_call_meter *cm,
 			int accmax, const char *passwd,
 			ofono_call_meter_set_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 
@@ -222,7 +228,7 @@ static void at_camm_set(struct ofono_call_meter *cm,
 
 	snprintf(buf, sizeof(buf), "AT+CAMM=\"%06X\",\"%s\"", accmax, passwd);
 
-	if (g_at_chat_send(chat, buf, none_prefix,
+	if (g_at_chat_send(cd->chat, buf, none_prefix,
 				generic_set_cb, cbd, g_free) > 0)
 		return;
 
@@ -276,14 +282,14 @@ error:
 static void at_cpuc_query(struct ofono_call_meter *cm,
 				ofono_call_meter_puct_query_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 
 	if (!cbd)
 		goto error;
 
 	cbd->user = "+CPUC:";
-	if (g_at_chat_send(chat, "AT+CPUC?", cpuc_prefix,
+	if (g_at_chat_send(cd->chat, "AT+CPUC?", cpuc_prefix,
 				cpuc_query_cb, cbd, g_free) > 0)
 		return;
 
@@ -298,7 +304,7 @@ static void at_cpuc_set(struct ofono_call_meter *cm, const char *currency,
 			double ppu, const char *passwd,
 			ofono_call_meter_set_cb_t cb, void *data)
 {
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 
@@ -308,7 +314,7 @@ static void at_cpuc_set(struct ofono_call_meter *cm, const char *currency,
 	snprintf(buf, sizeof(buf), "AT+CPUC=\"%s\",\"%f\",\"%s\"",
 			currency, ppu, passwd);
 
-	if (g_at_chat_send(chat, buf, none_prefix,
+	if (g_at_chat_send(cd->chat, buf, none_prefix,
 				generic_set_cb, cbd, g_free) > 0)
 		return;
 
@@ -335,10 +341,12 @@ static void at_call_meter_initialized(gboolean ok, GAtResult *result,
 					gpointer user_data)
 {
 	struct ofono_call_meter *cm = user_data;
-	GAtChat *chat = ofono_call_meter_get_data(cm);
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
 
-	g_at_chat_register(chat, "+CCCM:", cccm_notify, FALSE, cm, NULL);
-	g_at_chat_register(chat, "+CCWV", ccwv_notify, FALSE, cm, NULL);
+	cd->cccm_id = g_at_chat_register(cd->chat, "+CCCM:", cccm_notify, FALSE,
+						cm, NULL);
+	cd->ccwv_id = g_at_chat_register(cd->chat, "+CCWV", ccwv_notify, FALSE,
+						cm, NULL);
 
 	ofono_call_meter_register(cm);
 }
@@ -347,8 +355,13 @@ static int at_caoc_probe(struct ofono_call_meter *cm, unsigned int vendor,
 				void *data)
 {
 	GAtChat *chat = data;
+	struct cm_data *cd;
+
+	cd = g_new0(struct cm_data, 1);
 
-	ofono_call_meter_set_data(cm, chat);
+	cd->chat = chat;
+
+	ofono_call_meter_set_data(cm, cd);
 
 	g_at_chat_send(chat, "AT+CAOC=2", NULL, NULL, NULL, NULL);
 	g_at_chat_send(chat, "AT+CCWE=1", NULL,
@@ -359,6 +372,12 @@ static int at_caoc_probe(struct ofono_call_meter *cm, unsigned int vendor,
 
 static void at_caoc_remove(struct ofono_call_meter *cm)
 {
+	struct cm_data *cd = ofono_call_meter_get_data(cm);
+
+	g_at_chat_unregister(cd->chat, cd->cccm_id);
+	g_at_chat_unregister(cd->chat, cd->ccwv_id);
+
+	g_free(cd);
 }
 
 static struct ofono_call_meter_driver driver = {
-- 
1.7.0.4

[PATCH 10/10] voicecall: Unregister AT notifiers when removing

[Zhenhua Zhang]

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

Unregister AT notifiers when removing voicecall driver.
---
 drivers/atmodem/voicecall.c |   40 +++++++++++++++++++++++++++++++---------
 1 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/atmodem/voicecall.c b/drivers/atmodem/voicecall.c
index fce9144..16b3069 100644
--- a/drivers/atmodem/voicecall.c
+++ b/drivers/atmodem/voicecall.c
@@ -56,6 +56,13 @@ struct voicecall_data {
 	unsigned int local_release;
 	unsigned int clcc_source;
 	GAtChat *chat;
+	guint ring_id;
+	guint cring_id;
+	guint clip_id;
+	guint ccwa_id;
+	guint no_carrier_id;
+	guint no_answer_id;
+	guint busy_id;
 };
 
 struct release_id_req {
@@ -814,19 +821,26 @@ static void at_voicecall_initialized(gboolean ok, GAtResult *result,
 
 	DBG("voicecall_init: registering to notifications");
 
-	g_at_chat_register(vd->chat, "RING", ring_notify, FALSE, vc, NULL);
-	g_at_chat_register(vd->chat, "+CRING:", cring_notify, FALSE, vc, NULL);
-	g_at_chat_register(vd->chat, "+CLIP:", clip_notify, FALSE, vc, NULL);
-	g_at_chat_register(vd->chat, "+CCWA:", ccwa_notify, FALSE, vc, NULL);
+	vd->ring_id = g_at_chat_register(vd->chat, "RING", ring_notify,
+						FALSE, vc, NULL);
+	vd->cring_id = g_at_chat_register(vd->chat, "+CRING:", cring_notify,
+						FALSE, vc, NULL);
+	vd->clip_id = g_at_chat_register(vd->chat, "+CLIP:", clip_notify,
+						FALSE, vc, NULL);
+	vd->ccwa_id = g_at_chat_register(vd->chat, "+CCWA:", ccwa_notify,
+						FALSE, vc, NULL);
 
 	/* Modems with 'better' call progress indicators should
 	 * probably not even bother registering to these
 	 */
-	g_at_chat_register(vd->chat, "NO CARRIER",
-				no_carrier_notify, FALSE, vc, NULL);
-	g_at_chat_register(vd->chat, "NO ANSWER",
-				no_answer_notify, FALSE, vc, NULL);
-	g_at_chat_register(vd->chat, "BUSY", busy_notify, FALSE, vc, NULL);
+	vd->no_carrier_id = g_at_chat_register(vd->chat, "NO CARRIER",
+						no_carrier_notify,
+						FALSE, vc, NULL);
+	vd->no_answer_id = g_at_chat_register(vd->chat, "NO ANSWER",
+						no_answer_notify,
+						FALSE, vc, NULL);
+	vd->busy_id = g_at_chat_register(vd->chat, "BUSY", busy_notify,
+						FALSE, vc, NULL);
 
 	ofono_voicecall_register(vc);
 
@@ -863,6 +877,14 @@ static void at_voicecall_remove(struct ofono_voicecall *vc)
 	g_slist_foreach(vd->calls, (GFunc) g_free, NULL);
 	g_slist_free(vd->calls);
 
+	g_at_chat_unregister(vd->chat, vd->ring_id);
+	g_at_chat_unregister(vd->chat, vd->cring_id);
+	g_at_chat_unregister(vd->chat, vd->clip_id);
+	g_at_chat_unregister(vd->chat, vd->ccwa_id);
+	g_at_chat_unregister(vd->chat, vd->no_carrier_id);
+	g_at_chat_unregister(vd->chat, vd->no_answer_id);
+	g_at_chat_unregister(vd->chat, vd->busy_id);
+
 	ofono_voicecall_set_data(vc, NULL);
 
 	g_free(vd);
-- 
1.7.0.4

Re: [PATCH 0/10] Unregister AT notifiers when removing drivers

[Denis Kenzior]

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

Hi Zhenhua,

On 08/10/2010 09:43 AM, Zhenhua Zhang wrote:
> Hi,
> 
> This series unregister AT notifiers in removing various AT modem drivers when modem goes to offline mode. Please review them.
> 
> We should fix other modem drivers other than atmodem as well. I will send seperate patches for review.
> 
> To Emmanuel, could you try these patches to see wheter it fixes your problems? Thanks.
> 
> Regards,
> Zhenhua
> 

Your patches are fine, however there is an alternative we'd like to
explore first that might turn out to be better.  It does require some
surgery to GAtChat...

Regards,
-Denis

Re: [PATCH 0/10] Unregister AT notifiers when removing drivers

[Marcel Holtmann]

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

Hi Zhenhua,

> This series unregister AT notifiers in removing various AT modem drivers when modem goes to offline mode. Please review them.

just a heads up here that Denis and I talked about this. And we are
going to fix this inside GAtChat in the background for you. Trying to
fix this single handed in every atom driver is wrong.

This approach creates too much of a maintenance burden. And additional
code complexity and memory footprint doing it this way. So we have to
tackle it in a central place. And that is GAtChat.

Regards

Marcel

RE: [PATCH 0/10] Unregister AT notifiers when removing drivers

[Zhang, Zhenhua]

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

Hi Marcel,

Marcel Holtmann wrote:
> Hi Zhenhua,
> 
>> This series unregister AT notifiers in removing various AT modem
>> drivers when modem goes to offline mode. Please review them. 
> 
> just a heads up here that Denis and I talked about this. And we are
> going to fix this inside GAtChat in the background for you. Trying to
> fix this single handed in every atom driver is wrong.
> 
> This approach creates too much of a maintenance burden. And additional
> code complexity and memory footprint doing it this way. So we have to
> tackle it in a central place. And that is GAtChat.

Thanks for update. Yes. The fix by Denis looks smart and make sense to
me. By using fa?ade pattern, we don't require user to track
register/unregister of AT notifers any more.

> Regards
> 
> Marcel
> 
> 
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

Regards,
Zhenhua