[PATCH 1/6] qmi: unify common request header setup

[PATCH 1/6] qmi: unify common request header setup

[Jonas Bonn]

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

The service and control requests differ slightly in their headers, but
this difference is minor enough that we can handle it directly in the
request submission routine.  This patch unifies the header setup for the
two request types.
---
 drivers/qmimodem/qmi.c | 68 +++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index b60e0c0b..436f96a0 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -701,10 +701,30 @@ static void wakeup_writer(struct qmi_device *device)
 				can_write_data, device, write_watch_destroy);
 }
 
-static void __request_submit(struct qmi_device *device,
-				struct qmi_request *req, uint16_t transaction)
+static void __request_submit(struct qmi_device *device, struct qmi_request *req)
 {
-	req->tid = transaction;
+	struct qmi_mux_hdr *mux;
+
+	mux = req->buf;
+
+	if (mux->service == QMI_SERVICE_CONTROL) {
+		struct qmi_control_hdr* hdr;
+
+		hdr = req->buf + QMI_MUX_HDR_SIZE;
+		hdr->type = 0x00;
+		hdr->transaction = device->next_control_tid++;
+		if (device->next_control_tid == 0)
+			device->next_control_tid = 1;
+		req->tid = hdr->transaction;
+	} else {
+		struct qmi_service_hdr* hdr;
+		hdr = req->buf + QMI_MUX_HDR_SIZE;
+		hdr->type = 0x00;
+		hdr->transaction = device->next_service_tid++;
+		if (device->next_service_tid < 256)
+			device->next_service_tid = 256;
+		req->tid = hdr->transaction;
+	}
 
 	g_queue_push_tail(device->req_queue, req);
 
@@ -967,6 +987,9 @@ struct qmi_device *qmi_device_new(int fd)
 	device->service_list = g_hash_table_new_full(g_direct_hash,
 					g_direct_equal, NULL, service_destroy);
 
+	device->next_control_tid = 1;
+	device->next_service_tid = 256;
+
 	return device;
 }
 
@@ -1264,14 +1287,9 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func,
 		return false;
 	}
 
-	if (device->next_control_tid < 1)
-		device->next_control_tid = 1;
-
-	hdr->type = 0x00;
-	hdr->transaction = device->next_control_tid++;
 	data->tid = hdr->transaction;
 
-	__request_submit(device, req, hdr->transaction);
+	__request_submit(device, req);
 
 	data->timeout = g_timeout_add_seconds(5, discover_reply, data);
 	__qmi_device_discovery_started(device, &data->super);
@@ -1296,13 +1314,7 @@ static void release_client(struct qmi_device *device,
 		return;
 	}
 
-	if (device->next_control_tid < 1)
-		device->next_control_tid = 1;
-
-	hdr->type = 0x00;
-	hdr->transaction = device->next_control_tid++;
-
-	__request_submit(device, req, hdr->transaction);
+	__request_submit(device, req);
 }
 
 static void shutdown_destroy(gpointer user_data)
@@ -1397,13 +1409,7 @@ bool qmi_device_sync(struct qmi_device *device,
 			NULL, 0,
 			qmi_device_sync_callback, func_data, (void **) &hdr);
 
-	if (device->next_control_tid < 1)
-		device->next_control_tid = 1;
-
-	hdr->type = 0x00;
-	hdr->transaction = device->next_control_tid++;
-
-	__request_submit(device, req, hdr->transaction);
+	__request_submit(device, req);
 
 	return true;
 }
@@ -2032,13 +2038,7 @@ static void service_create_discover(uint8_t count,
 		return;
 	}
 
-	if (device->next_control_tid < 1)
-		device->next_control_tid = 1;
-
-	hdr->type = 0x00;
-	hdr->transaction = device->next_control_tid++;
-
-	__request_submit(device, req, hdr->transaction);
+	__request_submit(device, req);
 }
 
 static bool service_create(struct qmi_device *device, bool shared,
@@ -2327,13 +2327,7 @@ uint16_t qmi_service_send(struct qmi_service *service,
 
 	qmi_param_free(param);
 
-	if (device->next_service_tid < 256)
-		device->next_service_tid = 256;
-
-	hdr->type = 0x00;
-	hdr->transaction = device->next_service_tid++;
-
-	__request_submit(device, req, hdr->transaction);
+	__request_submit(device, req);
 
 	return hdr->transaction;
 }
-- 
2.15.1

[PATCH 2/6] qmi: request_alloc has no meaningful failure path

[Jonas Bonn]

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

The only way request_alloc can fail is if one of the memory allocation
routines fail to allocate memory.  However, Linux memory allocation
doesn't really fail in this manner; memory can be overcommited and the
out-of-memory reaper will take care of re-establishing the balance when
excess memory is actually accessed.

Given this, request_alloc will never return anything other than success
and the failure paths will never be exercised.
---
 drivers/qmimodem/qmi.c | 32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 436f96a0..a980839b 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -170,9 +170,7 @@ static struct qmi_request *__request_alloc(uint8_t service,
 	struct qmi_message_hdr *msg;
 	uint16_t headroom;
 
-	req = g_try_new0(struct qmi_request, 1);
-	if (!req)
-		return NULL;
+	req = g_new0(struct qmi_request, 1);
 
 	if (service == QMI_SERVICE_CONTROL)
 		headroom = QMI_CONTROL_HDR_SIZE;
@@ -181,11 +179,7 @@ static struct qmi_request *__request_alloc(uint8_t service,
 
 	req->len = QMI_MUX_HDR_SIZE + headroom + QMI_MESSAGE_HDR_SIZE + length;
 
-	req->buf = g_try_malloc(req->len);
-	if (!req->buf) {
-		g_free(req);
-		return NULL;
-	}
+	req->buf = g_malloc(req->len);
 
 	req->client = client;
 
@@ -1282,10 +1276,6 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func,
 	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
 			QMI_CTL_GET_VERSION_INFO,
 			NULL, 0, discover_callback, data, (void **) &hdr);
-	if (!req) {
-		g_free(data);
-		return false;
-	}
 
 	data->tid = hdr->transaction;
 
@@ -1309,10 +1299,6 @@ static void release_client(struct qmi_device *device,
 			QMI_CTL_RELEASE_CLIENT_ID,
 			release_req, sizeof(release_req),
 			func, user_data, (void **) &hdr);
-	if (!req) {
-		func(0x0000, 0x0000, NULL, user_data);
-		return;
-	}
 
 	__request_submit(device, req);
 }
@@ -2028,15 +2014,6 @@ static void service_create_discover(uint8_t count,
 			QMI_CTL_GET_CLIENT_ID,
 			client_req, sizeof(client_req),
 			service_create_callback, data, (void **) &hdr);
-	if (!req) {
-		if (data->timeout > 0)
-			g_source_remove(data->timeout);
-
-		data->timeout = g_timeout_add_seconds(0,
-						service_create_reply, data);
-		__qmi_device_discovery_started(device, &data->super);
-		return;
-	}
 
 	__request_submit(device, req);
 }
@@ -2320,11 +2297,6 @@ uint16_t qmi_service_send(struct qmi_service *service,
 				param ? param->length : 0,
 				service_send_callback, data, (void **) &hdr);
 
-	if (!req) {
-		g_free(data);
-		return 0;
-	}
-
 	qmi_param_free(param);
 
 	__request_submit(device, req);
-- 
2.15.1

[PATCH 3/6] qmi: drop header output parameter from request_alloc

[Jonas Bonn]

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

The only thing this output parameter is being used for now is for
getting the transaction ID.  Return the TID directly from
__submit_requesta and drop the 'head' parameter altogether.
---
 drivers/qmimodem/qmi.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index a980839b..08d437c1 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -163,7 +163,7 @@ static struct qmi_request *__request_alloc(uint8_t service,
 				uint8_t client, uint16_t message,
 				const void *data,
 				uint16_t length, qmi_message_func_t func,
-				void *user_data, void **head)
+				void *user_data)
 {
 	struct qmi_request *req;
 	struct qmi_mux_hdr *hdr;
@@ -203,8 +203,6 @@ static struct qmi_request *__request_alloc(uint8_t service,
 	req->callback = func;
 	req->user_data = user_data;
 
-	*head = req->buf + QMI_MUX_HDR_SIZE;
-
 	return req;
 }
 
@@ -695,7 +693,8 @@ static void wakeup_writer(struct qmi_device *device)
 				can_write_data, device, write_watch_destroy);
 }
 
-static void __request_submit(struct qmi_device *device, struct qmi_request *req)
+static uint16_t __request_submit(struct qmi_device *device,
+				struct qmi_request *req)
 {
 	struct qmi_mux_hdr *mux;
 
@@ -723,6 +722,8 @@ static void __request_submit(struct qmi_device *device, struct qmi_request *req)
 	g_queue_push_tail(device->req_queue, req);
 
 	wakeup_writer(device);
+
+	return req->tid;
 }
 
 static void service_notify(gpointer key, gpointer value, gpointer user_data)
@@ -1250,7 +1251,7 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func,
 {
 	struct discover_data *data;
 	struct qmi_request *req;
-	struct qmi_control_hdr *hdr;
+	uint8_t tid;
 
 	if (!device)
 		return false;
@@ -1275,11 +1276,11 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func,
 
 	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
 			QMI_CTL_GET_VERSION_INFO,
-			NULL, 0, discover_callback, data, (void **) &hdr);
+			NULL, 0, discover_callback, data);
 
-	data->tid = hdr->transaction;
+	tid = __request_submit(device, req);
 
-	__request_submit(device, req);
+	data->tid = tid;
 
 	data->timeout = g_timeout_add_seconds(5, discover_reply, data);
 	__qmi_device_discovery_started(device, &data->super);
@@ -1293,12 +1294,11 @@ static void release_client(struct qmi_device *device,
 {
 	unsigned char release_req[] = { 0x01, 0x02, 0x00, type, client_id };
 	struct qmi_request *req;
-	struct qmi_control_hdr *hdr;
 
 	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
 			QMI_CTL_RELEASE_CLIENT_ID,
 			release_req, sizeof(release_req),
-			func, user_data, (void **) &hdr);
+			func, user_data);
 
 	__request_submit(device, req);
 }
@@ -1378,7 +1378,6 @@ bool qmi_device_sync(struct qmi_device *device,
 		     qmi_sync_func_t func, void *user_data)
 {
 	struct qmi_request *req;
-	struct qmi_control_hdr *hdr;
 	struct sync_data *func_data;
 
 	if (!device)
@@ -1393,7 +1392,7 @@ bool qmi_device_sync(struct qmi_device *device,
 	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
 			QMI_CTL_SYNC,
 			NULL, 0,
-			qmi_device_sync_callback, func_data, (void **) &hdr);
+			qmi_device_sync_callback, func_data);
 
 	__request_submit(device, req);
 
@@ -1996,7 +1995,6 @@ static void service_create_discover(uint8_t count,
 	struct service_create_data *data = user_data;
 	struct qmi_device *device = data->device;
 	struct qmi_request *req;
-	struct qmi_control_hdr *hdr;
 	unsigned char client_req[] = { 0x01, 0x01, 0x00, data->type };
 	unsigned int i;
 
@@ -2013,7 +2011,7 @@ static void service_create_discover(uint8_t count,
 	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
 			QMI_CTL_GET_CLIENT_ID,
 			client_req, sizeof(client_req),
-			service_create_callback, data, (void **) &hdr);
+			service_create_callback, data);
 
 	__request_submit(device, req);
 }
@@ -2271,7 +2269,7 @@ uint16_t qmi_service_send(struct qmi_service *service,
 	struct qmi_device *device;
 	struct service_send_data *data;
 	struct qmi_request *req;
-	struct qmi_service_hdr *hdr;
+	uint16_t tid;
 
 	if (!service)
 		return 0;
@@ -2295,13 +2293,13 @@ uint16_t qmi_service_send(struct qmi_service *service,
 				message,
 				param ? param->data : NULL,
 				param ? param->length : 0,
-				service_send_callback, data, (void **) &hdr);
+				service_send_callback, data);
 
 	qmi_param_free(param);
 
-	__request_submit(device, req);
+	tid = __request_submit(device, req);
 
-	return hdr->transaction;
+	return tid;
 }
 
 bool qmi_service_cancel(struct qmi_service *service, uint16_t id)
-- 
2.15.1

[PATCH 4/6] qmi: assume version_list is up to date

[Jonas Bonn]

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

The way things are currently coded, the gobi plugin calls
qmi_device_discover and does nothing else until it succeeds.  As such,
we can safely assume that the version_list is set up when we go to
create a service.
---
 drivers/qmimodem/qmi.c | 56 +++++++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 08d437c1..750f3b00 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -1989,43 +1989,22 @@ done:
 	__qmi_device_discovery_complete(data->device, &data->super);
 }
 
-static void service_create_discover(uint8_t count,
-			const struct qmi_version *list, void *user_data)
-{
-	struct service_create_data *data = user_data;
-	struct qmi_device *device = data->device;
-	struct qmi_request *req;
-	unsigned char client_req[] = { 0x01, 0x01, 0x00, data->type };
-	unsigned int i;
-
-	__debug_device(device, "service create [type=%d]", data->type);
-
-	for (i = 0; i < count; i++) {
-		if (list[i].type == data->type) {
-			data->major = list[i].major;
-			data->minor = list[i].minor;
-			break;
-		}
-	}
-
-	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
-			QMI_CTL_GET_CLIENT_ID,
-			client_req, sizeof(client_req),
-			service_create_callback, data);
-
-	__request_submit(device, req);
-}
-
 static bool service_create(struct qmi_device *device, bool shared,
 				uint8_t type, qmi_create_func_t func,
 				void *user_data, qmi_destroy_func_t destroy)
 {
 	struct service_create_data *data;
+	unsigned char client_req[] = { 0x01, 0x01, 0x00, type };
+	struct qmi_request *req;
+	int i;
 
 	data = g_try_new0(struct service_create_data, 1);
 	if (!data)
 		return false;
 
+	if (!device->version_list)
+		return false;
+
 	data->super.destroy = service_create_data_free;
 	data->device = device;
 	data->shared = shared;
@@ -2034,20 +2013,23 @@ static bool service_create(struct qmi_device *device, bool shared,
 	data->user_data = user_data;
 	data->destroy = destroy;
 
-	if (device->version_list) {
-		service_create_discover(device->version_count,
-						device->version_list, data);
-		goto done;
-	}
+	__debug_device(device, "service create [type=%d]", type);
 
-	if (qmi_device_discover(device, service_create_discover, data, NULL))
-		goto done;
+	for (i = 0; i < device->version_count; i++) {
+		if (device->version_list[i].type == data->type) {
+			data->major = device->version_list[i].major;
+			data->minor = device->version_list[i].minor;
+			break;
+		}
+	}
 
-	g_free(data);
+	req = __request_alloc(QMI_SERVICE_CONTROL, 0x00,
+			QMI_CTL_GET_CLIENT_ID,
+			client_req, sizeof(client_req),
+			service_create_callback, data);
 
-	return false;
+	__request_submit(device, req);
 
-done:
 	data->timeout = g_timeout_add_seconds(8, service_create_reply, data);
 	__qmi_device_discovery_started(device, &data->super);
 
-- 
2.15.1

[PATCH 5/6] qmi: make version_list private

[Jonas Bonn]

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

---
 drivers/qmimodem/qmi.c | 47 +++++++++++++++++++++++++++++++---
 drivers/qmimodem/qmi.h | 14 ++++-------
 plugins/gobi.c         | 68 +++++++++++++++++++-------------------------------
 3 files changed, 74 insertions(+), 55 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 750f3b00..0ef84b50 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -47,6 +47,13 @@ struct discovery {
 	qmi_destroy_func_t destroy;
 };
 
+struct qmi_version {
+	uint8_t type;
+	uint16_t major;
+	uint16_t minor;
+	const char *name;
+};
+
 struct qmi_device {
 	int ref_count;
 	int fd;
@@ -1102,6 +1109,41 @@ static const void *tlv_get(const void *data, uint16_t size,
 	return NULL;
 }
 
+bool qmi_device_get_service_version(struct qmi_device* device, uint8_t type,
+					uint16_t *major, uint16_t *minor)
+{
+	struct qmi_version* info;
+	int i;
+
+	for (i = 0, info = device->version_list;
+			i < device->version_count;
+			i++, info++) {
+		if (info->type == type) {
+			*major = info->major;
+			*minor = info->minor;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+bool qmi_device_has_service(struct qmi_device* device, uint8_t type)
+{
+	struct qmi_version* info;
+	int i;
+
+	for (i = 0, info = device->version_list;
+			i < device->version_count;
+			i++, info++) {
+		if (info->type == type) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 struct discover_data {
 	struct discovery super;
 	struct qmi_device *device;
@@ -1201,7 +1243,7 @@ done:
 	device->version_count = count;
 
 	if (data->func)
-		data->func(count, list, data->user_data);
+		data->func(data->user_data);
 
 	__qmi_device_discovery_complete(data->device, &data->super);
 }
@@ -1237,8 +1279,7 @@ static gboolean discover_reply(gpointer user_data)
 	}
 
 	if (data->func)
-		data->func(device->version_count,
-				device->version_list, data->user_data);
+		data->func(data->user_data);
 
 	__qmi_device_discovery_complete(data->device, &data->super);
 	__request_free(req, NULL);
diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h
index e1801045..a5960e2a 100644
--- a/drivers/qmimodem/qmi.h
+++ b/drivers/qmimodem/qmi.h
@@ -61,13 +61,6 @@ enum qmi_device_expected_data_format {
 	QMI_DEVICE_EXPECTED_DATA_FORMAT_RAW_IP,
 };
 
-struct qmi_version {
-	uint8_t type;
-	uint16_t major;
-	uint16_t minor;
-	const char *name;
-};
-
 void qmi_free(void *ptr);
 
 typedef void (*qmi_destroy_func_t)(void *user_data);
@@ -78,8 +71,7 @@ struct qmi_device;
 typedef void (*qmi_debug_func_t)(const char *str, void *user_data);
 typedef void (*qmi_sync_func_t)(void *user_data);
 typedef void (*qmi_shutdown_func_t)(void *user_data);
-typedef void (*qmi_discover_func_t)(uint8_t count,
-			const struct qmi_version *list, void *user_data);
+typedef void (*qmi_discover_func_t)(void *user_data);
 
 struct qmi_device *qmi_device_new(int fd);
 
@@ -96,6 +88,10 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func,
 bool qmi_device_shutdown(struct qmi_device *device, qmi_shutdown_func_t func,
 				void *user_data, qmi_destroy_func_t destroy);
 
+bool qmi_device_has_service(struct qmi_device* device, uint8_t type);
+bool qmi_device_get_service_version(struct qmi_device* device, uint8_t type,
+					uint16_t *major, uint16_t *minor);
+
 bool qmi_device_sync(struct qmi_device *device,
 		     qmi_sync_func_t func, void *user_data);
 bool qmi_device_is_sync_supported(struct qmi_device *device);
diff --git a/plugins/gobi.c b/plugins/gobi.c
index dc466a86..85218913 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -264,56 +264,38 @@ static void create_shared_dms(void *user_data)
 				  create_dms_cb, modem, NULL);
 }
 
-static void discover_cb(uint8_t count, const struct qmi_version *list,
-							void *user_data)
+static void discover_cb(void *user_data)
 {
 	struct ofono_modem *modem = user_data;
 	struct gobi_data *data = ofono_modem_get_data(modem);
-	uint8_t i;
+	uint16_t major, minor;
 
 	DBG("");
 
-	for (i = 0; i < count; i++) {
-		DBG("%s %d.%d - %d", list[i].name, list[i].major, list[i].minor,
-				list[i].type);
-
-		switch (list[i].type) {
-		case QMI_SERVICE_DMS:
-			data->features |= GOBI_DMS;
-			break;
-		case QMI_SERVICE_NAS:
-			data->features |= GOBI_NAS;
-			break;
-		case QMI_SERVICE_WMS:
-			data->features |= GOBI_WMS;
-			break;
-		case QMI_SERVICE_WDS:
-			data->features |= GOBI_WDS;
-			break;
-		case QMI_SERVICE_WDA:
-			data->features |= GOBI_WDA;
-			break;
-		case QMI_SERVICE_PDS:
-			data->features |= GOBI_PDS;
-			break;
-		case QMI_SERVICE_PBM:
-			data->features |= GOBI_PBM;
-			break;
-		case QMI_SERVICE_UIM:
-			data->features |= GOBI_UIM;
-			break;
-		case QMI_SERVICE_CAT:
-			data->features |= GOBI_CAT;
-			break;
-		case QMI_SERVICE_CAT_OLD:
-			if (list[i].major > 0)
-				data->features |= GOBI_CAT_OLD;
-			break;
-		case QMI_SERVICE_VOICE:
+	if (qmi_device_has_service(data->device, QMI_SERVICE_DMS))
+		data->features |= GOBI_DMS;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_NAS))
+		data->features |= GOBI_NAS;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_WMS))
+		data->features |= GOBI_WMS;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_WDS))
+		data->features |= GOBI_WDS;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_WDA))
+		data->features |= GOBI_WDA;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_PDS))
+		data->features |= GOBI_PDS;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_PBM))
+		data->features |= GOBI_PBM;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_UIM))
+		data->features |= GOBI_UIM;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_CAT))
+		data->features |= GOBI_CAT;
+	if (qmi_device_get_service_version(data->device,
+				QMI_SERVICE_CAT_OLD, &major, &minor))
+		if (major > 0)
+			data->features |= GOBI_CAT_OLD;
+	if (qmi_device_has_service(data->device, QMI_SERVICE_VOICE))
 			data->features |= GOBI_VOICE;
-			break;
-		}
-	}
 
 	if (!(data->features & GOBI_DMS)) {
 		if (++data->discover_attempts < 3) {
-- 
2.15.1

[PATCH 6/6] qmi: make services always shared

[Jonas Bonn]

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

---
 drivers/qmimodem/qmi.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 0ef84b50..06a4cf4e 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -87,7 +87,6 @@ struct qmi_device {
 struct qmi_service {
 	int ref_count;
 	struct qmi_device *device;
-	bool shared;
 	uint8_t type;
 	uint16_t major;
 	uint16_t minor;
@@ -261,9 +260,6 @@ static gboolean __service_compare_shared(gpointer key, gpointer value,
 	struct qmi_service *service = value;
 	uint8_t type = GPOINTER_TO_UINT(user_data);
 
-	if (!service->shared)
-		return FALSE;
-
 	if (service->type == type)
 		return TRUE;
 
@@ -1936,7 +1932,6 @@ bool qmi_result_get_uint64(struct qmi_result *result, uint8_t type,
 struct service_create_data {
 	struct discovery super;
 	struct qmi_device *device;
-	bool shared;
 	uint8_t type;
 	uint16_t major;
 	uint16_t minor;
@@ -2007,7 +2002,6 @@ static void service_create_callback(uint16_t message, uint16_t length,
 
 	service->ref_count = 1;
 	service->device = data->device;
-	service->shared = data->shared;
 
 	service->type = data->type;
 	service->major = data->major;
@@ -2030,7 +2024,7 @@ done:
 	__qmi_device_discovery_complete(data->device, &data->super);
 }
 
-static bool service_create(struct qmi_device *device, bool shared,
+static bool service_create(struct qmi_device *device,
 				uint8_t type, qmi_create_func_t func,
 				void *user_data, qmi_destroy_func_t destroy)
 {
@@ -2048,7 +2042,6 @@ static bool service_create(struct qmi_device *device, bool shared,
 
 	data->super.destroy = service_create_data_free;
 	data->device = device;
-	data->shared = shared;
 	data->type = type;
 	data->func = func;
 	data->user_data = user_data;
@@ -2077,19 +2070,6 @@ static bool service_create(struct qmi_device *device, bool shared,
 	return true;
 }
 
-bool qmi_service_create(struct qmi_device *device,
-				uint8_t type, qmi_create_func_t func,
-				void *user_data, qmi_destroy_func_t destroy)
-{
-	if (!device || !func)
-		return false;
-
-	if (type == QMI_SERVICE_CONTROL)
-		return false;
-
-	return service_create(device, false, type, func, user_data, destroy);
-}
-
 struct service_create_shared_data {
 	struct discovery super;
 	struct qmi_service *service;
@@ -2165,7 +2145,15 @@ bool qmi_service_create_shared(struct qmi_device *device,
 		return 0;
 	}
 
-	return service_create(device, true, type, func, user_data, destroy);
+	return service_create(device, type, func, user_data, destroy);
+}
+
+bool qmi_service_create(struct qmi_device *device,
+				uint8_t type, qmi_create_func_t func,
+				void *user_data, qmi_destroy_func_t destroy)
+{
+	return qmi_service_create_shared(device, type, func,
+						user_data, destroy);
 }
 
 static void service_release_callback(uint16_t message, uint16_t length,
-- 
2.15.1

Re: [PATCH 1/6] qmi: unify common request header setup

[Denis Kenzior]

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

Hi Jonas,

On 04/03/2018 09:50 AM, Jonas Bonn wrote:
> The service and control requests differ slightly in their headers, but
> this difference is minor enough that we can handle it directly in the
> request submission routine.  This patch unifies the header setup for the
> two request types.
> ---
>   drivers/qmimodem/qmi.c | 68 +++++++++++++++++++++++---------------------------
>   1 file changed, 31 insertions(+), 37 deletions(-)
> 

Patches 1-4 applied, thanks.

Regards,
-Denis

Re: [PATCH 5/6] qmi: make version_list private

[Denis Kenzior]

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

Hi Jonas,

On 04/03/2018 09:50 AM, Jonas Bonn wrote:
> ---
>   drivers/qmimodem/qmi.c | 47 +++++++++++++++++++++++++++++++---
>   drivers/qmimodem/qmi.h | 14 ++++-------
>   plugins/gobi.c         | 68 +++++++++++++++++++-------------------------------
>   3 files changed, 74 insertions(+), 55 deletions(-)

I split this up into two commits per our patch submission guidelines.

> +bool qmi_device_get_service_version(struct qmi_device* device, uint8_t type,

And I had to fix a bunch of instances of misplaced '*' ;)

Regards,
-Denis

Re: [PATCH 6/6] qmi: make services always shared

[Denis Kenzior]

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

Hi Jonas,

On 04/03/2018 09:50 AM, Jonas Bonn wrote:
> ---
>   drivers/qmimodem/qmi.c | 32 ++++++++++----------------------
>   1 file changed, 10 insertions(+), 22 deletions(-)
> 

Applied, thanks.

Regards,
-Denis