* [PATCH 00/16] staging: vchiq_arm: code clean-up part 3
@ 2021-06-03 15:49 Stefan Wahren
2021-06-03 15:49 ` [PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group Stefan Wahren
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 02/16] staging: vchiq_arm: introduce free_bulk_waiter Stefan Wahren
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 02/16] staging: vchiq_arm: introduce free_bulk_waiter
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
2021-06-03 15:49 ` [PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 03/16] staging: vchiq_core: move internals to C source Stefan Wahren
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 03/16] staging: vchiq_core: move internals to C source
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
2021-06-03 15:49 ` [PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group Stefan Wahren
2021-06-03 15:49 ` [PATCH 02/16] staging: vchiq_arm: introduce free_bulk_waiter Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 04/16] staging: vchiq_core: get the rid of IS_POW2 Stefan Wahren
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 04/16] staging: vchiq_core: get the rid of IS_POW2
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (2 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 03/16] staging: vchiq_core: move internals to C source Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 05/16] staging: vchiq_core: get the rid of vchiq_static_assert Stefan Wahren
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 05/16] staging: vchiq_core: get the rid of vchiq_static_assert
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (3 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 04/16] staging: vchiq_core: get the rid of IS_POW2 Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 06/16] staging: vchiq_core: put spaces around operators Stefan Wahren
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 06/16] staging: vchiq_core: put spaces around operators
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (4 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 05/16] staging: vchiq_core: get the rid of vchiq_static_assert Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 07/16] staging: vchiq_core: avoid precedence issues Stefan Wahren
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 07/16] staging: vchiq_core: avoid precedence issues
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (5 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 06/16] staging: vchiq_core: put spaces around operators Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 08/16] staging: vchiq_core: use define for message type shift Stefan Wahren
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 08/16] staging: vchiq_core: use define for message type shift
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (6 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 07/16] staging: vchiq_core: avoid precedence issues Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 09/16] staging: vchiq_core: introduce message specific make macros Stefan Wahren
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 09/16] staging: vchiq_core: introduce message specific make macros
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (7 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 08/16] staging: vchiq_core: use define for message type shift Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:49 ` [PATCH 10/16] staging: vchiq_core: simplify WARN_ON conditions Stefan Wahren
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 10/16] staging: vchiq_core: simplify WARN_ON conditions
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (8 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 09/16] staging: vchiq_core: introduce message specific make macros Stefan Wahren
@ 2021-06-03 15:49 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 11/16] staging: vchiq_arm: tidy up service function naming Stefan Wahren
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 11/16] staging: vchiq_arm: tidy up service function naming
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (9 preceding siblings ...)
2021-06-03 15:49 ` [PATCH 10/16] staging: vchiq_core: simplify WARN_ON conditions Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 12/16] staging: vchiq_core: introduce process_free_data_message Stefan Wahren
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 12/16] staging: vchiq_core: introduce process_free_data_message
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (10 preceding siblings ...)
2021-06-03 15:50 ` [PATCH 11/16] staging: vchiq_arm: tidy up service function naming Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 13/16] staging: vchiq_core: reduce indentation in parse_open Stefan Wahren
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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("a_spinlock);
+ count = quota->message_use_count;
+ if (count > 0)
+ quota->message_use_count = count - 1;
+ spin_unlock("a_spinlock);
+
+ if (count == quota->message_quota) {
+ /*
+ * Signal the service that it
+ * has dropped below its quota
+ */
+ complete("a->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("a_spinlock);
+ count = quota->slot_use_count;
+ if (count > 0)
+ quota->slot_use_count = count - 1;
+ spin_unlock("a_spinlock);
+
+ if (count > 0) {
+ /*
+ * Signal the service in case
+ * it has dropped below its quota
+ */
+ complete("a->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("a_spinlock);
- count = quota->message_use_count;
- if (count > 0)
- quota->message_use_count = count - 1;
- spin_unlock("a_spinlock);
-
- if (count == quota->message_quota) {
- /*
- * Signal the service that it
- * has dropped below its quota
- */
- complete("a->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("a_spinlock);
- count = quota->slot_use_count;
- if (count > 0)
- quota->slot_use_count =
- count - 1;
- spin_unlock("a_spinlock);
-
- if (count > 0) {
- /*
- * Signal the service in case
- * it has dropped below its quota
- */
- complete("a->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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 13/16] staging: vchiq_core: reduce indentation in parse_open
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (11 preceding siblings ...)
2021-06-03 15:50 ` [PATCH 12/16] staging: vchiq_core: introduce process_free_data_message Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 14/16] staging: vchiq_core: store message id in local variable Stefan Wahren
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 14/16] staging: vchiq_core: store message id in local variable
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (12 preceding siblings ...)
2021-06-03 15:50 ` [PATCH 13/16] staging: vchiq_core: reduce indentation in parse_open Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 15/16] staging: vchiq_connected: move EXPORT_SYMBOL below the right function Stefan Wahren
2021-06-03 15:50 ` [PATCH 16/16] staging: vchiq_core: introduce handle_poll Stefan Wahren
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 15/16] staging: vchiq_connected: move EXPORT_SYMBOL below the right function
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (13 preceding siblings ...)
2021-06-03 15:50 ` [PATCH 14/16] staging: vchiq_core: store message id in local variable Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
2021-06-03 15:50 ` [PATCH 16/16] staging: vchiq_core: introduce handle_poll Stefan Wahren
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 16/16] staging: vchiq_core: introduce handle_poll
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
` (14 preceding siblings ...)
2021-06-03 15:50 ` [PATCH 15/16] staging: vchiq_connected: move EXPORT_SYMBOL below the right function Stefan Wahren
@ 2021-06-03 15:50 ` Stefan Wahren
15 siblings, 0 replies; 17+ messages in thread
From: Stefan Wahren @ 2021-06-03 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Saenz Julienne; +Cc: linux-staging, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-06-03 15:50 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 15:49 [PATCH 00/16] staging: vchiq_arm: code clean-up part 3 Stefan Wahren
2021-06-03 15:49 ` [PATCH 01/16] staging: vchiq_core: fix logic in poll_services_of_group Stefan Wahren
2021-06-03 15:49 ` [PATCH 02/16] staging: vchiq_arm: introduce free_bulk_waiter Stefan Wahren
2021-06-03 15:49 ` [PATCH 03/16] staging: vchiq_core: move internals to C source Stefan Wahren
2021-06-03 15:49 ` [PATCH 04/16] staging: vchiq_core: get the rid of IS_POW2 Stefan Wahren
2021-06-03 15:49 ` [PATCH 05/16] staging: vchiq_core: get the rid of vchiq_static_assert Stefan Wahren
2021-06-03 15:49 ` [PATCH 06/16] staging: vchiq_core: put spaces around operators Stefan Wahren
2021-06-03 15:49 ` [PATCH 07/16] staging: vchiq_core: avoid precedence issues Stefan Wahren
2021-06-03 15:49 ` [PATCH 08/16] staging: vchiq_core: use define for message type shift Stefan Wahren
2021-06-03 15:49 ` [PATCH 09/16] staging: vchiq_core: introduce message specific make macros Stefan Wahren
2021-06-03 15:49 ` [PATCH 10/16] staging: vchiq_core: simplify WARN_ON conditions Stefan Wahren
2021-06-03 15:50 ` [PATCH 11/16] staging: vchiq_arm: tidy up service function naming Stefan Wahren
2021-06-03 15:50 ` [PATCH 12/16] staging: vchiq_core: introduce process_free_data_message Stefan Wahren
2021-06-03 15:50 ` [PATCH 13/16] staging: vchiq_core: reduce indentation in parse_open Stefan Wahren
2021-06-03 15:50 ` [PATCH 14/16] staging: vchiq_core: store message id in local variable Stefan Wahren
2021-06-03 15:50 ` [PATCH 15/16] staging: vchiq_connected: move EXPORT_SYMBOL below the right function Stefan Wahren
2021-06-03 15:50 ` [PATCH 16/16] staging: vchiq_core: introduce handle_poll Stefan Wahren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).