Hi All,
This series was written in parallel with reading and understanding the
vchiq code. So excuse me for the lack of logic in the sequence of
patches.
The main focus was to delete as much code as possible, I've counted
around 550 lines, which is not bad. Apart from that there are some
patches enforcing proper kernel APIs usage.
The only patch that really changes code is the
vchiq_ioc_copy_element_data() rewrite.
The last commit updates the TODO list with some of my observations, I
realise some of the might be a little opinionated. If anything it's
going to force a discussion on the topic, which is nice.
It was developed on top of the latest linux-next, and was tested on a
RPIv3B+ with audio, video and running vchiq_test.
Regards,
Nicolas
===
Nicolas Saenz Julienne (16):
staging: vchiq_core: rework vchiq_get_config
staging: vchiq_arm: rework close/remove_service IOCTLS
staging: vchiq_shim: delete vchi_service_create
staging: vchiq_arm: use list_for_each_entry when accessing
bulk_waiter_list
staging: vchiq_arm: get rid of vchi_mh.h
staging: vchiq_arm: rework vchiq_ioc_copy_element_data
staging: vchiq-core: get rid of is_master distinction
staging: vchiq_core: remove unnecessary safety checks in
vchiq_init_state
staging: vchiq_core: do not initialize semaphores twice
staging: vchiq_core: don't add a wmb() before remote_event_signal()
staging: vchiq: use completions instead of semaphores
staging: vchiq_util: get rid of unneeded memory barriers
staging: vchiq_core: fix logic redundancy in parse_open
staging: vchiq_arm: rework probe and init functions
staging: vchiq_arm: fix open/release cdev functions
staging: vchiq: add more tasks to the TODO list
.../staging/vc04_services/interface/vchi/TODO | 42 ++
.../vc04_services/interface/vchi/vchi.h | 8 -
.../vc04_services/interface/vchi/vchi_mh.h | 42 --
.../interface/vchiq_arm/vchiq_2835_arm.c | 18 +-
.../interface/vchiq_arm/vchiq_arm.c | 598 ++++++++----------
.../interface/vchiq_arm/vchiq_core.c | 523 ++++-----------
.../interface/vchiq_arm/vchiq_core.h | 47 +-
.../interface/vchiq_arm/vchiq_if.h | 11 +-
.../interface/vchiq_arm/vchiq_shim.c | 32 -
.../interface/vchiq_arm/vchiq_util.c | 48 +-
.../interface/vchiq_arm/vchiq_util.h | 6 +-
11 files changed, 435 insertions(+), 940 deletions(-)
delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h
--
2.19.1
VCHIQ bulk transfers are what most people call DMA transfers. The CPU
sends a list of physical addresses to the VideoCore which then access
the memory directly without the need for CPU interaction. With this
setup we call the CPU the "slave" and the VideoCore the "master".
There seems to be an option to switch roles in vchiq. Which nobody is
using nor is properly implemented. So we get rid of the "is_master == 1"
option, and all the related code.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 12 +-
.../interface/vchiq_arm/vchiq_core.c | 300 +++---------------
.../interface/vchiq_arm/vchiq_core.h | 11 +-
3 files changed, 38 insertions(+), 285 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 014583cdf367..ecee54a31f8d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -163,7 +163,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
*(char **)&g_fragments_base[i * g_fragments_size] = NULL;
sema_init(&g_free_fragments_sema, MAX_FRAGMENTS);
- if (vchiq_init_state(state, vchiq_slot_zero, 0) != VCHIQ_SUCCESS)
+ if (vchiq_init_state(state, vchiq_slot_zero) != VCHIQ_SUCCESS)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -278,16 +278,6 @@ vchiq_complete_bulk(VCHIQ_BULK_T *bulk)
bulk->actual);
}
-void
-vchiq_transfer_bulk(VCHIQ_BULK_T *bulk)
-{
- /*
- * This should only be called on the master (VideoCore) side, but
- * provide an implementation to avoid the need for ifdefery.
- */
- BUG();
-}
-
void
vchiq_dump_platform_state(void *dump_context)
{
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 8c7bda2e7cb6..34a892011296 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -85,8 +85,6 @@ int vchiq_core_log_level = VCHIQ_LOG_DEFAULT;
int vchiq_core_msg_log_level = VCHIQ_LOG_DEFAULT;
int vchiq_sync_log_level = VCHIQ_LOG_DEFAULT;
-static atomic_t pause_bulks_count = ATOMIC_INIT(0);
-
static DEFINE_SPINLOCK(service_spinlock);
DEFINE_SPINLOCK(bulk_waiter_spinlock);
static DEFINE_SPINLOCK(quota_spinlock);
@@ -1222,32 +1220,7 @@ notify_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue,
(queue == &service->bulk_tx) ? 't' : 'r',
queue->process, queue->remote_notify, queue->remove);
- if (service->state->is_master) {
- while (queue->remote_notify != queue->process) {
- VCHIQ_BULK_T *bulk =
- &queue->bulks[BULK_INDEX(queue->remote_notify)];
- int msgtype = (bulk->dir == VCHIQ_BULK_TRANSMIT) ?
- VCHIQ_MSG_BULK_RX_DONE : VCHIQ_MSG_BULK_TX_DONE;
- int msgid = VCHIQ_MAKE_MSG(msgtype, service->localport,
- service->remoteport);
- /* Only reply to non-dummy bulk requests */
- if (bulk->remote_data) {
- status = queue_message(
- service->state,
- NULL,
- msgid,
- memcpy_copy_callback,
- &bulk->actual,
- 4,
- 0);
- if (status != VCHIQ_SUCCESS)
- break;
- }
- queue->remote_notify++;
- }
- } else {
- queue->remote_notify = queue->process;
- }
+ queue->remote_notify = queue->process;
if (status == VCHIQ_SUCCESS) {
while (queue->remove != queue->remote_notify) {
@@ -1385,63 +1358,6 @@ poll_services(VCHIQ_STATE_T *state)
}
}
-/* Called by the slot handler or application threads, holding the bulk mutex. */
-static int
-resolve_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue)
-{
- VCHIQ_STATE_T *state = service->state;
- int resolved = 0;
-
- while ((queue->process != queue->local_insert) &&
- (queue->process != queue->remote_insert)) {
- VCHIQ_BULK_T *bulk = &queue->bulks[BULK_INDEX(queue->process)];
-
- vchiq_log_trace(vchiq_core_log_level,
- "%d: rb:%d %cx - li=%x ri=%x p=%x",
- state->id, service->localport,
- (queue == &service->bulk_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));
-
- if (mutex_lock_killable(&state->bulk_transfer_mutex))
- break;
-
- vchiq_transfer_bulk(bulk);
- mutex_unlock(&state->bulk_transfer_mutex);
-
- if (SRVTRACE_ENABLED(service, VCHIQ_LOG_INFO)) {
- const char *header = (queue == &service->bulk_tx) ?
- "Send Bulk to" : "Recv Bulk from";
- if (bulk->actual != VCHIQ_BULK_ACTUAL_ABORTED)
- vchiq_log_info(SRVTRACE_LEVEL(service),
- "%s %c%c%c%c d:%d len:%d %pK<->%pK",
- header,
- VCHIQ_FOURCC_AS_4CHARS(
- service->base.fourcc),
- service->remoteport, bulk->size,
- bulk->data, bulk->remote_data);
- else
- vchiq_log_info(SRVTRACE_LEVEL(service),
- "%s %c%c%c%c d:%d ABORTED - tx len:%d,"
- " rx len:%d %pK<->%pK",
- header,
- VCHIQ_FOURCC_AS_4CHARS(
- service->base.fourcc),
- service->remoteport,
- bulk->size, bulk->remote_size,
- bulk->data, bulk->remote_data);
- }
-
- vchiq_complete_bulk(bulk);
- queue->process++;
- resolved++;
- }
- return resolved;
-}
-
/* Called with the bulk_mutex held */
static void
abort_outstanding_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue)
@@ -1492,65 +1408,6 @@ abort_outstanding_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue)
}
}
-/* Called from the slot handler thread */
-static void
-pause_bulks(VCHIQ_STATE_T *state)
-{
- if (unlikely(atomic_inc_return(&pause_bulks_count) != 1)) {
- WARN_ON_ONCE(1);
- atomic_set(&pause_bulks_count, 1);
- return;
- }
-
- /* Block bulk transfers from all services */
- mutex_lock(&state->bulk_transfer_mutex);
-}
-
-/* Called from the slot handler thread */
-static void
-resume_bulks(VCHIQ_STATE_T *state)
-{
- int i;
-
- if (unlikely(atomic_dec_return(&pause_bulks_count) != 0)) {
- WARN_ON_ONCE(1);
- atomic_set(&pause_bulks_count, 0);
- return;
- }
-
- /* Allow bulk transfers from all services */
- mutex_unlock(&state->bulk_transfer_mutex);
-
- if (state->deferred_bulks == 0)
- return;
-
- /* Deal with any bulks which had to be deferred due to being in
- * paused state. Don't try to match up to number of deferred bulks
- * in case we've had something come and close the service in the
- * interim - just process all bulk queues for all services */
- vchiq_log_info(vchiq_core_log_level, "%s: processing %d deferred bulks",
- __func__, state->deferred_bulks);
-
- for (i = 0; i < state->unused_service; i++) {
- VCHIQ_SERVICE_T *service = state->services[i];
- int resolved_rx = 0;
- int resolved_tx = 0;
-
- if (!service || (service->srvstate != VCHIQ_SRVSTATE_OPEN))
- continue;
-
- mutex_lock(&service->bulk_mutex);
- resolved_rx = resolve_bulks(service, &service->bulk_rx);
- resolved_tx = resolve_bulks(service, &service->bulk_tx);
- mutex_unlock(&service->bulk_mutex);
- if (resolved_rx)
- notify_bulks(service, &service->bulk_rx, 1);
- if (resolved_tx)
- notify_bulks(service, &service->bulk_tx, 1);
- }
- state->deferred_bulks = 0;
-}
-
static int
parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
{
@@ -1871,73 +1728,16 @@ parse_rx_slots(VCHIQ_STATE_T *state)
up(&state->connect);
break;
case VCHIQ_MSG_BULK_RX:
- case VCHIQ_MSG_BULK_TX: {
- VCHIQ_BULK_QUEUE_T *queue;
-
- WARN_ON(!state->is_master);
- queue = (type == VCHIQ_MSG_BULK_RX) ?
- &service->bulk_tx : &service->bulk_rx;
- if ((service->remoteport == remoteport)
- && (service->srvstate ==
- VCHIQ_SRVSTATE_OPEN)) {
- VCHIQ_BULK_T *bulk;
- int resolved = 0;
-
- DEBUG_TRACE(PARSE_LINE);
- if (mutex_lock_killable(
- &service->bulk_mutex) != 0) {
- DEBUG_TRACE(PARSE_LINE);
- goto bail_not_ready;
- }
-
- WARN_ON(!(queue->remote_insert < queue->remove +
- VCHIQ_NUM_SERVICE_BULKS));
- bulk = &queue->bulks[
- BULK_INDEX(queue->remote_insert)];
- bulk->remote_data =
- (void *)(long)((int *)header->data)[0];
- bulk->remote_size = ((int *)header->data)[1];
- wmb();
-
- vchiq_log_info(vchiq_core_log_level,
- "%d: prs %s@%pK (%d->%d) %x@%pK",
- state->id, msg_type_str(type),
- header, remoteport, localport,
- bulk->remote_size, bulk->remote_data);
-
- queue->remote_insert++;
-
- if (atomic_read(&pause_bulks_count)) {
- state->deferred_bulks++;
- vchiq_log_info(vchiq_core_log_level,
- "%s: deferring bulk (%d)",
- __func__,
- state->deferred_bulks);
- if (state->conn_state !=
- VCHIQ_CONNSTATE_PAUSE_SENT)
- vchiq_log_error(
- vchiq_core_log_level,
- "%s: bulks paused in "
- "unexpected state %s",
- __func__,
- conn_state_names[
- state->conn_state]);
- } else if (state->conn_state ==
- VCHIQ_CONNSTATE_CONNECTED) {
- DEBUG_TRACE(PARSE_LINE);
- resolved = resolve_bulks(service,
- queue);
- }
-
- mutex_unlock(&service->bulk_mutex);
- if (resolved)
- notify_bulks(service, queue,
- 1/*retry_poll*/);
- }
- } break;
+ case VCHIQ_MSG_BULK_TX:
+ /*
+ * We should never receive a bulk request from the
+ * other side since we're not setup to perform as the
+ * master.
+ */
+ WARN_ON(1);
+ break;
case VCHIQ_MSG_BULK_RX_DONE:
case VCHIQ_MSG_BULK_TX_DONE:
- WARN_ON(state->is_master);
if ((service->remoteport == remoteport)
&& (service->srvstate !=
VCHIQ_SRVSTATE_FREE)) {
@@ -2026,8 +1826,6 @@ parse_rx_slots(VCHIQ_STATE_T *state)
NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
== VCHIQ_RETRY)
goto bail_not_ready;
- if (state->is_master)
- pause_bulks(state);
}
/* At this point slot_mutex is held */
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSED);
@@ -2039,8 +1837,6 @@ parse_rx_slots(VCHIQ_STATE_T *state)
state->id, header, size);
/* Release the slot mutex */
mutex_unlock(&state->slot_mutex);
- if (state->is_master)
- resume_bulks(state);
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
vchiq_platform_resumed(state);
break;
@@ -2119,8 +1915,6 @@ slot_handler_func(void *v)
break;
case VCHIQ_CONNSTATE_PAUSING:
- if (state->is_master)
- pause_bulks(state);
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
NULL, NULL, 0,
@@ -2129,8 +1923,6 @@ slot_handler_func(void *v)
vchiq_set_conn_state(state,
VCHIQ_CONNSTATE_PAUSE_SENT);
} else {
- if (state->is_master)
- resume_bulks(state);
/* Retry later */
state->poll_needed = 1;
}
@@ -2145,8 +1937,6 @@ slot_handler_func(void *v)
VCHIQ_MAKE_MSG(VCHIQ_MSG_RESUME, 0, 0),
NULL, NULL, 0, QMFLAGS_NO_MUTEX_LOCK)
!= VCHIQ_RETRY) {
- if (state->is_master)
- resume_bulks(state);
vchiq_set_conn_state(state,
VCHIQ_CONNSTATE_CONNECTED);
vchiq_platform_resumed(state);
@@ -2364,8 +2154,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
}
VCHIQ_STATUS_T
-vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
- int is_master)
+vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero)
{
VCHIQ_SHARED_STATE_T *local;
VCHIQ_SHARED_STATE_T *remote;
@@ -2374,8 +2163,7 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
int i;
vchiq_log_warning(vchiq_core_log_level,
- "%s: slot_zero = %pK, is_master = %d",
- __func__, slot_zero, is_master);
+ "%s: slot_zero = %pK", __func__, slot_zero);
if (vchiq_states[0]) {
pr_err("%s: VCHIQ state already initialized\n", __func__);
@@ -2441,13 +2229,8 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
if (VCHIQ_VERSION < slot_zero->version)
slot_zero->version = VCHIQ_VERSION;
- if (is_master) {
- local = &slot_zero->master;
- remote = &slot_zero->slave;
- } else {
- local = &slot_zero->slave;
- remote = &slot_zero->master;
- }
+ local = &slot_zero->slave;
+ remote = &slot_zero->master;
if (local->initialised) {
vchiq_loud_error_header();
@@ -2455,16 +2238,13 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
vchiq_loud_error("local state has already been "
"initialised");
else
- vchiq_loud_error("master/slave mismatch - two %ss",
- is_master ? "master" : "slave");
+ vchiq_loud_error("master/slave mismatch two slaves");
vchiq_loud_error_footer();
return VCHIQ_ERROR;
}
memset(state, 0, sizeof(VCHIQ_STATE_T));
- state->is_master = is_master;
-
/*
initialize shared state pointers
*/
@@ -2971,7 +2751,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
mutex_lock(&state->sync_mutex);
/* fall through */
case VCHIQ_SRVSTATE_OPEN:
- if (state->is_master || close_recvd) {
+ if (close_recvd) {
if (!do_abort_bulks(service))
status = VCHIQ_RETRY;
}
@@ -3018,11 +2798,9 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
/* This happens when a process is killed mid-close */
break;
- if (!state->is_master) {
- if (!do_abort_bulks(service)) {
- status = VCHIQ_RETRY;
- break;
- }
+ if (!do_abort_bulks(service)) {
+ status = VCHIQ_RETRY;
+ break;
}
if (status == VCHIQ_SUCCESS)
@@ -3327,6 +3105,7 @@ VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
const int dir_msgtype = (dir == VCHIQ_BULK_TRANSMIT) ?
VCHIQ_MSG_BULK_TX : VCHIQ_MSG_BULK_RX;
VCHIQ_STATUS_T status = VCHIQ_ERROR;
+ int payload[2];
if (!service || service->srvstate != VCHIQ_SRVSTATE_OPEN ||
!offset || vchiq_check_service(service) != VCHIQ_SUCCESS)
@@ -3406,32 +3185,25 @@ VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
if (service->srvstate != VCHIQ_SRVSTATE_OPEN)
goto unlock_both_error_exit;
- if (state->is_master) {
- queue->local_insert++;
- if (resolve_bulks(service, queue))
- request_poll(state, service,
- (dir == VCHIQ_BULK_TRANSMIT) ?
- VCHIQ_POLL_TXNOTIFY : VCHIQ_POLL_RXNOTIFY);
- } else {
- int payload[2] = { (int)(long)bulk->data, bulk->size };
-
- status = queue_message(state,
- NULL,
- VCHIQ_MAKE_MSG(dir_msgtype,
- service->localport,
- service->remoteport),
- memcpy_copy_callback,
- &payload,
- sizeof(payload),
- QMFLAGS_IS_BLOCKING |
- QMFLAGS_NO_MUTEX_LOCK |
- QMFLAGS_NO_MUTEX_UNLOCK);
- if (status != VCHIQ_SUCCESS) {
- goto unlock_both_error_exit;
- }
- queue->local_insert++;
+ payload[0] = (int)(long)bulk->data;
+ payload[1] = bulk->size;
+ status = queue_message(state,
+ NULL,
+ VCHIQ_MAKE_MSG(dir_msgtype,
+ service->localport,
+ service->remoteport),
+ memcpy_copy_callback,
+ &payload,
+ sizeof(payload),
+ QMFLAGS_IS_BLOCKING |
+ QMFLAGS_NO_MUTEX_LOCK |
+ QMFLAGS_NO_MUTEX_UNLOCK);
+ if (status != VCHIQ_SUCCESS) {
+ goto unlock_both_error_exit;
}
+ queue->local_insert++;
+
mutex_unlock(&state->slot_mutex);
mutex_unlock(&service->bulk_mutex);
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 daada568f400..5b1696422e21 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -400,7 +400,6 @@ struct vchiq_state_struct {
int id;
int initialised;
VCHIQ_CONNSTATE_T conn_state;
- int is_master;
short version_common;
VCHIQ_SHARED_STATE_T *local;
@@ -489,10 +488,6 @@ struct vchiq_state_struct {
/* Signalled when a free data slot becomes available. */
struct semaphore data_quota_event;
- /* Incremented when there are bulk transfers which cannot be processed
- * whilst paused and must be processed on resume */
- int deferred_bulks;
-
struct state_stats_struct {
int slot_stalls;
int data_stalls;
@@ -529,8 +524,7 @@ extern VCHIQ_SLOT_ZERO_T *
vchiq_init_slots(void *mem_base, int mem_size);
extern VCHIQ_STATUS_T
-vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
- int is_master);
+vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero);
extern VCHIQ_STATUS_T
vchiq_connect_internal(VCHIQ_STATE_T *state, VCHIQ_INSTANCE_T instance);
@@ -625,9 +619,6 @@ unlock_service(VCHIQ_SERVICE_T *service);
extern VCHIQ_STATUS_T
vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, void *offset, int size, int dir);
-extern void
-vchiq_transfer_bulk(VCHIQ_BULK_T *bulk);
-
extern void
vchiq_complete_bulk(VCHIQ_BULK_T *bulk);
--
2.19.1
Both functions checked the minor number of the cdev prior running the
code. This was useless since the number of devices is already limited by
alloc_chrdev_region.
This removes the check and reindents the code where relevant.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 247 +++++++-----------
1 file changed, 100 insertions(+), 147 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 a7dcced79980..153a396d21bd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -63,8 +63,6 @@
#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX DEVICE_NAME "."
-#define VCHIQ_MINOR 0
-
/* Some per-instance constants */
#define MAX_COMPLETIONS 128
#define MAX_SERVICES 64
@@ -1950,195 +1948,150 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
#endif
-/****************************************************************************
-*
-* vchiq_open
-*
-***************************************************************************/
-
-static int
-vchiq_open(struct inode *inode, struct file *file)
+static int vchiq_open(struct inode *inode, struct file *file)
{
- int dev = iminor(inode) & 0x0f;
+ VCHIQ_STATE_T *state = vchiq_get_state();
+ VCHIQ_INSTANCE_T instance;
vchiq_log_info(vchiq_arm_log_level, "vchiq_open");
- switch (dev) {
- case VCHIQ_MINOR: {
- VCHIQ_STATE_T *state = vchiq_get_state();
- VCHIQ_INSTANCE_T instance;
- if (!state) {
- vchiq_log_error(vchiq_arm_log_level,
+ if (!state) {
+ vchiq_log_error(vchiq_arm_log_level,
"vchiq has no connection to VideoCore");
- return -ENOTCONN;
- }
-
- instance = kzalloc(sizeof(*instance), GFP_KERNEL);
- if (!instance)
- return -ENOMEM;
+ return -ENOTCONN;
+ }
- instance->state = state;
- instance->pid = current->tgid;
+ instance = kzalloc(sizeof(*instance), GFP_KERNEL);
+ if (!instance)
+ return -ENOMEM;
- vchiq_debugfs_add_instance(instance);
+ instance->state = state;
+ instance->pid = current->tgid;
- init_completion(&instance->insert_event);
- init_completion(&instance->remove_event);
- mutex_init(&instance->completion_mutex);
- mutex_init(&instance->bulk_waiter_list_mutex);
- INIT_LIST_HEAD(&instance->bulk_waiter_list);
+ vchiq_debugfs_add_instance(instance);
- file->private_data = instance;
- } break;
+ init_completion(&instance->insert_event);
+ init_completion(&instance->remove_event);
+ mutex_init(&instance->completion_mutex);
+ mutex_init(&instance->bulk_waiter_list_mutex);
+ INIT_LIST_HEAD(&instance->bulk_waiter_list);
- default:
- vchiq_log_error(vchiq_arm_log_level,
- "Unknown minor device: %d", dev);
- return -ENXIO;
- }
+ file->private_data = instance;
return 0;
}
-/****************************************************************************
-*
-* vchiq_release
-*
-***************************************************************************/
-
-static int
-vchiq_release(struct inode *inode, struct file *file)
+static int vchiq_release(struct inode *inode, struct file *file)
{
- int dev = iminor(inode) & 0x0f;
+ VCHIQ_INSTANCE_T instance = file->private_data;
+ VCHIQ_STATE_T *state = vchiq_get_state();
+ VCHIQ_SERVICE_T *service;
int ret = 0;
+ int i;
- switch (dev) {
- case VCHIQ_MINOR: {
- VCHIQ_INSTANCE_T instance = file->private_data;
- VCHIQ_STATE_T *state = vchiq_get_state();
- VCHIQ_SERVICE_T *service;
- int i;
+ vchiq_log_info(vchiq_arm_log_level, "%s: instance=%lx", __func__,
+ (unsigned long)instance);
- vchiq_log_info(vchiq_arm_log_level,
- "%s: instance=%lx",
- __func__, (unsigned long)instance);
+ if (!state) {
+ ret = -EPERM;
+ goto out;
+ }
- if (!state) {
- ret = -EPERM;
- goto out;
- }
+ /* Ensure videocore is awake to allow termination. */
+ vchiq_use_internal(instance->state, NULL, USE_TYPE_VCHIQ);
- /* Ensure videocore is awake to allow termination. */
- vchiq_use_internal(instance->state, NULL,
- USE_TYPE_VCHIQ);
+ mutex_lock(&instance->completion_mutex);
- mutex_lock(&instance->completion_mutex);
+ /* Wake the completion thread and ask it to exit */
+ instance->closing = 1;
+ complete(&instance->insert_event);
- /* Wake the completion thread and ask it to exit */
- instance->closing = 1;
- complete(&instance->insert_event);
+ mutex_unlock(&instance->completion_mutex);
- mutex_unlock(&instance->completion_mutex);
+ /* Wake the slot handler if the completion queue is full. */
+ complete(&instance->remove_event);
- /* Wake the slot handler if the completion queue is full. */
- complete(&instance->remove_event);
+ /* Mark all services for termination... */
+ i = 0;
+ while ((service = next_service_by_instance(state, instance, &i))) {
+ USER_SERVICE_T *user_service = service->base.userdata;
- /* Mark all services for termination... */
- i = 0;
- while ((service = next_service_by_instance(state, instance,
- &i)) != NULL) {
- USER_SERVICE_T *user_service = service->base.userdata;
+ /* Wake the slot handler if the msg queue is full. */
+ complete(&user_service->remove_event);
- /* Wake the slot handler if the msg queue is full. */
- complete(&user_service->remove_event);
+ vchiq_terminate_service_internal(service);
+ unlock_service(service);
+ }
- vchiq_terminate_service_internal(service);
- unlock_service(service);
- }
+ /* ...and wait for them to die */
+ i = 0;
+ while ((service = next_service_by_instance(state, instance, &i))) {
+ USER_SERVICE_T *user_service = service->base.userdata;
- /* ...and wait for them to die */
- i = 0;
- while ((service = next_service_by_instance(state, instance, &i))
- != NULL) {
- USER_SERVICE_T *user_service = service->base.userdata;
+ wait_for_completion(&service->remove_event);
+
+ BUG_ON(service->srvstate != VCHIQ_SRVSTATE_FREE);
- wait_for_completion(&service->remove_event);
+ spin_lock(&msg_queue_spinlock);
+
+ while (user_service->msg_remove != user_service->msg_insert) {
+ VCHIQ_HEADER_T *header;
+ int m = user_service->msg_remove & (MSG_QUEUE_SIZE - 1);
- BUG_ON(service->srvstate != VCHIQ_SRVSTATE_FREE);
+ header = user_service->msg_queue[m];
+ user_service->msg_remove++;
+ spin_unlock(&msg_queue_spinlock);
+ if (header)
+ vchiq_release_message(service->handle, header);
spin_lock(&msg_queue_spinlock);
+ }
- while (user_service->msg_remove !=
- user_service->msg_insert) {
- VCHIQ_HEADER_T *header;
- int m = user_service->msg_remove &
- (MSG_QUEUE_SIZE - 1);
+ spin_unlock(&msg_queue_spinlock);
- header = user_service->msg_queue[m];
- user_service->msg_remove++;
- spin_unlock(&msg_queue_spinlock);
+ unlock_service(service);
+ }
- if (header)
- vchiq_release_message(
- service->handle,
- header);
- spin_lock(&msg_queue_spinlock);
- }
+ /* Release any closed services */
+ while (instance->completion_remove !=
+ instance->completion_insert) {
+ VCHIQ_COMPLETION_DATA_T *completion;
+ VCHIQ_SERVICE_T *service;
- spin_unlock(&msg_queue_spinlock);
+ completion = &instance->completions[
+ instance->completion_remove & (MAX_COMPLETIONS - 1)];
+ service = completion->service_userdata;
+ if (completion->reason == VCHIQ_SERVICE_CLOSED) {
+ USER_SERVICE_T *user_service = service->base.userdata;
+ /* Wake any blocked user-thread */
+ if (instance->use_close_delivered)
+ complete(&user_service->close_event);
unlock_service(service);
}
+ instance->completion_remove++;
+ }
- /* Release any closed services */
- while (instance->completion_remove !=
- instance->completion_insert) {
- VCHIQ_COMPLETION_DATA_T *completion;
- VCHIQ_SERVICE_T *service;
-
- completion = &instance->completions[
- instance->completion_remove &
- (MAX_COMPLETIONS - 1)];
- service = completion->service_userdata;
- if (completion->reason == VCHIQ_SERVICE_CLOSED) {
- USER_SERVICE_T *user_service =
- service->base.userdata;
-
- /* Wake any blocked user-thread */
- if (instance->use_close_delivered)
- complete(&user_service->close_event);
- unlock_service(service);
- }
- instance->completion_remove++;
- }
-
- /* Release the PEER service count. */
- vchiq_release_internal(instance->state, NULL);
+ /* Release the PEER service count. */
+ vchiq_release_internal(instance->state, NULL);
- {
- struct bulk_waiter_node *waiter, *next;
+ {
+ 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);
- }
+ 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);
}
+ }
- vchiq_debugfs_remove_instance(instance);
+ vchiq_debugfs_remove_instance(instance);
- kfree(instance);
- file->private_data = NULL;
- } break;
-
- default:
- vchiq_log_error(vchiq_arm_log_level,
- "Unknown minor device: %d", dev);
- ret = -ENXIO;
- }
+ kfree(instance);
+ file->private_data = NULL;
out:
return ret;
@@ -3613,7 +3566,7 @@ static int __init vchiq_driver_init(void)
return PTR_ERR(vchiq_class);
}
- ret = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
+ ret = alloc_chrdev_region(&vchiq_devid, 0, 1, DEVICE_NAME);
if (ret) {
pr_err("Failed to allocate vchiq's chrdev region\n");
goto class_destroy;
--
2.19.1
The TODO list was missing some tasks needed before upstreaming the
device.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../staging/vc04_services/interface/vchi/TODO | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchi/TODO b/drivers/staging/vc04_services/interface/vchi/TODO
index 0b3ec75ff627..fc2752bc95b2 100644
--- a/drivers/staging/vc04_services/interface/vchi/TODO
+++ b/drivers/staging/vc04_services/interface/vchi/TODO
@@ -49,3 +49,45 @@ such as dev_info, dev_dbg, and friends.
A short top-down description of this driver's architecture (function of
kthreads, userspace, limitations) could be very helpful for reviewers.
+
+7) Review and comment memory barriers
+
+There is a heavy use of memory barriers in this driver, it would be very
+beneficial to go over all of them and, if correct, comment on their merits.
+Extra points to whomever confidently reviews the remote_event_*() family of
+functions.
+
+8) Get rid of custom function return values
+
+Most functions use a custom set of return values, we should force proper Linux
+error numbers. Special care is needed for VCHIQ_RETRY.
+
+9) Reformat core code with more sane indentations
+
+The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
+indentation deep making it very unpleasant to read. This is specially relevant
+in the character driver ioctl code and in the core thread functions.
+
+10) Reorganize file structure: Move char driver to it's own file and join both
+platform files
+
+The cdev is defined alongside with the platform code in vchiq_arm.c. It would
+be nice to completely decouple it from the actual core code. For instance to be
+able to use bcm2835-audio without having /dev/vchiq created. One could argue
+it's better for security reasons or general cleanliness. It could even be
+interesting to create two different kernel modules, something the likes of
+vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
+
+The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
+
+12) Get rid of all the struct typedefs
+
+Most structs are typedefd, it's not encouraged in the kernel.
+
+13) Get rid of all non essential global structures and create a proper per
+device structure
+
+The first thing one generally sees in a probe function is a memory allocation
+for all the device specific data. This structure is then passed all over the
+driver. This is good practice since it makes the driver work regardless of the
+number of devices probed.
--
2.19.1
The concept of VCHI_MEM_HANDLE_T is introduced by this header file and
was meant to be used with bulk transfers. After a quick look in
vchiq_core.c it is pretty clear that it actually accomplishes nothing
nor alters the bulk transfers in any way.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../vc04_services/interface/vchi/vchi.h | 3 --
.../vc04_services/interface/vchi/vchi_mh.h | 42 -------------------
.../interface/vchiq_arm/vchiq_2835_arm.c | 6 +--
.../interface/vchiq_arm/vchiq_arm.c | 27 ++++++------
.../interface/vchiq_arm/vchiq_core.c | 17 ++++----
.../interface/vchiq_arm/vchiq_core.h | 10 ++---
.../interface/vchiq_arm/vchiq_if.h | 8 ++--
7 files changed, 27 insertions(+), 86 deletions(-)
delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h b/drivers/staging/vc04_services/interface/vchi/vchi.h
index 379a16ebfd5b..e326926eac31 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -36,7 +36,6 @@
#include "interface/vchi/vchi_cfg.h"
#include "interface/vchi/vchi_common.h"
-#include "vchi_mh.h"
/******************************************************************************
Global defs
@@ -239,7 +238,6 @@ extern int32_t vchi_bulk_queue_receive(VCHI_SERVICE_HANDLE_T handle,
// Prepare interface for a transfer from the other side into relocatable memory.
int32_t vchi_bulk_queue_receive_reloc(const VCHI_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T h_dst,
uint32_t offset,
uint32_t data_size,
const VCHI_FLAGS_T flags,
@@ -261,7 +259,6 @@ extern int32_t vchi_bulk_queue_transmit(VCHI_SERVICE_HANDLE_T handle,
#endif
extern int32_t vchi_bulk_queue_transmit_reloc(VCHI_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T h_src,
uint32_t offset,
uint32_t data_size,
VCHI_FLAGS_T flags,
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
deleted file mode 100644
index 198bd076b666..000000000000
--- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/**
- * Copyright (c) 2010-2012 Broadcom. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions, and the following disclaimer,
- * without modification.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. The names of the above-listed copyright holders may not be used
- * to endorse or promote products derived from this software without
- * specific prior written permission.
- *
- * ALTERNATIVELY, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2, as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
- * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef VCHI_MH_H_
-#define VCHI_MH_H_
-
-#include <linux/types.h>
-
-typedef int32_t VCHI_MEM_HANDLE_T;
-#define VCHI_MEM_HANDLE_INVALID 0
-
-#endif
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 83d740feab96..014583cdf367 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -247,13 +247,10 @@ remote_event_signal(REMOTE_EVENT_T *event)
}
VCHIQ_STATUS_T
-vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle,
- void *offset, int size, int dir)
+vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, void *offset, int size, int dir)
{
struct vchiq_pagelist_info *pagelistinfo;
- WARN_ON(memhandle != VCHI_MEM_HANDLE_INVALID);
-
pagelistinfo = create_pagelist((char __user *)offset, size,
(dir == VCHIQ_BULK_RECEIVE)
? PAGELIST_READ
@@ -262,7 +259,6 @@ vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle,
if (!pagelistinfo)
return VCHIQ_ERROR;
- bulk->handle = memhandle;
bulk->data = (void *)(unsigned long)pagelistinfo->dma_addr;
/*
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 5f55c708ade8..34160cc3b8bd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -419,9 +419,9 @@ vchiq_bulk_transmit(VCHIQ_SERVICE_HANDLE_T handle, const void *data,
switch (mode) {
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
- status = vchiq_bulk_transfer(handle,
- VCHI_MEM_HANDLE_INVALID, (void *)data, size, userdata,
- mode, VCHIQ_BULK_TRANSMIT);
+ status = vchiq_bulk_transfer(handle, (void *)data, size,
+ userdata, mode,
+ VCHIQ_BULK_TRANSMIT);
break;
case VCHIQ_BULK_MODE_BLOCKING:
status = vchiq_blocking_bulk_transfer(handle,
@@ -444,9 +444,8 @@ vchiq_bulk_receive(VCHIQ_SERVICE_HANDLE_T handle, void *data,
switch (mode) {
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
- status = vchiq_bulk_transfer(handle,
- VCHI_MEM_HANDLE_INVALID, data, size, userdata,
- mode, VCHIQ_BULK_RECEIVE);
+ status = vchiq_bulk_transfer(handle, data, size, userdata,
+ mode, VCHIQ_BULK_RECEIVE);
break;
case VCHIQ_BULK_MODE_BLOCKING:
status = vchiq_blocking_bulk_transfer(handle,
@@ -513,9 +512,8 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *data,
}
}
- status = vchiq_bulk_transfer(handle, VCHI_MEM_HANDLE_INVALID,
- data, size, &waiter->bulk_waiter, VCHIQ_BULK_MODE_BLOCKING,
- dir);
+ status = vchiq_bulk_transfer(handle, data, size, &waiter->bulk_waiter,
+ VCHIQ_BULK_MODE_BLOCKING, dir);
if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
!waiter->bulk_waiter.bulk) {
VCHIQ_BULK_T *bulk = waiter->bulk_waiter.bulk;
@@ -1149,14 +1147,13 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
current->pid);
args.userdata = &waiter->bulk_waiter;
}
- status = vchiq_bulk_transfer
- (args.handle,
- VCHI_MEM_HANDLE_INVALID,
- args.data, args.size,
- args.userdata, args.mode,
- dir);
+
+ status = vchiq_bulk_transfer(args.handle, args.data, args.size,
+ args.userdata, args.mode, dir);
+
if (!waiter)
break;
+
if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
!waiter->bulk_waiter.bulk) {
if (waiter->bulk_waiter.bulk) {
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 89f1ccdc3b98..8c7bda2e7cb6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3313,10 +3313,10 @@ vchiq_remove_service(VCHIQ_SERVICE_HANDLE_T handle)
* When called in blocking mode, the userdata field points to a bulk_waiter
* structure.
*/
-VCHIQ_STATUS_T
-vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T memhandle, void *offset, int size, void *userdata,
- VCHIQ_BULK_MODE_T mode, VCHIQ_BULK_DIR_T dir)
+VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
+ void *offset, int size, void *userdata,
+ VCHIQ_BULK_MODE_T mode,
+ VCHIQ_BULK_DIR_T dir)
{
VCHIQ_SERVICE_T *service = find_service_by_handle(handle);
VCHIQ_BULK_QUEUE_T *queue;
@@ -3328,10 +3328,8 @@ vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
VCHIQ_MSG_BULK_TX : VCHIQ_MSG_BULK_RX;
VCHIQ_STATUS_T status = VCHIQ_ERROR;
- if (!service ||
- (service->srvstate != VCHIQ_SRVSTATE_OPEN) ||
- ((memhandle == VCHI_MEM_HANDLE_INVALID) && (offset == NULL)) ||
- (vchiq_check_service(service) != VCHIQ_SUCCESS))
+ if (!service || service->srvstate != VCHIQ_SRVSTATE_OPEN ||
+ !offset || vchiq_check_service(service) != VCHIQ_SUCCESS)
goto error_exit;
switch (mode) {
@@ -3388,8 +3386,7 @@ vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
bulk->size = size;
bulk->actual = VCHIQ_BULK_ACTUAL_ABORTED;
- if (vchiq_prepare_bulk_data(bulk, memhandle, offset, size, dir) !=
- VCHIQ_SUCCESS)
+ if (vchiq_prepare_bulk_data(bulk, offset, size, dir) != VCHIQ_SUCCESS)
goto unlock_error_exit;
wmb();
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 10deb5745dda..daada568f400 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -242,7 +242,6 @@ typedef struct vchiq_bulk_struct {
short mode;
short dir;
void *userdata;
- VCHI_MEM_HANDLE_T handle;
void *data;
int size;
void *remote_data;
@@ -566,9 +565,9 @@ extern void
remote_event_pollall(VCHIQ_STATE_T *state);
extern VCHIQ_STATUS_T
-vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T memhandle, void *offset, int size, void *userdata,
- VCHIQ_BULK_MODE_T mode, VCHIQ_BULK_DIR_T dir);
+vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *offset, int size,
+ void *userdata, VCHIQ_BULK_MODE_T mode,
+ VCHIQ_BULK_DIR_T dir);
extern void
vchiq_dump_state(void *dump_context, VCHIQ_STATE_T *state);
@@ -624,8 +623,7 @@ unlock_service(VCHIQ_SERVICE_T *service);
** implementations must be provided. */
extern VCHIQ_STATUS_T
-vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk,
- VCHI_MEM_HANDLE_T memhandle, void *offset, int size, int dir);
+vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, void *offset, int size, int dir);
extern void
vchiq_transfer_bulk(VCHIQ_BULK_T *bulk);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
index 87829a244465..7b948a173e29 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
@@ -34,8 +34,6 @@
#ifndef VCHIQ_IF_H
#define VCHIQ_IF_H
-#include "interface/vchi/vchi_mh.h"
-
#define VCHIQ_SERVICE_HANDLE_INVALID 0
#define VCHIQ_SLOT_SIZE 4096
@@ -156,11 +154,11 @@ extern VCHIQ_STATUS_T vchiq_bulk_receive(VCHIQ_SERVICE_HANDLE_T service,
void *data, unsigned int size, void *userdata,
VCHIQ_BULK_MODE_T mode);
extern VCHIQ_STATUS_T vchiq_bulk_transmit_handle(VCHIQ_SERVICE_HANDLE_T service,
- VCHI_MEM_HANDLE_T handle, const void *offset, unsigned int size,
+ const void *offset, unsigned int size,
void *userdata, VCHIQ_BULK_MODE_T mode);
extern VCHIQ_STATUS_T vchiq_bulk_receive_handle(VCHIQ_SERVICE_HANDLE_T service,
- VCHI_MEM_HANDLE_T handle, void *offset, unsigned int size,
- void *userdata, VCHIQ_BULK_MODE_T mode);
+ void *offset, unsigned int size, void *userdata,
+ VCHIQ_BULK_MODE_T mode);
extern int vchiq_get_client_id(VCHIQ_SERVICE_HANDLE_T service);
extern void *vchiq_get_service_userdata(VCHIQ_SERVICE_HANDLE_T service);
extern int vchiq_get_service_fourcc(VCHIQ_SERVICE_HANDLE_T service);
--
2.19.1
It is preferred in the kernel to avoid using semaphores to wait for
events, as they are optimised for the opposite situation; where the
common case is that they are available and may block only occasionally.
FYI see this thread: https://lkml.org/lkml/2008/4/11/323.
Also completions are semantically more explicit in this case.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 60 ++++++-----
.../interface/vchiq_arm/vchiq_core.c | 100 +++++++++---------
.../interface/vchiq_arm/vchiq_core.h | 26 ++---
.../interface/vchiq_arm/vchiq_util.c | 16 +--
.../interface/vchiq_arm/vchiq_util.h | 6 +-
5 files changed, 106 insertions(+), 102 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 1cdfdb714abc..383013a92939 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -44,7 +44,7 @@
#include <linux/highmem.h>
#include <linux/pagemap.h>
#include <linux/bug.h>
-#include <linux/semaphore.h>
+#include <linux/completion.h>
#include <linux/list.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -121,9 +121,9 @@ typedef struct user_service_struct {
int message_available_pos;
int msg_insert;
int msg_remove;
- struct semaphore insert_event;
- struct semaphore remove_event;
- struct semaphore close_event;
+ struct completion insert_event;
+ struct completion remove_event;
+ struct completion close_event;
VCHIQ_HEADER_T * msg_queue[MSG_QUEUE_SIZE];
} USER_SERVICE_T;
@@ -138,8 +138,8 @@ struct vchiq_instance_struct {
VCHIQ_COMPLETION_DATA_T completions[MAX_COMPLETIONS];
int completion_insert;
int completion_remove;
- struct semaphore insert_event;
- struct semaphore remove_event;
+ struct completion insert_event;
+ struct completion remove_event;
struct mutex completion_mutex;
int connected;
@@ -562,7 +562,8 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason,
vchiq_log_trace(vchiq_arm_log_level,
"%s - completion queue full", __func__);
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
- if (down_interruptible(&instance->remove_event) != 0) {
+ if (wait_for_completion_interruptible(
+ &instance->remove_event)) {
vchiq_log_info(vchiq_arm_log_level,
"service_callback interrupted");
return VCHIQ_RETRY;
@@ -600,7 +601,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason,
insert++;
instance->completion_insert = insert;
- up(&instance->insert_event);
+ complete(&instance->insert_event);
return VCHIQ_SUCCESS;
}
@@ -673,7 +674,8 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header,
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- if (down_interruptible(&user_service->remove_event)
+ if (wait_for_completion_interruptible(
+ &user_service->remove_event)
!= 0) {
vchiq_log_info(vchiq_arm_log_level,
"%s interrupted", __func__);
@@ -705,7 +707,7 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header,
}
spin_unlock(&msg_queue_spinlock);
- up(&user_service->insert_event);
+ complete(&user_service->insert_event);
header = NULL;
}
@@ -745,7 +747,7 @@ static void close_delivered(USER_SERVICE_T *user_service)
unlock_service(user_service->service);
/* Wake the user-thread blocked in close_ or remove_service */
- up(&user_service->close_event);
+ complete(&user_service->close_event);
user_service->close_pending = 0;
}
@@ -867,7 +869,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (status == VCHIQ_SUCCESS) {
/* Wake the completion thread and ask it to exit */
instance->closing = 1;
- up(&instance->insert_event);
+ complete(&instance->insert_event);
}
break;
@@ -948,9 +950,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
instance->completion_remove - 1;
user_service->msg_insert = 0;
user_service->msg_remove = 0;
- sema_init(&user_service->insert_event, 0);
- sema_init(&user_service->remove_event, 0);
- sema_init(&user_service->close_event, 0);
+ init_completion(&user_service->insert_event);
+ init_completion(&user_service->remove_event);
+ init_completion(&user_service->close_event);
if (args.is_open) {
status = vchiq_open_service_internal
@@ -1007,7 +1009,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
has been closed until the client library calls the
CLOSE_DELIVERED ioctl, signalling close_event. */
if (user_service->close_pending &&
- down_interruptible(&user_service->close_event))
+ wait_for_completion_interruptible(
+ &user_service->close_event))
status = VCHIQ_RETRY;
break;
}
@@ -1182,7 +1185,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
mutex_unlock(&instance->completion_mutex);
- rc = down_interruptible(&instance->insert_event);
+ rc = wait_for_completion_interruptible(
+ &instance->insert_event);
mutex_lock(&instance->completion_mutex);
if (rc != 0) {
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
@@ -1310,7 +1314,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
if (ret != 0)
- up(&instance->remove_event);
+ complete(&instance->remove_event);
mutex_unlock(&instance->completion_mutex);
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
} break;
@@ -1350,8 +1354,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
do {
spin_unlock(&msg_queue_spinlock);
DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- if (down_interruptible(
- &user_service->insert_event) != 0) {
+ if (wait_for_completion_interruptible(
+ &user_service->insert_event)) {
vchiq_log_info(vchiq_arm_log_level,
"DEQUEUE_MESSAGE interrupted");
ret = -EINTR;
@@ -1373,7 +1377,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
user_service->msg_remove++;
spin_unlock(&msg_queue_spinlock);
- up(&user_service->remove_event);
+ complete(&user_service->remove_event);
if (header == NULL)
ret = -ENOTCONN;
else if (header->size <= args.bufsize) {
@@ -1979,8 +1983,8 @@ vchiq_open(struct inode *inode, struct file *file)
vchiq_debugfs_add_instance(instance);
- sema_init(&instance->insert_event, 0);
- sema_init(&instance->remove_event, 0);
+ init_completion(&instance->insert_event);
+ init_completion(&instance->remove_event);
mutex_init(&instance->completion_mutex);
mutex_init(&instance->bulk_waiter_list_mutex);
INIT_LIST_HEAD(&instance->bulk_waiter_list);
@@ -2033,12 +2037,12 @@ vchiq_release(struct inode *inode, struct file *file)
/* Wake the completion thread and ask it to exit */
instance->closing = 1;
- up(&instance->insert_event);
+ complete(&instance->insert_event);
mutex_unlock(&instance->completion_mutex);
/* Wake the slot handler if the completion queue is full. */
- up(&instance->remove_event);
+ complete(&instance->remove_event);
/* Mark all services for termination... */
i = 0;
@@ -2047,7 +2051,7 @@ vchiq_release(struct inode *inode, struct file *file)
USER_SERVICE_T *user_service = service->base.userdata;
/* Wake the slot handler if the msg queue is full. */
- up(&user_service->remove_event);
+ complete(&user_service->remove_event);
vchiq_terminate_service_internal(service);
unlock_service(service);
@@ -2059,7 +2063,7 @@ vchiq_release(struct inode *inode, struct file *file)
!= NULL) {
USER_SERVICE_T *user_service = service->base.userdata;
- down(&service->remove_event);
+ wait_for_completion(&service->remove_event);
BUG_ON(service->srvstate != VCHIQ_SRVSTATE_FREE);
@@ -2103,7 +2107,7 @@ vchiq_release(struct inode *inode, struct file *file)
/* Wake any blocked user-thread */
if (instance->use_close_delivered)
- up(&user_service->close_event);
+ complete(&user_service->close_event);
unlock_service(service);
}
instance->completion_remove++;
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 5791c2b670fa..a45cdd08e209 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -376,7 +376,7 @@ mark_service_closing_internal(VCHIQ_SERVICE_T *service, int sh_thread)
/* Unblock any sending thread. */
service_quota = &state->service_quotas[service->localport];
- up(&service_quota->quota_event);
+ complete(&service_quota->quota_event);
}
static void
@@ -432,9 +432,9 @@ remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
event->armed = 1;
dsb(sy);
if (!event->fired) {
- if (down_interruptible(
- (struct semaphore *)
- ((char *)state + event->event)) != 0) {
+ if (wait_for_completion_interruptible(
+ (struct completion *)
+ ((char *)state + event->event))) {
event->armed = 0;
return 0;
}
@@ -451,7 +451,7 @@ static inline void
remote_event_signal_local(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
{
event->armed = 0;
- up((struct semaphore *)((char *)state + event->event));
+ complete((struct completion *)((char *)state + event->event));
}
static inline void
@@ -581,7 +581,7 @@ reserve_space(VCHIQ_STATE_T *state, size_t space, int is_blocking)
/* If there is no free slot... */
- if (down_trylock(&state->slot_available_event) != 0) {
+ if (!try_wait_for_completion(&state->slot_available_event)) {
/* ...wait for one. */
VCHIQ_STATS_INC(state, slot_stalls);
@@ -592,13 +592,13 @@ reserve_space(VCHIQ_STATE_T *state, size_t space, int is_blocking)
remote_event_signal(&state->remote->trigger);
if (!is_blocking ||
- (down_interruptible(
- &state->slot_available_event) != 0))
+ (wait_for_completion_interruptible(
+ &state->slot_available_event)))
return NULL; /* No space available */
}
if (tx_pos == (state->slot_queue_available * VCHIQ_SLOT_SIZE)) {
- up(&state->slot_available_event);
+ complete(&state->slot_available_event);
pr_warn("%s: invalid tx_pos: %d\n", __func__, tx_pos);
return NULL;
}
@@ -678,7 +678,7 @@ process_free_queue(VCHIQ_STATE_T *state, BITSET_T *service_found, size_t length)
/* Signal the service that it
** has dropped below its quota
*/
- up(&service_quota->quota_event);
+ complete(&service_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)",
@@ -703,7 +703,7 @@ process_free_queue(VCHIQ_STATE_T *state, BITSET_T *service_found, size_t length)
/* Signal the service in case
** it has dropped below its
** quota */
- up(&service_quota->quota_event);
+ complete(&service_quota->quota_event);
vchiq_log_trace(
vchiq_core_log_level,
"%d: pfq:%d %x@%pK - slot_use->%d",
@@ -744,7 +744,7 @@ process_free_queue(VCHIQ_STATE_T *state, BITSET_T *service_found, size_t length)
count - 1;
spin_unlock("a_spinlock);
if (count == state->data_quota)
- up(&state->data_quota_event);
+ complete(&state->data_quota_event);
}
/*
@@ -754,7 +754,7 @@ process_free_queue(VCHIQ_STATE_T *state, BITSET_T *service_found, size_t length)
mb();
state->slot_queue_available = slot_queue_available;
- up(&state->slot_available_event);
+ complete(&state->slot_available_event);
}
}
@@ -862,8 +862,8 @@ queue_message(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
spin_unlock("a_spinlock);
mutex_unlock(&state->slot_mutex);
- if (down_interruptible(&state->data_quota_event)
- != 0)
+ if (wait_for_completion_interruptible(
+ &state->data_quota_event))
return VCHIQ_RETRY;
mutex_lock(&state->slot_mutex);
@@ -873,7 +873,7 @@ queue_message(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
if ((tx_end_index == state->previous_data_index) ||
(state->data_use_count < state->data_quota)) {
/* Pass the signal on to other waiters */
- up(&state->data_quota_event);
+ complete(&state->data_quota_event);
break;
}
}
@@ -893,8 +893,8 @@ queue_message(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
service_quota->slot_use_count);
VCHIQ_SERVICE_STATS_INC(service, quota_stalls);
mutex_unlock(&state->slot_mutex);
- if (down_interruptible(&service_quota->quota_event)
- != 0)
+ if (wait_for_completion_interruptible(
+ &service_quota->quota_event))
return VCHIQ_RETRY;
if (service->closing)
return VCHIQ_ERROR;
@@ -1251,7 +1251,7 @@ notify_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue,
waiter = bulk->userdata;
if (waiter) {
waiter->actual = bulk->actual;
- up(&waiter->event);
+ complete(&waiter->event);
}
spin_unlock(&bulk_waiter_spinlock);
} else if (bulk->mode ==
@@ -1274,7 +1274,7 @@ notify_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue,
}
queue->remove++;
- up(&service->bulk_remove_event);
+ complete(&service->bulk_remove_event);
}
if (!retry_poll)
status = VCHIQ_SUCCESS;
@@ -1667,7 +1667,7 @@ parse_rx_slots(VCHIQ_STATE_T *state)
service->remoteport = remoteport;
vchiq_set_service_state(service,
VCHIQ_SRVSTATE_OPEN);
- up(&service->remove_event);
+ complete(&service->remove_event);
} else
vchiq_log_error(vchiq_core_log_level,
"OPENACK received in state %s",
@@ -1721,7 +1721,7 @@ parse_rx_slots(VCHIQ_STATE_T *state)
"%d: prs CONNECT@%pK", state->id, header);
state->version_common = ((VCHIQ_SLOT_ZERO_T *)
state->slot_data)->version;
- up(&state->connect);
+ complete(&state->connect);
break;
case VCHIQ_MSG_BULK_RX:
case VCHIQ_MSG_BULK_TX:
@@ -2055,7 +2055,7 @@ sync_func(void *v)
vchiq_set_service_state(service,
VCHIQ_SRVSTATE_OPENSYNC);
service->sync = 1;
- up(&service->remove_event);
+ complete(&service->remove_event);
}
release_message_sync(state, header);
break;
@@ -2194,33 +2194,33 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero)
initialize events and mutexes
*/
- sema_init(&state->connect, 0);
+ init_completion(&state->connect);
mutex_init(&state->mutex);
- sema_init(&state->trigger_event, 0);
- sema_init(&state->recycle_event, 0);
- sema_init(&state->sync_trigger_event, 0);
- sema_init(&state->sync_release_event, 0);
+ init_completion(&state->trigger_event);
+ init_completion(&state->recycle_event);
+ init_completion(&state->sync_trigger_event);
+ init_completion(&state->sync_release_event);
mutex_init(&state->slot_mutex);
mutex_init(&state->recycle_mutex);
mutex_init(&state->sync_mutex);
mutex_init(&state->bulk_transfer_mutex);
- sema_init(&state->slot_available_event, 0);
- sema_init(&state->slot_remove_event, 0);
- sema_init(&state->data_quota_event, 0);
+ init_completion(&state->slot_available_event);
+ init_completion(&state->slot_remove_event);
+ init_completion(&state->data_quota_event);
state->slot_queue_available = 0;
for (i = 0; i < VCHIQ_MAX_SERVICES; i++) {
VCHIQ_SERVICE_QUOTA_T *service_quota =
&state->service_quotas[i];
- sema_init(&service_quota->quota_event, 0);
+ init_completion(&service_quota->quota_event);
}
for (i = local->slot_first; i <= local->slot_last; i++) {
local->slot_queue[state->slot_queue_available++] = i;
- up(&state->slot_available_event);
+ complete(&state->slot_available_event);
}
state->default_slot_quota = state->slot_queue_available/2;
@@ -2354,8 +2354,8 @@ vchiq_add_service_internal(VCHIQ_STATE_T *state,
service->service_use_count = 0;
init_bulk_queue(&service->bulk_tx);
init_bulk_queue(&service->bulk_rx);
- sema_init(&service->remove_event, 0);
- sema_init(&service->bulk_remove_event, 0);
+ init_completion(&service->remove_event);
+ init_completion(&service->bulk_remove_event);
mutex_init(&service->bulk_mutex);
memset(&service->stats, 0, sizeof(service->stats));
@@ -2470,7 +2470,7 @@ vchiq_open_service_internal(VCHIQ_SERVICE_T *service, int client_id)
QMFLAGS_IS_BLOCKING);
if (status == VCHIQ_SUCCESS) {
/* Wait for the ACK/NAK */
- if (down_interruptible(&service->remove_event) != 0) {
+ if (wait_for_completion_interruptible(&service->remove_event)) {
status = VCHIQ_RETRY;
vchiq_release_service_internal(service);
} else if ((service->srvstate != VCHIQ_SRVSTATE_OPEN) &&
@@ -2622,7 +2622,7 @@ close_service_complete(VCHIQ_SERVICE_T *service, int failstate)
if (is_server)
service->closing = 0;
- up(&service->remove_event);
+ complete(&service->remove_event);
}
} else
vchiq_set_service_state(service, failstate);
@@ -2663,7 +2663,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
vchiq_set_service_state(service,
VCHIQ_SRVSTATE_LISTENING);
}
- up(&service->remove_event);
+ complete(&service->remove_event);
} else
vchiq_free_service_internal(service);
break;
@@ -2672,7 +2672,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
/* The open was rejected - tell the user */
vchiq_set_service_state(service,
VCHIQ_SRVSTATE_CLOSEWAIT);
- up(&service->remove_event);
+ complete(&service->remove_event);
} else {
/* Shutdown mid-open - let the other side know */
status = queue_message(state, service,
@@ -2805,7 +2805,7 @@ vchiq_free_service_internal(VCHIQ_SERVICE_T *service)
vchiq_set_service_state(service, VCHIQ_SRVSTATE_FREE);
- up(&service->remove_event);
+ complete(&service->remove_event);
/* Release the initial lock */
unlock_service(service);
@@ -2837,11 +2837,11 @@ vchiq_connect_internal(VCHIQ_STATE_T *state, VCHIQ_INSTANCE_T instance)
}
if (state->conn_state == VCHIQ_CONNSTATE_CONNECTING) {
- if (down_interruptible(&state->connect) != 0)
+ if (wait_for_completion_interruptible(&state->connect))
return VCHIQ_RETRY;
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
- up(&state->connect);
+ complete(&state->connect);
}
return VCHIQ_SUCCESS;
@@ -2936,7 +2936,7 @@ vchiq_close_service(VCHIQ_SERVICE_HANDLE_T handle)
}
while (1) {
- if (down_interruptible(&service->remove_event) != 0) {
+ if (wait_for_completion_interruptible(&service->remove_event)) {
status = VCHIQ_RETRY;
break;
}
@@ -2997,7 +2997,7 @@ vchiq_remove_service(VCHIQ_SERVICE_HANDLE_T handle)
request_poll(service->state, service, VCHIQ_POLL_REMOVE);
}
while (1) {
- if (down_interruptible(&service->remove_event) != 0) {
+ if (wait_for_completion_interruptible(&service->remove_event)) {
status = VCHIQ_RETRY;
break;
}
@@ -3054,7 +3054,7 @@ VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
break;
case VCHIQ_BULK_MODE_BLOCKING:
bulk_waiter = (struct bulk_waiter *)userdata;
- sema_init(&bulk_waiter->event, 0);
+ init_completion(&bulk_waiter->event);
bulk_waiter->actual = 0;
bulk_waiter->bulk = NULL;
break;
@@ -3080,8 +3080,8 @@ VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
VCHIQ_SERVICE_STATS_INC(service, bulk_stalls);
do {
mutex_unlock(&service->bulk_mutex);
- if (down_interruptible(&service->bulk_remove_event)
- != 0) {
+ if (wait_for_completion_interruptible(
+ &service->bulk_remove_event)) {
status = VCHIQ_RETRY;
goto error_exit;
}
@@ -3157,7 +3157,7 @@ VCHIQ_STATUS_T vchiq_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
if (bulk_waiter) {
bulk_waiter->bulk = bulk;
- if (down_interruptible(&bulk_waiter->event) != 0)
+ if (wait_for_completion_interruptible(&bulk_waiter->event))
status = VCHIQ_RETRY;
else if (bulk_waiter->actual == VCHIQ_BULK_ACTUAL_ABORTED)
status = VCHIQ_ERROR;
@@ -3326,7 +3326,7 @@ vchiq_set_service_option(VCHIQ_SERVICE_HANDLE_T handle,
service_quota->message_use_count)) {
/* Signal the service that it may have
** dropped below its quota */
- up(&service_quota->quota_event);
+ complete(&service_quota->quota_event);
}
status = VCHIQ_SUCCESS;
}
@@ -3347,7 +3347,7 @@ vchiq_set_service_option(VCHIQ_SERVICE_HANDLE_T handle,
service_quota->slot_use_count))
/* Signal the service that it may have
** dropped below its quota */
- up(&service_quota->quota_event);
+ complete(&service_quota->quota_event);
status = VCHIQ_SUCCESS;
}
} break;
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 5b1696422e21..b76281f7510e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -35,7 +35,7 @@
#define VCHIQ_CORE_H
#include <linux/mutex.h>
-#include <linux/semaphore.h>
+#include <linux/completion.h>
#include <linux/kthread.h>
#include "vchiq_cfg.h"
@@ -307,8 +307,8 @@ typedef struct vchiq_service_struct {
VCHIQ_BULK_QUEUE_T bulk_tx;
VCHIQ_BULK_QUEUE_T bulk_rx;
- struct semaphore remove_event;
- struct semaphore bulk_remove_event;
+ struct completion remove_event;
+ struct completion bulk_remove_event;
struct mutex bulk_mutex;
struct service_stats_struct {
@@ -337,7 +337,7 @@ typedef struct vchiq_service_quota_struct {
unsigned short slot_use_count;
unsigned short message_quota;
unsigned short message_use_count;
- struct semaphore quota_event;
+ struct completion quota_event;
int previous_tx_index;
} VCHIQ_SERVICE_QUOTA_T;
@@ -410,7 +410,7 @@ struct vchiq_state_struct {
unsigned short default_message_quota;
/* Event indicating connect message received */
- struct semaphore connect;
+ struct completion connect;
/* Mutex protecting services */
struct mutex mutex;
@@ -426,16 +426,16 @@ struct vchiq_state_struct {
struct task_struct *sync_thread;
/* Local implementation of the trigger remote event */
- struct semaphore trigger_event;
+ struct completion trigger_event;
/* Local implementation of the recycle remote event */
- struct semaphore recycle_event;
+ struct completion recycle_event;
/* Local implementation of the sync trigger remote event */
- struct semaphore sync_trigger_event;
+ struct completion sync_trigger_event;
/* Local implementation of the sync release remote event */
- struct semaphore sync_release_event;
+ struct completion sync_release_event;
char *tx_data;
char *rx_data;
@@ -481,12 +481,12 @@ struct vchiq_state_struct {
int unused_service;
/* Signalled when a free slot becomes available. */
- struct semaphore slot_available_event;
+ struct completion slot_available_event;
- struct semaphore slot_remove_event;
+ struct completion slot_remove_event;
/* Signalled when a free data slot becomes available. */
- struct semaphore data_quota_event;
+ struct completion data_quota_event;
struct state_stats_struct {
int slot_stalls;
@@ -505,7 +505,7 @@ struct vchiq_state_struct {
struct bulk_waiter {
VCHIQ_BULK_T *bulk;
- struct semaphore event;
+ struct completion event;
int actual;
};
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
index 2e52f07bbaa9..44b954daa74a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
@@ -48,8 +48,8 @@ int vchiu_queue_init(VCHIU_QUEUE_T *queue, int size)
queue->write = 0;
queue->initialized = 1;
- sema_init(&queue->pop, 0);
- sema_init(&queue->push, 0);
+ init_completion(&queue->pop);
+ init_completion(&queue->push);
queue->storage = kcalloc(size, sizeof(VCHIQ_HEADER_T *), GFP_KERNEL);
if (!queue->storage) {
@@ -80,7 +80,7 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header)
return;
while (queue->write == queue->read + queue->size) {
- if (down_interruptible(&queue->pop) != 0)
+ if (wait_for_completion_interruptible(&queue->pop))
flush_signals(current);
}
@@ -100,17 +100,17 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header)
queue->write++;
- up(&queue->push);
+ complete(&queue->push);
}
VCHIQ_HEADER_T *vchiu_queue_peek(VCHIU_QUEUE_T *queue)
{
while (queue->write == queue->read) {
- if (down_interruptible(&queue->push) != 0)
+ if (wait_for_completion_interruptible(&queue->push))
flush_signals(current);
}
- up(&queue->push); // We haven't removed anything from the queue.
+ complete(&queue->push); // We haven't removed anything from the queue.
/*
* Read from queue->storage must be visible after read from
@@ -126,7 +126,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue)
VCHIQ_HEADER_T *header;
while (queue->write == queue->read) {
- if (down_interruptible(&queue->push) != 0)
+ if (wait_for_completion_interruptible(&queue->push))
flush_signals(current);
}
@@ -146,7 +146,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue)
queue->read++;
- up(&queue->pop);
+ complete(&queue->pop);
return header;
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h
index 5a1540d349d3..b226227fe5bc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h
@@ -35,7 +35,7 @@
#define VCHIQ_UTIL_H
#include <linux/types.h>
-#include <linux/semaphore.h>
+#include <linux/completion.h>
#include <linux/mutex.h>
#include <linux/bitops.h>
#include <linux/kthread.h>
@@ -60,8 +60,8 @@ typedef struct {
int write;
int initialized;
- struct semaphore pop;
- struct semaphore push;
+ struct completion pop;
+ struct completion push;
VCHIQ_HEADER_T **storage;
} VCHIU_QUEUE_T;
--
2.19.1
vchiq_init_state() initialises a series of semaphores to then call
remote_event_create() on the same semaphores, which initializes them
again. We get rid of the second initialization.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 11 +++++------
1 file changed, 5 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 dee5ea7bfe4f..8b23ea5322e8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -418,12 +418,11 @@ vchiq_set_conn_state(VCHIQ_STATE_T *state, VCHIQ_CONNSTATE_T newstate)
}
static inline void
-remote_event_create(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
+remote_event_create(REMOTE_EVENT_T *event)
{
event->armed = 0;
/* Don't clear the 'fired' flag because it may already have been set
** by the other side. */
- sema_init((struct semaphore *)((char *)state + event->event), 0);
}
static inline int
@@ -2237,18 +2236,18 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero)
state->data_quota = state->slot_queue_available - 1;
local->trigger.event = offsetof(VCHIQ_STATE_T, trigger_event);
- remote_event_create(state, &local->trigger);
+ remote_event_create(&local->trigger);
local->tx_pos = 0;
local->recycle.event = offsetof(VCHIQ_STATE_T, recycle_event);
- remote_event_create(state, &local->recycle);
+ remote_event_create(&local->recycle);
local->slot_queue_recycle = state->slot_queue_available;
local->sync_trigger.event = offsetof(VCHIQ_STATE_T, sync_trigger_event);
- remote_event_create(state, &local->sync_trigger);
+ remote_event_create(&local->sync_trigger);
local->sync_release.event = offsetof(VCHIQ_STATE_T, sync_release_event);
- remote_event_create(state, &local->sync_release);
+ remote_event_create(&local->sync_release);
/* At start-of-day, the slot is empty and available */
((VCHIQ_HEADER_T *)SLOT_DATA_FROM_INDEX(state, local->slot_sync))->msgid
--
2.19.1
We update sync to reflect that the firmware version is compatible with
that option. We don't need to check both of them again further down the
code.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 +---
1 file changed, 1 insertion(+), 3 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 a45cdd08e209..5ee667d46eb5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1461,9 +1461,7 @@ parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
service->sync = 0;
/* Acknowledge the OPEN */
- if (service->sync &&
- (state->version_common >=
- VCHIQ_VERSION_SYNCHRONOUS_MODE)) {
+ if (service->sync) {
if (queue_message_sync(
state,
NULL,
--
2.19.1
All the memory operations featured in this file modify/access memory
that is only accessed by the CPU. So we can assume that all the memory
barrier handling done by the completion routines is good enough for us.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_util.c | 32 -------------------
1 file changed, 32 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
index 44b954daa74a..4b8554bc647e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
@@ -84,20 +84,7 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header)
flush_signals(current);
}
- /*
- * Write to queue->storage must be visible after read from
- * queue->read
- */
- smp_mb();
-
queue->storage[queue->write & (queue->size - 1)] = header;
-
- /*
- * Write to queue->storage must be visible before write to
- * queue->write
- */
- smp_wmb();
-
queue->write++;
complete(&queue->push);
@@ -112,12 +99,6 @@ VCHIQ_HEADER_T *vchiu_queue_peek(VCHIU_QUEUE_T *queue)
complete(&queue->push); // We haven't removed anything from the queue.
- /*
- * Read from queue->storage must be visible after read from
- * queue->write
- */
- smp_rmb();
-
return queue->storage[queue->read & (queue->size - 1)];
}
@@ -130,20 +111,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue)
flush_signals(current);
}
- /*
- * Read from queue->storage must be visible after read from
- * queue->write
- */
- smp_rmb();
-
header = queue->storage[queue->read & (queue->size - 1)];
-
- /*
- * Read from queue->storage must be visible before write to
- * queue->read
- */
- smp_mb();
-
queue->read++;
complete(&queue->pop);
--
2.19.1
The resulting code is way more readeable and intuitive compared to plain
list_for_each.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 52 ++++++-------------
1 file changed, 16 insertions(+), 36 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 d88dd7415e1e..5f55c708ade8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -280,16 +280,11 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance)
"%s(%p): returning %d", __func__, instance, status);
if (status == VCHIQ_SUCCESS) {
- struct list_head *pos, *next;
+ struct bulk_waiter_node *waiter, *next;
- list_for_each_safe(pos, next,
- &instance->bulk_waiter_list) {
- struct bulk_waiter_node *waiter;
-
- waiter = list_entry(pos,
- struct bulk_waiter_node,
- list);
- list_del(pos);
+ 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);
@@ -473,7 +468,6 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *data,
VCHIQ_SERVICE_T *service;
VCHIQ_STATUS_T status;
struct bulk_waiter_node *waiter = NULL;
- struct list_head *pos;
service = find_service_by_handle(handle);
if (!service)
@@ -484,13 +478,9 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *data,
unlock_service(service);
mutex_lock(&instance->bulk_waiter_list_mutex);
- list_for_each(pos, &instance->bulk_waiter_list) {
- if (list_entry(pos, struct bulk_waiter_node,
- list)->pid == current->pid) {
- waiter = list_entry(pos,
- struct bulk_waiter_node,
- list);
- list_del(pos);
+ list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
+ if (waiter->pid == current->pid) {
+ list_del(&waiter->list);
break;
}
}
@@ -1135,21 +1125,16 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
ret = -ENOMEM;
break;
}
+
args.userdata = &waiter->bulk_waiter;
} else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
- struct list_head *pos;
-
mutex_lock(&instance->bulk_waiter_list_mutex);
- list_for_each(pos, &instance->bulk_waiter_list) {
- if (list_entry(pos, struct bulk_waiter_node,
- list)->pid == current->pid) {
- waiter = list_entry(pos,
- struct bulk_waiter_node,
- list);
- list_del(pos);
+ list_for_each_entry(waiter, &instance->bulk_waiter_list,
+ list) {
+ if (waiter->pid == current->pid) {
+ list_del(&waiter->list);
break;
}
-
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
if (!waiter) {
@@ -2158,16 +2143,11 @@ vchiq_release(struct inode *inode, struct file *file)
vchiq_release_internal(instance->state, NULL);
{
- struct list_head *pos, *next;
-
- list_for_each_safe(pos, next,
- &instance->bulk_waiter_list) {
- struct bulk_waiter_node *waiter;
+ struct bulk_waiter_node *waiter, *next;
- waiter = list_entry(pos,
- struct bulk_waiter_node,
- list);
- list_del(pos);
+ 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);
--
2.19.1
Moves the allocation of a chardev region and class creation to the init
function of the driver since those functions are meant to be run on a
per driver basis, as opposed to the code run in the probe function which
is run in a per device basis.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 71 ++++++++++++-------
1 file changed, 45 insertions(+), 26 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 383013a92939..a7dcced79980 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -166,7 +166,6 @@ static struct cdev vchiq_cdev;
static dev_t vchiq_devid;
static VCHIQ_STATE_T g_state;
static struct class *vchiq_class;
-static struct device *vchiq_dev;
static DEFINE_SPINLOCK(msg_queue_spinlock);
static struct platform_device *bcm2835_camera;
@@ -3552,34 +3551,19 @@ static int vchiq_probe(struct platform_device *pdev)
if (err != 0)
goto failed_platform_init;
- err = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
- if (err != 0) {
- vchiq_log_error(vchiq_arm_log_level,
- "Unable to allocate device number");
- goto failed_platform_init;
- }
cdev_init(&vchiq_cdev, &vchiq_fops);
vchiq_cdev.owner = THIS_MODULE;
err = cdev_add(&vchiq_cdev, vchiq_devid, 1);
if (err != 0) {
vchiq_log_error(vchiq_arm_log_level,
"Unable to register device");
- goto failed_cdev_add;
+ goto failed_platform_init;
}
- /* create sysfs entries */
- vchiq_class = class_create(THIS_MODULE, DEVICE_NAME);
- err = PTR_ERR(vchiq_class);
- if (IS_ERR(vchiq_class))
- goto failed_class_create;
-
- vchiq_dev = device_create(vchiq_class, NULL,
- vchiq_devid, NULL, "vchiq");
- err = PTR_ERR(vchiq_dev);
- if (IS_ERR(vchiq_dev))
+ if (IS_ERR(device_create(vchiq_class, &pdev->dev, vchiq_devid,
+ NULL, "vchiq")))
goto failed_device_create;
- /* create debugfs entries */
vchiq_debugfs_init();
vchiq_log_info(vchiq_arm_log_level,
@@ -3594,11 +3578,7 @@ static int vchiq_probe(struct platform_device *pdev)
return 0;
failed_device_create:
- class_destroy(vchiq_class);
-failed_class_create:
cdev_del(&vchiq_cdev);
-failed_cdev_add:
- unregister_chrdev_region(vchiq_devid, 1);
failed_platform_init:
vchiq_log_warning(vchiq_arm_log_level, "could not load vchiq");
return err;
@@ -3609,9 +3589,7 @@ static int vchiq_remove(struct platform_device *pdev)
platform_device_unregister(bcm2835_camera);
vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
- class_destroy(vchiq_class);
cdev_del(&vchiq_cdev);
- unregister_chrdev_region(vchiq_devid, 1);
return 0;
}
@@ -3624,7 +3602,48 @@ static struct platform_driver vchiq_driver = {
.probe = vchiq_probe,
.remove = vchiq_remove,
};
-module_platform_driver(vchiq_driver);
+
+static int __init vchiq_driver_init(void)
+{
+ int ret;
+
+ vchiq_class = class_create(THIS_MODULE, DEVICE_NAME);
+ if (IS_ERR(vchiq_class)) {
+ pr_err("Failed to create vchiq class\n");
+ return PTR_ERR(vchiq_class);
+ }
+
+ ret = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
+ if (ret) {
+ pr_err("Failed to allocate vchiq's chrdev region\n");
+ goto class_destroy;
+ }
+
+ ret = platform_driver_register(&vchiq_driver);
+ if (ret) {
+ pr_err("Failed to register vchiq driver\n");
+ goto region_unregister;
+ }
+
+ return 0;
+
+region_unregister:
+ platform_driver_unregister(&vchiq_driver);
+
+class_destroy:
+ class_destroy(vchiq_class);
+
+ return ret;
+}
+module_init(vchiq_driver_init);
+
+static void __exit vchiq_driver_exit(void)
+{
+ platform_driver_unregister(&vchiq_driver);
+ unregister_chrdev_region(vchiq_devid, 1);
+ class_destroy(vchiq_class);
+}
+module_exit(vchiq_driver_exit);
MODULE_LICENSE("Dual BSD/GPL");
MODULE_DESCRIPTION("Videocore VCHIQ driver");
--
2.19.1
vchiq_init_state() checks the initial contents of slot_zero are correct.
These are set in vchiq_init_slots(), using the same hard-coded defaults
as the checks. Both functions are called sequentially and Video Core
isn't yet aware of the slot's address. There is no way the contents of
slot_zero changed in between functions, making the checks useless.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_core.c | 59 -------------------
1 file changed, 59 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 34a892011296..dee5ea7bfe4f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2170,65 +2170,6 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero)
return VCHIQ_ERROR;
}
- /* Check the input configuration */
-
- if (slot_zero->magic != VCHIQ_MAGIC) {
- vchiq_loud_error_header();
- vchiq_loud_error("Invalid VCHIQ magic value found.");
- vchiq_loud_error("slot_zero=%pK: magic=%x (expected %x)",
- slot_zero, slot_zero->magic, VCHIQ_MAGIC);
- vchiq_loud_error_footer();
- return VCHIQ_ERROR;
- }
-
- if (slot_zero->version < VCHIQ_VERSION_MIN) {
- vchiq_loud_error_header();
- vchiq_loud_error("Incompatible VCHIQ versions found.");
- vchiq_loud_error("slot_zero=%pK: VideoCore version=%d (minimum %d)",
- slot_zero, slot_zero->version, VCHIQ_VERSION_MIN);
- vchiq_loud_error("Restart with a newer VideoCore image.");
- vchiq_loud_error_footer();
- return VCHIQ_ERROR;
- }
-
- if (VCHIQ_VERSION < slot_zero->version_min) {
- vchiq_loud_error_header();
- vchiq_loud_error("Incompatible VCHIQ versions found.");
- vchiq_loud_error("slot_zero=%pK: version=%d (VideoCore minimum %d)",
- slot_zero, VCHIQ_VERSION, slot_zero->version_min);
- vchiq_loud_error("Restart with a newer kernel.");
- vchiq_loud_error_footer();
- return VCHIQ_ERROR;
- }
-
- if ((slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T)) ||
- (slot_zero->slot_size != VCHIQ_SLOT_SIZE) ||
- (slot_zero->max_slots != VCHIQ_MAX_SLOTS) ||
- (slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE)) {
- vchiq_loud_error_header();
- if (slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T))
- vchiq_loud_error("slot_zero=%pK: slot_zero_size=%d (expected %d)",
- slot_zero, slot_zero->slot_zero_size,
- (int)sizeof(VCHIQ_SLOT_ZERO_T));
- if (slot_zero->slot_size != VCHIQ_SLOT_SIZE)
- vchiq_loud_error("slot_zero=%pK: slot_size=%d (expected %d)",
- slot_zero, slot_zero->slot_size,
- VCHIQ_SLOT_SIZE);
- if (slot_zero->max_slots != VCHIQ_MAX_SLOTS)
- vchiq_loud_error("slot_zero=%pK: max_slots=%d (expected %d)",
- slot_zero, slot_zero->max_slots,
- VCHIQ_MAX_SLOTS);
- if (slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE)
- vchiq_loud_error("slot_zero=%pK: max_slots_per_side=%d (expected %d)",
- slot_zero, slot_zero->max_slots_per_side,
- VCHIQ_MAX_SLOTS_PER_SIDE);
- vchiq_loud_error_footer();
- return VCHIQ_ERROR;
- }
-
- if (VCHIQ_VERSION < slot_zero->version)
- slot_zero->version = VCHIQ_VERSION;
-
local = &slot_zero->slave;
remote = &slot_zero->master;
--
2.19.1
The implementation of both IOCTLS was the same except for one function
call. This joins both implementations and updates the code to avoid
unneeded indentations.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 66 +++++++------------
1 file changed, 24 insertions(+), 42 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 6d503392341e..d88dd7415e1e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1019,55 +1019,37 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
} break;
- case VCHIQ_IOC_CLOSE_SERVICE: {
+ case VCHIQ_IOC_CLOSE_SERVICE:
+ case VCHIQ_IOC_REMOVE_SERVICE: {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+ USER_SERVICE_T *user_service;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
- USER_SERVICE_T *user_service =
- (USER_SERVICE_T *)service->base.userdata;
- /* close_pending is false on first entry, and when the
- wait in vchiq_close_service has been interrupted. */
- if (!user_service->close_pending) {
- status = vchiq_close_service(service->handle);
- if (status != VCHIQ_SUCCESS)
- break;
- }
-
- /* close_pending is true once the underlying service
- has been closed until the client library calls the
- CLOSE_DELIVERED ioctl, signalling close_event. */
- if (user_service->close_pending &&
- down_interruptible(&user_service->close_event))
- status = VCHIQ_RETRY;
- } else
+ if (!service) {
ret = -EINVAL;
- } break;
+ break;
+ }
- case VCHIQ_IOC_REMOVE_SERVICE: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+ user_service = service->base.userdata;
- service = find_service_for_instance(instance, handle);
- if (service != NULL) {
- USER_SERVICE_T *user_service =
- (USER_SERVICE_T *)service->base.userdata;
- /* close_pending is false on first entry, and when the
- wait in vchiq_close_service has been interrupted. */
- if (!user_service->close_pending) {
- status = vchiq_remove_service(service->handle);
- if (status != VCHIQ_SUCCESS)
- break;
- }
+ /* close_pending is false on first entry, and when the
+ wait in vchiq_close_service has been interrupted. */
+ if (!user_service->close_pending) {
+ status = (cmd == VCHIQ_IOC_CLOSE_SERVICE) ?
+ vchiq_close_service(service->handle) :
+ vchiq_remove_service(service->handle);
+ if (status != VCHIQ_SUCCESS)
+ break;
+ }
- /* close_pending is true once the underlying service
- has been closed until the client library calls the
- CLOSE_DELIVERED ioctl, signalling close_event. */
- if (user_service->close_pending &&
- down_interruptible(&user_service->close_event))
- status = VCHIQ_RETRY;
- } else
- ret = -EINVAL;
- } break;
+ /* close_pending is true once the underlying service
+ has been closed until the client library calls the
+ CLOSE_DELIVERED ioctl, signalling close_event. */
+ if (user_service->close_pending &&
+ down_interruptible(&user_service->close_event))
+ status = VCHIQ_RETRY;
+ break;
+ }
case VCHIQ_IOC_USE_SERVICE:
case VCHIQ_IOC_RELEASE_SERVICE: {
--
2.19.1
It's the first thing remote_event_signal() does.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ----
1 file changed, 4 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 8b23ea5322e8..5791c2b670fa 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1137,9 +1137,6 @@ queue_message_sync(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
size);
}
- /* Make sure the new header is visible to the peer. */
- wmb();
-
remote_event_signal(&state->remote->sync_trigger);
if (VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_PAUSE)
@@ -3269,7 +3266,6 @@ static void
release_message_sync(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
{
header->msgid = VCHIQ_MSGID_PADDING;
- wmb();
remote_event_signal(&state->remote->sync_release);
}
--
2.19.1
The function is passed to vchiq_core.c for it to go trough all the
transfer elements (an array of pointers to data) and copy them into the
actual transfer memory (contiguous memory).
The logic in the function was "copy an element and return, except when
the element is empty, in which case look for the next non-empty element
and copy it. The function will be called as many times as necessary until
all the elements are copied".
Now, this approach already forces the function to loop around elements
and felt convoluted, so it was changed to a more straightforward "Copy
all the elements into memory as long as they fit".
The resulting function is shorter and simpler.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 89 +++++++------------
1 file changed, 31 insertions(+), 58 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 34160cc3b8bd..1cdfdb714abc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -752,74 +752,48 @@ static void close_delivered(USER_SERVICE_T *user_service)
}
struct vchiq_io_copy_callback_context {
- struct vchiq_element *current_element;
- size_t current_element_offset;
+ struct vchiq_element *element;
+ size_t element_offset;
unsigned long elements_to_go;
- size_t current_offset;
};
-static ssize_t
-vchiq_ioc_copy_element_data(
- void *context,
- void *dest,
- size_t offset,
- size_t maxsize)
+static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
+ size_t offset, size_t maxsize)
{
- long res;
+ struct vchiq_io_copy_callback_context *cc = context;
+ size_t total_bytes_copied = 0;
size_t bytes_this_round;
- struct vchiq_io_copy_callback_context *copy_context =
- (struct vchiq_io_copy_callback_context *)context;
-
- if (offset != copy_context->current_offset)
- return 0;
-
- if (!copy_context->elements_to_go)
- return 0;
-
- /*
- * Complex logic here to handle the case of 0 size elements
- * in the middle of the array of elements.
- *
- * Need to skip over these 0 size elements.
- */
- while (1) {
- bytes_this_round = min(copy_context->current_element->size -
- copy_context->current_element_offset,
- maxsize);
-
- if (bytes_this_round)
- break;
- copy_context->elements_to_go--;
- copy_context->current_element++;
- copy_context->current_element_offset = 0;
+ while (total_bytes_copied < maxsize) {
+ if (!cc->elements_to_go)
+ return total_bytes_copied;
- if (!copy_context->elements_to_go)
- return 0;
- }
+ if (!cc->element->size) {
+ cc->elements_to_go--;
+ cc->element++;
+ cc->element_offset = 0;
+ continue;
+ }
- res = copy_from_user(dest,
- copy_context->current_element->data +
- copy_context->current_element_offset,
- bytes_this_round);
+ bytes_this_round = min(cc->element->size - cc->element_offset,
+ maxsize - total_bytes_copied);
- if (res != 0)
- return -EFAULT;
+ if (copy_from_user(dest + total_bytes_copied,
+ cc->element->data + cc->element_offset,
+ bytes_this_round))
+ return -EFAULT;
- copy_context->current_element_offset += bytes_this_round;
- copy_context->current_offset += bytes_this_round;
+ cc->element_offset += bytes_this_round;
+ total_bytes_copied += bytes_this_round;
- /*
- * Check if done with current element, and if so advance to the next.
- */
- if (copy_context->current_element_offset ==
- copy_context->current_element->size) {
- copy_context->elements_to_go--;
- copy_context->current_element++;
- copy_context->current_element_offset = 0;
+ if (cc->element_offset == cc->element->size) {
+ cc->elements_to_go--;
+ cc->element++;
+ cc->element_offset = 0;
+ }
}
- return bytes_this_round;
+ return maxsize;
}
/**************************************************************************
@@ -836,10 +810,9 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
unsigned long i;
size_t total_size = 0;
- context.current_element = elements;
- context.current_element_offset = 0;
+ context.element = elements;
+ context.element_offset = 0;
context.elements_to_go = count;
- context.current_offset = 0;
for (i = 0; i < count; i++) {
if (!elements[i].data && elements[i].size != 0)
--
2.19.1
No one is using the API neither in the actual staging tree nor in the
downstream tree (https://github.com/raspberrypi/linux).
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../vc04_services/interface/vchi/vchi.h | 5 ---
.../interface/vchiq_arm/vchiq_shim.c | 32 -------------------
2 files changed, 37 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h b/drivers/staging/vc04_services/interface/vchi/vchi.h
index 01381904775d..379a16ebfd5b 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -113,11 +113,6 @@ extern uint32_t vchi_current_time(VCHI_INSTANCE_T instance_handle);
/******************************************************************************
Global service API
*****************************************************************************/
-// Routine to create a named service
-extern int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
- SERVICE_CREATION_T *setup,
- VCHI_SERVICE_HANDLE_T *handle);
-
// Routine to destroy a service
extern int32_t vchi_service_destroy(const VCHI_SERVICE_HANDLE_T handle);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index c3223fcdaf87..81cac68f4b78 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -660,38 +660,6 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle,
}
EXPORT_SYMBOL(vchi_service_open);
-int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
- SERVICE_CREATION_T *setup,
- VCHI_SERVICE_HANDLE_T *handle)
-{
- VCHIQ_INSTANCE_T instance = (VCHIQ_INSTANCE_T)instance_handle;
- struct shim_service *service = service_alloc(instance, setup);
-
- *handle = (VCHI_SERVICE_HANDLE_T)service;
-
- if (service) {
- VCHIQ_SERVICE_PARAMS_T params;
- VCHIQ_STATUS_T status;
-
- memset(¶ms, 0, sizeof(params));
- params.fourcc = setup->service_id;
- params.callback = shim_callback;
- params.userdata = service;
- params.version = setup->version.version;
- params.version_min = setup->version.version_min;
- status = vchiq_add_service(instance, ¶ms, &service->handle);
-
- if (status != VCHIQ_SUCCESS) {
- service_free(service);
- service = NULL;
- *handle = NULL;
- }
- }
-
- return (service != NULL) ? 0 : -1;
-}
-EXPORT_SYMBOL(vchi_service_create);
-
int32_t vchi_service_close(const VCHI_SERVICE_HANDLE_T handle)
{
int32_t ret = -1;
--
2.19.1
The function is overly complicated for what it's ultimately achieving.
It's simply filling up a structure.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 12 ++++----
.../interface/vchiq_arm/vchiq_core.c | 30 +++++--------------
.../interface/vchiq_arm/vchiq_if.h | 3 +-
3 files changed, 14 insertions(+), 31 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 ea789376de0f..6d503392341e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1480,13 +1480,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
ret = -EINVAL;
break;
}
- status = vchiq_get_config(instance, args.config_size, &config);
- if (status == VCHIQ_SUCCESS) {
- if (copy_to_user((void __user *)args.pconfig,
- &config, args.config_size) != 0) {
- ret = -EFAULT;
- break;
- }
+
+ vchiq_get_config(&config);
+ if (copy_to_user(args.pconfig, &config, args.config_size)) {
+ ret = -EFAULT;
+ break;
}
} break;
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 7642ced31436..89f1ccdc3b98 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3583,28 +3583,14 @@ vchiq_get_peer_version(VCHIQ_SERVICE_HANDLE_T handle, short *peer_version)
return status;
}
-VCHIQ_STATUS_T
-vchiq_get_config(VCHIQ_INSTANCE_T instance,
- int config_size, VCHIQ_CONFIG_T *pconfig)
-{
- VCHIQ_CONFIG_T config;
-
- (void)instance;
-
- config.max_msg_size = VCHIQ_MAX_MSG_SIZE;
- config.bulk_threshold = VCHIQ_MAX_MSG_SIZE;
- config.max_outstanding_bulks = VCHIQ_NUM_SERVICE_BULKS;
- config.max_services = VCHIQ_MAX_SERVICES;
- config.version = VCHIQ_VERSION;
- config.version_min = VCHIQ_VERSION_MIN;
-
- if (config_size > sizeof(VCHIQ_CONFIG_T))
- return VCHIQ_ERROR;
-
- memcpy(pconfig, &config,
- min(config_size, (int)(sizeof(VCHIQ_CONFIG_T))));
-
- return VCHIQ_SUCCESS;
+void vchiq_get_config(VCHIQ_CONFIG_T *config)
+{
+ config->max_msg_size = VCHIQ_MAX_MSG_SIZE;
+ config->bulk_threshold = VCHIQ_MAX_MSG_SIZE;
+ config->max_outstanding_bulks = VCHIQ_NUM_SERVICE_BULKS;
+ config->max_services = VCHIQ_MAX_SERVICES;
+ config->version = VCHIQ_VERSION;
+ config->version_min = VCHIQ_VERSION_MIN;
}
VCHIQ_STATUS_T
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
index e4109a83e628..87829a244465 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
@@ -164,8 +164,7 @@ extern VCHIQ_STATUS_T vchiq_bulk_receive_handle(VCHIQ_SERVICE_HANDLE_T service,
extern int vchiq_get_client_id(VCHIQ_SERVICE_HANDLE_T service);
extern void *vchiq_get_service_userdata(VCHIQ_SERVICE_HANDLE_T service);
extern int vchiq_get_service_fourcc(VCHIQ_SERVICE_HANDLE_T service);
-extern VCHIQ_STATUS_T vchiq_get_config(VCHIQ_INSTANCE_T instance,
- int config_size, VCHIQ_CONFIG_T *pconfig);
+extern void vchiq_get_config(VCHIQ_CONFIG_T *config);
extern VCHIQ_STATUS_T vchiq_set_service_option(VCHIQ_SERVICE_HANDLE_T service,
VCHIQ_SERVICE_OPTION_T option, int value);
--
2.19.1
Hi Nicolas,
> Nicolas Saenz Julienne <[email protected]> hat am 14. November 2018 um 13:59 geschrieben:
>
>
> Hi All,
>
> This series was written in parallel with reading and understanding the
> vchiq code. So excuse me for the lack of logic in the sequence of
> patches.
>
> The main focus was to delete as much code as possible, I've counted
> around 550 lines, which is not bad. Apart from that there are some
> patches enforcing proper kernel APIs usage.
>
> The only patch that really changes code is the
> vchiq_ioc_copy_element_data() rewrite.
>
> The last commit updates the TODO list with some of my observations, I
> realise some of the might be a little opinionated. If anything it's
> going to force a discussion on the topic, which is nice.
>
> It was developed on top of the latest linux-next, and was tested on a
> RPIv3B+ with audio, video and running vchiq_test.
>
> Regards,
> Nicolas
>
without a changelog i won't start a review.
Regards
Stefan
On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
> Hi Nicolas,
>
> > Nicolas Saenz Julienne <[email protected]> hat am 14. November 2018 um 13:59 geschrieben:
> >
> >
> > Hi All,
> >
> > This series was written in parallel with reading and understanding the
> > vchiq code. So excuse me for the lack of logic in the sequence of
> > patches.
> >
> > The main focus was to delete as much code as possible, I've counted
> > around 550 lines, which is not bad. Apart from that there are some
> > patches enforcing proper kernel APIs usage.
> >
> > The only patch that really changes code is the
> > vchiq_ioc_copy_element_data() rewrite.
> >
> > The last commit updates the TODO list with some of my observations, I
> > realise some of the might be a little opinionated. If anything it's
> > going to force a discussion on the topic, which is nice.
> >
> > It was developed on top of the latest linux-next, and was tested on a
> > RPIv3B+ with audio, video and running vchiq_test.
> >
> > Regards,
> > Nicolas
> >
>
> without a changelog i won't start a review.
What do you mean by this? The individual patches have a "changelog" in
them, this is just a summary of the overall changes.
What do you feel is missing here?
thanks,
greg k-h
On Tue, 2018-11-20 at 10:57 +0100, Greg KH wrote:
> On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
> > Hi Nicolas,
> >
> > > Nicolas Saenz Julienne <[email protected]> hat am 14.
> > > November 2018 um 13:59 geschrieben:
> > >
> > >
> > > Hi All,
> > >
> > > This series was written in parallel with reading and
> > > understanding the
> > > vchiq code. So excuse me for the lack of logic in the sequence of
> > > patches.
> > >
> > > The main focus was to delete as much code as possible, I've
> > > counted
> > > around 550 lines, which is not bad. Apart from that there are
> > > some
> > > patches enforcing proper kernel APIs usage.
> > >
> > > The only patch that really changes code is the
> > > vchiq_ioc_copy_element_data() rewrite.
> > >
> > > The last commit updates the TODO list with some of my
> > > observations, I
> > > realise some of the might be a little opinionated. If anything
> > > it's
> > > going to force a discussion on the topic, which is nice.
> > >
> > > It was developed on top of the latest linux-next, and was tested
> > > on a
> > > RPIv3B+ with audio, video and running vchiq_test.
> > >
> > > Regards,
> > > Nicolas
> > >
> >
> > without a changelog i won't start a review.
>
> What do you mean by this? The individual patches have a "changelog"
> in
> them, this is just a summary of the overall changes.
>
> What do you feel is missing here?
Hi Greg,
I did send an RFC version of this, to which Stephan made several
comments. I should have added the changelog to the proper series
submission. I've been busy with work, but I'll send the updated version
during the day.
Regards,
Nicolas
>
> thanks,
>
> greg k-h
Am 20.11.18 um 11:04 schrieb Nicolas Saenz Julienne:
> On Tue, 2018-11-20 at 10:57 +0100, Greg KH wrote:
>> On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
>>> Hi Nicolas,
>>>
>>>
>>> without a changelog i won't start a review.
>> What do you mean by this? The individual patches have a "changelog"
>> in
>> them, this is just a summary of the overall changes.
>>
>> What do you feel is missing here?
> Hi Greg,
> I did send an RFC version of this, to which Stephan made several
> comments. I should have added the changelog to the proper series
> submission. I've been busy with work, but I'll send the updated version
> during the day.
Thanks Stefan
>
> Regards,
> Nicolas
>
>> thanks,
>>
>> greg k-h