Merge "soc: qcom: port some changes from kernel 4.4"
diff --git a/drivers/soc/qcom/spcom.c b/drivers/soc/qcom/spcom.c
index f381f16..1c7c4a1 100644
--- a/drivers/soc/qcom/spcom.c
+++ b/drivers/soc/qcom/spcom.c
@@ -53,28 +53,25 @@
 /* Uncomment the line below to test spcom against modem rather than SP */
 /* #define SPCOM_TEST_HLOS_WITH_MODEM 1 */
 
-/* Uncomment the line below to enable debug messages */
-/* #define DEBUG 1 */
-
 #define pr_fmt(fmt)	"spcom [%s]: " fmt, __func__
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/device.h>
-#include <linux/slab.h>
-#include <linux/fs.h>
-#include <linux/cdev.h>
-#include <linux/errno.h>
-#include <linux/printk.h>
-#include <linux/bitops.h>
-#include <linux/completion.h>
-#include <linux/poll.h>
-#include <linux/dma-mapping.h>
+#include <linux/kernel.h>	/* min() */
+#include <linux/module.h>	/* MODULE_LICENSE */
+#include <linux/device.h>	/* class_create() */
+#include <linux/slab.h>	/* kzalloc() */
+#include <linux/fs.h>		/* file_operations */
+#include <linux/cdev.h>	/* cdev_add() */
+#include <linux/errno.h>	/* EINVAL, ETIMEDOUT */
+#include <linux/printk.h>	/* pr_err() */
+#include <linux/bitops.h>	/* BIT(x) */
+#include <linux/completion.h>	/* wait_for_completion_timeout() */
+#include <linux/poll.h>	/* POLLOUT */
+#include <linux/dma-mapping.h>	/* dma_alloc_coherent() */
 #include <linux/platform_device.h>
-#include <linux/of.h>
+#include <linux/of.h>		/* of_property_count_strings() */
 #include <linux/workqueue.h>
-#include <linux/delay.h>
-#include <linux/msm_ion.h>
+#include <linux/delay.h>	/* msleep() */
+#include <linux/msm_ion.h>	/* msm_ion_client_create() */
 
 #include <soc/qcom/glink.h>
 #include <soc/qcom/smem.h>
@@ -82,7 +79,7 @@
 
 #include <uapi/linux/spcom.h>
 
-#include "glink_private.h"
+#include "glink_private.h" /* glink_ssr() */
 
 /* "SPCM" string */
 #define SPCOM_MAGIC_ID	((uint32_t)(0x5350434D))
@@ -220,9 +217,9 @@ struct spcom_channel {
 	bool tx_abort;
 
 	/* rx data info */
-	int rx_buf_size;	/* allocated rx buffer size */
+	size_t rx_buf_size;	/* allocated rx buffer size */
 	bool rx_buf_ready;
-	int actual_rx_size;	/* actual data size received */
+	size_t actual_rx_size;	/* actual data size received */
 	const void *glink_rx_buf;
 
 	/* ION lock/unlock support */
@@ -276,6 +273,7 @@ static void spcom_notify_rx_abort(void *handle, const void *priv,
 				  const void *pkt_priv);
 static struct spcom_channel *spcom_find_channel_by_name(const char *name);
 static int spcom_unlock_ion_buf(struct spcom_channel *ch, int fd);
+static void spcom_rx_abort_pending_server(void);
 
 /**
  * spcom_is_ready() - driver is initialized and ready.
@@ -301,6 +299,10 @@ static inline bool spcom_is_channel_open(struct spcom_channel *ch)
  */
 static inline bool spcom_is_channel_connected(struct spcom_channel *ch)
 {
+	/* Channel must be open before it gets connected */
+	if (!spcom_is_channel_open(ch))
+		return false;
+
 	return (ch->glink_state == GLINK_CONNECTED);
 }
 
@@ -316,6 +318,10 @@ static int spcom_create_predefined_channels_chardev(void)
 {
 	int i;
 	int ret;
+	static bool is_predefined_created;
+
+	if (is_predefined_created)
+		return 0;
 
 	for (i = 0; i < SPCOM_MAX_CHANNELS; i++) {
 		const char *name = spcom_dev->predefined_ch_name[i];
@@ -330,6 +336,8 @@ static int spcom_create_predefined_channels_chardev(void)
 		}
 	}
 
+	is_predefined_created = true;
+
 	return 0;
 }
 
@@ -352,6 +360,16 @@ static void spcom_link_state_notif_cb(struct glink_link_state_cb_info *cb_info,
 	struct spcom_channel *ch = NULL;
 	const char *ch_name = "sp_kernel";
 
+	if (!cb_info) {
+		pr_err("invalid NULL cb_info param\n");
+		return;
+	}
+
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return;
+	}
+
 	spcom_dev->link_state = cb_info->link_state;
 
 	pr_debug("spcom_link_state_notif_cb called. transport = %s edge = %s\n",
@@ -375,6 +393,9 @@ static void spcom_link_state_notif_cb(struct glink_link_state_cb_info *cb_info,
 			pr_err("failed to find channel [%s].\n", ch_name);
 		else
 			spcom_unlock_ion_buf(ch, SPCOM_ION_FD_UNLOCK_ALL);
+
+		pr_debug("Rx-Abort pending servers.\n");
+		spcom_rx_abort_pending_server();
 		break;
 	default:
 		pr_err("unknown link_state [%d].\n", cb_info->link_state);
@@ -396,13 +417,17 @@ static void spcom_notify_rx(void *handle,
 	struct spcom_channel *ch = (struct spcom_channel *) priv;
 
 	if (!ch) {
-		pr_err("invalid ch parameter.\n");
+		pr_err("invalid NULL channel param\n");
+		return;
+	}
+	if (!buf) {
+		pr_err("invalid NULL buf param\n");
 		return;
 	}
 
-	pr_debug("ch [%s] rx size [%d].\n", ch->name, (int) size);
+	pr_debug("ch [%s] rx size [%zu]\n", ch->name, size);
 
-	ch->actual_rx_size = (int) size;
+	ch->actual_rx_size = size;
 	ch->glink_rx_buf = (void *) buf;
 
 	complete_all(&ch->rx_done);
@@ -421,7 +446,11 @@ static void spcom_notify_tx_done(void *handle,
 	int *tx_buf = (int *) buf;
 
 	if (!ch) {
-		pr_err("invalid ch parameter.\n");
+		pr_err("invalid NULL channel param\n");
+		return;
+	}
+	if (!buf) {
+		pr_err("invalid NULL buf param\n");
 		return;
 	}
 
@@ -446,11 +475,15 @@ static void spcom_notify_state(void *handle, const void *priv,
 	int ret;
 	struct spcom_channel *ch = (struct spcom_channel *) priv;
 
+	if (!ch) {
+		pr_err("invalid NULL channel param\n");
+		return;
+	}
+
 	switch (event) {
 	case GLINK_CONNECTED:
 		pr_debug("GLINK_CONNECTED, ch name [%s].\n", ch->name);
-		complete_all(&ch->connect);
-
+		ch->glink_state = event;
 		/*
 		 * if spcom_notify_state() is called within glink_open()
 		 * then ch->glink_handle is not updated yet.
@@ -466,10 +499,11 @@ static void spcom_notify_state(void *handle, const void *priv,
 		if (ret) {
 			pr_err("glink_queue_rx_intent() err [%d]\n", ret);
 		} else {
-			pr_debug("rx buf is ready, size [%d].\n",
+			pr_debug("rx buf is ready, size [%zu].\n",
 				 ch->rx_buf_size);
 			ch->rx_buf_ready = true;
 		}
+		complete_all(&ch->connect);
 		break;
 	case GLINK_LOCAL_DISCONNECTED:
 		/*
@@ -477,6 +511,7 @@ static void spcom_notify_state(void *handle, const void *priv,
 		 * only after *both* sides closed the channel.
 		 */
 		pr_debug("GLINK_LOCAL_DISCONNECTED, ch [%s].\n", ch->name);
+		ch->glink_state = event;
 		complete_all(&ch->disconnect);
 		break;
 	case GLINK_REMOTE_DISCONNECTED:
@@ -487,6 +522,8 @@ static void spcom_notify_state(void *handle, const void *priv,
 		 */
 		pr_err("GLINK_REMOTE_DISCONNECTED, ch [%s].\n", ch->name);
 
+		ch->glink_state = event;
+
 		/*
 		 * Abort any blocking read() operation.
 		 * The glink notification might be after REMOTE_DISCONNECT.
@@ -504,8 +541,6 @@ static void spcom_notify_state(void *handle, const void *priv,
 		       (int) event, ch->name);
 		return;
 	}
-
-	ch->glink_state = event;
 }
 
 /**
@@ -539,9 +574,14 @@ static void spcom_notify_rx_abort(void *handle, const void *priv,
 {
 	struct spcom_channel *ch = (struct spcom_channel *) priv;
 
+	if (!ch) {
+		pr_err("invalid NULL channel param\n");
+		return;
+	}
+
 	pr_debug("ch [%s] pending rx aborted.\n", ch->name);
 
-	if (spcom_is_channel_connected(ch) && (!ch->rx_abort)) {
+	if (spcom_is_channel_open(ch) && (!ch->rx_abort)) {
 		ch->rx_abort = true;
 		complete_all(&ch->rx_done);
 	}
@@ -559,6 +599,11 @@ static void spcom_notify_tx_abort(void *handle, const void *priv,
 {
 	struct spcom_channel *ch = (struct spcom_channel *) priv;
 
+	if (!ch) {
+		pr_err("invalid NULL channel param\n");
+		return;
+	}
+
 	pr_debug("ch [%s] pending tx aborted.\n", ch->name);
 
 	if (spcom_is_channel_connected(ch) && (!ch->tx_abort)) {
@@ -672,12 +717,11 @@ static int spcom_open(struct spcom_channel *ch, unsigned int timeout_msec)
 
 	/* only one client/server may use the channel */
 	if (ch->ref_count) {
-		pr_err("channel [%s] already in use.\n", name);
-		goto exit_err;
+		pr_err("channel [%s] is BUSY, already in use by pid [%d].\n",
+			name, ch->pid);
+		mutex_unlock(&ch->lock);
+		return -EBUSY;
 	}
-	ch->ref_count++;
-	ch->pid = current_pid();
-	ch->txn_id = INITIAL_TXN_ID;
 
 	pr_debug("ch [%s] opened by PID [%d], count [%d]\n",
 		 name, ch->pid, ch->ref_count);
@@ -702,7 +746,12 @@ static int spcom_open(struct spcom_channel *ch, unsigned int timeout_msec)
 	} else {
 		pr_debug("glink_open [%s] ok.\n", name);
 	}
+
+	/* init channel context after successful open */
 	ch->glink_handle = handle;
+	ch->ref_count++;
+	ch->pid = current_pid();
+	ch->txn_id = INITIAL_TXN_ID;
 
 	pr_debug("Wait for connection on channel [%s] timeout_msec [%d].\n",
 		 name, timeout_msec);
@@ -776,6 +825,8 @@ static int spcom_close(struct spcom_channel *ch)
  * @size: buffer size
  *
  * ACK is expected within a very short time (few msec).
+ *
+ * Return: 0 on successful operation, negative value otherwise.
  */
 static int spcom_tx(struct spcom_channel *ch,
 		    void *buf,
@@ -840,13 +891,15 @@ static int spcom_tx(struct spcom_channel *ch,
  * @size: buffer size
  *
  * ACK is expected within a very short time (few msec).
+ *
+ * Return: size in bytes on success, negative value on failure.
  */
 static int spcom_rx(struct spcom_channel *ch,
 		     void *buf,
 		     uint32_t size,
 		     uint32_t timeout_msec)
 {
-	int ret;
+	int ret = -1;
 	unsigned long jiffies = msecs_to_jiffies(timeout_msec);
 	long timeleft = 1;
 
@@ -854,7 +907,7 @@ static int spcom_rx(struct spcom_channel *ch,
 
 	/* check for already pending data */
 	if (ch->actual_rx_size) {
-		pr_debug("already pending data size [%d].\n",
+		pr_debug("already pending data size [%zu]\n",
 			 ch->actual_rx_size);
 		goto copy_buf;
 	}
@@ -871,23 +924,24 @@ static int spcom_rx(struct spcom_channel *ch,
 
 	if (timeleft == 0) {
 		pr_err("rx_done timeout [%d] msec expired.\n", timeout_msec);
-		goto exit_err;
+		mutex_unlock(&ch->lock);
+		return -ETIMEDOUT;
 	} else if (ch->rx_abort) {
-		pr_err("rx aborted.\n");
-		goto exit_err;
+		mutex_unlock(&ch->lock);
+		return -ERESTART; /* probably SSR */
 	} else if (ch->actual_rx_size) {
-		pr_debug("actual_rx_size is [%d].\n", ch->actual_rx_size);
+		pr_debug("actual_rx_size is [%zu]\n", ch->actual_rx_size);
 	} else {
 		pr_err("actual_rx_size is zero.\n");
 		goto exit_err;
 	}
 
+copy_buf:
 	if (!ch->glink_rx_buf) {
 		pr_err("invalid glink_rx_buf.\n");
 		goto exit_err;
 	}
 
-copy_buf:
 	/* Copy from glink buffer to spcom buffer */
 	size = min_t(int, ch->actual_rx_size, size);
 	memcpy(buf, ch->glink_rx_buf, size);
@@ -905,7 +959,7 @@ static int spcom_rx(struct spcom_channel *ch,
 		pr_err("glink_queue_rx_intent() failed, ret [%d]", ret);
 		goto exit_err;
 	} else {
-		pr_debug("queue rx_buf, size [%d].\n", ch->rx_buf_size);
+		pr_debug("queue rx_buf, size [%zu]\n", ch->rx_buf_size);
 	}
 
 	mutex_unlock(&ch->lock);
@@ -925,6 +979,8 @@ static int spcom_rx(struct spcom_channel *ch,
  * Server needs the size of the next request to allocate a request buffer.
  * Initially used intent-request, however this complicated the remote side,
  * so both sides are not using glink_tx() with INTENT_REQ anymore.
+ *
+ * Return: size in bytes on success, negative value on failure.
  */
 static int spcom_get_next_request_size(struct spcom_channel *ch)
 {
@@ -936,15 +992,22 @@ static int spcom_get_next_request_size(struct spcom_channel *ch)
 
 	/* check if already got it via callback */
 	if (ch->actual_rx_size) {
-		pr_debug("next-req-size already ready ch [%s] size [%d].\n",
+		pr_debug("next-req-size already ready ch [%s] size [%zu]\n",
 			 ch->name, ch->actual_rx_size);
 		goto exit_ready;
 	}
 
 	pr_debug("Wait for Rx Done, ch [%s].\n", ch->name);
 	wait_for_completion(&ch->rx_done);
+
+	/* Check Rx Abort on SP reset */
+	if (ch->rx_abort) {
+		pr_err("rx aborted.\n");
+		goto exit_error;
+	}
+
 	if (ch->actual_rx_size <= 0) {
-		pr_err("invalid rx size [%d] ch [%s].\n",
+		pr_err("invalid rx size [%zu] ch [%s]\n",
 		       ch->actual_rx_size, ch->name);
 		goto exit_error;
 	}
@@ -968,6 +1031,27 @@ static int spcom_get_next_request_size(struct spcom_channel *ch)
 
 }
 
+/**
+ * spcom_rx_abort_pending_server() - abort pending server rx on SSR.
+ *
+ * Server that is waiting for request, but has no client connected,
+ * will not get RX-ABORT or REMOTE-DISCONNECT notification,
+ * that should cancel the server pending rx operation.
+ */
+static void spcom_rx_abort_pending_server(void)
+{
+	int i;
+
+	for (i = 0 ; i < ARRAY_SIZE(spcom_dev->channels); i++) {
+		struct spcom_channel *ch = &spcom_dev->channels[i];
+
+		if (ch->is_server) {
+			pr_debug("rx-abort server on ch [%s].\n", ch->name);
+			spcom_notify_rx_abort(NULL, ch, NULL);
+		}
+	}
+}
+
 /*======================================================================*/
 /*		General API for kernel drivers				*/
 /*======================================================================*/
@@ -979,6 +1063,9 @@ static int spcom_get_next_request_size(struct spcom_channel *ch)
  */
 bool spcom_is_sp_subsystem_link_up(void)
 {
+	if (spcom_dev == NULL)
+		return false;
+
 	return (spcom_dev->link_state == GLINK_LINK_STATE_UP);
 }
 EXPORT_SYMBOL(spcom_is_sp_subsystem_link_up);
@@ -1001,6 +1088,11 @@ struct spcom_client *spcom_register_client(struct spcom_client_info *info)
 	struct spcom_channel *ch;
 	struct spcom_client *client;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+			return NULL;
+	}
+
 	if (!info) {
 		pr_err("Invalid parameter.\n");
 			return NULL;
@@ -1042,17 +1134,26 @@ int spcom_unregister_client(struct spcom_client *client)
 {
 	struct spcom_channel *ch;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return -ENODEV;
+	}
+
 	if (!client) {
-		pr_err("Invalid parameter.\n");
+		pr_err("Invalid client parameter.\n");
 		return -EINVAL;
 	}
 
 	ch = client->ch;
-
-	kfree(client);
+	if (!ch) {
+		pr_err("Invalid channel.\n");
+		return -EINVAL;
+	}
 
 	spcom_close(ch);
 
+	kfree(client);
+
 	return 0;
 }
 EXPORT_SYMBOL(spcom_unregister_client);
@@ -1069,6 +1170,8 @@ EXPORT_SYMBOL(spcom_unregister_client);
  * @timeout_msec: timeout waiting for response.
  *
  * The timeout depends on the specific request handling time at the remote side.
+ *
+ * Return: number of rx bytes on success, negative value on failure.
  */
 int spcom_client_send_message_sync(struct spcom_client	*client,
 				    void	*req_ptr,
@@ -1080,12 +1183,21 @@ int spcom_client_send_message_sync(struct spcom_client	*client,
 	int ret;
 	struct spcom_channel *ch;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return -ENODEV;
+	}
+
 	if (!client || !req_ptr || !resp_ptr) {
 		pr_err("Invalid parameter.\n");
 		return -EINVAL;
 	}
 
 	ch = client->ch;
+	if (!ch) {
+		pr_err("Invalid channel.\n");
+		return -EINVAL;
+	}
 
 	/* Check if remote side connect */
 	if (!spcom_is_channel_connected(ch)) {
@@ -1120,13 +1232,25 @@ EXPORT_SYMBOL(spcom_client_send_message_sync);
 bool spcom_client_is_server_connected(struct spcom_client *client)
 {
 	bool connected;
+	struct spcom_channel *ch;
+
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return false;
+	}
 
 	if (!client) {
 		pr_err("Invalid parameter.\n");
+		return false;
+	}
+
+	ch = client->ch;
+	if (!ch) {
+		pr_err("Invalid channel.\n");
 		return -EINVAL;
 	}
 
-	connected = spcom_is_channel_connected(client->ch);
+	connected = spcom_is_channel_connected(ch);
 
 	return connected;
 }
@@ -1150,6 +1274,11 @@ struct spcom_server *spcom_register_service(struct spcom_service_info *info)
 	struct spcom_channel *ch;
 	struct spcom_server *server;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return NULL;
+	}
+
 	if (!info) {
 		pr_err("Invalid parameter.\n");
 		return NULL;
@@ -1188,17 +1317,26 @@ int spcom_unregister_service(struct spcom_server *server)
 {
 	struct spcom_channel *ch;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return -ENODEV;
+	}
+
 	if (!server) {
-		pr_err("Invalid parameter.\n");
+		pr_err("Invalid server parameter.\n");
 		return -EINVAL;
 	}
 
 	ch = server->ch;
-
-	kfree(server);
+	if (!ch) {
+		pr_err("Invalid channel parameter.\n");
+		return -EINVAL;
+	}
 
 	spcom_close(ch);
 
+	kfree(server);
+
 	return 0;
 }
 EXPORT_SYMBOL(spcom_unregister_service);
@@ -1208,7 +1346,7 @@ EXPORT_SYMBOL(spcom_unregister_service);
  *
  * @server: server handle
  *
- * Return: request size in bytes.
+ * Return: size in bytes on success, negative value on failure.
  */
 int spcom_server_get_next_request_size(struct spcom_server *server)
 {
@@ -1221,6 +1359,10 @@ int spcom_server_get_next_request_size(struct spcom_server *server)
 	}
 
 	ch = server->ch;
+	if (!ch) {
+		pr_err("Invalid channel.\n");
+		return -EINVAL;
+	}
 
 	/* Check if remote side connect */
 	if (!spcom_is_channel_connected(ch)) {
@@ -1243,7 +1385,7 @@ EXPORT_SYMBOL(spcom_server_get_next_request_size);
  * @req_ptr: request buffer pointer
  * @req_size: max request size
  *
- * Return: request size in bytes.
+ * Return: size in bytes on success, negative value on failure.
  */
 int spcom_server_wait_for_request(struct spcom_server	*server,
 				  void			*req_ptr,
@@ -1252,12 +1394,21 @@ int spcom_server_wait_for_request(struct spcom_server	*server,
 	int ret;
 	struct spcom_channel *ch;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return -ENODEV;
+	}
+
 	if (!server || !req_ptr) {
 		pr_err("Invalid parameter.\n");
 		return -EINVAL;
 	}
 
 	ch = server->ch;
+	if (!ch) {
+		pr_err("Invalid channel.\n");
+		return -EINVAL;
+	}
 
 	/* Check if remote side connect */
 	if (!spcom_is_channel_connected(ch)) {
@@ -1285,12 +1436,21 @@ int spcom_server_send_response(struct spcom_server	*server,
 	int ret;
 	struct spcom_channel *ch;
 
+	if (!spcom_is_ready()) {
+		pr_err("spcom is not ready.\n");
+		return -ENODEV;
+	}
+
 	if (!server || !resp_ptr) {
 		pr_err("Invalid parameter.\n");
 		return -EINVAL;
 	}
 
 	ch = server->ch;
+	if (!ch) {
+		pr_err("Invalid channel.\n");
+		return -EINVAL;
+	}
 
 	/* Check if remote side connect */
 	if (!spcom_is_channel_connected(ch)) {
@@ -1322,6 +1482,7 @@ static int spcom_handle_create_channel_command(void *cmd_buf, int cmd_size)
 	int ret = 0;
 	struct spcom_user_create_channel_command *cmd = cmd_buf;
 	const char *ch_name;
+	const size_t maxlen = sizeof(cmd->ch_name);
 
 	if (cmd_size != sizeof(*cmd)) {
 		pr_err("cmd_size [%d] , expected [%d].\n",
@@ -1330,6 +1491,10 @@ static int spcom_handle_create_channel_command(void *cmd_buf, int cmd_size)
 	}
 
 	ch_name = cmd->ch_name;
+	if (strnlen(cmd->ch_name, maxlen) == maxlen) {
+		pr_err("channel name is not NULL terminated\n");
+		return -EINVAL;
+	}
 
 	pr_debug("ch_name [%s].\n", ch_name);
 
@@ -1468,7 +1633,7 @@ static int modify_ion_addr(void *buf,
 
 	/* Get ION handle from fd */
 	handle = ion_import_dma_buf_fd(spcom_dev->ion_client, fd);
-	if (handle == NULL) {
+	if (IS_ERR_OR_NULL(handle)) {
 		pr_err("fail to get ion handle.\n");
 		return -EINVAL;
 	}
@@ -1629,18 +1794,23 @@ static int spcom_handle_lock_ion_buf_command(struct spcom_channel *ch,
 
 	/* Get ION handle from fd - this increments the ref count */
 	ion_handle = ion_import_dma_buf_fd(spcom_dev->ion_client, fd);
-	if (ion_handle == NULL) {
+	if (IS_ERR_OR_NULL(ion_handle)) {
 		pr_err("fail to get ion handle.\n");
 		return -EINVAL;
 	}
+
 	pr_debug("ion handle ok.\n");
 
+	/* ION buf lock doesn't involve any rx/tx data to SP. */
+	mutex_lock(&ch->lock);
+
 	/* Check if this ION buffer is already locked */
 	for (i = 0 ; i < ARRAY_SIZE(ch->ion_handle_table) ; i++) {
 		if (ch->ion_handle_table[i] == ion_handle) {
-			pr_debug("fd [%d] ion buf is already locked.\n", fd);
+			pr_err("fd [%d] ion buf is already locked.\n", fd);
 			/* decrement back the ref count */
 			ion_free(spcom_dev->ion_client, ion_handle);
+			mutex_unlock(&ch->lock);
 			return -EINVAL;
 		}
 	}
@@ -1650,11 +1820,19 @@ static int spcom_handle_lock_ion_buf_command(struct spcom_channel *ch,
 		if (ch->ion_handle_table[i] == NULL) {
 			ch->ion_handle_table[i] = ion_handle;
 			ch->ion_fd_table[i] = fd;
-			pr_debug("locked ion buf#[%d], fd [%d].\n", i, fd);
+			pr_debug("ch [%s] locked ion buf #%d, fd [%d].\n",
+				ch->name, i, fd);
+			mutex_unlock(&ch->lock);
 			return 0;
 		}
 	}
 
+	pr_err("no free entry to store ion handle of fd [%d].\n", fd);
+	/* decrement back the ref count */
+	ion_free(spcom_dev->ion_client, ion_handle);
+
+	mutex_unlock(&ch->lock);
+
 	return -EFAULT;
 }
 
@@ -1684,20 +1862,24 @@ static int spcom_unlock_ion_buf(struct spcom_channel *ch, int fd)
 		/* unlock all ION buf */
 		for (i = 0 ; i < ARRAY_SIZE(ch->ion_handle_table) ; i++) {
 			if (ch->ion_handle_table[i] != NULL) {
+				pr_debug("unlocked ion buf #%d fd [%d].\n",
+					i, ch->ion_fd_table[i]);
 				ion_free(ion_client, ch->ion_handle_table[i]);
 				ch->ion_handle_table[i] = NULL;
 				ch->ion_fd_table[i] = -1;
-				pr_debug("unlocked ion buf#[%d].\n", i);
 			}
 		}
 	} else {
 		/* unlock specific ION buf */
 		for (i = 0 ; i < ARRAY_SIZE(ch->ion_handle_table) ; i++) {
+			if (ch->ion_handle_table[i] == NULL)
+				continue;
 			if (ch->ion_fd_table[i] == fd) {
+				pr_debug("unlocked ion buf #%d fd [%d].\n",
+					i, ch->ion_fd_table[i]);
 				ion_free(ion_client, ch->ion_handle_table[i]);
 				ch->ion_handle_table[i] = NULL;
 				ch->ion_fd_table[i] = -1;
-				pr_debug("unlocked ion buf#[%d].\n", i);
 				found = true;
 				break;
 			}
@@ -1731,8 +1913,13 @@ static int spcom_handle_unlock_ion_buf_command(struct spcom_channel *ch,
 		return -EINVAL;
 	}
 
+	/* ION buf unlock doesn't involve any rx/tx data to SP. */
+	mutex_lock(&ch->lock);
+
 	ret = spcom_unlock_ion_buf(ch, fd);
 
+	mutex_unlock(&ch->lock);
+
 	return ret;
 }
 
@@ -1766,9 +1953,9 @@ static int spcom_handle_write(struct spcom_channel *ch,
 	int swap_id;
 	char cmd_name[5] = {0}; /* debug only */
 
-	/* opcode field is the minimum length of cmd */
-	if (buf_size < sizeof(cmd->cmd_id)) {
-		pr_err("Invalid argument user buffer size %d.\n", buf_size);
+	/* Minimal command should have command-id and argument */
+	if (buf_size < sizeof(struct spcom_user_command)) {
+		pr_err("Command buffer size [%d] too small\n", buf_size);
 		return -EINVAL;
 	}
 
@@ -1813,7 +2000,7 @@ static int spcom_handle_write(struct spcom_channel *ch,
  * @buf:	command buffer.
  * @size:	command buffer size.
  *
- * Return: size in bytes.
+ * Return: size in bytes on success, negative value on failure.
  */
 static int spcom_handle_get_req_size(struct spcom_channel *ch,
 				      void *buf,
@@ -1841,7 +2028,7 @@ static int spcom_handle_get_req_size(struct spcom_channel *ch,
  * @buf:	command buffer.
  * @size:	command buffer size.
  *
- * Return: size in bytes.
+ * Return: size in bytes on success, negative value on failure.
  */
 static int spcom_handle_read_req_resp(struct spcom_channel *ch,
 				       void *buf,
@@ -1861,7 +2048,7 @@ static int spcom_handle_read_req_resp(struct spcom_channel *ch,
 
 	/* Check param validity */
 	if (size > SPCOM_MAX_RESPONSE_SIZE) {
-		pr_err("ch [%s] inavlid size [%d].\n",
+		pr_err("ch [%s] invalid size [%d].\n",
 			ch->name, size);
 		return -EINVAL;
 	}
@@ -1884,7 +2071,8 @@ static int spcom_handle_read_req_resp(struct spcom_channel *ch,
 	ret = spcom_rx(ch, rx_buf, rx_buf_size, timeout_msec);
 	if (ret < 0) {
 		pr_err("rx error %d.\n", ret);
-		goto exit_err;
+		kfree(rx_buf);
+		return ret;
 	} else {
 		size = ret; /* actual_rx_size */
 	}
@@ -1924,7 +2112,7 @@ static int spcom_handle_read_req_resp(struct spcom_channel *ch,
  * A special size SPCOM_GET_NEXT_REQUEST_SIZE, which is bigger than the max
  * response/request tells the kernel that user space only need the size.
  *
- * Return: size in bytes.
+ * Return: size in bytes on success, negative value on failure.
  */
 static int spcom_handle_read(struct spcom_channel *ch,
 			      void *buf,
@@ -1932,8 +2120,8 @@ static int spcom_handle_read(struct spcom_channel *ch,
 {
 	if (size == SPCOM_GET_NEXT_REQUEST_SIZE) {
 		pr_debug("get next request size, ch [%s].\n", ch->name);
-		size = spcom_handle_get_req_size(ch, buf, size);
 		ch->is_server = true;
+		size = spcom_handle_get_req_size(ch, buf, size);
 	} else {
 		pr_debug("get request/response, ch [%s].\n", ch->name);
 		size = spcom_handle_read_req_resp(ch, buf, size);
@@ -1988,6 +2176,10 @@ static int spcom_device_open(struct inode *inode, struct file *filp)
 	struct spcom_channel *ch;
 	const char *name = file_to_filename(filp);
 
+	/* silent error message until spss link is up */
+	if (!spcom_is_sp_subsystem_link_up())
+		return -ENODEV;
+
 	pr_debug("Open file [%s].\n", name);
 
 	if (strcmp(name, DEVICE_NAME) == 0) {
@@ -2006,8 +2198,6 @@ static int spcom_device_open(struct inode *inode, struct file *filp)
 		return -ENODEV;
 	}
 
-	filp->private_data = ch;
-
 	ret = spcom_open(ch, OPEN_CHANNEL_TIMEOUT_MSEC);
 	if (ret == -ETIMEDOUT) {
 		pr_err("Connection timeout channel [%s].\n", name);
@@ -2016,6 +2206,8 @@ static int spcom_device_open(struct inode *inode, struct file *filp)
 		return ret;
 	}
 
+	filp->private_data = ch;
+
 	pr_debug("finished.\n");
 
 	return 0;
@@ -2036,7 +2228,6 @@ static int spcom_device_release(struct inode *inode, struct file *filp)
 {
 	struct spcom_channel *ch;
 	const char *name = file_to_filename(filp);
-	bool connected = false;
 
 	pr_debug("Close file [%s].\n", name);
 
@@ -2058,19 +2249,18 @@ static int spcom_device_release(struct inode *inode, struct file *filp)
 	}
 
 	/* channel might be already closed or disconnected */
-	if (spcom_is_channel_open(ch) && spcom_is_channel_connected(ch))
-		connected = true;
+	if (!spcom_is_channel_open(ch)) {
+		pr_err("ch [%s] already closed.\n", name);
+		return 0;
+	}
 
 	reinit_completion(&ch->disconnect);
 
 	spcom_close(ch);
 
-	if (connected) {
-		pr_debug("Wait for event GLINK_LOCAL_DISCONNECTED, ch [%s].\n",
-			 name);
-		wait_for_completion(&ch->disconnect);
-		pr_debug("GLINK_LOCAL_DISCONNECTED signaled, ch [%s].\n", name);
-	}
+	pr_debug("Wait for event GLINK_LOCAL_DISCONNECTED, ch [%s].\n", name);
+	wait_for_completion(&ch->disconnect);
+	pr_debug("GLINK_LOCAL_DISCONNECTED signaled, ch [%s].\n", name);
 
 	return 0;
 }
@@ -2102,8 +2292,8 @@ static ssize_t spcom_device_write(struct file *filp,
 
 	ch = filp->private_data;
 	if (!ch) {
-		pr_debug("invalid ch pointer.\n");
-		/* Allow some special commands via /dev/spcom and /dev/sp_ssr */
+		pr_err("invalid ch pointer, command not allowed.\n");
+		return -EINVAL;
 	} else {
 		/* Check if remote side connect */
 		if (!spcom_is_channel_connected(ch)) {
@@ -2147,7 +2337,7 @@ static ssize_t spcom_device_write(struct file *filp,
 }
 
 /**
- * spcom_device_read() - handle channel file write() from user space.
+ * spcom_device_read() - handle channel file read() from user space.
  *
  * @filp: file pointer
  *
@@ -2173,12 +2363,28 @@ static ssize_t spcom_device_read(struct file *filp, char __user *user_buff,
 
 	ch = filp->private_data;
 
+	if (ch == NULL) {
+		pr_err("invalid ch pointer, file [%s].\n", name);
+		return -EINVAL;
+	}
+
+	if (!spcom_is_channel_open(ch)) {
+		pr_err("ch is not open, file [%s].\n", name);
+		return -EINVAL;
+	}
+
 	buf = kzalloc(size, GFP_KERNEL);
 	if (buf == NULL)
 		return -ENOMEM;
 
-	actual_size = spcom_handle_read(ch, buf, size);
-	if ((actual_size <= 0) || (actual_size > size)) {
+	ret = spcom_handle_read(ch, buf, size);
+	if (ret < 0) {
+		pr_err("read error [%d].\n", ret);
+		kfree(buf);
+		return ret;
+	}
+	actual_size = ret;
+	if ((actual_size == 0) || (actual_size > size)) {
 		pr_err("invalid actual_size [%d].\n", actual_size);
 		kfree(buf);
 		return -EFAULT;
@@ -2254,6 +2460,10 @@ static unsigned int spcom_device_poll(struct file *filp,
 		done = (spcom_dev->link_state == GLINK_LINK_STATE_UP);
 		break;
 	case SPCOM_POLL_CH_CONNECT:
+		if (ch == NULL) {
+			pr_err("invalid ch pointer, file [%s].\n", name);
+			return -EINVAL;
+		}
 		pr_debug("ch [%s] SPCOM_POLL_CH_CONNECT.\n", name);
 		if (wait) {
 			reinit_completion(&ch->connect);
@@ -2329,7 +2539,7 @@ static int spcom_create_channel_chardev(const char *name)
 	devt = spcom_dev->device_no + spcom_dev->channel_count;
 	priv = ch;
 	dev = device_create(cls, parent, devt, priv, name);
-	if (!dev) {
+	if (IS_ERR(dev)) {
 		pr_err("device_create failed.\n");
 		kfree(cdev);
 		return -ENODEV;
@@ -2382,7 +2592,7 @@ static int __init spcom_register_chardev(void)
 				  spcom_dev->device_no, priv,
 				  DEVICE_NAME);
 
-	if (!spcom_dev->class_dev) {
+	if (IS_ERR(spcom_dev->class_dev)) {
 		pr_err("class_device_create failed %d\n", ret);
 		ret = -ENOMEM;
 		goto exit_destroy_class;
@@ -2435,6 +2645,11 @@ static int spcom_parse_dt(struct device_node *np)
 
 	pr_debug("num of predefined channels [%d].\n", num_ch);
 
+	if (num_ch > ARRAY_SIZE(spcom_dev->predefined_ch_name)) {
+		pr_err("too many predefined channels [%d].\n", num_ch);
+		return -EINVAL;
+	}
+
 	for (i = 0; i < num_ch; i++) {
 		ret = of_property_read_string_index(np, propname, i, &name);
 		if (ret) {
@@ -2500,21 +2715,23 @@ static int spcom_probe(struct platform_device *pdev)
 	pr_debug("register_link_state_cb(), transport [%s] edge [%s]\n",
 		link_info.transport, link_info.edge);
 	notif_handle = glink_register_link_state_cb(&link_info, spcom_dev);
-	if (!notif_handle) {
+	if (IS_ERR(notif_handle)) {
 		pr_err("glink_register_link_state_cb(), err [%d]\n", ret);
 		goto fail_reg_chardev;
 	}
 
 	spcom_dev->ion_client = msm_ion_client_create(DEVICE_NAME);
-	if (spcom_dev->ion_client == NULL) {
+	if (IS_ERR(spcom_dev->ion_client)) {
 		pr_err("fail to create ion client.\n");
-		goto fail_reg_chardev;
+		goto fail_ion_client;
 	}
 
 	pr_info("Driver Initialization ok.\n");
 
 	return 0;
 
+fail_ion_client:
+	glink_unregister_link_state_cb(notif_handle);
 fail_reg_chardev:
 	pr_err("Failed to init driver.\n");
 	spcom_unregister_chrdev();
@@ -2552,7 +2769,7 @@ static int __init spcom_init(void)
 	if (ret)
 		pr_err("spcom_driver register failed %d\n", ret);
 
-	return 0;
+	return ret;
 }
 module_init(spcom_init);