[PATCH 00/16] staging: vchiq_arm: code clean-up part 3

[PATCH 00/16] staging: vchiq_arm: code clean-up part 3

[Stefan Wahren]

This series mostly addresses the code readability. Additionally it contains
a fix for a change from my last series.

Stefan Wahren (16):
  staging: vchiq_core: fix logic in poll_services_of_group
  staging: vchiq_arm: introduce free_bulk_waiter
  staging: vchiq_core: move internals to C source
  staging: vchiq_core: get the rid of IS_POW2
  staging: vchiq_core: get the rid of vchiq_static_assert
  staging: vchiq_core: put spaces around operators
  staging: vchiq_core: avoid precedence issues
  staging: vchiq_core: use define for message type shift
  staging: vchiq_core: introduce message specific make macros
  staging: vchiq_core: simplify WARN_ON conditions
  staging: vchiq_arm: tidy up service function naming
  staging: vchiq_core: introduce process_free_data_message
  staging: vchiq_core: reduce indentation in parse_open
  staging: vchiq_core: store message id in local variable
  staging: vchiq_connected: move EXPORT_SYMBOL below the right function
  staging: vchiq_core: introduce handle_poll

 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  71 ++-
 .../interface/vchiq_arm/vchiq_connected.c          |   2 +-
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 606 ++++++++++++---------
 .../vc04_services/interface/vchiq_arm/vchiq_core.h |  79 +--
 4 files changed, 375 insertions(+), 383 deletions(-)

-- 
2.7.4

[PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group

[Stefan Wahren]

Commit "staging: vchiq_core: avoid indention in poll_services_of_group"
tries to reduce the indention, but the parts regarding the third if statement
breaks the logic. So restore them and prevent vchiq_test from hanging during
the ping test.

Fixes: 2f440843a7d4 ("staging: vchiq_core: avoid indention in poll_services_of_group")
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 28 ++++++++++------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 47bb0af..bc6e123 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1340,23 +1340,21 @@ poll_services_of_group(struct vchiq_state *state, int group)
 			continue;
 
 		service_flags = atomic_xchg(&service->poll_flags, 0);
-		if ((service_flags & BIT(VCHIQ_POLL_REMOVE)) == 0)
-			continue;
-
-		vchiq_log_info(vchiq_core_log_level, "%d: ps - remove %d<->%d",
-			       state->id, service->localport,
-			       service->remoteport);
+		if (service_flags & BIT(VCHIQ_POLL_REMOVE)) {
+			vchiq_log_info(vchiq_core_log_level, "%d: ps - remove %d<->%d",
+				       state->id, service->localport,
+				       service->remoteport);
 
-		/*
-		 * Make it look like a client, because
-		 * it must be removed and not left in
-		 * the LISTENING state.
-		 */
-		service->public_fourcc = VCHIQ_FOURCC_INVALID;
+			/*
+			 * Make it look like a client, because
+			 * it must be removed and not left in
+			 * the LISTENING state.
+			 */
+			service->public_fourcc = VCHIQ_FOURCC_INVALID;
 
-		if (vchiq_close_service_internal(service, NO_CLOSE_RECVD) !=
-						 VCHIQ_SUCCESS) {
-			request_poll(state, service, VCHIQ_POLL_REMOVE);
+			if (vchiq_close_service_internal(service, NO_CLOSE_RECVD) !=
+							 VCHIQ_SUCCESS)
+				request_poll(state, service, VCHIQ_POLL_REMOVE);
 		} else if (service_flags & BIT(VCHIQ_POLL_TERMINATE)) {
 			vchiq_log_info(vchiq_core_log_level,
 				"%d: ps - terminate %d<->%d",
-- 
2.7.4

[PATCH 02/16] staging: vchiq_arm: introduce free_bulk_waiter

[Stefan Wahren]

Avoid the copy & paste of freeing the bulk waiter and move it into a
separate function. Found by CPD.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 37 ++++++++++------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 344f35a..12a294c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -199,11 +199,24 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
 }
 EXPORT_SYMBOL(vchiq_initialise);
 
+static void free_bulk_waiter(struct vchiq_instance *instance)
+{
+	struct bulk_waiter_node *waiter, *next;
+
+	list_for_each_entry_safe(waiter, next,
+				 &instance->bulk_waiter_list, list) {
+		list_del(&waiter->list);
+		vchiq_log_info(vchiq_arm_log_level,
+				"bulk_waiter - cleaned up %pK for pid %d",
+				waiter, waiter->pid);
+		kfree(waiter);
+	}
+}
+
 enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
 {
 	enum vchiq_status status = VCHIQ_SUCCESS;
 	struct vchiq_state *state = instance->state;
-	struct bulk_waiter_node *waiter, *next;
 
 	if (mutex_lock_killable(&state->mutex))
 		return VCHIQ_RETRY;
@@ -216,14 +229,7 @@ enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
 	vchiq_log_trace(vchiq_core_log_level,
 		"%s(%p): returning %d", __func__, instance, status);
 
-	list_for_each_entry_safe(waiter, next,
-				 &instance->bulk_waiter_list, list) {
-		list_del(&waiter->list);
-		vchiq_log_info(vchiq_arm_log_level,
-				"bulk_waiter - cleaned up %pK for pid %d",
-				waiter, waiter->pid);
-		kfree(waiter);
-	}
+	free_bulk_waiter(instance);
 	kfree(instance);
 
 	return status;
@@ -1943,18 +1949,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 	/* Release the PEER service count. */
 	vchiq_release_internal(instance->state, NULL);
 
-	{
-		struct bulk_waiter_node *waiter, *next;
-
-		list_for_each_entry_safe(waiter, next,
-					 &instance->bulk_waiter_list, list) {
-			list_del(&waiter->list);
-			vchiq_log_info(vchiq_arm_log_level,
-				"bulk_waiter - cleaned up %pK for pid %d",
-				waiter, waiter->pid);
-			kfree(waiter);
-		}
-	}
+	free_bulk_waiter(instance);
 
 	vchiq_debugfs_remove_instance(instance);
 
-- 
2.7.4

[PATCH 03/16] staging: vchiq_core: move internals to C source

[Stefan Wahren]

There is no need to export those definitions, so keep them in the
source file.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 61 ++++++++++++++++++++++
 .../vc04_services/interface/vchiq_arm/vchiq_core.h | 61 ----------------------
 2 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index bc6e123..b808f91 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -17,6 +17,59 @@
 
 #define VCHIQ_SLOT_HANDLER_STACK 8192
 
+#define VCHIQ_MSG_PADDING            0  /* -                                 */
+#define VCHIQ_MSG_CONNECT            1  /* -                                 */
+#define VCHIQ_MSG_OPEN               2  /* + (srcport, -), fourcc, client_id */
+#define VCHIQ_MSG_OPENACK            3  /* + (srcport, dstport)              */
+#define VCHIQ_MSG_CLOSE              4  /* + (srcport, dstport)              */
+#define VCHIQ_MSG_DATA               5  /* + (srcport, dstport)              */
+#define VCHIQ_MSG_BULK_RX            6  /* + (srcport, dstport), data, size  */
+#define VCHIQ_MSG_BULK_TX            7  /* + (srcport, dstport), data, size  */
+#define VCHIQ_MSG_BULK_RX_DONE       8  /* + (srcport, dstport), actual      */
+#define VCHIQ_MSG_BULK_TX_DONE       9  /* + (srcport, dstport), actual      */
+#define VCHIQ_MSG_PAUSE             10  /* -                                 */
+#define VCHIQ_MSG_RESUME            11  /* -                                 */
+#define VCHIQ_MSG_REMOTE_USE        12  /* -                                 */
+#define VCHIQ_MSG_REMOTE_RELEASE    13  /* -                                 */
+#define VCHIQ_MSG_REMOTE_USE_ACTIVE 14  /* -                                 */
+
+#define VCHIQ_PORT_MAX                 (VCHIQ_MAX_SERVICES - 1)
+#define VCHIQ_PORT_FREE                0x1000
+#define VCHIQ_PORT_IS_VALID(port)      (port < VCHIQ_PORT_FREE)
+#define VCHIQ_MAKE_MSG(type, srcport, dstport) \
+	((type<<24) | (srcport<<12) | (dstport<<0))
+#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)msgid >> 24)
+#define VCHIQ_MSG_SRCPORT(msgid) \
+	(unsigned short)(((unsigned int)msgid >> 12) & 0xfff)
+#define VCHIQ_MSG_DSTPORT(msgid) \
+	((unsigned short)msgid & 0xfff)
+
+/* Ensure the fields are wide enough */
+vchiq_static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
+	== 0);
+vchiq_static_assert(VCHIQ_MSG_TYPE(VCHIQ_MAKE_MSG(0, VCHIQ_PORT_MAX, 0)) == 0);
+vchiq_static_assert((unsigned int)VCHIQ_PORT_MAX <
+	(unsigned int)VCHIQ_PORT_FREE);
+
+#define VCHIQ_MSGID_PADDING            VCHIQ_MAKE_MSG(VCHIQ_MSG_PADDING, 0, 0)
+#define VCHIQ_MSGID_CLAIMED            0x40000000
+
+#define VCHIQ_FOURCC_INVALID           0x00000000
+#define VCHIQ_FOURCC_IS_LEGAL(fourcc)  (fourcc != VCHIQ_FOURCC_INVALID)
+
+#define VCHIQ_BULK_ACTUAL_ABORTED -1
+
+#if VCHIQ_ENABLE_STATS
+#define VCHIQ_STATS_INC(state, stat) (state->stats. stat++)
+#define VCHIQ_SERVICE_STATS_INC(service, stat) (service->stats. stat++)
+#define VCHIQ_SERVICE_STATS_ADD(service, stat, addend) \
+	(service->stats. stat += addend)
+#else
+#define VCHIQ_STATS_INC(state, stat) ((void)0)
+#define VCHIQ_SERVICE_STATS_INC(service, stat) ((void)0)
+#define VCHIQ_SERVICE_STATS_ADD(service, stat, addend) ((void)0)
+#endif
+
 #define HANDLE_STATE_SHIFT 12
 
 #define SLOT_INFO_FROM_INDEX(state, index) (state->slot_info + (index))
@@ -61,6 +114,14 @@ enum {
 	QMFLAGS_NO_MUTEX_UNLOCK = BIT(2)
 };
 
+enum {
+	VCHIQ_POLL_TERMINATE,
+	VCHIQ_POLL_REMOVE,
+	VCHIQ_POLL_TXNOTIFY,
+	VCHIQ_POLL_RXNOTIFY,
+	VCHIQ_POLL_COUNT
+};
+
 /* we require this for consistency between endpoints */
 vchiq_static_assert(sizeof(struct vchiq_header) == 8);
 vchiq_static_assert(IS_POW2(sizeof(struct vchiq_header)));
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 5264677..db93495 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -81,54 +81,12 @@ vchiq_static_assert(IS_POW2(VCHIQ_MAX_SLOTS_PER_SIDE));
 #define VCHIQ_SLOT_ZERO_SLOTS  DIV_ROUND_UP(sizeof(struct vchiq_slot_zero), \
 					    VCHIQ_SLOT_SIZE)
 
-#define VCHIQ_MSG_PADDING            0  /* -                                 */
-#define VCHIQ_MSG_CONNECT            1  /* -                                 */
-#define VCHIQ_MSG_OPEN               2  /* + (srcport, -), fourcc, client_id */
-#define VCHIQ_MSG_OPENACK            3  /* + (srcport, dstport)              */
-#define VCHIQ_MSG_CLOSE              4  /* + (srcport, dstport)              */
-#define VCHIQ_MSG_DATA               5  /* + (srcport, dstport)              */
-#define VCHIQ_MSG_BULK_RX            6  /* + (srcport, dstport), data, size  */
-#define VCHIQ_MSG_BULK_TX            7  /* + (srcport, dstport), data, size  */
-#define VCHIQ_MSG_BULK_RX_DONE       8  /* + (srcport, dstport), actual      */
-#define VCHIQ_MSG_BULK_TX_DONE       9  /* + (srcport, dstport), actual      */
-#define VCHIQ_MSG_PAUSE             10  /* -                                 */
-#define VCHIQ_MSG_RESUME            11  /* -                                 */
-#define VCHIQ_MSG_REMOTE_USE        12  /* -                                 */
-#define VCHIQ_MSG_REMOTE_RELEASE    13  /* -                                 */
-#define VCHIQ_MSG_REMOTE_USE_ACTIVE 14  /* -                                 */
-
-#define VCHIQ_PORT_MAX                 (VCHIQ_MAX_SERVICES - 1)
-#define VCHIQ_PORT_FREE                0x1000
-#define VCHIQ_PORT_IS_VALID(port)      (port < VCHIQ_PORT_FREE)
-#define VCHIQ_MAKE_MSG(type, srcport, dstport) \
-	((type<<24) | (srcport<<12) | (dstport<<0))
-#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)msgid >> 24)
-#define VCHIQ_MSG_SRCPORT(msgid) \
-	(unsigned short)(((unsigned int)msgid >> 12) & 0xfff)
-#define VCHIQ_MSG_DSTPORT(msgid) \
-	((unsigned short)msgid & 0xfff)
-
 #define VCHIQ_FOURCC_AS_4CHARS(fourcc)	\
 	((fourcc) >> 24) & 0xff, \
 	((fourcc) >> 16) & 0xff, \
 	((fourcc) >>  8) & 0xff, \
 	(fourcc) & 0xff
 
-/* Ensure the fields are wide enough */
-vchiq_static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
-	== 0);
-vchiq_static_assert(VCHIQ_MSG_TYPE(VCHIQ_MAKE_MSG(0, VCHIQ_PORT_MAX, 0)) == 0);
-vchiq_static_assert((unsigned int)VCHIQ_PORT_MAX <
-	(unsigned int)VCHIQ_PORT_FREE);
-
-#define VCHIQ_MSGID_PADDING            VCHIQ_MAKE_MSG(VCHIQ_MSG_PADDING, 0, 0)
-#define VCHIQ_MSGID_CLAIMED            0x40000000
-
-#define VCHIQ_FOURCC_INVALID           0x00000000
-#define VCHIQ_FOURCC_IS_LEGAL(fourcc)  (fourcc != VCHIQ_FOURCC_INVALID)
-
-#define VCHIQ_BULK_ACTUAL_ABORTED -1
-
 typedef uint32_t BITSET_T;
 
 vchiq_static_assert((sizeof(BITSET_T) * 8) == 32);
@@ -140,17 +98,6 @@ vchiq_static_assert((sizeof(BITSET_T) * 8) == 32);
 #define BITSET_SET(bs, b)     (bs[BITSET_WORD(b)] |= BITSET_BIT(b))
 #define BITSET_CLR(bs, b)     (bs[BITSET_WORD(b)] &= ~BITSET_BIT(b))
 
-#if VCHIQ_ENABLE_STATS
-#define VCHIQ_STATS_INC(state, stat) (state->stats. stat++)
-#define VCHIQ_SERVICE_STATS_INC(service, stat) (service->stats. stat++)
-#define VCHIQ_SERVICE_STATS_ADD(service, stat, addend) \
-	(service->stats. stat += addend)
-#else
-#define VCHIQ_STATS_INC(state, stat) ((void)0)
-#define VCHIQ_SERVICE_STATS_INC(service, stat) ((void)0)
-#define VCHIQ_SERVICE_STATS_ADD(service, stat, addend) ((void)0)
-#endif
-
 enum {
 	DEBUG_ENTRIES,
 #if VCHIQ_ENABLE_DEBUG
@@ -212,14 +159,6 @@ enum {
 	VCHIQ_SRVSTATE_CLOSED
 };
 
-enum {
-	VCHIQ_POLL_TERMINATE,
-	VCHIQ_POLL_REMOVE,
-	VCHIQ_POLL_TXNOTIFY,
-	VCHIQ_POLL_RXNOTIFY,
-	VCHIQ_POLL_COUNT
-};
-
 enum vchiq_bulk_dir {
 	VCHIQ_BULK_TRANSMIT,
 	VCHIQ_BULK_RECEIVE
-- 
2.7.4

[PATCH 04/16] staging: vchiq_core: get the rid of IS_POW2

[Stefan Wahren]

The macro IS_POW2 is only used to ensure some size are powers of 2.
Better use BUILD_BUG_ON_NOT_POWER_OF_2 for this. Since this must be done
in a function, merge all these checks in a new function.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c      | 17 +++++++++++++----
 .../vc04_services/interface/vchiq_arm/vchiq_core.h      |  7 -------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index b808f91..32016ea 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -124,12 +124,19 @@ enum {
 
 /* we require this for consistency between endpoints */
 vchiq_static_assert(sizeof(struct vchiq_header) == 8);
-vchiq_static_assert(IS_POW2(sizeof(struct vchiq_header)));
-vchiq_static_assert(IS_POW2(VCHIQ_NUM_CURRENT_BULKS));
-vchiq_static_assert(IS_POW2(VCHIQ_NUM_SERVICE_BULKS));
-vchiq_static_assert(IS_POW2(VCHIQ_MAX_SERVICES));
 vchiq_static_assert(VCHIQ_VERSION >= VCHIQ_VERSION_MIN);
 
+static inline void check_sizes(void)
+{
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_SLOT_SIZE);
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_MAX_SLOTS);
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_MAX_SLOTS_PER_SIDE);
+	BUILD_BUG_ON_NOT_POWER_OF_2(sizeof(struct vchiq_header));
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_NUM_CURRENT_BULKS);
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_NUM_SERVICE_BULKS);
+	BUILD_BUG_ON_NOT_POWER_OF_2(VCHIQ_MAX_SERVICES);
+}
+
 /* Run time control of log level, based on KERN_XXX level. */
 int vchiq_core_log_level = VCHIQ_LOG_DEFAULT;
 int vchiq_core_msg_log_level = VCHIQ_LOG_DEFAULT;
@@ -2206,6 +2213,8 @@ vchiq_init_slots(void *mem_base, int mem_size)
 	int num_slots = (mem_size - mem_align)/VCHIQ_SLOT_SIZE;
 	int first_data_slot = VCHIQ_SLOT_ZERO_SLOTS;
 
+	check_sizes();
+
 	/* Ensure there is enough memory to run an absolutely minimum system */
 	num_slots -= first_data_slot;
 
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index db93495..df51418 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -69,13 +69,6 @@
 	extern int vchiq_static_assert[(cond) ? 1 : -1]
 #endif
 
-#define IS_POW2(x) (x && ((x & (x - 1)) == 0))
-
-/* Ensure that the slot size and maximum number of slots are powers of 2 */
-vchiq_static_assert(IS_POW2(VCHIQ_SLOT_SIZE));
-vchiq_static_assert(IS_POW2(VCHIQ_MAX_SLOTS));
-vchiq_static_assert(IS_POW2(VCHIQ_MAX_SLOTS_PER_SIDE));
-
 #define VCHIQ_SLOT_MASK        (VCHIQ_SLOT_SIZE - 1)
 #define VCHIQ_SLOT_QUEUE_MASK  (VCHIQ_MAX_SLOTS_PER_SIDE - 1)
 #define VCHIQ_SLOT_ZERO_SLOTS  DIV_ROUND_UP(sizeof(struct vchiq_slot_zero), \
-- 
2.7.4

[PATCH 05/16] staging: vchiq_core: get the rid of vchiq_static_assert

[Stefan Wahren]

Use static_assert from the kernel instead of opencode it.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c  |  2 +-
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 10 +++++-----
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h |  7 +------
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 12a294c..32278b4 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -139,7 +139,7 @@ static const char *const ioctl_names[] = {
 	"CLOSE_DELIVERED"
 };
 
-vchiq_static_assert(ARRAY_SIZE(ioctl_names) == (VCHIQ_IOC_MAX + 1));
+static_assert(ARRAY_SIZE(ioctl_names) == (VCHIQ_IOC_MAX + 1));
 
 static enum vchiq_status
 vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 32016ea..7fe0583 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -45,10 +45,10 @@
 	((unsigned short)msgid & 0xfff)
 
 /* Ensure the fields are wide enough */
-vchiq_static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
+static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
 	== 0);
-vchiq_static_assert(VCHIQ_MSG_TYPE(VCHIQ_MAKE_MSG(0, VCHIQ_PORT_MAX, 0)) == 0);
-vchiq_static_assert((unsigned int)VCHIQ_PORT_MAX <
+static_assert(VCHIQ_MSG_TYPE(VCHIQ_MAKE_MSG(0, VCHIQ_PORT_MAX, 0)) == 0);
+static_assert((unsigned int)VCHIQ_PORT_MAX <
 	(unsigned int)VCHIQ_PORT_FREE);
 
 #define VCHIQ_MSGID_PADDING            VCHIQ_MAKE_MSG(VCHIQ_MSG_PADDING, 0, 0)
@@ -123,8 +123,8 @@ enum {
 };
 
 /* we require this for consistency between endpoints */
-vchiq_static_assert(sizeof(struct vchiq_header) == 8);
-vchiq_static_assert(VCHIQ_VERSION >= VCHIQ_VERSION_MIN);
+static_assert(sizeof(struct vchiq_header) == 8);
+static_assert(VCHIQ_VERSION >= VCHIQ_VERSION_MIN);
 
 static inline void check_sizes(void)
 {
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index df51418..8d8e104 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -64,11 +64,6 @@
 #define vchiq_loud_error(...) \
 	vchiq_log_error(vchiq_core_log_level, "===== " __VA_ARGS__)
 
-#ifndef vchiq_static_assert
-#define vchiq_static_assert(cond) __attribute__((unused)) \
-	extern int vchiq_static_assert[(cond) ? 1 : -1]
-#endif
-
 #define VCHIQ_SLOT_MASK        (VCHIQ_SLOT_SIZE - 1)
 #define VCHIQ_SLOT_QUEUE_MASK  (VCHIQ_MAX_SLOTS_PER_SIDE - 1)
 #define VCHIQ_SLOT_ZERO_SLOTS  DIV_ROUND_UP(sizeof(struct vchiq_slot_zero), \
@@ -82,7 +77,7 @@
 
 typedef uint32_t BITSET_T;
 
-vchiq_static_assert((sizeof(BITSET_T) * 8) == 32);
+static_assert((sizeof(BITSET_T) * 8) == 32);
 
 #define BITSET_SIZE(b)        ((b + 31) >> 5)
 #define BITSET_WORD(b)        (b >> 5)
-- 
2.7.4

[PATCH 06/16] staging: vchiq_core: put spaces around operators

[Stefan Wahren]

This fixes the checkpatch issues regarding missing spaces around operators.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c   | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 7fe0583..3570132 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -37,7 +37,7 @@
 #define VCHIQ_PORT_FREE                0x1000
 #define VCHIQ_PORT_IS_VALID(port)      (port < VCHIQ_PORT_FREE)
 #define VCHIQ_MAKE_MSG(type, srcport, dstport) \
-	((type<<24) | (srcport<<12) | (dstport<<0))
+	((type << 24) | (srcport << 12) | (dstport << 0))
 #define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)msgid >> 24)
 #define VCHIQ_MSG_SRCPORT(msgid) \
 	(unsigned short)(((unsigned int)msgid >> 12) & 0xfff)
@@ -2210,7 +2210,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
 		(int)((VCHIQ_SLOT_SIZE - (long)mem_base) & VCHIQ_SLOT_MASK);
 	struct vchiq_slot_zero *slot_zero =
 		(struct vchiq_slot_zero *)(mem_base + mem_align);
-	int num_slots = (mem_size - mem_align)/VCHIQ_SLOT_SIZE;
+	int num_slots = (mem_size - mem_align) / VCHIQ_SLOT_SIZE;
 	int first_data_slot = VCHIQ_SLOT_ZERO_SLOTS;
 
 	check_sizes();
@@ -2237,9 +2237,9 @@ vchiq_init_slots(void *mem_base, int mem_size)
 
 	slot_zero->master.slot_sync = first_data_slot;
 	slot_zero->master.slot_first = first_data_slot + 1;
-	slot_zero->master.slot_last = first_data_slot + (num_slots/2) - 1;
-	slot_zero->slave.slot_sync = first_data_slot + (num_slots/2);
-	slot_zero->slave.slot_first = first_data_slot + (num_slots/2) + 1;
+	slot_zero->master.slot_last = first_data_slot + (num_slots / 2) - 1;
+	slot_zero->slave.slot_sync = first_data_slot + (num_slots / 2);
+	slot_zero->slave.slot_first = first_data_slot + (num_slots / 2) + 1;
 	slot_zero->slave.slot_last = first_data_slot + num_slots - 1;
 
 	return slot_zero;
@@ -2309,7 +2309,7 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
 		complete(&state->slot_available_event);
 	}
 
-	state->default_slot_quota = state->slot_queue_available/2;
+	state->default_slot_quota = state->slot_queue_available / 2;
 	state->default_message_quota =
 		min((unsigned short)(state->default_slot_quota * 256),
 		(unsigned short)~0);
-- 
2.7.4

[PATCH 07/16] staging: vchiq_core: avoid precedence issues

[Stefan Wahren]

Add () around macro argument to avoid precedence issues

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 3570132..0afac46 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -35,14 +35,14 @@
 
 #define VCHIQ_PORT_MAX                 (VCHIQ_MAX_SERVICES - 1)
 #define VCHIQ_PORT_FREE                0x1000
-#define VCHIQ_PORT_IS_VALID(port)      (port < VCHIQ_PORT_FREE)
+#define VCHIQ_PORT_IS_VALID(port)      ((port) < VCHIQ_PORT_FREE)
 #define VCHIQ_MAKE_MSG(type, srcport, dstport) \
-	((type << 24) | (srcport << 12) | (dstport << 0))
-#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)msgid >> 24)
+	(((type) << 24) | ((srcport) << 12) | ((dstport) << 0))
+#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)(msgid) >> 24)
 #define VCHIQ_MSG_SRCPORT(msgid) \
-	(unsigned short)(((unsigned int)msgid >> 12) & 0xfff)
+	(unsigned short)(((unsigned int)(msgid) >> 12) & 0xfff)
 #define VCHIQ_MSG_DSTPORT(msgid) \
-	((unsigned short)msgid & 0xfff)
+	((unsigned short)(msgid) & 0xfff)
 
 /* Ensure the fields are wide enough */
 static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
@@ -55,7 +55,7 @@ static_assert((unsigned int)VCHIQ_PORT_MAX <
 #define VCHIQ_MSGID_CLAIMED            0x40000000
 
 #define VCHIQ_FOURCC_INVALID           0x00000000
-#define VCHIQ_FOURCC_IS_LEGAL(fourcc)  (fourcc != VCHIQ_FOURCC_INVALID)
+#define VCHIQ_FOURCC_IS_LEGAL(fourcc)  ((fourcc) != VCHIQ_FOURCC_INVALID)
 
 #define VCHIQ_BULK_ACTUAL_ABORTED -1
 
@@ -84,7 +84,7 @@ static_assert((unsigned int)VCHIQ_PORT_MAX <
 #define SLOT_QUEUE_INDEX_FROM_POS_MASKED(pos) \
 	(SLOT_QUEUE_INDEX_FROM_POS(pos) & VCHIQ_SLOT_QUEUE_MASK)
 
-#define BULK_INDEX(x) (x & (VCHIQ_NUM_SERVICE_BULKS - 1))
+#define BULK_INDEX(x) ((x) & (VCHIQ_NUM_SERVICE_BULKS - 1))
 
 #define SRVTRACE_LEVEL(srv) \
 	(((srv) && (srv)->trace) ? VCHIQ_LOG_TRACE : vchiq_core_msg_log_level)
-- 
2.7.4

[PATCH 08/16] staging: vchiq_core: use define for message type shift

[Stefan Wahren]

Make it clear and use a define for shifting the message type.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 0afac46..f6ada95 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -33,12 +33,14 @@
 #define VCHIQ_MSG_REMOTE_RELEASE    13  /* -                                 */
 #define VCHIQ_MSG_REMOTE_USE_ACTIVE 14  /* -                                 */
 
+#define TYPE_SHIFT 24
+
 #define VCHIQ_PORT_MAX                 (VCHIQ_MAX_SERVICES - 1)
 #define VCHIQ_PORT_FREE                0x1000
 #define VCHIQ_PORT_IS_VALID(port)      ((port) < VCHIQ_PORT_FREE)
 #define VCHIQ_MAKE_MSG(type, srcport, dstport) \
-	(((type) << 24) | ((srcport) << 12) | ((dstport) << 0))
-#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)(msgid) >> 24)
+	(((type) << TYPE_SHIFT) | ((srcport) << 12) | ((dstport) << 0))
+#define VCHIQ_MSG_TYPE(msgid)          ((unsigned int)(msgid) >> TYPE_SHIFT)
 #define VCHIQ_MSG_SRCPORT(msgid) \
 	(unsigned short)(((unsigned int)(msgid) >> 12) & 0xfff)
 #define VCHIQ_MSG_DSTPORT(msgid) \
-- 
2.7.4

[PATCH 09/16] staging: vchiq_core: introduce message specific make macros

[Stefan Wahren]

VCHIQ_MAKE_MSG isn't optimal because a lot of message types doesn't need
a parameter. So better define a macro for every message type which is used.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 64 ++++++++++------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index f6ada95..99624ca 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -46,6 +46,20 @@
 #define VCHIQ_MSG_DSTPORT(msgid) \
 	((unsigned short)(msgid) & 0xfff)
 
+#define MAKE_CONNECT			(VCHIQ_MSG_CONNECT << TYPE_SHIFT)
+#define MAKE_OPEN(srcport) \
+	((VCHIQ_MSG_OPEN << TYPE_SHIFT) | ((srcport) << 12))
+#define MAKE_OPENACK(srcport, dstport) \
+	((VCHIQ_MSG_OPENACK << TYPE_SHIFT) | ((srcport) << 12) | ((dstport) << 0))
+#define MAKE_CLOSE(srcport, dstport) \
+	((VCHIQ_MSG_CLOSE << TYPE_SHIFT) | ((srcport) << 12) | ((dstport) << 0))
+#define MAKE_DATA(srcport, dstport) \
+	((VCHIQ_MSG_DATA << TYPE_SHIFT) | ((srcport) << 12) | ((dstport) << 0))
+#define MAKE_PAUSE			(VCHIQ_MSG_PAUSE << TYPE_SHIFT)
+#define MAKE_RESUME			(VCHIQ_MSG_RESUME << TYPE_SHIFT)
+#define MAKE_REMOTE_USE			(VCHIQ_MSG_REMOTE_USE << TYPE_SHIFT)
+#define MAKE_REMOTE_USE_ACTIVE		(VCHIQ_MSG_REMOTE_USE_ACTIVE << TYPE_SHIFT)
+
 /* Ensure the fields are wide enough */
 static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
 	== 0);
@@ -1566,10 +1580,8 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 					if (queue_message_sync(
 						state,
 						NULL,
-						VCHIQ_MAKE_MSG(
-							VCHIQ_MSG_OPENACK,
-							service->localport,
-							remoteport),
+						MAKE_OPENACK(service->localport,
+							     remoteport),
 						memcpy_copy_callback,
 						&ack_payload,
 						sizeof(ack_payload),
@@ -1578,8 +1590,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 				} else {
 					if (queue_message(state,
 							NULL,
-							VCHIQ_MAKE_MSG(
-							VCHIQ_MSG_OPENACK,
+							MAKE_OPENACK(
 							service->localport,
 							remoteport),
 						memcpy_copy_callback,
@@ -1603,8 +1614,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 
 fail_open:
 	/* No available service, or an invalid request - send a CLOSE */
-	if (queue_message(state, NULL,
-		VCHIQ_MAKE_MSG(VCHIQ_MSG_CLOSE, 0, VCHIQ_MSG_SRCPORT(msgid)),
+	if (queue_message(state, NULL, MAKE_CLOSE(0, VCHIQ_MSG_SRCPORT(msgid)),
 		NULL, NULL, 0, 0) == VCHIQ_RETRY)
 		goto bail_not_ready;
 
@@ -1885,8 +1895,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 		}
 		if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
 			/* Send a PAUSE in response */
-			if (queue_message(state, NULL,
-				VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
+			if (queue_message(state, NULL, MAKE_PAUSE,
 				NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
 			    == VCHIQ_RETRY)
 				goto bail_not_ready;
@@ -2018,8 +2027,7 @@ slot_handler_func(void *v)
 				break;
 
 			case VCHIQ_CONNSTATE_PAUSING:
-				if (queue_message(state, NULL,
-					VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
+				if (queue_message(state, NULL, MAKE_PAUSE,
 					NULL, NULL, 0,
 					QMFLAGS_NO_MUTEX_UNLOCK)
 				    != VCHIQ_RETRY) {
@@ -2032,8 +2040,7 @@ slot_handler_func(void *v)
 				break;
 
 			case VCHIQ_CONNSTATE_RESUMING:
-				if (queue_message(state, NULL,
-					VCHIQ_MAKE_MSG(VCHIQ_MSG_RESUME, 0, 0),
+				if (queue_message(state, NULL, MAKE_RESUME,
 					NULL, NULL, 0, QMFLAGS_NO_MUTEX_LOCK)
 					!= VCHIQ_RETRY) {
 					vchiq_set_conn_state(state,
@@ -2613,10 +2620,7 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
 	service->client_id = client_id;
 	vchiq_use_service_internal(service);
 	status = queue_message(service->state,
-			       NULL,
-			       VCHIQ_MAKE_MSG(VCHIQ_MSG_OPEN,
-					      service->localport,
-					      0),
+			       NULL, MAKE_OPEN(service->localport),
 			       memcpy_copy_callback,
 			       &payload,
 			       sizeof(payload),
@@ -2841,9 +2845,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 		} else {
 			/* Shutdown mid-open - let the other side know */
 			status = queue_message(state, service,
-				VCHIQ_MAKE_MSG
-				(VCHIQ_MSG_CLOSE,
-				service->localport,
+				MAKE_CLOSE(service->localport,
 				VCHIQ_MSG_DSTPORT(service->remoteport)),
 				NULL, NULL, 0, 0);
 		}
@@ -2862,9 +2864,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 
 		if (status == VCHIQ_SUCCESS)
 			status = queue_message(state, service,
-				VCHIQ_MAKE_MSG
-				(VCHIQ_MSG_CLOSE,
-				service->localport,
+				MAKE_CLOSE(service->localport,
 				VCHIQ_MSG_DSTPORT(service->remoteport)),
 				NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK);
 
@@ -2992,8 +2992,7 @@ vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instanc
 	}
 
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) {
-		if (queue_message(state, NULL,
-			VCHIQ_MAKE_MSG(VCHIQ_MSG_CONNECT, 0, 0), NULL, NULL,
+		if (queue_message(state, NULL, MAKE_CONNECT, NULL, NULL,
 			0, QMFLAGS_IS_BLOCKING) == VCHIQ_RETRY)
 			return VCHIQ_RETRY;
 
@@ -3342,15 +3341,13 @@ vchiq_queue_message(unsigned int handle,
 	switch (service->srvstate) {
 	case VCHIQ_SRVSTATE_OPEN:
 		status = queue_message(service->state, service,
-				VCHIQ_MAKE_MSG(VCHIQ_MSG_DATA,
-					service->localport,
+				MAKE_DATA(service->localport,
 					service->remoteport),
 				copy_callback, context, size, 1);
 		break;
 	case VCHIQ_SRVSTATE_OPENSYNC:
 		status = queue_message_sync(service->state, service,
-				VCHIQ_MAKE_MSG(VCHIQ_MSG_DATA,
-					service->localport,
+				MAKE_DATA(service->localport,
 					service->remoteport),
 				copy_callback, context, size, 1);
 		break;
@@ -3813,9 +3810,7 @@ enum vchiq_status vchiq_send_remote_use(struct vchiq_state *state)
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
 		return VCHIQ_RETRY;
 
-	return queue_message(state, NULL,
-			     VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE, 0, 0),
-			     NULL, NULL, 0, 0);
+	return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0);
 }
 
 enum vchiq_status vchiq_send_remote_use_active(struct vchiq_state *state)
@@ -3823,8 +3818,7 @@ enum vchiq_status vchiq_send_remote_use_active(struct vchiq_state *state)
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
 		return VCHIQ_RETRY;
 
-	return queue_message(state, NULL,
-			     VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE_ACTIVE, 0, 0),
+	return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE,
 			     NULL, NULL, 0, 0);
 }
 
-- 
2.7.4

[PATCH 10/16] staging: vchiq_core: simplify WARN_ON conditions

[Stefan Wahren]

During a recent review Dan Carpenter noticed a double negation in a WARN_ON.
But a quick search revealed more unnecessary complex WARN_ON conditions.
This change should simplify them.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c   | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 99624ca..2e1b218 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -924,7 +924,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
 
 	stride = calc_stride(size);
 
-	WARN_ON(!(stride <= VCHIQ_SLOT_SIZE));
+	WARN_ON(stride > VCHIQ_SLOT_SIZE);
 
 	if (!(flags & QMFLAGS_NO_MUTEX_LOCK) &&
 	    mutex_lock_killable(&state->slot_mutex))
@@ -1480,8 +1480,8 @@ abort_outstanding_bulks(struct vchiq_service *service,
 		service->state->id, service->localport, is_tx ? 't' : 'r',
 		queue->local_insert, queue->remote_insert, queue->process);
 
-	WARN_ON(!((int)(queue->local_insert - queue->process) >= 0));
-	WARN_ON(!((int)(queue->remote_insert - queue->process) >= 0));
+	WARN_ON((int)(queue->local_insert - queue->process) < 0);
+	WARN_ON((int)(queue->remote_insert - queue->process) < 0);
 
 	while ((queue->process != queue->local_insert) ||
 		(queue->process != queue->remote_insert)) {
@@ -1729,7 +1729,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 
 	switch (type) {
 	case VCHIQ_MSG_OPEN:
-		WARN_ON(!(VCHIQ_MSG_DSTPORT(msgid) == 0));
+		WARN_ON(VCHIQ_MSG_DSTPORT(msgid));
 		if (!parse_open(state, header))
 			goto bail_not_ready;
 		break;
@@ -1756,7 +1756,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 		}
 		break;
 	case VCHIQ_MSG_CLOSE:
-		WARN_ON(size != 0); /* There should be no data */
+		WARN_ON(size); /* There should be no data */
 
 		vchiq_log_info(vchiq_core_log_level,
 			"%d: prs CLOSE@%pK (%d->%d)",
@@ -1958,7 +1958,7 @@ parse_rx_slots(struct vchiq_state *state)
 		if (!state->rx_data) {
 			int rx_index;
 
-			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
+			WARN_ON(state->rx_pos & VCHIQ_SLOT_MASK);
 			rx_index = remote->slot_queue[
 				SLOT_QUEUE_INDEX_FROM_POS_MASKED(state->rx_pos)];
 			state->rx_data = (char *)SLOT_DATA_FROM_INDEX(state,
-- 
2.7.4

[PATCH 11/16] staging: vchiq_arm: tidy up service function naming

[Stefan Wahren]

During a recent review Dan Carpenter reported that the function naming of
(un)lock_service is misleading. They are like wrapper around kref_get /
kref_put and don't have anything to do with locking. So use a name which
represent more the actual behavior. Btw add the vchiq prefix to avoid
possible name conflicts.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 32 +++++++--------
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 46 +++++++++++-----------
 .../vc04_services/interface/vchiq_arm/vchiq_core.h |  4 +-
 3 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 32278b4..72e9576 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -431,7 +431,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
 
 	instance = service->instance;
 
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	mutex_lock(&instance->bulk_waiter_list_mutex);
 	list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
@@ -539,7 +539,7 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
 		 * Take an extra reference, to be held until
 		 * this CLOSED notification is delivered.
 		 */
-		lock_service(user_service->service);
+		vchiq_service_get(user_service->service);
 		if (instance->use_close_delivered)
 			user_service->close_pending = 1;
 	}
@@ -686,7 +686,7 @@ static void close_delivered(struct user_service *user_service)
 
 	if (user_service->close_pending) {
 		/* Allow the underlying service to be culled */
-		unlock_service(user_service->service);
+		vchiq_service_put(user_service->service);
 
 		/* Wake the user-thread blocked in close_ or remove_service */
 		complete(&user_service->close_event);
@@ -915,7 +915,7 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
 	}
 	DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
 out:
-	unlock_service(service);
+	vchiq_service_put(service);
 	return ret;
 }
 
@@ -1001,7 +1001,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 		ret = put_user(mode_waiting, mode);
 	}
 out:
-	unlock_service(service);
+	vchiq_service_put(service);
 	if (ret)
 		return ret;
 	else if (status == VCHIQ_ERROR)
@@ -1181,7 +1181,7 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
 
 		if ((completion->reason == VCHIQ_SERVICE_CLOSED) &&
 		    !instance->use_close_delivered)
-			unlock_service(service);
+			vchiq_service_put(service);
 
 		/*
 		 * FIXME: address space mismatch, does bulk_userdata
@@ -1243,7 +1243,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		while ((service = next_service_by_instance(instance->state,
 			instance, &i))) {
 			status = vchiq_remove_service(service->handle);
-			unlock_service(service);
+			vchiq_service_put(service);
 			if (status != VCHIQ_SUCCESS)
 				break;
 		}
@@ -1508,7 +1508,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	}
 
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 
 	if (ret == 0) {
 		if (status == VCHIQ_ERROR)
@@ -1641,7 +1641,7 @@ vchiq_compat_ioctl_queue_message(struct file *file,
 
 		if (copy_from_user(&element32, args.elements,
 				   sizeof(element32))) {
-			unlock_service(service);
+			vchiq_service_put(service);
 			return -EFAULT;
 		}
 
@@ -1655,7 +1655,7 @@ vchiq_compat_ioctl_queue_message(struct file *file,
 	} else {
 		ret = -EINVAL;
 	}
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	return ret;
 }
@@ -1891,7 +1891,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 		complete(&user_service->remove_event);
 
 		vchiq_terminate_service_internal(service);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 
 	/* ...and wait for them to die */
@@ -1902,7 +1902,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 		wait_for_completion(&service->remove_event);
 
 		if (WARN_ON(service->srvstate != VCHIQ_SRVSTATE_FREE)) {
-			unlock_service(service);
+			vchiq_service_put(service);
 			break;
 		}
 
@@ -1923,7 +1923,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 
 		spin_unlock(&msg_queue_spinlock);
 
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 
 	/* Release any closed services */
@@ -1941,7 +1941,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 			/* Wake any blocked user-thread */
 			if (instance->use_close_delivered)
 				complete(&user_service->close_event);
-			unlock_service(service);
+			vchiq_service_put(service);
 		}
 		instance->completion_remove++;
 	}
@@ -2444,7 +2444,7 @@ vchiq_use_service(unsigned int handle)
 	if (service) {
 		ret = vchiq_use_internal(service->state, service,
 				USE_TYPE_SERVICE);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 	return ret;
 }
@@ -2458,7 +2458,7 @@ vchiq_release_service(unsigned int handle)
 
 	if (service) {
 		ret = vchiq_release_internal(service->state, service);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 	return ret;
 }
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 2e1b218..af2effa 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -368,7 +368,7 @@ next_service_by_instance(struct vchiq_state *state,
 }
 
 void
-lock_service(struct vchiq_service *service)
+vchiq_service_get(struct vchiq_service *service)
 {
 	if (!service) {
 		WARN(1, "%s service is NULL\n", __func__);
@@ -391,7 +391,7 @@ static void service_release(struct kref *kref)
 }
 
 void
-unlock_service(struct vchiq_service *service)
+vchiq_service_put(struct vchiq_service *service)
 {
 	if (!service) {
 		WARN(1, "%s: service is NULL\n", __func__);
@@ -1454,7 +1454,7 @@ poll_services_of_group(struct vchiq_state *state, int group)
 			notify_bulks(service, &service->bulk_tx, RETRY_POLL);
 		if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
 			notify_bulks(service, &service->bulk_rx, RETRY_POLL);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 }
 
@@ -1560,7 +1560,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 					service->version, service->version_min,
 					version, version_min);
 				vchiq_loud_error_footer();
-				unlock_service(service);
+				vchiq_service_put(service);
 				service = NULL;
 				goto fail_open;
 			}
@@ -1607,7 +1607,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 			}
 
 			/* Success - the message has been dealt with */
-			unlock_service(service);
+			vchiq_service_put(service);
 			return 1;
 		}
 	}
@@ -1622,7 +1622,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 
 bail_not_ready:
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 
 	return 0;
 }
@@ -1678,7 +1678,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 			 * the connected service
 			 */
 			if (service)
-				unlock_service(service);
+				vchiq_service_put(service);
 			service = get_connected_service(state,
 				remoteport);
 			if (service)
@@ -1934,7 +1934,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 
 bail_not_ready:
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 
 	return ret;
 }
@@ -2190,7 +2190,7 @@ sync_func(void *v)
 			break;
 		}
 
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 
 	return 0;
@@ -2972,7 +2972,7 @@ vchiq_free_service_internal(struct vchiq_service *service)
 	complete(&service->remove_event);
 
 	/* Release the initial lock */
-	unlock_service(service);
+	vchiq_service_put(service);
 }
 
 enum vchiq_status
@@ -2988,7 +2988,7 @@ vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instanc
 		if (service->srvstate == VCHIQ_SRVSTATE_HIDDEN)
 			vchiq_set_service_state(service,
 				VCHIQ_SRVSTATE_LISTENING);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) {
@@ -3021,7 +3021,7 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
 	while ((service = next_service_by_instance(state, instance,
 		&i)) !=	NULL) {
 		(void)vchiq_remove_service(service->handle);
-		unlock_service(service);
+		vchiq_service_put(service);
 	}
 }
 
@@ -3042,7 +3042,7 @@ vchiq_close_service(unsigned int handle)
 	if ((service->srvstate == VCHIQ_SRVSTATE_FREE) ||
 	    (service->srvstate == VCHIQ_SRVSTATE_LISTENING) ||
 	    (service->srvstate == VCHIQ_SRVSTATE_HIDDEN)) {
-		unlock_service(service);
+		vchiq_service_put(service);
 		return VCHIQ_ERROR;
 	}
 
@@ -3078,7 +3078,7 @@ vchiq_close_service(unsigned int handle)
 	    (service->srvstate != VCHIQ_SRVSTATE_LISTENING))
 		status = VCHIQ_ERROR;
 
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	return status;
 }
@@ -3099,7 +3099,7 @@ vchiq_remove_service(unsigned int handle)
 		service->state->id, service->localport);
 
 	if (service->srvstate == VCHIQ_SRVSTATE_FREE) {
-		unlock_service(service);
+		vchiq_service_put(service);
 		return VCHIQ_ERROR;
 	}
 
@@ -3139,7 +3139,7 @@ vchiq_remove_service(unsigned int handle)
 	    (service->srvstate != VCHIQ_SRVSTATE_FREE))
 		status = VCHIQ_ERROR;
 
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	return status;
 }
@@ -3284,7 +3284,7 @@ enum vchiq_status vchiq_bulk_transfer(unsigned int handle,
 		queue->local_insert, queue->remote_insert, queue->process);
 
 waiting:
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	status = VCHIQ_SUCCESS;
 
@@ -3307,7 +3307,7 @@ enum vchiq_status vchiq_bulk_transfer(unsigned int handle,
 
 error_exit:
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 	return status;
 }
 
@@ -3358,7 +3358,7 @@ vchiq_queue_message(unsigned int handle,
 
 error_exit:
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 
 	return status;
 }
@@ -3417,7 +3417,7 @@ vchiq_release_message(unsigned int handle,
 		release_message_sync(state, header);
 	}
 
-	unlock_service(service);
+	vchiq_service_put(service);
 }
 EXPORT_SYMBOL(vchiq_release_message);
 
@@ -3448,7 +3448,7 @@ vchiq_get_peer_version(unsigned int handle, short *peer_version)
 
 exit:
 	if (service)
-		unlock_service(service);
+		vchiq_service_put(service);
 	return status;
 }
 EXPORT_SYMBOL(vchiq_get_peer_version);
@@ -3532,7 +3532,7 @@ vchiq_set_service_option(unsigned int handle,
 	default:
 		break;
 	}
-	unlock_service(service);
+	vchiq_service_put(service);
 
 	return ret;
 }
@@ -3673,7 +3673,7 @@ int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
 
 		if (service) {
 			err = vchiq_dump_service_state(dump_context, service);
-			unlock_service(service);
+			vchiq_service_put(service);
 			if (err)
 				return err;
 		}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 8d8e104..957fea1 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -554,10 +554,10 @@ next_service_by_instance(struct vchiq_state *state,
 			 int *pidx);
 
 extern void
-lock_service(struct vchiq_service *service);
+vchiq_service_get(struct vchiq_service *service);
 
 extern void
-unlock_service(struct vchiq_service *service);
+vchiq_service_put(struct vchiq_service *service);
 
 extern enum vchiq_status
 vchiq_queue_message(unsigned int handle,
-- 
2.7.4

[PATCH 12/16] staging: vchiq_core: introduce process_free_data_message

[Stefan Wahren]

This moves the special handling for data messages from process_free_queue()
into a new function. After this process_free_queue() has less extreme
indentation and should be easier to read.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 124 +++++++++++----------
 1 file changed, 64 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index af2effa..84dcbc2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -717,6 +717,68 @@ reserve_space(struct vchiq_state *state, size_t space, int is_blocking)
 						(tx_pos & VCHIQ_SLOT_MASK));
 }
 
+static void
+process_free_data_message(struct vchiq_state *state, BITSET_T *service_found,
+			  struct vchiq_header *header)
+{
+	int msgid = header->msgid;
+	int port = VCHIQ_MSG_SRCPORT(msgid);
+	struct vchiq_service_quota *quota = &state->service_quotas[port];
+	int count;
+
+	spin_lock(&quota_spinlock);
+	count = quota->message_use_count;
+	if (count > 0)
+		quota->message_use_count = count - 1;
+	spin_unlock(&quota_spinlock);
+
+	if (count == quota->message_quota) {
+		/*
+		 * Signal the service that it
+		 * has dropped below its quota
+		 */
+		complete(&quota->quota_event);
+	} else if (count == 0) {
+		vchiq_log_error(vchiq_core_log_level,
+			"service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
+			port,
+			quota->message_use_count,
+			header, msgid, header->msgid,
+			header->size);
+		WARN(1, "invalid message use count\n");
+	}
+	if (!BITSET_IS_SET(service_found, port)) {
+		/* Set the found bit for this service */
+		BITSET_SET(service_found, port);
+
+		spin_lock(&quota_spinlock);
+		count = quota->slot_use_count;
+		if (count > 0)
+			quota->slot_use_count = count - 1;
+		spin_unlock(&quota_spinlock);
+
+		if (count > 0) {
+			/*
+			 * Signal the service in case
+			 * it has dropped below its quota
+			 */
+			complete(&quota->quota_event);
+			vchiq_log_trace(vchiq_core_log_level,
+				"%d: pfq:%d %x@%pK - slot_use->%d",
+				state->id, port,
+				header->size, header,
+				count - 1);
+		} else {
+			vchiq_log_error(vchiq_core_log_level,
+					"service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
+				port, count, header,
+				msgid, header->msgid,
+				header->size);
+			WARN(1, "bad slot use count\n");
+		}
+	}
+}
+
 /* Called by the recycle thread. */
 static void
 process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
@@ -767,66 +829,8 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
 			int msgid = header->msgid;
 
 			if (VCHIQ_MSG_TYPE(msgid) == VCHIQ_MSG_DATA) {
-				int port = VCHIQ_MSG_SRCPORT(msgid);
-				struct vchiq_service_quota *quota =
-					&state->service_quotas[port];
-				int count;
-
-				spin_lock(&quota_spinlock);
-				count = quota->message_use_count;
-				if (count > 0)
-					quota->message_use_count = count - 1;
-				spin_unlock(&quota_spinlock);
-
-				if (count == quota->message_quota) {
-					/*
-					 * Signal the service that it
-					 * has dropped below its quota
-					 */
-					complete(&quota->quota_event);
-				} else if (count == 0) {
-					vchiq_log_error(vchiq_core_log_level,
-						"service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
-						port,
-						quota->message_use_count,
-						header, msgid, header->msgid,
-						header->size);
-					WARN(1, "invalid message use count\n");
-				}
-				if (!BITSET_IS_SET(service_found, port)) {
-					/* Set the found bit for this service */
-					BITSET_SET(service_found, port);
-
-					spin_lock(&quota_spinlock);
-					count = quota->slot_use_count;
-					if (count > 0)
-						quota->slot_use_count =
-							count - 1;
-					spin_unlock(&quota_spinlock);
-
-					if (count > 0) {
-						/*
-						 * Signal the service in case
-						 * it has dropped below its quota
-						 */
-						complete(&quota->quota_event);
-						vchiq_log_trace(
-							vchiq_core_log_level,
-							"%d: pfq:%d %x@%pK - slot_use->%d",
-							state->id, port,
-							header->size, header,
-							count - 1);
-					} else {
-						vchiq_log_error(
-							vchiq_core_log_level,
-								"service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
-							port, count, header,
-							msgid, header->msgid,
-							header->size);
-						WARN(1, "bad slot use count\n");
-					}
-				}
-
+				process_free_data_message(state, service_found,
+							  header);
 				data_found = 1;
 			}
 
-- 
2.7.4

[PATCH 13/16] staging: vchiq_core: reduce indentation in parse_open

[Stefan Wahren]

The function parse_open() already has bail out sections like fail_open.
So use a goto for the other error cases like payload size is too small
and listening service cannot be found. This avoids two indentation levels.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 146 ++++++++++-----------
 1 file changed, 70 insertions(+), 76 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 84dcbc2..3d543c5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1526,96 +1526,90 @@ abort_outstanding_bulks(struct vchiq_service *service,
 static int
 parse_open(struct vchiq_state *state, struct vchiq_header *header)
 {
+	const struct vchiq_open_payload *payload;
 	struct vchiq_service *service = NULL;
 	int msgid, size;
-	unsigned int localport, remoteport;
+	unsigned int localport, remoteport, fourcc;
+	short version, version_min;
 
 	msgid = header->msgid;
 	size = header->size;
 	localport = VCHIQ_MSG_DSTPORT(msgid);
 	remoteport = VCHIQ_MSG_SRCPORT(msgid);
-	if (size >= sizeof(struct vchiq_open_payload)) {
-		const struct vchiq_open_payload *payload =
-			(struct vchiq_open_payload *)header->data;
-		unsigned int fourcc;
-
-		fourcc = payload->fourcc;
-		vchiq_log_info(vchiq_core_log_level,
-			"%d: prs OPEN@%pK (%d->'%c%c%c%c')",
-			state->id, header, localport,
-			VCHIQ_FOURCC_AS_4CHARS(fourcc));
-
-		service = get_listening_service(state, fourcc);
+	if (size < sizeof(struct vchiq_open_payload))
+		goto fail_open;
 
-		if (service) {
-			/* A matching service exists */
-			short version = payload->version;
-			short version_min = payload->version_min;
-
-			if ((service->version < version_min) ||
-				(version < service->version_min)) {
-				/* Version mismatch */
-				vchiq_loud_error_header();
-				vchiq_loud_error("%d: service %d (%c%c%c%c) "
-					"version mismatch - local (%d, min %d)"
-					" vs. remote (%d, min %d)",
-					state->id, service->localport,
-					VCHIQ_FOURCC_AS_4CHARS(fourcc),
-					service->version, service->version_min,
-					version, version_min);
-				vchiq_loud_error_footer();
-				vchiq_service_put(service);
-				service = NULL;
-				goto fail_open;
-			}
-			service->peer_version = version;
+	payload = (struct vchiq_open_payload *)header->data;
+	fourcc = payload->fourcc;
+	vchiq_log_info(vchiq_core_log_level,
+		"%d: prs OPEN@%pK (%d->'%c%c%c%c')",
+		state->id, header, localport,
+		VCHIQ_FOURCC_AS_4CHARS(fourcc));
 
-			if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
-				struct vchiq_openack_payload ack_payload = {
-					service->version
-				};
-
-				if (state->version_common <
-				    VCHIQ_VERSION_SYNCHRONOUS_MODE)
-					service->sync = 0;
-
-				/* Acknowledge the OPEN */
-				if (service->sync) {
-					if (queue_message_sync(
-						state,
-						NULL,
-						MAKE_OPENACK(service->localport,
-							     remoteport),
-						memcpy_copy_callback,
-						&ack_payload,
-						sizeof(ack_payload),
-						0) == VCHIQ_RETRY)
-						goto bail_not_ready;
-				} else {
-					if (queue_message(state,
-							NULL,
-							MAKE_OPENACK(
-							service->localport,
-							remoteport),
-						memcpy_copy_callback,
-						&ack_payload,
-						sizeof(ack_payload),
-						0) == VCHIQ_RETRY)
-						goto bail_not_ready;
-				}
+	service = get_listening_service(state, fourcc);
+	if (!service)
+		goto fail_open;
 
-				/* The service is now open */
-				vchiq_set_service_state(service,
-					service->sync ? VCHIQ_SRVSTATE_OPENSYNC
-					: VCHIQ_SRVSTATE_OPEN);
-			}
+	/* A matching service exists */
+	version = payload->version;
+	version_min = payload->version_min;
 
-			/* Success - the message has been dealt with */
-			vchiq_service_put(service);
-			return 1;
+	if ((service->version < version_min) ||
+		(version < service->version_min)) {
+		/* Version mismatch */
+		vchiq_loud_error_header();
+		vchiq_loud_error("%d: service %d (%c%c%c%c) "
+			"version mismatch - local (%d, min %d)"
+			" vs. remote (%d, min %d)",
+			state->id, service->localport,
+			VCHIQ_FOURCC_AS_4CHARS(fourcc),
+			service->version, service->version_min,
+			version, version_min);
+		vchiq_loud_error_footer();
+		vchiq_service_put(service);
+		service = NULL;
+		goto fail_open;
+	}
+	service->peer_version = version;
+
+	if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
+		struct vchiq_openack_payload ack_payload = {
+			service->version
+		};
+
+		if (state->version_common <
+		    VCHIQ_VERSION_SYNCHRONOUS_MODE)
+			service->sync = 0;
+
+		/* Acknowledge the OPEN */
+		if (service->sync) {
+			if (queue_message_sync(state, NULL,
+				MAKE_OPENACK(service->localport, remoteport),
+				memcpy_copy_callback,
+				&ack_payload,
+				sizeof(ack_payload),
+				0) == VCHIQ_RETRY)
+				goto bail_not_ready;
+		} else {
+			if (queue_message(state, NULL,
+				MAKE_OPENACK(service->localport, remoteport),
+				memcpy_copy_callback,
+				&ack_payload,
+				sizeof(ack_payload),
+				0) == VCHIQ_RETRY)
+				goto bail_not_ready;
 		}
+
+		/* The service is now open */
+		vchiq_set_service_state(service,
+			service->sync ? VCHIQ_SRVSTATE_OPENSYNC
+			: VCHIQ_SRVSTATE_OPEN);
 	}
 
+	/* Success - the message has been dealt with */
+	vchiq_service_put(service);
+	return 1;
+
 fail_open:
 	/* No available service, or an invalid request - send a CLOSE */
 	if (queue_message(state, NULL, MAKE_CLOSE(0, VCHIQ_MSG_SRCPORT(msgid)),
-- 
2.7.4

[PATCH 14/16] staging: vchiq_core: store message id in local variable

[Stefan Wahren]

Some queue_message() calls are still rather complex to read. So store
the message ids in a local variable in those cases.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 28 ++++++++++------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 3d543c5..a2a2472 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1576,6 +1576,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 		struct vchiq_openack_payload ack_payload = {
 			service->version
 		};
+		int openack_id = MAKE_OPENACK(service->localport, remoteport);
 
 		if (state->version_common <
 		    VCHIQ_VERSION_SYNCHRONOUS_MODE)
@@ -1583,16 +1584,14 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 
 		/* Acknowledge the OPEN */
 		if (service->sync) {
-			if (queue_message_sync(state, NULL,
-				MAKE_OPENACK(service->localport, remoteport),
+			if (queue_message_sync(state, NULL, openack_id,
 				memcpy_copy_callback,
 				&ack_payload,
 				sizeof(ack_payload),
 				0) == VCHIQ_RETRY)
 				goto bail_not_ready;
 		} else {
-			if (queue_message(state, NULL,
-				MAKE_OPENACK(service->localport, remoteport),
+			if (queue_message(state, NULL, openack_id,
 				memcpy_copy_callback,
 				&ack_payload,
 				sizeof(ack_payload),
@@ -2804,6 +2803,8 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 	struct vchiq_state *state = service->state;
 	enum vchiq_status status = VCHIQ_SUCCESS;
 	int is_server = (service->public_fourcc != VCHIQ_FOURCC_INVALID);
+	int close_id = MAKE_CLOSE(service->localport,
+				  VCHIQ_MSG_DSTPORT(service->remoteport));
 
 	vchiq_log_info(vchiq_core_log_level, "%d: csi:%d,%d (%s)",
 		service->state->id, service->localport, close_recvd,
@@ -2842,9 +2843,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 			complete(&service->remove_event);
 		} else {
 			/* Shutdown mid-open - let the other side know */
-			status = queue_message(state, service,
-				MAKE_CLOSE(service->localport,
-				VCHIQ_MSG_DSTPORT(service->remoteport)),
+			status = queue_message(state, service, close_id,
 				NULL, NULL, 0, 0);
 		}
 		break;
@@ -2861,9 +2860,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 		release_service_messages(service);
 
 		if (status == VCHIQ_SUCCESS)
-			status = queue_message(state, service,
-				MAKE_CLOSE(service->localport,
-				VCHIQ_MSG_DSTPORT(service->remoteport)),
+			status = queue_message(state, service, close_id,
 				NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK);
 
 		if (status != VCHIQ_SUCCESS) {
@@ -3318,6 +3315,7 @@ vchiq_queue_message(unsigned int handle,
 {
 	struct vchiq_service *service = find_service_by_handle(handle);
 	enum vchiq_status status = VCHIQ_ERROR;
+	int data_id;
 
 	if (!service)
 		goto error_exit;
@@ -3336,17 +3334,15 @@ vchiq_queue_message(unsigned int handle,
 		goto error_exit;
 	}
 
+	data_id = MAKE_DATA(service->localport, service->remoteport);
+
 	switch (service->srvstate) {
 	case VCHIQ_SRVSTATE_OPEN:
-		status = queue_message(service->state, service,
-				MAKE_DATA(service->localport,
-					service->remoteport),
+		status = queue_message(service->state, service, data_id,
 				copy_callback, context, size, 1);
 		break;
 	case VCHIQ_SRVSTATE_OPENSYNC:
-		status = queue_message_sync(service->state, service,
-				MAKE_DATA(service->localport,
-					service->remoteport),
+		status = queue_message_sync(service->state, service, data_id,
 				copy_callback, context, size, 1);
 		break;
 	default:
-- 
2.7.4

[PATCH 15/16] staging: vchiq_connected: move EXPORT_SYMBOL below the right function

[Stefan Wahren]

It's more intuitive to have the EXPORT_SYMBOL() call below the matching
definition.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
index 3023fa9..0ee96d1 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
@@ -52,6 +52,7 @@ void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
 	}
 	mutex_unlock(&g_connected_mutex);
 }
+EXPORT_SYMBOL(vchiq_add_connected_callback);
 
 /*
  * This function is called by the vchiq stack once it has been connected to
@@ -73,4 +74,3 @@ void vchiq_call_connected_callbacks(void)
 	g_connected = 1;
 	mutex_unlock(&g_connected_mutex);
 }
-EXPORT_SYMBOL(vchiq_add_connected_callback);
-- 
2.7.4

[PATCH 16/16] staging: vchiq_core: introduce handle_poll

[Stefan Wahren]

The function slot_handler_func() has very deep indentations. Moving the
poll handling into separate function could improve the readability.
Use the return value to keep the poll_needed handling at the same place.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 92 ++++++++++++----------
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index a2a2472..4f43e42 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1992,6 +1992,56 @@ parse_rx_slots(struct vchiq_state *state)
 	}
 }
 
+/**
+ * handle_poll() - handle service polling and other rare conditions
+ * @state:  vchiq state struct
+ *
+ * Context: Process context
+ *
+ * Return:
+ * * 0        - poll handled successful
+ * * -EAGAIN  - retry later
+ */
+static int
+handle_poll(struct vchiq_state *state)
+{
+	switch (state->conn_state) {
+	case VCHIQ_CONNSTATE_CONNECTED:
+		/* Poll the services as requested */
+		poll_services(state);
+		break;
+
+	case VCHIQ_CONNSTATE_PAUSING:
+		if (queue_message(state, NULL, MAKE_PAUSE, NULL, NULL, 0,
+				  QMFLAGS_NO_MUTEX_UNLOCK) != VCHIQ_RETRY) {
+			vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSE_SENT);
+		} else {
+			/* Retry later */
+			return -EAGAIN;
+		}
+		break;
+
+	case VCHIQ_CONNSTATE_RESUMING:
+		if (queue_message(state, NULL, MAKE_RESUME, NULL, NULL, 0,
+				  QMFLAGS_NO_MUTEX_LOCK) != VCHIQ_RETRY) {
+			vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
+		} else {
+			/*
+			 * This should really be impossible,
+			 * since the PAUSE should have flushed
+			 * through outstanding messages.
+			 */
+			vchiq_log_error(vchiq_core_log_level,
+				"Failed to send RESUME message");
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 /* Called by the slot handler thread */
 static int
 slot_handler_func(void *v)
@@ -2010,52 +2060,14 @@ slot_handler_func(void *v)
 
 		DEBUG_TRACE(SLOT_HANDLER_LINE);
 		if (state->poll_needed) {
-
 			state->poll_needed = 0;
 
 			/*
 			 * Handle service polling and other rare conditions here
 			 * out of the mainline code
 			 */
-			switch (state->conn_state) {
-			case VCHIQ_CONNSTATE_CONNECTED:
-				/* Poll the services as requested */
-				poll_services(state);
-				break;
-
-			case VCHIQ_CONNSTATE_PAUSING:
-				if (queue_message(state, NULL, MAKE_PAUSE,
-					NULL, NULL, 0,
-					QMFLAGS_NO_MUTEX_UNLOCK)
-				    != VCHIQ_RETRY) {
-					vchiq_set_conn_state(state,
-						VCHIQ_CONNSTATE_PAUSE_SENT);
-				} else {
-					/* Retry later */
-					state->poll_needed = 1;
-				}
-				break;
-
-			case VCHIQ_CONNSTATE_RESUMING:
-				if (queue_message(state, NULL, MAKE_RESUME,
-					NULL, NULL, 0, QMFLAGS_NO_MUTEX_LOCK)
-					!= VCHIQ_RETRY) {
-					vchiq_set_conn_state(state,
-						VCHIQ_CONNSTATE_CONNECTED);
-				} else {
-					/*
-					 * This should really be impossible,
-					 * since the PAUSE should have flushed
-					 * through outstanding messages.
-					 */
-					vchiq_log_error(vchiq_core_log_level,
-						"Failed to send RESUME message");
-				}
-				break;
-			default:
-				break;
-			}
-
+			if (handle_poll(state) == -EAGAIN)
+				state->poll_needed = 1;
 		}
 
 		DEBUG_TRACE(SLOT_HANDLER_LINE);
-- 
2.7.4