linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] In-kernel QMI handling
@ 2017-08-04 14:59 Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err Bjorn Andersson
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

This series starts by moving the common definitions of the QMUX protocol to the
uapi header, as they are shared with clients - both in kernel and userspace.

This series then introduces in-kernel helper functions for aiding the handling
of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
exchanging messages between the majority of QRTR clients and services.

It then adds an abstractions to reduce the duplication of common code in
drivers that needs to query the name server and send and receive encoded
messages to a remote service.

Finally it introduces a sample implementation for showing QRTR and the QMI
helpers in action. The sample device instantiates in response to finding the
"test service" and implements the "test protocol".

Bjorn Andersson (6):
  net: qrtr: Invoke sk_error_report() after setting sk_err
  net: qrtr: Move constants to header file
  net: qrtr: Add control packet definition to uapi
  soc: qcom: Introduce QMI encoder/decoder
  soc: qcom: Introduce QMI helpers
  samples: Introduce Qualcomm QRTR sample client

 drivers/soc/qcom/Kconfig          |   8 +
 drivers/soc/qcom/Makefile         |   3 +
 drivers/soc/qcom/qmi_encdec.c     | 812 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/qmi_interface.c  | 540 +++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h      | 249 ++++++++++++
 include/uapi/linux/qrtr.h         |  35 ++
 net/qrtr/qrtr.c                   |  16 +-
 samples/Kconfig                   |   8 +
 samples/Makefile                  |   2 +-
 samples/qrtr/Makefile             |   1 +
 samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++
 11 files changed, 2261 insertions(+), 16 deletions(-)
 create mode 100644 drivers/soc/qcom/qmi_encdec.c
 create mode 100644 drivers/soc/qcom/qmi_interface.c
 create mode 100644 include/linux/soc/qcom/qmi.h
 create mode 100644 samples/qrtr/Makefile
 create mode 100644 samples/qrtr/qrtr_sample_client.c

-- 
2.12.0

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 2/6] net: qrtr: Move constants to header file Bjorn Andersson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

Rather than manually waking up any context sleeping on the sock to
signal an error we should call sk_error_report(). This has the added
benefit that in-kernel consumers can override this notificatino with
its own callback.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 5586609afa27..2058b27821a4 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -541,7 +541,7 @@ static void qrtr_reset_ports(void)
 
 		sock_hold(&ipc->sk);
 		ipc->sk.sk_err = ENETRESET;
-		wake_up_interruptible(sk_sleep(&ipc->sk));
+		ipc->sk.sk_error_report(&ipc->sk);
 		sock_put(&ipc->sk);
 	}
 	mutex_unlock(&qrtr_port_lock);
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] net: qrtr: Move constants to header file
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 3/6] net: qrtr: Add control packet definition to uapi Bjorn Andersson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

The constants are used by both the name server and clients, so make
their value explicit and move them to the uapi header.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 include/uapi/linux/qrtr.h | 3 +++
 net/qrtr/qrtr.c           | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 9d76c566f66e..63e8803e4d90 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -4,6 +4,9 @@
 #include <linux/socket.h>
 #include <linux/types.h>
 
+#define QRTR_NODE_BCAST	0xffffffffu
+#define QRTR_PORT_CTRL	0xfffffffeu
+
 struct sockaddr_qrtr {
 	__kernel_sa_family_t sq_family;
 	__u32 sq_node;
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 2058b27821a4..0d7d3968414e 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -61,8 +61,6 @@ struct qrtr_hdr {
 } __packed;
 
 #define QRTR_HDR_SIZE sizeof(struct qrtr_hdr)
-#define QRTR_NODE_BCAST ((unsigned int)-1)
-#define QRTR_PORT_CTRL ((unsigned int)-2)
 
 struct qrtr_sock {
 	/* WARNING: sk must be the first member */
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] net: qrtr: Add control packet definition to uapi
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 2/6] net: qrtr: Move constants to header file Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 4/6] soc: qcom: Introduce QMI encoder/decoder Bjorn Andersson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

The QMUX protocol specification defines structure of the special control
packet messages being sent between handlers of the control port.

Add these to the uapi header, as this structure and the associated types
are shared between the kernel and all userspace handlers of control
messages.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 include/uapi/linux/qrtr.h | 32 ++++++++++++++++++++++++++++++++
 net/qrtr/qrtr.c           | 12 ------------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 63e8803e4d90..179af64846e0 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -13,4 +13,36 @@ struct sockaddr_qrtr {
 	__u32 sq_port;
 };
 
+enum qrtr_pkt_type {
+	QRTR_TYPE_DATA		= 1,
+	QRTR_TYPE_HELLO		= 2,
+	QRTR_TYPE_BYE		= 3,
+	QRTR_TYPE_NEW_SERVER	= 4,
+	QRTR_TYPE_DEL_SERVER	= 5,
+	QRTR_TYPE_DEL_CLIENT	= 6,
+	QRTR_TYPE_RESUME_TX	= 7,
+	QRTR_TYPE_EXIT          = 8,
+	QRTR_TYPE_PING          = 9,
+	QRTR_TYPE_NEW_LOOKUP	= 10,
+	QRTR_TYPE_DEL_LOOKUP	= 11,
+};
+
+struct qrtr_ctrl_pkt {
+	__le32 cmd;
+
+	union {
+		struct {
+			__le32 service;
+			__le32 instance;
+			__le32 node;
+			__le32 port;
+		} server;
+
+		struct {
+			__le32 node;
+			__le32 port;
+		} client;
+	};
+} __packed;
+
 #endif /* _LINUX_QRTR_H */
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 0d7d3968414e..fac7cd6ea445 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -26,18 +26,6 @@
 #define QRTR_MIN_EPH_SOCKET 0x4000
 #define QRTR_MAX_EPH_SOCKET 0x7fff
 
-enum qrtr_pkt_type {
-	QRTR_TYPE_DATA		= 1,
-	QRTR_TYPE_HELLO		= 2,
-	QRTR_TYPE_BYE		= 3,
-	QRTR_TYPE_NEW_SERVER	= 4,
-	QRTR_TYPE_DEL_SERVER	= 5,
-	QRTR_TYPE_DEL_CLIENT	= 6,
-	QRTR_TYPE_RESUME_TX	= 7,
-	QRTR_TYPE_EXIT		= 8,
-	QRTR_TYPE_PING		= 9,
-};
-
 /**
  * struct qrtr_hdr - (I|R)PCrouter packet header
  * @version: protocol version
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] soc: qcom: Introduce QMI encoder/decoder
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
                   ` (2 preceding siblings ...)
  2017-08-04 14:59 ` [PATCH 3/6] net: qrtr: Add control packet definition to uapi Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 5/6] soc: qcom: Introduce QMI helpers Bjorn Andersson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

Add the helper library for encoding and decoding QMI encoded messages.
The implementation is taken from lib/qmi_encdec.c of the Qualcomm kernel
(msm-3.18).

Modifications has been made to the public API, source buffers has been
made const and the debug-logging part was omitted, for now.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/Kconfig      |   8 +
 drivers/soc/qcom/Makefile     |   2 +
 drivers/soc/qcom/qmi_encdec.c | 812 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h  | 116 ++++++
 4 files changed, 938 insertions(+)
 create mode 100644 drivers/soc/qcom/qmi_encdec.c
 create mode 100644 include/linux/soc/qcom/qmi.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 9fca977ef18d..2541ae07ad2a 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -24,6 +24,14 @@ config QCOM_PM
 	  modes. It interface with various system drivers to put the cores in
 	  low power modes.
 
+config QCOM_QMI_HELPERS
+	bool
+	help
+	  Helper library for handling QMI encoded messages.  QMI encoded
+	  messages are used in communication between the majority of QRTR
+	  clients and this helpers provide the common functionality needed for
+	  doing this from a kernel driver.
+
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 414f0de274fa..27b60da7a062 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,8 @@
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
+obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
+qmi_helpers-y	+= qmi_encdec.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
 obj-$(CONFIG_QCOM_SMEM) +=	smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
new file mode 100644
index 000000000000..4eb3099c64e7
--- /dev/null
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -0,0 +1,812 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/soc/qcom/qmi.h>
+
+#define QMI_ENCDEC_ENCODE_TLV(type, length, p_dst) do { \
+	*p_dst++ = type; \
+	*p_dst++ = ((uint8_t)((length) & 0xFF)); \
+	*p_dst++ = ((uint8_t)(((length) >> 8) & 0xFF)); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_TLV(p_type, p_length, p_src) do { \
+	*p_type = (uint8_t)*p_src++; \
+	*p_length = (uint8_t)*p_src++; \
+	*p_length |= ((uint8_t)*p_src) << 8; \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_N_BYTES(p_dst, p_src, size) \
+do { \
+	memcpy(p_dst, p_src, size); \
+	p_dst = (uint8_t *)p_dst + size; \
+	p_src = (uint8_t *)p_src + size; \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
+do { \
+	memcpy(p_dst, p_src, size); \
+	p_dst = (uint8_t *)p_dst + size; \
+	p_src = (uint8_t *)p_src + size; \
+} while (0)
+
+#define UPDATE_ENCODE_VARIABLES(temp_si, buf_dst, \
+				encoded_bytes, tlv_len, encode_tlv, rc) \
+do { \
+	buf_dst = (uint8_t *)buf_dst + rc; \
+	encoded_bytes += rc; \
+	tlv_len += rc; \
+	temp_si = temp_si + 1; \
+	encode_tlv = 1; \
+} while (0)
+
+#define UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc) \
+do { \
+	buf_src = (uint8_t *)buf_src + rc; \
+	decoded_bytes += rc; \
+} while (0)
+
+#define TLV_LEN_SIZE sizeof(uint16_t)
+#define TLV_TYPE_SIZE sizeof(uint8_t)
+#define OPTIONAL_TLV_TYPE_START 0x10
+
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+		      const void *in_c_struct, uint32_t out_buf_len,
+		      int enc_level);
+
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+		      const void *in_buf, uint32_t in_buf_len, int dec_level);
+
+/**
+ * skip_to_next_elem() - Skip to next element in the structure to be encoded
+ * @ei_array: Struct info describing the element to be skipped.
+ * @level: Depth level of encoding/decoding to identify nested structures.
+ *
+ * Returns struct info of the next element that can be encoded.
+ *
+ * This function is used while encoding optional elements. If the flag
+ * corresponding to an optional element is not set, then encoding the
+ * optional element can be skipped. This function can be used to perform
+ * that operation.
+ */
+static struct qmi_elem_info *skip_to_next_elem(struct qmi_elem_info *ei_array,
+					   int level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t tlv_type;
+
+	if (level > 1) {
+		temp_ei = temp_ei + 1;
+	} else {
+		do {
+			tlv_type = temp_ei->tlv_type;
+			temp_ei = temp_ei + 1;
+		} while (tlv_type == temp_ei->tlv_type);
+	}
+
+	return temp_ei;
+}
+
+/**
+ * qmi_calc_min_msg_len() - Calculate the minimum length of a QMI message
+ * @ei_array: Struct info array describing the structure.
+ * @level: Level to identify the depth of the nested structures.
+ *
+ * Returns expected minimum length of the QMI message or 0 on error.
+ */
+static int qmi_calc_min_msg_len(struct qmi_elem_info *ei_array,
+				int level)
+{
+	int min_msg_len = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	if (!ei_array)
+		return min_msg_len;
+
+	while (temp_ei->data_type != QMI_EOTI) {
+		/* Optional elements do not count in minimum length */
+		if (temp_ei->data_type == QMI_OPT_FLAG) {
+			temp_ei = skip_to_next_elem(temp_ei, level);
+			continue;
+		}
+
+		if (temp_ei->data_type == QMI_DATA_LEN) {
+			min_msg_len += (temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t));
+			temp_ei++;
+			continue;
+		} else if (temp_ei->data_type == QMI_STRUCT) {
+			min_msg_len += qmi_calc_min_msg_len(temp_ei->ei_array,
+							    (level + 1));
+			temp_ei++;
+		} else if (temp_ei->data_type == QMI_STRING) {
+			if (level > 1)
+				min_msg_len += temp_ei->elem_len <= U8_MAX ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			min_msg_len += temp_ei->elem_len * temp_ei->elem_size;
+			temp_ei++;
+		} else {
+			min_msg_len += (temp_ei->elem_len * temp_ei->elem_size);
+			temp_ei++;
+		}
+
+		/*
+		 * Type & Length info. not prepended for elements in the
+		 * nested structure.
+		 */
+		if (level == 1)
+			min_msg_len += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+	}
+	return min_msg_len;
+}
+
+/**
+ * qmi_encode_basic_elem() - Encodes elements of basic/primary data type
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @elem_len: Number of elements, in the buf_src, to be encoded.
+ * @elem_size: Size of a single instance of the element to be encoded.
+ *
+ * Returns the number of bytes of encoded information.
+ *
+ * This function encodes the "elem_len" number of data elements, each of
+ * size "elem_size" bytes from the source buffer "buf_src" and stores the
+ * encoded information in the destination buffer "buf_dst". The elements are
+ * of primary data type which include uint8_t - uint64_t or similar. This
+ * function returns the number of bytes of encoded information.
+ */
+static int qmi_encode_basic_elem(void *buf_dst, const void *buf_src,
+				 uint32_t elem_len, uint32_t elem_size)
+{
+	uint32_t i, rc = 0;
+
+	for (i = 0; i < elem_len; i++) {
+		QMI_ENCDEC_ENCODE_N_BYTES(buf_dst, buf_src, elem_size);
+		rc += elem_size;
+	}
+
+	return rc;
+}
+
+/**
+ * qmi_encode_struct_elem() - Encodes elements of struct data type
+ * @ei_array: Struct info array descibing the struct element.
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @elem_len: Number of elements, in the buf_src, to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Depth of the nested structure from the main structure.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ *
+ * This function encodes the "elem_len" number of struct elements, each of
+ * size "ei_array->elem_size" bytes from the source buffer "buf_src" and
+ * stores the encoded information in the destination buffer "buf_dst". The
+ * elements are of struct data type which includes any C structure. This
+ * function returns the number of bytes of encoded information.
+ */
+static int qmi_encode_struct_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t elem_len, uint32_t out_buf_len,
+				  int enc_level)
+{
+	int i, rc, encoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	for (i = 0; i < elem_len; i++) {
+		rc = qmi_encode(temp_ei->ei_array, buf_dst, buf_src,
+				out_buf_len - encoded_bytes, enc_level);
+		if (rc < 0) {
+			pr_err("%s: STRUCT Encode failure\n", __func__);
+			return rc;
+		}
+		buf_dst = buf_dst + rc;
+		buf_src = buf_src + temp_ei->elem_size;
+		encoded_bytes += rc;
+	}
+
+	return encoded_bytes;
+}
+
+/**
+ * qmi_encode_string_elem() - Encodes elements of string data type
+ * @ei_array: Struct info array descibing the string element.
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Depth of the string element from the main structure.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ *
+ * This function encodes a string element of maximum length "ei_array->elem_len"
+ * bytes from the source buffer "buf_src" and stores the encoded information in
+ * the destination buffer "buf_dst". This function returns the number of bytes
+ * of encoded information.
+ */
+static int qmi_encode_string_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t out_buf_len, int enc_level)
+{
+	int rc;
+	int encoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint32_t string_len = 0;
+	uint32_t string_len_sz = 0;
+
+	string_len = strlen(buf_src);
+	string_len_sz = temp_ei->elem_len <= U8_MAX ?
+			sizeof(uint8_t) : sizeof(uint16_t);
+	if (string_len > temp_ei->elem_len) {
+		pr_err("%s: String to be encoded is longer - %d > %d\n",
+			__func__, string_len, temp_ei->elem_len);
+		return -EINVAL;
+	}
+
+	if (enc_level == 1) {
+		if (string_len + TLV_LEN_SIZE + TLV_TYPE_SIZE >
+		    out_buf_len) {
+			pr_err("%s: Output len %d > Out Buf len %d\n",
+				__func__, string_len, out_buf_len);
+			return -ETOOSMALL;
+		}
+	} else {
+		if (string_len + string_len_sz > out_buf_len) {
+			pr_err("%s: Output len %d > Out Buf len %d\n",
+				__func__, string_len, out_buf_len);
+			return -ETOOSMALL;
+		}
+		rc = qmi_encode_basic_elem(buf_dst, &string_len,
+					   1, string_len_sz);
+		encoded_bytes += rc;
+	}
+
+	rc = qmi_encode_basic_elem(buf_dst + encoded_bytes, buf_src,
+				   string_len, temp_ei->elem_size);
+	encoded_bytes += rc;
+	return encoded_bytes;
+}
+
+/**
+ * qmi_encode() - Core Encode Function
+ * @ei_array: Struct info array describing the structure to be encoded.
+ * @out_buf: Buffer to hold the encoded QMI message.
+ * @in_c_struct: Pointer to the C structure to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Encode level to indicate the depth of the nested structure,
+ *             within the main structure, being encoded.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ */
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+		      const void *in_c_struct, uint32_t out_buf_len,
+		      int enc_level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t opt_flag_value = 0;
+	uint32_t data_len_value = 0, data_len_sz;
+	uint8_t *buf_dst = (uint8_t *)out_buf;
+	uint8_t *tlv_pointer;
+	uint32_t tlv_len;
+	uint8_t tlv_type;
+	uint32_t encoded_bytes = 0;
+	const void *buf_src;
+	int encode_tlv = 0;
+	int rc;
+
+	tlv_pointer = buf_dst;
+	tlv_len = 0;
+	if (enc_level == 1)
+		buf_dst = buf_dst + (TLV_LEN_SIZE + TLV_TYPE_SIZE);
+
+	while (temp_ei->data_type != QMI_EOTI) {
+		buf_src = in_c_struct + temp_ei->offset;
+		tlv_type = temp_ei->tlv_type;
+
+		if (temp_ei->is_array == NO_ARRAY) {
+			data_len_value = 1;
+		} else if (temp_ei->is_array == STATIC_ARRAY) {
+			data_len_value = temp_ei->elem_len;
+		} else if (data_len_value <= 0 ||
+			    temp_ei->elem_len < data_len_value) {
+			pr_err("%s: Invalid data length\n", __func__);
+			return -EINVAL;
+		}
+
+		switch (temp_ei->data_type) {
+		case QMI_OPT_FLAG:
+			rc = qmi_encode_basic_elem(&opt_flag_value, buf_src,
+						   1, sizeof(uint8_t));
+			if (opt_flag_value)
+				temp_ei = temp_ei + 1;
+			else
+				temp_ei = skip_to_next_elem(temp_ei, enc_level);
+			break;
+
+		case QMI_DATA_LEN:
+			memcpy(&data_len_value, buf_src, temp_ei->elem_size);
+			data_len_sz = temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			/* Check to avoid out of range buffer access */
+			if ((data_len_sz + encoded_bytes + TLV_LEN_SIZE +
+			    TLV_TYPE_SIZE) > out_buf_len) {
+				pr_err("%s: Too Small Buffer @DATA_LEN\n",
+					__func__);
+				return -ETOOSMALL;
+			}
+			rc = qmi_encode_basic_elem(buf_dst, &data_len_value,
+						   1, data_len_sz);
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			if (!data_len_value)
+				temp_ei = skip_to_next_elem(temp_ei, enc_level);
+			else
+				encode_tlv = 0;
+			break;
+
+		case QMI_UNSIGNED_1_BYTE:
+		case QMI_UNSIGNED_2_BYTE:
+		case QMI_UNSIGNED_4_BYTE:
+		case QMI_UNSIGNED_8_BYTE:
+		case QMI_SIGNED_2_BYTE_ENUM:
+		case QMI_SIGNED_4_BYTE_ENUM:
+			/* Check to avoid out of range buffer access */
+			if (((data_len_value * temp_ei->elem_size) +
+			    encoded_bytes + TLV_LEN_SIZE + TLV_TYPE_SIZE) >
+			    out_buf_len) {
+				pr_err("%s: Too Small Buffer @data_type:%d\n",
+					__func__, temp_ei->data_type);
+				return -ETOOSMALL;
+			}
+			rc = qmi_encode_basic_elem(buf_dst, buf_src,
+				data_len_value, temp_ei->elem_size);
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+
+		case QMI_STRUCT:
+			rc = qmi_encode_struct_elem(temp_ei, buf_dst, buf_src,
+				data_len_value, (out_buf_len - encoded_bytes),
+				(enc_level + 1));
+			if (rc < 0)
+				return rc;
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+
+		case QMI_STRING:
+			rc = qmi_encode_string_elem(temp_ei, buf_dst, buf_src,
+				out_buf_len - encoded_bytes, enc_level);
+			if (rc < 0)
+				return rc;
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+		default:
+			pr_err("%s: Unrecognized data type\n", __func__);
+			return -EINVAL;
+
+		}
+
+		if (encode_tlv && enc_level == 1) {
+			QMI_ENCDEC_ENCODE_TLV(tlv_type, tlv_len, tlv_pointer);
+			encoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			tlv_pointer = buf_dst;
+			tlv_len = 0;
+			buf_dst = buf_dst + TLV_LEN_SIZE + TLV_TYPE_SIZE;
+			encode_tlv = 0;
+		}
+	}
+	return encoded_bytes;
+}
+
+/**
+ * qmi_decode_basic_elem() - Decodes elements of basic/primary data type
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @elem_len: Number of elements to be decoded.
+ * @elem_size: Size of a single instance of the element to be decoded.
+ *
+ * Returns the total size of the decoded data elements, in bytes.
+ *
+ * This function decodes the "elem_len" number of elements in QMI wire format,
+ * each of size "elem_size" bytes from the source buffer "buf_src" and stores
+ * the decoded elements in the destination buffer "buf_dst". The elements are
+ * of primary data type which include uint8_t - uint64_t or similar. This
+ * function returns the number of bytes of decoded information.
+ */
+static int qmi_decode_basic_elem(void *buf_dst, const void *buf_src,
+				 uint32_t elem_len, uint32_t elem_size)
+{
+	uint32_t i, rc = 0;
+
+	for (i = 0; i < elem_len; i++) {
+		QMI_ENCDEC_DECODE_N_BYTES(buf_dst, buf_src, elem_size);
+		rc += elem_size;
+	}
+
+	return rc;
+}
+
+/**
+ * qmi_decode_struct_elem() - Decodes elements of struct data type
+ * @ei_array: Struct info array descibing the struct element.
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @elem_len: Number of elements to be decoded.
+ * @tlv_len: Total size of the encoded inforation corresponding to
+ *           this struct element.
+ * @dec_level: Depth of the nested structure from the main structure.
+ *
+ * Returns the total size of the decoded data elements on success, negative
+ * errno on error.
+ *
+ * This function decodes the "elem_len" number of elements in QMI wire format,
+ * each of size "(tlv_len/elem_len)" bytes from the source buffer "buf_src"
+ * and stores the decoded elements in the destination buffer "buf_dst". The
+ * elements are of struct data type which includes any C structure. This
+ * function returns the number of bytes of decoded information.
+ */
+static int qmi_decode_struct_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t elem_len, uint32_t tlv_len,
+				  int dec_level)
+{
+	int i, rc, decoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	for (i = 0; i < elem_len && decoded_bytes < tlv_len; i++) {
+		rc = qmi_decode(temp_ei->ei_array, buf_dst, buf_src,
+				tlv_len - decoded_bytes, dec_level);
+		if (rc < 0)
+			return rc;
+		buf_src = buf_src + rc;
+		buf_dst = buf_dst + temp_ei->elem_size;
+		decoded_bytes += rc;
+	}
+
+	if ((dec_level <= 2 && decoded_bytes != tlv_len) ||
+	    (dec_level > 2 && (i < elem_len || decoded_bytes > tlv_len))) {
+		pr_err("%s: Fault in decoding: dl(%d), db(%d), tl(%d), i(%d), el(%d)\n",
+			__func__, dec_level, decoded_bytes, tlv_len,
+			i, elem_len);
+		return -EFAULT;
+	}
+	return decoded_bytes;
+}
+
+/**
+ * qmi_decode_string_elem() - Decodes elements of string data type
+ * @ei_array: Struct info array descibing the string element.
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @tlv_len: Total size of the encoded inforation corresponding to
+ *           this string element.
+ * @dec_level: Depth of the string element from the main structure.
+ *
+
+ * Returns the total size of the decoded data elements on success, negative
+ * errno on error.
+
+ *
+ * This function decodes the string element of maximum length
+ * "ei_array->elem_len" from the source buffer "buf_src" and puts it into
+ * the destination buffer "buf_dst". This function returns number of bytes
+ * decoded from the input buffer.
+ */
+static int qmi_decode_string_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t tlv_len, int dec_level)
+{
+	int rc;
+	int decoded_bytes = 0;
+	uint32_t string_len = 0;
+	uint32_t string_len_sz = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	if (dec_level == 1) {
+		string_len = tlv_len;
+	} else {
+		string_len_sz = temp_ei->elem_len <= U8_MAX ?
+				sizeof(uint8_t) : sizeof(uint16_t);
+		rc = qmi_decode_basic_elem(&string_len, buf_src,
+					   1, string_len_sz);
+		decoded_bytes += rc;
+	}
+
+	if (string_len > temp_ei->elem_len) {
+		pr_err("%s: String len %d > Max Len %d\n",
+			__func__, string_len, temp_ei->elem_len);
+		return -ETOOSMALL;
+	} else if (string_len > tlv_len) {
+		pr_err("%s: String len %d > Input Buffer Len %d\n",
+			__func__, string_len, tlv_len);
+		return -EFAULT;
+	}
+
+	rc = qmi_decode_basic_elem(buf_dst, buf_src + decoded_bytes,
+				   string_len, temp_ei->elem_size);
+	*((char *)buf_dst + string_len) = '\0';
+	decoded_bytes += rc;
+	return decoded_bytes;
+}
+
+/**
+ * find_ei() - Find element info corresponding to TLV Type
+ * @ei_array: Struct info array of the message being decoded.
+ * @type: TLV Type of the element being searched.
+ *
+ * Returns pointer to struct info, if found
+ *
+ * Every element that got encoded in the QMI message will have a type
+ * information associated with it. While decoding the QMI message,
+ * this function is used to find the struct info regarding the element
+ * that corresponds to the type being decoded.
+ */
+static struct qmi_elem_info *find_ei(struct qmi_elem_info *ei_array,
+				   uint32_t type)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	while (temp_ei->data_type != QMI_EOTI) {
+		if (temp_ei->tlv_type == (uint8_t)type)
+			return temp_ei;
+		temp_ei = temp_ei + 1;
+	}
+	return NULL;
+}
+
+/**
+ * qmi_decode() - Core Decode Function
+ * @ei_array: Struct info array describing the structure to be decoded.
+ * @out_c_struct: Buffer to hold the decoded C struct
+ * @in_buf: Buffer containing the QMI message to be decoded
+ * @in_buf_len: Length of the QMI message to be decoded
+ * @dec_level: Decode level to indicate the depth of the nested structure,
+ *             within the main structure, being decoded
+ *
+ * Returns the number of bytes of decoded information on success, negative
+ * errno on error.
+ */
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+		      const void *in_buf, uint32_t in_buf_len,
+		      int dec_level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t opt_flag_value = 1;
+	uint32_t data_len_value = 0, data_len_sz = 0;
+	uint8_t *buf_dst = out_c_struct;
+	const uint8_t *tlv_pointer;
+	uint32_t tlv_len = 0;
+	uint32_t tlv_type;
+	uint32_t decoded_bytes = 0;
+	const void *buf_src = in_buf;
+	int rc;
+
+	while (decoded_bytes < in_buf_len) {
+		if (dec_level >= 2 && temp_ei->data_type == QMI_EOTI)
+			return decoded_bytes;
+
+		if (dec_level == 1) {
+			tlv_pointer = buf_src;
+			QMI_ENCDEC_DECODE_TLV(&tlv_type,
+					      &tlv_len, tlv_pointer);
+			buf_src += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			decoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			temp_ei = find_ei(ei_array, tlv_type);
+			if (!temp_ei && (tlv_type < OPTIONAL_TLV_TYPE_START)) {
+				pr_err("%s: Inval element info\n", __func__);
+				return -EINVAL;
+			} else if (!temp_ei) {
+				UPDATE_DECODE_VARIABLES(buf_src,
+						decoded_bytes, tlv_len);
+				continue;
+			}
+		} else {
+			/*
+			 * No length information for elements in nested
+			 * structures. So use remaining decodable buffer space.
+			 */
+			tlv_len = in_buf_len - decoded_bytes;
+		}
+
+		buf_dst = out_c_struct + temp_ei->offset;
+		if (temp_ei->data_type == QMI_OPT_FLAG) {
+			memcpy(buf_dst, &opt_flag_value, sizeof(uint8_t));
+			temp_ei = temp_ei + 1;
+			buf_dst = out_c_struct + temp_ei->offset;
+		}
+
+		if (temp_ei->data_type == QMI_DATA_LEN) {
+			data_len_sz = temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			rc = qmi_decode_basic_elem(&data_len_value, buf_src,
+						   1, data_len_sz);
+			memcpy(buf_dst, &data_len_value, sizeof(uint32_t));
+			temp_ei = temp_ei + 1;
+			buf_dst = out_c_struct + temp_ei->offset;
+			tlv_len -= data_len_sz;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+		}
+
+		if (temp_ei->is_array == NO_ARRAY) {
+			data_len_value = 1;
+		} else if (temp_ei->is_array == STATIC_ARRAY) {
+			data_len_value = temp_ei->elem_len;
+		} else if (data_len_value > temp_ei->elem_len) {
+			pr_err("%s: Data len %d > max spec %d\n",
+				__func__, data_len_value, temp_ei->elem_len);
+			return -ETOOSMALL;
+		}
+
+		switch (temp_ei->data_type) {
+		case QMI_UNSIGNED_1_BYTE:
+		case QMI_UNSIGNED_2_BYTE:
+		case QMI_UNSIGNED_4_BYTE:
+		case QMI_UNSIGNED_8_BYTE:
+		case QMI_SIGNED_2_BYTE_ENUM:
+		case QMI_SIGNED_4_BYTE_ENUM:
+			rc = qmi_decode_basic_elem(buf_dst, buf_src,
+				data_len_value, temp_ei->elem_size);
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		case QMI_STRUCT:
+			rc = qmi_decode_struct_elem(temp_ei, buf_dst, buf_src,
+				data_len_value, tlv_len, (dec_level + 1));
+			if (rc < 0)
+				return rc;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		case QMI_STRING:
+			rc = qmi_decode_string_elem(temp_ei, buf_dst, buf_src,
+						     tlv_len, dec_level);
+			if (rc < 0)
+				return rc;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		default:
+			pr_err("%s: Unrecognized data type\n", __func__);
+			return -EINVAL;
+		}
+		temp_ei = temp_ei + 1;
+	}
+	return decoded_bytes;
+}
+
+/**
+ * qmi_encode_message() - Encode C structure as QMI encoded message
+ * @type:	Type of QMI message
+ * @msg_id:	Message ID of the message
+ * @len:	Passed as max length of the message, updated to actual size
+ * @txn_id:	Transaction ID
+ * @ei:		QMI message descriptor
+ * @c_struct:	Reference to structure to encode
+ *
+ * Returns buffer with encoded message, or negative ERR_PTR() on error
+ */
+void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
+			 unsigned int txn_id, struct qmi_elem_info *ei,
+			 const void *c_struct)
+{
+	struct qmi_header *hdr;
+	void *msg;
+	int ret;
+
+	/* Check the possibility of a zero length QMI message */
+	if (!c_struct) {
+		ret = qmi_calc_min_msg_len(ei, 1);
+		if (ret) {
+			pr_err("%s: Calc. len %d != 0, but NULL c_struct\n",
+				__func__, ret);
+			return ERR_PTR(-EINVAL);
+		} else {
+			return NULL;
+		}
+	}
+
+	if (!ei)
+		return ERR_PTR(-EINVAL);
+
+	hdr = msg = kzalloc(sizeof(*hdr) + *len, GFP_KERNEL);
+	if (!msg)
+		return ERR_PTR(-ENOMEM);
+
+	ret = qmi_encode(ei, msg + sizeof(*hdr), c_struct, *len, 1);
+	if (ret < 0) {
+		kfree(msg);
+		return ERR_PTR(ret);
+	}
+
+	hdr->type = type;
+	hdr->txn_id = txn_id;
+	hdr->msg_id = msg_id;
+	hdr->msg_len = ret;
+
+	*len = sizeof(*hdr) + ret;
+
+	return msg;
+}
+EXPORT_SYMBOL(qmi_encode_message);
+
+/**
+ * qmi_decode_message() - Decode QMI encoded message to C structure
+ * @buf:	Buffer with encoded message
+ * @len:	Amount of data in @buf
+ * @ei:		QMI message descriptor
+ * @c_struct:	Reference to structure to decode into
+ *
+ * Returns the number of bytes of decoded information on success, negative
+ * errno on error.
+ */
+int qmi_decode_message(const void *buf, size_t len,
+		       struct qmi_elem_info *ei, void *c_struct)
+{
+	if (!ei)
+		return -EINVAL;
+
+	if (!c_struct || !buf || !len)
+		return -EINVAL;
+
+	return qmi_decode(ei, c_struct, buf + sizeof(struct qmi_header),
+			  len - sizeof(struct qmi_header), 1);
+}
+EXPORT_SYMBOL(qmi_decode_message);
+
+/* Common header in all QMI responses */
+struct qmi_elem_info qmi_response_type_v01_ei[] = {
+	{
+		.data_type	= QMI_SIGNED_2_BYTE_ENUM,
+		.elem_len	= 1,
+		.elem_size	= sizeof(uint16_t),
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= offsetof(struct qmi_response_type_v01,
+					   result),
+		.ei_array	= NULL,
+	},
+	{
+		.data_type      = QMI_SIGNED_2_BYTE_ENUM,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint16_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = QMI_COMMON_TLV_TYPE,
+		.offset         = offsetof(struct qmi_response_type_v01,
+					   error),
+		.ei_array       = NULL,
+	},
+	{
+		.data_type	= QMI_EOTI,
+		.elem_len	= 0,
+		.elem_size	= 0,
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= 0,
+		.ei_array	= NULL,
+	},
+};
+EXPORT_SYMBOL(qmi_response_type_v01_ei);
+
+MODULE_DESCRIPTION("QMI encoder/decoder helper");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
new file mode 100644
index 000000000000..c012a1e9e24b
--- /dev/null
+++ b/include/linux/soc/qcom/qmi.h
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2017, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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.
+ */
+#ifndef __QMI_HELPERS_H__
+#define __QMI_HELPERS_H__
+
+#include <linux/qrtr.h>
+#include <linux/types.h>
+
+/**
+ * qmi_header - wireformat header of QMI messages
+ * @type:	type of message
+ * @txn_id:	transaction id
+ * @msg_id:	message id
+ * @msg_len:	length of message payload following header
+ */
+struct qmi_header {
+	uint8_t type;
+	uint16_t txn_id;
+	uint16_t msg_id;
+	uint16_t msg_len;
+} __packed;
+
+#define QMI_REQUEST	0
+#define QMI_RESPONSE	2
+#define QMI_INDICATION	4
+
+#define QMI_COMMON_TLV_TYPE 0
+
+enum qmi_elem_type {
+	QMI_EOTI,
+	QMI_OPT_FLAG,
+	QMI_DATA_LEN,
+	QMI_UNSIGNED_1_BYTE,
+	QMI_UNSIGNED_2_BYTE,
+	QMI_UNSIGNED_4_BYTE,
+	QMI_UNSIGNED_8_BYTE,
+	QMI_SIGNED_2_BYTE_ENUM,
+	QMI_SIGNED_4_BYTE_ENUM,
+	QMI_STRUCT,
+	QMI_STRING,
+};
+
+enum qmi_array_type {
+	NO_ARRAY,
+	STATIC_ARRAY,
+	VAR_LEN_ARRAY,
+};
+
+/**
+ * struct qmi_elem_info - describes how to encode a single QMI element
+ * @data_type:	Data type of this element.
+ * @elem_len:	Array length of this element, if an array.
+ * @elem_size:	Size of a single instance of this data type.
+ * @is_array:	Array type of this element.
+ * @tlv_type:	QMI message specific type to identify which element
+ *		is present in an incoming message.
+ * @offset:	Specifies the offset of the first instance of this
+ *		element in the data structure.
+ * @ei_array:	Null-terminated array of @qmi_elem_info to describe nested
+ *		structures.
+ */
+struct qmi_elem_info {
+	enum qmi_elem_type data_type;
+	uint32_t elem_len;
+	uint32_t elem_size;
+	enum qmi_array_type is_array;
+	uint8_t tlv_type;
+	uint32_t offset;
+	struct qmi_elem_info *ei_array;
+};
+
+#define QMI_RESULT_SUCCESS_V01			0
+#define QMI_RESULT_FAILURE_V01			1
+
+#define QMI_ERR_NONE_V01			0
+#define QMI_ERR_MALFORMED_MSG_V01		1
+#define QMI_ERR_NO_MEMORY_V01			2
+#define QMI_ERR_INTERNAL_V01			3
+#define QMI_ERR_CLIENT_IDS_EXHAUSTED_V01	5
+#define QMI_ERR_INVALID_ID_V01			41
+#define QMI_ERR_ENCODING_V01			58
+#define QMI_ERR_INCOMPATIBLE_STATE_V01		90
+#define QMI_ERR_NOT_SUPPORTED_V01		94
+
+/**
+ * qmi_response_type_v01 - common response header (decoded)
+ * @result:	result of the transaction
+ * @error:	error value, when @result is QMI_RESULT_FAILURE_V01
+ */
+struct qmi_response_type_v01 {
+	u32 result;
+	u32 error;
+};
+
+extern struct qmi_elem_info qmi_response_type_v01_ei[];
+
+void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
+			 unsigned int txn_id, struct qmi_elem_info *ei,
+			 const void *c_struct);
+
+int qmi_decode_message(const void *buf, size_t len,
+		       struct qmi_elem_info *ei, void *c_struct);
+
+
+#endif
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] soc: qcom: Introduce QMI helpers
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
                   ` (3 preceding siblings ...)
  2017-08-04 14:59 ` [PATCH 4/6] soc: qcom: Introduce QMI encoder/decoder Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 14:59 ` [PATCH 6/6] samples: Introduce Qualcomm QRTR sample client Bjorn Andersson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

Drivers that needs to communicate with a remote QMI service all has to
perform the operations of discovering the service, encoding and decoding
the messages and operate the socket. This introduces an abstraction for
these common operations, reducing most of the duplication in such cases.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/Makefile        |   1 +
 drivers/soc/qcom/qmi_interface.c | 540 +++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h     | 133 ++++++++++
 3 files changed, 674 insertions(+)
 create mode 100644 drivers/soc/qcom/qmi_interface.c

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 27b60da7a062..812402ae9cfa 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
 obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
 qmi_helpers-y	+= qmi_encdec.o
+qmi_helpers-y	+= qmi_interface.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
 obj-$(CONFIG_QCOM_SMEM) +=	smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
new file mode 100644
index 000000000000..41853a9becfd
--- /dev/null
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -0,0 +1,540 @@
+/*
+ * Sample QRTR client driver
+ *
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/qrtr.h>
+#include <linux/net.h>
+#include <linux/completion.h>
+#include <linux/idr.h>
+#include <linux/string.h>
+#include <net/sock.h>
+#include <linux/workqueue.h>
+#include <linux/soc/qcom/qmi.h>
+
+/**
+ * qrtr_client_new_server() - handler of NEW_SERVER control message
+ * @qrtr:	qrtr handle
+ * @node:	node of the new server
+ * @port:	port of the new server
+ *
+ * Calls the new_server callback to inform the client about a newly registered
+ * server matching the currently registered service lookup.
+ */
+static void qrtr_client_new_server(struct qrtr_handle *qrtr,
+				   unsigned int node, unsigned int port)
+{
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct qrtr_service *service;
+	int ret;
+
+	if (!ops->new_server)
+		return;
+
+	/* Ignore EOF marker */
+	if (!node && !port)
+		return;
+
+	service = kzalloc(sizeof(*service), GFP_KERNEL);
+	if (!service)
+		return;
+
+	service->node = node;
+	service->port = port;
+
+	ret = ops->new_server(qrtr, service);
+	if (ret < 0)
+		kfree(service);
+	else
+		list_add(&service->list_node, &qrtr->services);
+}
+
+/**
+ * qrtr_client_del_server() - handler of DEL_SERVER control message
+ * @qrtr:	qrtr handle
+ * @node:	node of the dying server, a value of -1 matches all nodes
+ * @port:	port of the dying server, a value of -1 matches all ports
+ *
+ * Calls the del_server callback for each previously seen server, allowing the
+ * client to react to the disappearing server.
+ */
+static void qrtr_client_del_server(struct qrtr_handle *qrtr,
+				   unsigned int node, unsigned int port)
+{
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct qrtr_service *service;
+	struct qrtr_service *tmp;
+
+	list_for_each_entry_safe(service, tmp, &qrtr->services, list_node) {
+		if (node != -1 && service->node != node)
+			continue;
+		if (port != -1 && service->port != port)
+			continue;
+
+		if (ops->del_server)
+			ops->del_server(qrtr, service);
+
+		list_del(&service->list_node);
+		kfree(service);
+	}
+}
+
+static void qrtr_client_ctrl_pkt(struct qrtr_handle *qrtr,
+				 const void *buf, size_t len)
+{
+	const struct qrtr_ctrl_pkt *pkt = buf;
+
+	if (len < sizeof(struct qrtr_ctrl_pkt)) {
+		pr_debug("ignoring short control packet\n");
+		return;
+	}
+
+	switch (le32_to_cpu(pkt->cmd)) {
+	case QRTR_TYPE_NEW_SERVER:
+		qrtr_client_new_server(qrtr,
+				       le32_to_cpu(pkt->server.node),
+				       le32_to_cpu(pkt->server.port));
+		break;
+	case QRTR_TYPE_DEL_SERVER:
+		qrtr_client_del_server(qrtr,
+				       le32_to_cpu(pkt->server.node),
+				       le32_to_cpu(pkt->server.port));
+		break;
+	}
+}
+
+static void qrtr_client_data_ready_work(struct work_struct *work)
+{
+	struct qrtr_handle *qrtr = container_of(work, struct qrtr_handle, work);
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = { .msg_name = &sq, .msg_namelen = sizeof(sq) };
+	struct kvec iv;
+	ssize_t msglen;
+
+	for (;;) {
+		iv.iov_base = qrtr->recv_buf;
+		iv.iov_len = qrtr->recv_buf_size;
+
+		msglen = kernel_recvmsg(qrtr->sock, &msg, &iv, 1, iv.iov_len,
+					MSG_DONTWAIT);
+		if (msglen == -EAGAIN)
+			break;
+
+		if (msglen == -ENETRESET) {
+			if (ops->net_reset)
+				ops->net_reset(qrtr);
+			break;
+		}
+
+		if (msglen < 0) {
+			pr_err("qrtr_handle recvmsg failed: %zd\n", msglen);
+			break;
+		}
+
+		if (sq.sq_node == qrtr->sq.sq_node &&
+		    sq.sq_port == QRTR_PORT_CTRL) {
+			qrtr_client_ctrl_pkt(qrtr, qrtr->recv_buf, msglen);
+		} else {
+			if (!ops->msg_handler)
+				continue;
+
+			ops->msg_handler(qrtr, &sq, qrtr->recv_buf, msglen);
+		}
+	}
+}
+
+static void qrtr_client_schedule_worker(struct sock *sk)
+{
+	struct qrtr_handle *qrtr = sk->sk_user_data;
+
+	/*
+	 * This will be NULL if we receive data while being in
+	 * qrtr_client_release()
+	 */
+	if (!qrtr)
+		return;
+
+	queue_work(qrtr->wq, &qrtr->work);
+}
+
+/**
+ * qrtr_client_init() - initialize a qrtr_handle
+ * @qrtr:		reference to qrtr handle
+ * @recv_buf_size:	maximum size of received messages
+ * @ops:		qrtr_handle_ops struct with callbacks
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This function initializes @qrtr to allow sending, receiving and handling
+ * QRTR control messages and data packets.
+ */
+int qrtr_client_init(struct qrtr_handle *qrtr, size_t recv_buf_size,
+		     struct qrtr_handle_ops *ops)
+{
+	struct sockaddr_qrtr sq;
+	struct socket *sock;
+	int ret;
+	int sl = sizeof(sq);
+
+	if (recv_buf_size < sizeof(struct qrtr_ctrl_pkt))
+		recv_buf_size = sizeof(struct qrtr_ctrl_pkt);
+
+	INIT_LIST_HEAD(&qrtr->services);
+	INIT_WORK(&qrtr->work, qrtr_client_data_ready_work);
+
+	qrtr->wq = alloc_workqueue("qrtr_handle", WQ_UNBOUND, 1);
+	if (!qrtr->wq)
+		return -ENOMEM;
+
+	qrtr->ops = *ops;
+	qrtr->recv_buf_size = recv_buf_size;
+	qrtr->recv_buf = kzalloc(recv_buf_size, GFP_KERNEL);
+	if (!qrtr->recv_buf)
+		return -ENOMEM;
+
+	ret = sock_create_kern(&init_net, AF_QIPCRTR, SOCK_DGRAM,
+			       PF_QIPCRTR, &sock);
+	if (ret < 0)
+		goto err_free_recv_buf;
+
+	ret = kernel_getsockname(sock, (struct sockaddr *)&sq, &sl);
+	if (ret < 0)
+		goto err_release_sock;
+
+	qrtr->sock = sock;
+	qrtr->sq = sq;
+
+	sock->sk->sk_user_data = qrtr;
+	sock->sk->sk_data_ready = qrtr_client_schedule_worker;
+	sock->sk->sk_error_report = qrtr_client_schedule_worker;
+
+	return 0;
+err_release_sock:
+	sock_release(sock);
+
+err_free_recv_buf:
+	kfree(qrtr->recv_buf);
+
+	return ret;
+}
+EXPORT_SYMBOL(qrtr_client_init);
+
+/**
+ * qrtr_client_release() - tear down a qrtr_handle
+ * @qrtr:	qrtr handle
+ *
+ * This will tear down the qrtr handle, stop handling of any incoming messages
+ * and release the underlying socket.
+ */
+void qrtr_client_release(struct qrtr_handle *qrtr)
+{
+	struct socket *sock = qrtr->sock;
+
+	sock->sk->sk_user_data = NULL;
+	cancel_work_sync(&qrtr->work);
+
+	kfree(qrtr->recv_buf);
+
+	qrtr_client_del_server(qrtr, -1, -1);
+
+	sock_release(sock);
+	qrtr->sock = NULL;
+
+	destroy_workqueue(qrtr->wq);
+}
+EXPORT_SYMBOL(qrtr_client_release);
+
+/**
+ * qrtr_client_new_lookup() - register a new lookup with the name service
+ * @qrtr:	qrtr handle
+ * @service:	service id of the request
+ * @instance:	instance id of the request
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * Registering a lookup query with the name server will cause the name server
+ * to send NEW_SERVER and DEL_SERVER control messages to this socket as
+ * matching services are registered.
+ */
+int qrtr_client_new_lookup(struct qrtr_handle *qrtr,
+			   unsigned int service, unsigned int instance)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = {0};
+	struct kvec iv = { &pkt, sizeof(pkt) };
+	int ret;
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_NEW_LOOKUP);
+	pkt.server.service = cpu_to_le32(service);
+	pkt.server.instance = cpu_to_le32(instance);
+
+	sq.sq_family = qrtr->sq.sq_family;
+	sq.sq_node = qrtr->sq.sq_node;
+	sq.sq_port = QRTR_PORT_CTRL;
+
+	msg.msg_name = &sq;
+	msg.msg_namelen = sizeof(sq);
+
+	ret = kernel_sendmsg(qrtr->sock, &msg, &iv, 1, sizeof(pkt));
+	if (ret < 0)
+		pr_err("failed to send lookup registration: %d\n", ret);
+
+	return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL(qrtr_client_new_lookup);
+
+/**
+ * qmi_txn_init() - allocate transaction id within the given QMI handle
+ * @qmi:	QMI handle
+ * @txn:	transaction context
+ * @ei:		description of how to decode a matching response (optional)
+ * @c_struct:	pointer to the object to decode the response into (optional)
+ *
+ * Returns transaction id on success, negative errno on failure.
+ *
+ * This allocates a transaction id within the QMI handle. If @ei and @c_struct
+ * are specified any responses to this transaction will be decoded as described
+ * by @ei into @c_struct.
+ */
+int qmi_txn_init(struct qmi_handle *qmi, struct qmi_txn *txn,
+		 struct qmi_elem_info *ei, void *c_struct)
+{
+	int ret;
+
+	memset(txn, 0, sizeof(*txn));
+
+	init_completion(&txn->completion);
+	txn->qmi = qmi;
+	txn->ei = ei;
+	txn->dest = c_struct;
+
+	mutex_lock(&qmi->txn_lock);
+	ret = idr_alloc_cyclic(&qmi->txns, txn, 0, INT_MAX, GFP_KERNEL);
+	if (ret < 0)
+		pr_err("failed to allocate transaction id\n");
+
+	txn->id = ret;
+	mutex_unlock(&qmi->txn_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qmi_txn_init);
+
+/**
+ * qmi_txn_wait() - wait for a response on a transaction
+ * @txn:	transaction handle
+ * @timeout:	timeout, in jiffies
+ *
+ * Returns the transaction response on success, negative errno on failure.
+ *
+ * If the transaction is decoded by the means of @ei and @c_struct the return
+ * value will be the returned value of qmi_decode_message(), otherwise it's up
+ * to the specified message handler to fill out the result.
+ */
+int qmi_txn_wait(struct qmi_txn *txn, unsigned long timeout)
+{
+	struct qmi_handle *qmi = txn->qmi;
+	int ret;
+
+	ret = wait_for_completion_interruptible_timeout(&txn->completion,
+							timeout);
+
+	mutex_lock(&qmi->txn_lock);
+	idr_remove(&qmi->txns, txn->id);
+	mutex_unlock(&qmi->txn_lock);
+
+	if (ret < 0)
+		return ret;
+	else if (ret == 0)
+		return -ETIMEDOUT;
+	else
+		return txn->result;
+}
+EXPORT_SYMBOL(qmi_txn_wait);
+
+/**
+ * qmi_txn_cancel() - cancel an ongoing transaction
+ * @txn:	transaction id
+ */
+void qmi_txn_cancel(struct qmi_txn *txn)
+{
+	struct qmi_handle *qmi = txn->qmi;
+
+	mutex_lock(&qmi->txn_lock);
+	idr_remove(&qmi->txns, txn->id);
+	mutex_unlock(&qmi->txn_lock);
+}
+EXPORT_SYMBOL(qmi_txn_cancel);
+
+static void qmi_client_handle_data(struct qrtr_handle *qrtr,
+				   struct sockaddr_qrtr *sq,
+				   const void *buf, size_t len)
+{
+	const struct qmi_header *hdr = buf;
+	struct qmi_handle *qmi = container_of(qrtr, struct qmi_handle, qrtr);
+	struct qmi_msg_handler *handler;
+	struct qmi_txn *txn = NULL;
+	void *dest;
+	int ret;
+
+	if (len < sizeof(*hdr)) {
+		pr_err("ignoring short QMI packet\n");
+		return;
+	}
+
+	mutex_lock(&qmi->txn_lock);
+
+	/* If this is a response, find the matching transaction handle */
+	if (hdr->type == QMI_RESPONSE)
+		txn = idr_find(&qmi->txns, hdr->txn_id);
+
+	if (txn && txn->dest && txn->ei) {
+		ret = qmi_decode_message(buf, len, txn->ei, txn->dest);
+		if (ret < 0)
+			pr_err("failed to decode incoming message\n");
+
+		txn->result = ret;
+		complete(&txn->completion);
+	} else if (qmi->handlers) {
+		for (handler = qmi->handlers; handler->fn; handler++) {
+			if (handler->type == hdr->type &&
+			    handler->msg_id == hdr->msg_id)
+				break;
+		}
+
+		if (!handler->fn)
+			goto out;
+
+		dest = kzalloc(handler->decoded_size, GFP_KERNEL);
+		if (!dest)
+			goto out;
+
+		ret = qmi_decode_message(buf, len, handler->ei, dest);
+		if (ret < 0)
+			pr_err("failed to decode incoming message\n");
+		else
+			handler->fn(qmi, sq, txn, dest);
+
+		kfree(dest);
+	}
+
+out:
+	mutex_unlock(&qmi->txn_lock);
+}
+
+/**
+ * qmi_client_init() - initialize a QMI client handle
+ * @qmi:	QMI handle to initialize
+ * @max_msg_len: maximum size of incoming message
+ * @handlers:	NULL-terminated list of QMI message handlers
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This initializes the QMI client handle to allow sending and receiving QMI
+ * messages. As messages are received the appropriate handler will be invoked.
+ */
+int qmi_client_init(struct qmi_handle *qmi, size_t max_msg_len,
+		    struct qmi_msg_handler *handlers)
+{
+	struct qrtr_handle_ops ops = { 0 };
+	int ret;
+
+	mutex_init(&qmi->txn_lock);
+	idr_init(&qmi->txns);
+
+	ops.msg_handler = qmi_client_handle_data;
+
+	ret = qrtr_client_init(&qmi->qrtr, max_msg_len, &ops);
+	if (ret < 0)
+		return ret;
+
+	qmi->handlers = handlers;
+
+	return 0;
+}
+EXPORT_SYMBOL(qmi_client_init);
+
+/**
+ * qrtr_client_release() - release the QMI client handle
+ * @qmi:	QMI client handle
+ *
+ * This closes the underlying socket and stops any handling of QMI messages.
+ */
+void qmi_client_release(struct qmi_handle *qmi)
+{
+	qrtr_client_release(&qmi->qrtr);
+
+	idr_destroy(&qmi->txns);
+
+}
+EXPORT_SYMBOL(qmi_client_release);
+
+/**
+ * qmi_send_message() - send a QMI message
+ * @qmi:	QMI client handle
+ * @sq:		destination sockaddr
+ * @txn:	transaction object to use for the message
+ * @type:	type of message to send
+ * @msg_id:	message id
+ * @len:	max length of the QMI message
+ * @ei:		QMI message description
+ * @c_struct:	object to be encoded
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This function encodes @c_struct using @ei into a message of type @type,
+ * with @msg_id and @txn into a buffer of maximum size @len, and sends this to
+ * @sq.
+ */
+ssize_t qmi_send_message(struct qmi_handle *qmi,
+			 struct sockaddr_qrtr *sq, struct qmi_txn *txn,
+			 int type, int msg_id, size_t len,
+			 struct qmi_elem_info *ei, const void *c_struct)
+{
+	struct qrtr_handle *qrtr = &qmi->qrtr;
+	struct msghdr msghdr = {0};
+	struct kvec iv;
+	void *msg;
+	int ret;
+
+	msg = qmi_encode_message(type,
+				 msg_id, &len,
+				 txn->id, ei,
+				 c_struct);
+	if (IS_ERR(msg))
+		return PTR_ERR(msg);
+
+	iv.iov_base = msg;
+	iv.iov_len = len;
+
+	if (sq) {
+		msghdr.msg_name = sq;
+		msghdr.msg_namelen = sizeof(*sq);
+	}
+
+	ret = kernel_sendmsg(qrtr->sock, &msghdr, &iv, 1, len);
+	if (ret < 0)
+		pr_err("failed to send QMI message\n");
+
+	kfree(msg);
+
+	return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL(qmi_send_message);
diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
index c012a1e9e24b..add7570fa14b 100644
--- a/include/linux/soc/qcom/qmi.h
+++ b/include/linux/soc/qcom/qmi.h
@@ -105,6 +105,135 @@ struct qmi_response_type_v01 {
 
 extern struct qmi_elem_info qmi_response_type_v01_ei[];
 
+/**
+ * struct qrtr_service - context to track lookup-results
+ * @node:	node of the service
+ * @port:	port of the service
+ * @cookie:	handle for client's use
+ * @list_node:	list_head for house keeping
+ */
+struct qrtr_service {
+	unsigned int node;
+	unsigned int port;
+
+	void *cookie;
+	struct list_head list_node;
+};
+
+struct qrtr_handle;
+
+/**
+ * struct qrtr_handle_ops - callbacks from qrtr_handle
+ * @new_server:		invoked as a new_server message arrives
+ * @del_server:		invoked as a del_server message arrives
+ * @net_reset:		invoked as the name server is restarted
+ * @msg_handler:	invoked as a non-control message arrives
+ */
+struct qrtr_handle_ops {
+	int (*new_server)(struct qrtr_handle *, struct qrtr_service *);
+	void (*del_server)(struct qrtr_handle *, struct qrtr_service *);
+	void (*net_reset)(struct qrtr_handle *);
+	void (*msg_handler)(struct qrtr_handle *, struct sockaddr_qrtr *,
+			    const void *, size_t);
+};
+
+/**
+ * struct qrtr_handle - qrtr client context
+ * @sock:	socket handle
+ * @sq:		sockaddr of @sock
+ * @work:	work for handling incoming messages
+ * @wq:		workqueue to post @work on
+ * @recv_buf:	scratch buffer for handling incoming messages
+ * @recv_buf_size:	size of @recv_buf
+ * @services:	list of services advertised to the client
+ * @ops:	reference to callbacks
+ */
+struct qrtr_handle {
+	struct socket *sock;
+	struct sockaddr_qrtr sq;
+
+	struct work_struct work;
+	struct workqueue_struct *wq;
+
+	void *recv_buf;
+	size_t recv_buf_size;
+
+	struct list_head services;
+
+	struct qrtr_handle_ops ops;
+};
+
+/**
+ * struct qmi_txn - transaction context
+ * @qmi:	QMI handle this transaction is associated with
+ * @id:		transaction id
+ * @completion:	completion object as the transaction receives a response
+ * @result:	result code for the completed transaction
+ * @ei:		description of the QMI encoded response (optional)
+ * @dest:	destination buffer to decode message into (optional)
+ */
+struct qmi_txn {
+	struct qmi_handle *qmi;
+
+	int id;
+
+	struct completion completion;
+	int result;
+
+	struct qmi_elem_info *ei;
+	void *dest;
+};
+
+/**
+ * struct qmi_msg_handler - description of QMI message handler
+ * @type:	type of message
+ * @msg_id:	message id
+ * @ei:		description of the QMI encoded message
+ * @decoded_size:	size of the decoded object
+ * @fn:		function to invoke as the message is decoded
+ */
+struct qmi_msg_handler {
+	unsigned int type;
+	unsigned int msg_id;
+
+	struct qmi_elem_info *ei;
+
+	size_t decoded_size;
+	void (*fn)(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
+		   struct qmi_txn *txn, const void *decoded);
+};
+
+/**
+ * struct qmi_handle - QMI client handle
+ * @qrtr:	qrtr handle backing the QMI client
+ * @txns:	outstanding transactions
+ * @txn_lock:	lock for modifications of @txns
+ * @handlers:	list of handlers for incoming messages
+ */
+struct qmi_handle {
+	struct qrtr_handle qrtr;
+
+	struct idr txns;
+	struct mutex txn_lock;
+
+	struct qmi_msg_handler *handlers;
+};
+
+int qrtr_client_init(struct qrtr_handle *qrtr, size_t recv_buf_size,
+		     struct qrtr_handle_ops *ops);
+void qrtr_client_release(struct qrtr_handle *qrtr);
+int qrtr_client_new_lookup(struct qrtr_handle *qrtr,
+			   unsigned int service, unsigned int instance);
+
+int qmi_client_init(struct qmi_handle *qmi, size_t max_msg_len,
+		    struct qmi_msg_handler *handlers);
+void qmi_client_release(struct qmi_handle *qmi);
+
+ssize_t qmi_send_message(struct qmi_handle *qmi,
+			 struct sockaddr_qrtr *sq, struct qmi_txn *txn,
+			 int type, int msg_id, size_t len,
+			 struct qmi_elem_info *ei, const void *c_struct);
+
 void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
 			 unsigned int txn_id, struct qmi_elem_info *ei,
 			 const void *c_struct);
@@ -112,5 +241,9 @@ void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
 int qmi_decode_message(const void *buf, size_t len,
 		       struct qmi_elem_info *ei, void *c_struct);
 
+int qmi_txn_init(struct qmi_handle *qmi, struct qmi_txn *txn,
+		 struct qmi_elem_info *ei, void *c_struct);
+int qmi_txn_wait(struct qmi_txn *txn, unsigned long timeout);
+void qmi_txn_cancel(struct qmi_txn *txn);
 
 #endif
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] samples: Introduce Qualcomm QRTR sample client
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
                   ` (4 preceding siblings ...)
  2017-08-04 14:59 ` [PATCH 5/6] soc: qcom: Introduce QMI helpers Bjorn Andersson
@ 2017-08-04 14:59 ` Bjorn Andersson
  2017-08-04 15:36 ` [PATCH 0/6] In-kernel QMI handling Dan Williams
  2017-08-08 11:02 ` Bjørn Mork
  7 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

Introduce a sample driver that register for server notifications and
spawn clients for each available test service (service 15). The spawned
clients implements the interface for encoding "ping" and "data" requests
and decode the responses from the remote.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 samples/Kconfig                   |   8 +
 samples/Makefile                  |   2 +-
 samples/qrtr/Makefile             |   1 +
 samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 613 insertions(+), 1 deletion(-)
 create mode 100644 samples/qrtr/Makefile
 create mode 100644 samples/qrtr/qrtr_sample_client.c

diff --git a/samples/Kconfig b/samples/Kconfig
index 9cb63188d3ef..18796928ab58 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -62,6 +62,14 @@ config SAMPLE_KDB
 	  Build an example of how to dynamically add the hello
 	  command to the kdb shell.
 
+config SAMPLE_QRTR_CLIENT
+	tristate "Build qrtr client sample -- loadable modules only"
+	depends on m
+	select QCOM_QMI_HELPERS
+	help
+	  Build an qrtr client sample driver, which demonstrates how to
+	  communicate with a remote QRTR service, using QMI encoded messages.
+
 config SAMPLE_RPMSG_CLIENT
 	tristate "Build rpmsg client sample -- loadable modules only"
 	depends on RPMSG && m
diff --git a/samples/Makefile b/samples/Makefile
index db54e766ddb1..4bf64276860c 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -3,4 +3,4 @@
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ trace_events/ livepatch/ \
 			   hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
 			   configfs/ connector/ v4l/ trace_printk/ blackfin/ \
-			   vfio-mdev/ statx/
+			   vfio-mdev/ statx/ qrtr/
diff --git a/samples/qrtr/Makefile b/samples/qrtr/Makefile
new file mode 100644
index 000000000000..3f2c2cfdf2e7
--- /dev/null
+++ b/samples/qrtr/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SAMPLE_QRTR_CLIENT) += qrtr_sample_client.o
diff --git a/samples/qrtr/qrtr_sample_client.c b/samples/qrtr/qrtr_sample_client.c
new file mode 100644
index 000000000000..ccb359de4340
--- /dev/null
+++ b/samples/qrtr/qrtr_sample_client.c
@@ -0,0 +1,603 @@
+/*
+ * Sample QRTR client driver
+ *
+ * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/qrtr.h>
+#include <linux/net.h>
+#include <linux/completion.h>
+#include <linux/idr.h>
+#include <linux/string.h>
+#include <net/sock.h>
+#include <linux/soc/qcom/qmi.h>
+
+#define PING_REQ1_TLV_TYPE		0x1
+#define PING_RESP1_TLV_TYPE		0x2
+#define PING_OPT1_TLV_TYPE		0x10
+#define PING_OPT2_TLV_TYPE		0x11
+
+#define DATA_REQ1_TLV_TYPE		0x1
+#define DATA_RESP1_TLV_TYPE		0x2
+#define DATA_OPT1_TLV_TYPE		0x10
+#define DATA_OPT2_TLV_TYPE		0x11
+
+#define TEST_MED_DATA_SIZE_V01		8192
+#define TEST_MAX_NAME_SIZE_V01		255
+
+#define TEST_PING_REQ_MSG_ID_V01	0x20
+#define TEST_DATA_REQ_MSG_ID_V01	0x21
+
+#define TEST_PING_REQ_MAX_MSG_LEN_V01	266
+#define TEST_DATA_REQ_MAX_MSG_LEN_V01	8456
+
+struct test_name_type_v01 {
+	uint32_t name_len;
+	char name[TEST_MAX_NAME_SIZE_V01];
+};
+
+static struct qmi_elem_info test_name_type_v01_ei[] = {
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len	= 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= offsetof(struct test_name_type_v01,
+					   name_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MAX_NAME_SIZE_V01,
+		.elem_size      = sizeof(char),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = QMI_COMMON_TLV_TYPE,
+		.offset         = offsetof(struct test_name_type_v01,
+					   name),
+	},
+	{}
+};
+
+struct test_ping_req_msg_v01 {
+	char ping[4];
+
+	uint8_t client_name_valid;
+	struct test_name_type_v01 client_name;
+};
+
+struct qmi_elem_info test_ping_req_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = 4,
+		.elem_size      = sizeof(char),
+		.is_array       = STATIC_ARRAY,
+		.tlv_type       = PING_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   ping),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   client_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   client_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_ping_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+
+	uint8_t pong_valid;
+	char pong[4];
+
+	uint8_t service_name_valid;
+	struct test_name_type_v01 service_name;
+};
+
+struct qmi_elem_info test_ping_resp_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct qmi_response_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_RESP1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   resp),
+		.ei_array       = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   pong_valid),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = 4,
+		.elem_size      = sizeof(char),
+		.is_array       = STATIC_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   pong),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   service_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   service_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_data_req_msg_v01 {
+	uint32_t data_len;
+	uint8_t data[TEST_MED_DATA_SIZE_V01];
+
+	uint8_t client_name_valid;
+	struct test_name_type_v01 client_name;
+};
+
+struct qmi_elem_info test_data_req_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint32_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   data_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MED_DATA_SIZE_V01,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = DATA_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   data),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   client_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   client_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_data_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+
+	uint8_t data_valid;
+	uint32_t data_len;
+	uint8_t data[TEST_MED_DATA_SIZE_V01];
+
+	uint8_t service_name_valid;
+	struct test_name_type_v01 service_name;
+};
+
+struct qmi_elem_info test_data_resp_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct qmi_response_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_RESP1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   resp),
+		.ei_array       = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data_valid),
+	},
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint32_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MED_DATA_SIZE_V01,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   service_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   service_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+/*
+ * ping_pong_store() - ping_pong attribute store handler
+ * @dev:	sample device context
+ * @attr:	the ping_pong attribute
+ * @buf:	write buffer
+ * @count:	length of @buf
+ *
+ * Returns @count, or negative errno on failure.
+ *
+ * This function allows user space to send out a ping_pong QMI encoded message
+ * to the associated remote test service and will return with the result of the
+ * transaction. It serves as an example of how to provide a custom response
+ * handler.
+ */
+static ssize_t ping_pong_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct qmi_handle *qmi = dev_get_drvdata(dev);
+	struct test_ping_req_msg_v01 req = {0};
+	struct qmi_txn txn;
+	int ret;
+
+	memcpy(req.ping, "ping", sizeof(req.ping));
+
+	ret = qmi_txn_init(qmi, &txn, NULL, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = qmi_send_message(qmi, NULL, &txn,
+			       QMI_REQUEST,
+			       TEST_PING_REQ_MSG_ID_V01,
+			       TEST_PING_REQ_MAX_MSG_LEN_V01,
+			       test_ping_req_msg_v01_ei, &req);
+	if (ret < 0) {
+		qmi_txn_cancel(&txn);
+		return ret;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0)
+		count = ret;
+
+	return count;
+}
+static DEVICE_ATTR_WO(ping_pong);
+
+static void ping_pong_cb(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
+			 struct qmi_txn *txn, const void *data)
+{
+	const struct test_ping_resp_msg_v01 *resp = data;
+
+	if (!txn) {
+		pr_err("spurious ping response\n");
+		return;
+	}
+
+	if (resp->resp.result == QMI_RESULT_FAILURE_V01)
+		txn->result = -ENXIO;
+	else if (!resp->pong_valid || memcmp(resp->pong, "pong", 4))
+		txn->result = -EINVAL;
+
+	complete(&txn->completion);
+}
+
+/*
+ * data_store() - data attribute store handler
+ * @dev:	sample device context
+ * @attr:	the data attribute
+ * @buf:	buffer with message to encode
+ * @count:	length of @buf
+ *
+ * Returns @count, or negative errno on failure.
+ *
+ * This function allows user space to send out a data QMI encoded message to
+ * the associated remote test service and will return with the result of the
+ * transaction. It serves as an example of how to have the QMI helpers decode a
+ * transaction response into a provided object automatically.
+ */
+static ssize_t data_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct qmi_handle *qmi = dev_get_drvdata(dev);
+	struct test_data_resp_msg_v01 *resp;
+	struct test_data_req_msg_v01 *req;
+	struct qmi_txn txn;
+	int ret;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+	if (!resp) {
+		kfree(req);
+		return -ENOMEM;
+	}
+
+	req->data_len = min_t(size_t, sizeof(req->data), count);
+	memcpy(req->data, buf, req->data_len);
+
+	ret = qmi_txn_init(qmi, &txn, test_data_resp_msg_v01_ei, resp);
+	if (ret < 0) {
+		count = ret;
+		goto out;
+	}
+
+	ret = qmi_send_message(qmi, NULL, &txn,
+			       QMI_REQUEST,
+			       TEST_DATA_REQ_MSG_ID_V01,
+			       TEST_DATA_REQ_MAX_MSG_LEN_V01,
+			       test_data_req_msg_v01_ei, req);
+	if (ret < 0) {
+		qmi_txn_cancel(&txn);
+		count = ret;
+		goto out;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0) {
+		count = ret;
+	} else if (!resp->data_valid ||
+		   resp->data_len != req->data_len ||
+		   memcmp(resp->data, req->data, req->data_len)) {
+		pr_err("response data doesn't match expectation\n");
+		count = -EINVAL;
+	}
+
+out:
+	kfree(resp);
+	kfree(req);
+
+	return count;
+}
+static DEVICE_ATTR_WO(data);
+
+static struct attribute *qrtr_dev_attrs[] = {
+	&dev_attr_ping_pong.attr,
+	&dev_attr_data.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(qrtr_dev);
+
+static struct qmi_msg_handler qrtr_sample_handlers[] = {
+	{
+		.type = QMI_RESPONSE,
+		.msg_id = TEST_PING_REQ_MSG_ID_V01,
+		.ei = test_ping_resp_msg_v01_ei,
+		.decoded_size = sizeof(struct test_ping_req_msg_v01),
+		.fn = ping_pong_cb
+	},
+	{}
+};
+
+static int qrtr_sample_probe(struct platform_device *pdev)
+{
+	struct qrtr_handle *qrtr;
+	struct qmi_handle *qmi;
+	struct sockaddr_qrtr *sq;
+	int ret;
+
+	qmi = devm_kzalloc(&pdev->dev, sizeof(*qmi), GFP_KERNEL);
+	if (!qmi)
+		return -ENOMEM;
+
+	qrtr = &qmi->qrtr;
+
+	ret = qmi_client_init(qmi, TEST_DATA_REQ_MAX_MSG_LEN_V01,
+			      qrtr_sample_handlers);
+	if (ret < 0)
+		return ret;
+
+	sq = dev_get_platdata(&pdev->dev);
+	ret = kernel_connect(qrtr->sock, (struct sockaddr *)sq,
+			     sizeof(*sq), 0);
+	if (ret < 0) {
+		pr_err("failed to connect to remote service port\n");
+		qmi_client_release(qmi);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qmi);
+
+	return 0;
+}
+
+static int qrtr_sample_remove(struct platform_device *pdev)
+{
+	struct qmi_handle *qmi = platform_get_drvdata(pdev);
+
+	qmi_client_release(qmi);
+
+	return 0;
+}
+
+static struct platform_driver qrtr_sample_driver = {
+	.probe = qrtr_sample_probe,
+	.remove = qrtr_sample_remove,
+	.driver = {
+		.name = "qrtr_sample_client",
+	},
+};
+
+static int qrtr_sample_new_server(struct qrtr_handle *qrtr,
+				  struct qrtr_service *service)
+{
+	struct platform_device *pdev;
+	struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port };
+	char name[32];
+	int ret;
+
+	snprintf(name, sizeof(name), "qrtr_sample_client@%d:%d",
+		 service->node, service->port);
+
+	pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+	if (!pdev)
+		return -ENOMEM;
+
+	ret = platform_device_add_data(pdev, &sq, sizeof(sq));
+	if (ret)
+		goto err_put_device;
+
+	pdev->dev.groups = qrtr_dev_groups;
+	pdev->driver_override = (char *)qrtr_sample_driver.driver.name;
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto err_put_device;
+
+	service->cookie = pdev;
+
+	return 0;
+
+err_put_device:
+	platform_device_put(pdev);
+
+	return ret;
+}
+
+static void qrtr_sample_del_server(struct qrtr_handle *qrtr,
+				   struct qrtr_service *service)
+{
+	struct platform_device *pdev = service->cookie;
+
+	platform_device_unregister(pdev);
+}
+
+static struct qrtr_handle lookup_client;
+
+static struct qrtr_handle_ops lookup_ops;
+
+static void qrtr_sample_net_reset_work(struct work_struct *work)
+{
+	int ret;
+
+	qrtr_client_release(&lookup_client);
+
+	ret = qrtr_client_init(&lookup_client, 0, &lookup_ops);
+	if (ret < 0)
+		return;
+
+	qrtr_client_new_lookup(&lookup_client, 15, 0);
+}
+static DECLARE_WORK(net_reset_work, qrtr_sample_net_reset_work);
+
+static void qrtr_sample_net_reset(struct qrtr_handle *qrtr)
+{
+	schedule_work(&net_reset_work);
+}
+
+static struct qrtr_handle_ops lookup_ops = {
+	.new_server = qrtr_sample_new_server,
+	.del_server = qrtr_sample_del_server,
+	.net_reset = qrtr_sample_net_reset,
+};
+
+static int qrtr_sample_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&qrtr_sample_driver);
+	if (ret)
+		return ret;
+
+	ret = qrtr_client_init(&lookup_client, 0, &lookup_ops);
+	if (ret < 0)
+		goto err_unregister_driver;
+
+	qrtr_client_new_lookup(&lookup_client, 15, 0);
+
+	return 0;
+
+err_unregister_driver:
+	platform_driver_unregister(&qrtr_sample_driver);
+
+	return ret;
+}
+
+static void qrtr_sample_exit(void)
+{
+	qrtr_client_release(&lookup_client);
+
+	platform_driver_unregister(&qrtr_sample_driver);
+}
+
+module_init(qrtr_sample_init);
+module_exit(qrtr_sample_exit);
+
+MODULE_DESCRIPTION("Sample QRTR client driver");
+MODULE_LICENSE("GPL v2");
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
                   ` (5 preceding siblings ...)
  2017-08-04 14:59 ` [PATCH 6/6] samples: Introduce Qualcomm QRTR sample client Bjorn Andersson
@ 2017-08-04 15:36 ` Dan Williams
  2017-08-07 17:38   ` Bjorn Andersson
  2017-08-08 11:02 ` Bjørn Mork
  7 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2017-08-04 15:36 UTC (permalink / raw)
  To: Bjorn Andersson, David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

On Fri, 2017-08-04 at 07:59 -0700, Bjorn Andersson wrote:
> This series starts by moving the common definitions of the QMUX
> protocol to the
> uapi header, as they are shared with clients - both in kernel and
> userspace.
> 
> This series then introduces in-kernel helper functions for aiding the
> handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> used in
> exchanging messages between the majority of QRTR clients and
> services.

This raises a few red-flags for me.  So far, we've kept almost
everything QMI related in userspace and handled all QMI control-channel 
messages from libraries like libqmi or uqmi via the cdc-wdm driver and
the "rmnet" interface via the qmi_wwan driver.  The kernel drivers just
serve as the transport.

Can you describe what kinds of in-kernel drivers need to actually parse
QMI messages as part of their operation?

Dan

> It then adds an abstractions to reduce the duplication of common code
> in
> drivers that needs to query the name server and send and receive
> encoded
> messages to a remote service.
> 
> Finally it introduces a sample implementation for showing QRTR and
> the QMI
> helpers in action. The sample device instantiates in response to
> finding the
> "test service" and implements the "test protocol".
> 
> Bjorn Andersson (6):
>   net: qrtr: Invoke sk_error_report() after setting sk_err
>   net: qrtr: Move constants to header file
>   net: qrtr: Add control packet definition to uapi
>   soc: qcom: Introduce QMI encoder/decoder
>   soc: qcom: Introduce QMI helpers
>   samples: Introduce Qualcomm QRTR sample client
> 
>  drivers/soc/qcom/Kconfig          |   8 +
>  drivers/soc/qcom/Makefile         |   3 +
>  drivers/soc/qcom/qmi_encdec.c     | 812
> ++++++++++++++++++++++++++++++++++++++
>  drivers/soc/qcom/qmi_interface.c  | 540 +++++++++++++++++++++++++
>  include/linux/soc/qcom/qmi.h      | 249 ++++++++++++
>  include/uapi/linux/qrtr.h         |  35 ++
>  net/qrtr/qrtr.c                   |  16 +-
>  samples/Kconfig                   |   8 +
>  samples/Makefile                  |   2 +-
>  samples/qrtr/Makefile             |   1 +
>  samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++
>  11 files changed, 2261 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/soc/qcom/qmi_encdec.c
>  create mode 100644 drivers/soc/qcom/qmi_interface.c
>  create mode 100644 include/linux/soc/qcom/qmi.h
>  create mode 100644 samples/qrtr/Makefile
>  create mode 100644 samples/qrtr/qrtr_sample_client.c
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-04 15:36 ` [PATCH 0/6] In-kernel QMI handling Dan Williams
@ 2017-08-07 17:38   ` Bjorn Andersson
  2017-08-07 19:19     ` Marcel Holtmann
  0 siblings, 1 reply; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-07 17:38 UTC (permalink / raw)
  To: Dan Williams
  Cc: David S. Miller, Andy Gross, David Brown, linux-arm-msm,
	linux-soc, netdev, linux-kernel

On Fri 04 Aug 08:36 PDT 2017, Dan Williams wrote:

> On Fri, 2017-08-04 at 07:59 -0700, Bjorn Andersson wrote:
> > This series starts by moving the common definitions of the QMUX
> > protocol to the
> > uapi header, as they are shared with clients - both in kernel and
> > userspace.
> > 
> > This series then introduces in-kernel helper functions for aiding the
> > handling
> > of QMI encoded messages in the kernel. QMI encoding is a wire-format
> > used in
> > exchanging messages between the majority of QRTR clients and
> > services.
> 
> This raises a few red-flags for me.

I'm glad it does. In discussions with the responsible team within
Qualcomm I've highlighted a number of concerns about enabling this
support in the kernel. Together we're continuously looking into what
should be pushed out to user space, and trying to not introduce
unnecessary new users.

> So far, we've kept almost everything QMI related in userspace and
> handled all QMI control-channel messages from libraries like libqmi or
> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
> driver.  The kernel drivers just serve as the transport.
> 

The path that was taken to support the MSM-style devices was to
implement net/qrtr, which exposes a socket interface to abstract the
physical transports (QMUX or IPCROUTER in Qualcomm terminology).

As I share you view on letting the kernel handle the transportation only
the task of keeping track of registered services (service id -> node and
port mapping) was done in a user space process and so far we've only
ever have to deal with QMI encoded messages in various user space tools.

> Can you describe what kinds of in-kernel drivers need to actually parse
> QMI messages as part of their operation?
> 

= "sysmon"
sysmon is a mechanism in Qualcomm devices to notify remote processors
when other remote processors goes away. This is required for such things
as letting the modem firmware know that the audio path is gone in the
case that the adsp abruptly disappears - i.e. when the adsp hits a
watchdog bite and doesn't have a chance to close the modem<->audio
communication channels.

In previous platforms the sysmon messages was just handled by packed
structs, but this was changed to being done using QMI and IPCROUTER
recently.

These messages needs to be sent before we attempt to restart the remote
processor and doing this from user space causes synchronization issues.

= Qualcomm slimbus implementation
The slimbus implementation in Qualcomm MSM devices expects a few QMI
encoded control messages to be sent for configuration and power
management purposes. So in order to get audio from the audio blocks to
the codec we need to send a few QMI encoded messages in the Qualcomm
slimbus driver.

As this is used to communicate power management states from the kernel
driver I see it infeasible to do this from user space.

= IPQ8074 WiFi driver
In many Qualcomm SoC the WiFi solution is split between an in-SoC core
that does protocol handling and an off-chip RF module. The communication
of control messages with the protocol core is done over shared memory
transports (SMD or GLINK) and has in the past (WCN3620, 3660 and 3680)
been done with packed structs. In the latest incarnation this is
replaced by QMI-encoded messages.

Qualcomm is trying to move forward in upstreaming a driver for this, as
part of their push to get IPQ8074 support upstream. I have not reviewed
the new version of this driver, but the driver for the previous
generation dealt with a mixture of control messages, dealing with DMA
channels and direct hardware control - so this does indeed look to
require in-kernel QMI messaging.

Regards,
Bjorn

> Dan
> 
> > It then adds an abstractions to reduce the duplication of common code
> > in
> > drivers that needs to query the name server and send and receive
> > encoded
> > messages to a remote service.
> > 
> > Finally it introduces a sample implementation for showing QRTR and
> > the QMI
> > helpers in action. The sample device instantiates in response to
> > finding the
> > "test service" and implements the "test protocol".
> > 
> > Bjorn Andersson (6):
> >   net: qrtr: Invoke sk_error_report() after setting sk_err
> >   net: qrtr: Move constants to header file
> >   net: qrtr: Add control packet definition to uapi
> >   soc: qcom: Introduce QMI encoder/decoder
> >   soc: qcom: Introduce QMI helpers
> >   samples: Introduce Qualcomm QRTR sample client
> > 
> >  drivers/soc/qcom/Kconfig          |   8 +
> >  drivers/soc/qcom/Makefile         |   3 +
> >  drivers/soc/qcom/qmi_encdec.c     | 812
> > ++++++++++++++++++++++++++++++++++++++
> >  drivers/soc/qcom/qmi_interface.c  | 540 +++++++++++++++++++++++++
> >  include/linux/soc/qcom/qmi.h      | 249 ++++++++++++
> >  include/uapi/linux/qrtr.h         |  35 ++
> >  net/qrtr/qrtr.c                   |  16 +-
> >  samples/Kconfig                   |   8 +
> >  samples/Makefile                  |   2 +-
> >  samples/qrtr/Makefile             |   1 +
> >  samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++
> >  11 files changed, 2261 insertions(+), 16 deletions(-)
> >  create mode 100644 drivers/soc/qcom/qmi_encdec.c
> >  create mode 100644 drivers/soc/qcom/qmi_interface.c
> >  create mode 100644 include/linux/soc/qcom/qmi.h
> >  create mode 100644 samples/qrtr/Makefile
> >  create mode 100644 samples/qrtr/qrtr_sample_client.c
> > 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-07 17:38   ` Bjorn Andersson
@ 2017-08-07 19:19     ` Marcel Holtmann
  2017-08-08  4:45       ` Bjorn Andersson
  0 siblings, 1 reply; 16+ messages in thread
From: Marcel Holtmann @ 2017-08-07 19:19 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Dan Williams, David S. Miller, Andy Gross, David Brown,
	linux-arm-msm, linux-soc, Netdev list, linux-kernel

Hi Bjorn,

>>> This series starts by moving the common definitions of the QMUX
>>> protocol to the
>>> uapi header, as they are shared with clients - both in kernel and
>>> userspace.
>>> 
>>> This series then introduces in-kernel helper functions for aiding the
>>> handling
>>> of QMI encoded messages in the kernel. QMI encoding is a wire-format
>>> used in
>>> exchanging messages between the majority of QRTR clients and
>>> services.
>> 
>> This raises a few red-flags for me.
> 
> I'm glad it does. In discussions with the responsible team within
> Qualcomm I've highlighted a number of concerns about enabling this
> support in the kernel. Together we're continuously looking into what
> should be pushed out to user space, and trying to not introduce
> unnecessary new users.
> 
>> So far, we've kept almost everything QMI related in userspace and
>> handled all QMI control-channel messages from libraries like libqmi or
>> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
>> driver.  The kernel drivers just serve as the transport.
>> 
> 
> The path that was taken to support the MSM-style devices was to
> implement net/qrtr, which exposes a socket interface to abstract the
> physical transports (QMUX or IPCROUTER in Qualcomm terminology).
> 
> As I share you view on letting the kernel handle the transportation only
> the task of keeping track of registered services (service id -> node and
> port mapping) was done in a user space process and so far we've only
> ever have to deal with QMI encoded messages in various user space tools.

I think that the transport and multiplexing can be in the kernel as long as it is done as proper subsystem. Similar to Phonet or CAIF. Meaning it should have a well defined socket interface that can be easily used from userspace, but also a clean in-kernel interface handling.

If Qualcomm is supportive of this effort and is willing to actually assist and/or open some of the specs or interface descriptions, then this is a good thing. Service registration and cleanup is really done best in the kernel. Same applies to multiplexing. Trying to do multiplexing in userspace is always cumbersome and leads to overhead that is of no gain. For example within oFono, we had to force everything to go via oFono since it was the only sane way of handling it. Other approaches were error prone and full of race conditions. You need a central entity that can clean up.

For the definition of an UAPI to share some code, I am actually not sure that is such a good idea. For example the QMI code in oFono follows a way simpler approach. And I am not convinced that all the macros are actually beneficial. For example, the whole netlink macros are pretty cumbersome. Adding some Documentation/qmi.txt on how the wire format looks like and what is expected seems to be a way better approach.

Regards

Marcel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-07 19:19     ` Marcel Holtmann
@ 2017-08-08  4:45       ` Bjorn Andersson
  2017-08-08  6:15         ` Marcel Holtmann
  0 siblings, 1 reply; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-08  4:45 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Dan Williams, David S. Miller, Andy Gross, David Brown,
	linux-arm-msm, linux-soc, Netdev list, linux-kernel

On Mon 07 Aug 12:19 PDT 2017, Marcel Holtmann wrote:

> Hi Bjorn,
> 
> >>> This series starts by moving the common definitions of the QMUX
> >>> protocol to the
> >>> uapi header, as they are shared with clients - both in kernel and
> >>> userspace.
> >>> 
> >>> This series then introduces in-kernel helper functions for aiding the
> >>> handling
> >>> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> >>> used in
> >>> exchanging messages between the majority of QRTR clients and
> >>> services.
> >> 
> >> This raises a few red-flags for me.
> > 
> > I'm glad it does. In discussions with the responsible team within
> > Qualcomm I've highlighted a number of concerns about enabling this
> > support in the kernel. Together we're continuously looking into what
> > should be pushed out to user space, and trying to not introduce
> > unnecessary new users.
> > 
> >> So far, we've kept almost everything QMI related in userspace and
> >> handled all QMI control-channel messages from libraries like libqmi or
> >> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
> >> driver.  The kernel drivers just serve as the transport.
> >> 
> > 
> > The path that was taken to support the MSM-style devices was to
> > implement net/qrtr, which exposes a socket interface to abstract the
> > physical transports (QMUX or IPCROUTER in Qualcomm terminology).
> > 
> > As I share you view on letting the kernel handle the transportation only
> > the task of keeping track of registered services (service id -> node and
> > port mapping) was done in a user space process and so far we've only
> > ever have to deal with QMI encoded messages in various user space tools.
> 
> I think that the transport and multiplexing can be in the kernel as
> long as it is done as proper subsystem. Similar to Phonet or CAIF.
> Meaning it should have a well defined socket interface that can be
> easily used from userspace, but also a clean in-kernel interface
> handling.
> 

In a mobile Qualcomm device there's a few different components involved
here: message routing, QMUX protocol and QMI-encoding.

The downstream Qualcomm kernel implements the two first in the
IPCROUTER, upstream this is split between the kernel net/qrtr and a user
space service-register implementing the QMUX protocol for knowing where
services are located.

The common encoding of messages passed between endpoints of the message
routing is QMI, which is made an affair totally that of each client.

> If Qualcomm is supportive of this effort and is willing to actually
> assist and/or open some of the specs or interface descriptions, then
> this is a good thing. Service registration and cleanup is really done
> best in the kernel. Same applies to multiplexing. Trying to do
> multiplexing in userspace is always cumbersome and leads to overhead
> that is of no gain. For example within oFono, we had to force
> everything to go via oFono since it was the only sane way of handling
> it. Other approaches were error prone and full of race conditions. You
> need a central entity that can clean up.
> 

The current upstream solution depends on a collaboration between
net/qrtr and the user space service register for figuring out whom to
send messages to. After that muxing et al is handled by the socket
interface and service registry does not need to be involved.

Qualcomm is very supporting of this solution and we're collaborating on
transitioning "downstream" to use this implementation.

> For the definition of an UAPI to share some code, I am actually not
> sure that is such a good idea. For example the QMI code in oFono
> follows a way simpler approach. And I am not convinced that all the
> macros are actually beneficial. For example, the whole netlink macros
> are pretty cumbersome. Adding some Documentation/qmi.txt on how the
> wire format looks like and what is expected seems to be a way better
> approach.
> 

The socket interface provided by the kernel expects some knowledge of
the QMUX protocol, for service management. The majority of this
knowledge is already public, but I agree that it would be good to gather
this in a document. The common data structure for the control message is
what I've put in the uapi, as this is used by anyone dealing with
control messages.

When it comes to the QMI-encoded messages these are application
specific, just like e.g. protobuf definitions are application specific.

As the core infrastructure is becoming available upstream and boards
like the DB410c and DB820c aim to be supported by open solutions we will
have a natural place to discuss publication of at least some of the
application level protocols.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-08  4:45       ` Bjorn Andersson
@ 2017-08-08  6:15         ` Marcel Holtmann
  0 siblings, 0 replies; 16+ messages in thread
From: Marcel Holtmann @ 2017-08-08  6:15 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Dan Williams, David S. Miller, Andy Gross, David Brown,
	linux-arm-msm, linux-soc, Netdev list, linux-kernel

Hi Bjorn,

>>>>> This series starts by moving the common definitions of the QMUX
>>>>> protocol to the
>>>>> uapi header, as they are shared with clients - both in kernel and
>>>>> userspace.
>>>>> 
>>>>> This series then introduces in-kernel helper functions for aiding the
>>>>> handling
>>>>> of QMI encoded messages in the kernel. QMI encoding is a wire-format
>>>>> used in
>>>>> exchanging messages between the majority of QRTR clients and
>>>>> services.
>>>> 
>>>> This raises a few red-flags for me.
>>> 
>>> I'm glad it does. In discussions with the responsible team within
>>> Qualcomm I've highlighted a number of concerns about enabling this
>>> support in the kernel. Together we're continuously looking into what
>>> should be pushed out to user space, and trying to not introduce
>>> unnecessary new users.
>>> 
>>>> So far, we've kept almost everything QMI related in userspace and
>>>> handled all QMI control-channel messages from libraries like libqmi or
>>>> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
>>>> driver.  The kernel drivers just serve as the transport.
>>>> 
>>> 
>>> The path that was taken to support the MSM-style devices was to
>>> implement net/qrtr, which exposes a socket interface to abstract the
>>> physical transports (QMUX or IPCROUTER in Qualcomm terminology).
>>> 
>>> As I share you view on letting the kernel handle the transportation only
>>> the task of keeping track of registered services (service id -> node and
>>> port mapping) was done in a user space process and so far we've only
>>> ever have to deal with QMI encoded messages in various user space tools.
>> 
>> I think that the transport and multiplexing can be in the kernel as
>> long as it is done as proper subsystem. Similar to Phonet or CAIF.
>> Meaning it should have a well defined socket interface that can be
>> easily used from userspace, but also a clean in-kernel interface
>> handling.
>> 
> 
> In a mobile Qualcomm device there's a few different components involved
> here: message routing, QMUX protocol and QMI-encoding.
> 
> The downstream Qualcomm kernel implements the two first in the
> IPCROUTER, upstream this is split between the kernel net/qrtr and a user
> space service-register implementing the QMUX protocol for knowing where
> services are located.

as long as all of QMUX moves into the kernel and userspace doesn’t need to know about QMUX anymore, that would be good. The cross termination of QMUX in kernel space and userspace is a really bad idea. It is even worse if userspace has to do service registration. That is just a recipe for disaster.

One extra thing to keep in mind is that all the USB dongle should register with such a new QMI subsystem. And have their network interfaces being proper children of the QMI node. And please do not forget QMI passthrough via MBIM. Just saying we move some QMUX code into the kernel is not enough. It really needs to be a proper subsystem with a proper hierarchy of the child devices.

> The common encoding of messages passed between endpoints of the message
> routing is QMI, which is made an affair totally that of each client.
> 
>> If Qualcomm is supportive of this effort and is willing to actually
>> assist and/or open some of the specs or interface descriptions, then
>> this is a good thing. Service registration and cleanup is really done
>> best in the kernel. Same applies to multiplexing. Trying to do
>> multiplexing in userspace is always cumbersome and leads to overhead
>> that is of no gain. For example within oFono, we had to force
>> everything to go via oFono since it was the only sane way of handling
>> it. Other approaches were error prone and full of race conditions. You
>> need a central entity that can clean up.
>> 
> 
> The current upstream solution depends on a collaboration between
> net/qrtr and the user space service register for figuring out whom to
> send messages to. After that muxing et al is handled by the socket
> interface and service registry does not need to be involved.
> 
> Qualcomm is very supporting of this solution and we're collaborating on
> transitioning "downstream" to use this implementation.

It would be good if someone looks into oFono and makes sure that it works there as well. I would prefer at least some initial patches to proof-point the kernel APIs. oFono is a full telephony stack. So if you can make that one work, then you are most likely on the right track.

>> For the definition of an UAPI to share some code, I am actually not
>> sure that is such a good idea. For example the QMI code in oFono
>> follows a way simpler approach. And I am not convinced that all the
>> macros are actually beneficial. For example, the whole netlink macros
>> are pretty cumbersome. Adding some Documentation/qmi.txt on how the
>> wire format looks like and what is expected seems to be a way better
>> approach.
>> 
> 
> The socket interface provided by the kernel expects some knowledge of
> the QMUX protocol, for service management. The majority of this
> knowledge is already public, but I agree that it would be good to gather
> this in a document. The common data structure for the control message is
> what I've put in the uapi, as this is used by anyone dealing with
> control messages.
> 
> When it comes to the QMI-encoded messages these are application
> specific, just like e.g. protobuf definitions are application specific.

That is fine, but what I like to see is at least a documentation that clearly documents the boundaries and examples on how the socket interface is suppose to use. For example using DMS to get the model id etc. would be a simple enough example that could be easily added.

> As the core infrastructure is becoming available upstream and boards
> like the DB410c and DB820c aim to be supported by open solutions we will
> have a natural place to discuss publication of at least some of the
> application level protocols.

I would really welcome public documentation of it. That would be great.

Regards

Marcel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
                   ` (6 preceding siblings ...)
  2017-08-04 15:36 ` [PATCH 0/6] In-kernel QMI handling Dan Williams
@ 2017-08-08 11:02 ` Bjørn Mork
  2017-08-08 11:13   ` Marcel Holtmann
  2017-08-08 22:42   ` Bjorn Andersson
  7 siblings, 2 replies; 16+ messages in thread
From: Bjørn Mork @ 2017-08-08 11:02 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: David S. Miller, Andy Gross, David Brown, linux-arm-msm,
	linux-soc, netdev, linux-kernel

Bjorn Andersson <bjorn.andersson@linaro.org> writes:

> This series starts by moving the common definitions of the QMUX protocol to the
> uapi header, as they are shared with clients - both in kernel and userspace.
>
> This series then introduces in-kernel helper functions for aiding the handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
> exchanging messages between the majority of QRTR clients and services.

Interesting!  I tried to add some QMI handling in the kernel a few years
ago, but was thankfully voted down.  See
https://www.spinics.net/lists/netdev/msg183101.html and the following
discussion. I am convinced that was the right decision, for the client
side at least. The protocol is just too extensive and ever-growing to be
implemented in the kernel. We would be catching up forever.

Note that I had very limited knowledge of the protocol at the time I
wrote that driver.  Still have, in fact :-)


Bjørn

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-08 11:02 ` Bjørn Mork
@ 2017-08-08 11:13   ` Marcel Holtmann
  2017-08-08 22:42   ` Bjorn Andersson
  1 sibling, 0 replies; 16+ messages in thread
From: Marcel Holtmann @ 2017-08-08 11:13 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Bjorn Andersson, David S. Miller, Andy Gross, David Brown,
	linux-arm-msm, linux-soc, Netdev list, linux-kernel

Hi Bjorn,

>> This series starts by moving the common definitions of the QMUX protocol to the
>> uapi header, as they are shared with clients - both in kernel and userspace.
>> 
>> This series then introduces in-kernel helper functions for aiding the handling
>> of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
>> exchanging messages between the majority of QRTR clients and services.
> 
> Interesting!  I tried to add some QMI handling in the kernel a few years
> ago, but was thankfully voted down.  See
> https://www.spinics.net/lists/netdev/msg183101.html and the following
> discussion. I am convinced that was the right decision, for the client
> side at least. The protocol is just too extensive and ever-growing to be
> implemented in the kernel. We would be catching up forever.

I think that even back then I said, that it has to be done as a proper subsystem if it has a chance to be in the kernel. So something similar to Phonet and CAIF where the service registration is handled by the kernel, but applications can be fully in userspace. None of this is actually brand new Qualcomm design since Nokia has had its Phonet long before QMI existed.

The real importance is that Qualcomm is behind this and wants to get this done a clean way with a proper API. The /dev/qmi thing was a pretty broken interface. Any subsystem has to support multiple QMI devices. Even if this is unlikely in a phone design, it has to be supported so that attaching two USB QMI based dongles does not end up with some pointless errors.

Regards

Marcel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-08 11:02 ` Bjørn Mork
  2017-08-08 11:13   ` Marcel Holtmann
@ 2017-08-08 22:42   ` Bjorn Andersson
  2017-08-09  0:48     ` Dan Williams
  1 sibling, 1 reply; 16+ messages in thread
From: Bjorn Andersson @ 2017-08-08 22:42 UTC (permalink / raw)
  To: Bj?rn Mork
  Cc: David S. Miller, Andy Gross, David Brown, linux-arm-msm,
	linux-soc, netdev, linux-kernel

On Tue 08 Aug 04:02 PDT 2017, Bj?rn Mork wrote:

> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> 
> > This series starts by moving the common definitions of the QMUX protocol to the
> > uapi header, as they are shared with clients - both in kernel and userspace.
> >
> > This series then introduces in-kernel helper functions for aiding the handling
> > of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
> > exchanging messages between the majority of QRTR clients and services.
> 
> Interesting!  I tried to add some QMI handling in the kernel a few years
> ago, but was thankfully voted down.  See
> https://www.spinics.net/lists/netdev/msg183101.html and the following
> discussion. I am convinced that was the right decision, for the client
> side at least. The protocol is just too extensive and ever-growing to be
> implemented in the kernel. We would be catching up forever.
> 
> Note that I had very limited knowledge of the protocol at the time I
> wrote that driver.  Still have, in fact :-)
> 

Thanks for the pointer, I definitely think there's more work to be done
here to figure out the proper way to interact with these devices.

But I think that Dan's reply shows a huge source of confusion here; the
acronym "QMI" covers a large amount of different things - and means
different things for different people.

In the modem world QMI seems to mean a defined set of logical endpoints
that accepts TLV-encoded messages to do modem-related things. But the
TLV-encoding is used for non-modem related services and the only common
denominator of everything called QMI is the TLV-encoding.


Due to my limited exposure to the USB attached "QMI thingies" I haven't
previously looked into the exact differences. The proposed patches aimed
to support implementing a few non-modem-related clients using
QMI-encoded messages over ipcrouter.

Looking at your patch above, and oPhono, seems to highlight a few
important differences that will take some thinking to overcome.

= Transport
The transport header in the USB case is your struct qmux, which contains
the type of message (in "flags") and the transaction id. The "service"
in the QMUX header matches the service id being communicated with. But
in order to communicate with a service it seems like one requests a
client-id from the control service.

In the smartphone world (with shared memory communication) the transport
is ipcrouter - with a header very similar to UDP - and there's no
information about the payload, it provides only the means of delivering
messages from one address/port to another address/port. A typical
smartphone has 3-4 nodes (modem, sensors, audio etc) and ports are
dynamically allocated. The control messages in the QMUX protocol (not
the same QMUX protocol as in the USB case!) are used for clients to find
the mapping from service id to a port on the given address.  The source
port is dynamically allocated in this case.

= QMI-encoded messages
The list of TLV-entries have a "QMI header" prepended in both cases, but
in the QMUX case the header consists only of "msgid" and length.

In the ipcrouter case the transport doesn't carry any information
regarding the payload, so the header prepended the TLV entries includes
"type", transaction id, "msg_id" and length.


It looks as if once past the differences in the transport and QMI
message header the messages (TLV-encoded data) are the same. But I'm not
yet sure about how we can hide the transport differences.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] In-kernel QMI handling
  2017-08-08 22:42   ` Bjorn Andersson
@ 2017-08-09  0:48     ` Dan Williams
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2017-08-09  0:48 UTC (permalink / raw)
  To: Bjorn Andersson, Bj?rn Mork
  Cc: David S. Miller, Andy Gross, David Brown, linux-arm-msm,
	linux-soc, netdev, linux-kernel

On Tue, 2017-08-08 at 15:42 -0700, Bjorn Andersson wrote:
> On Tue 08 Aug 04:02 PDT 2017, Bj?rn Mork wrote:
> 
> > Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> > 
> > > This series starts by moving the common definitions of the QMUX
> > > protocol to the
> > > uapi header, as they are shared with clients - both in kernel and
> > > userspace.
> > > 
> > > This series then introduces in-kernel helper functions for aiding
> > > the handling
> > > of QMI encoded messages in the kernel. QMI encoding is a wire-
> > > format used in
> > > exchanging messages between the majority of QRTR clients and
> > > services.
> > 
> > Interesting!  I tried to add some QMI handling in the kernel a few
> > years
> > ago, but was thankfully voted down.  See
> > https://www.spinics.net/lists/netdev/msg183101.html and the
> > following
> > discussion. I am convinced that was the right decision, for the
> > client
> > side at least. The protocol is just too extensive and ever-growing
> > to be
> > implemented in the kernel. We would be catching up forever.
> > 
> > Note that I had very limited knowledge of the protocol at the time
> > I
> > wrote that driver.  Still have, in fact :-)
> > 
> 
> Thanks for the pointer, I definitely think there's more work to be
> done
> here to figure out the proper way to interact with these devices.
> 
> But I think that Dan's reply shows a huge source of confusion here;
> the
> acronym "QMI" covers a large amount of different things - and means
> different things for different people.

I would agree, sorry for any confusion caused.  Great discussion so
far.

> In the modem world QMI seems to mean a defined set of logical
> endpoints
> that accepts TLV-encoded messages to do modem-related things. But the
> TLV-encoding is used for non-modem related services and the only
> common
> denominator of everything called QMI is the TLV-encoding.
> 
> 
> Due to my limited exposure to the USB attached "QMI thingies" I
> haven't
> previously looked into the exact differences. The proposed patches
> aimed
> to support implementing a few non-modem-related clients using
> QMI-encoded messages over ipcrouter.
> 
> Looking at your patch above, and oPhono, seems to highlight a few
> important differences that will take some thinking to overcome.
> 
> = Transport
> The transport header in the USB case is your struct qmux, which
> contains
> the type of message (in "flags") and the transaction id. The
> "service"
> in the QMUX header matches the service id being communicated with.
> But
> in order to communicate with a service it seems like one requests a
> client-id from the control service.

Correct.  You cannot talk to a service on the modem without getting an
allocated client ID from the CTL service, which has a well-defined
client ID.

> In the smartphone world (with shared memory communication) the
> transport
> is ipcrouter - with a header very similar to UDP - and there's no
> information about the payload, it provides only the means of
> delivering

Can you explain a bit about the relationship of SMD to [I/R]PC, qrtr,
and QMI?  A couple years ago there was smd_qmi.c (like for the Nexus 4
with APQ8064 and a discrete MDM9215) which from a 10 minute fresh look
appears to just push QMUX+QMI via SMD rather than being backed by the
RPC/IPC stuff.  I could be wrong, there's a lot of indirection there
and it may well end up going over the router.  But that's buried deeper
than a 10m look for me.

Is it perhaps only with on-chip blocks where the QMUX/QMI/qrtr/irpc
stuff you describe here is used?  If so, perhaps that's the distinction
to be made.  I'll let you correct me here since you clearly know more
than I about the internals of these devices.

> messages from one address/port to another address/port. A typical
> smartphone has 3-4 nodes (modem, sensors, audio etc) and ports are
> dynamically allocated. The control messages in the QMUX protocol (not
> the same QMUX protocol as in the USB case!) are used for clients to
> find
> the mapping from service id to a port on the given address.  The
> source
> port is dynamically allocated in this case.
> 
> = QMI-encoded messages
> The list of TLV-entries have a "QMI header" prepended in both cases,
> but
> in the QMUX case the header consists only of "msgid" and length.
> 
> In the ipcrouter case the transport doesn't carry any information
> regarding the payload, so the header prepended the TLV entries
> includes
> "type", transaction id, "msg_id" and length.

I'll assume that in this case, because the client has already found out
how to contact the target service directly, that it has no use for a
"fat" QMUX header that includes the client ID and service stuff.

I don't really have an issue with the kernel doing "thin" QMUX-related
stuff.  That's pretty simple.

> It looks as if once past the differences in the transport and QMI
> message header the messages (TLV-encoded data) are the same. But I'm
> not
> yet sure about how we can hide the transport differences.

QMI itself is really just a header + TLVs.  So it makes sense that it
would be loosely repurposed since it's pretty generic.  All the
interesting stuff is actually in the services themselves and the
messages that the services respond to.

What's confusing me a lot so far is exactly *how* QMI itself gets used
in-kernel.  Instead of the samples that you've provided in this
patchset, could you point me to an actual in-kernel driver that would
use something like qmi_send_message()?  I can't properly evaluate
whether I have further comments on your approach without some specific
in-kernel use-cases.

Thanks,
Dan

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-08-09  0:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 14:59 [PATCH 0/6] In-kernel QMI handling Bjorn Andersson
2017-08-04 14:59 ` [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err Bjorn Andersson
2017-08-04 14:59 ` [PATCH 2/6] net: qrtr: Move constants to header file Bjorn Andersson
2017-08-04 14:59 ` [PATCH 3/6] net: qrtr: Add control packet definition to uapi Bjorn Andersson
2017-08-04 14:59 ` [PATCH 4/6] soc: qcom: Introduce QMI encoder/decoder Bjorn Andersson
2017-08-04 14:59 ` [PATCH 5/6] soc: qcom: Introduce QMI helpers Bjorn Andersson
2017-08-04 14:59 ` [PATCH 6/6] samples: Introduce Qualcomm QRTR sample client Bjorn Andersson
2017-08-04 15:36 ` [PATCH 0/6] In-kernel QMI handling Dan Williams
2017-08-07 17:38   ` Bjorn Andersson
2017-08-07 19:19     ` Marcel Holtmann
2017-08-08  4:45       ` Bjorn Andersson
2017-08-08  6:15         ` Marcel Holtmann
2017-08-08 11:02 ` Bjørn Mork
2017-08-08 11:13   ` Marcel Holtmann
2017-08-08 22:42   ` Bjorn Andersson
2017-08-09  0:48     ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).