2022-12-19 12:07:46

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 0/7] staging: vc04_services: Remove custom return values

The series removes the custom return values from functions
and replaces them with linux error codes. This address the TODO
vchiq interface:
* Get rid of custom function return values

Changes in V2:
- Patch 3/7 now reports for specific errors like -ENOMEM, -EHOSTDOWN
- Patch 5/7 reports -ENOTCONN instead of -EINVAL and adds a "Fixes" tag

Umang Jain (7):
staging: vc04_services: Replace vchiq_status return type to int
staging: vc04_services: Drop VCHIQ_SUCCESS usage
staging: vc04_services: Drop VCHIQ_ERROR usage
staging: vc04_services: Drop VCHIQ_RETRY usage
vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect
staging: vc04_services: Drop enum vchiq_status remnants
staging: vc04_services: vchiq: Drop custom return values from TODO

.../bcm2835-audio/bcm2835-vchiq.c | 12 +-
.../include/linux/raspberrypi/vchiq.h | 65 +++---
drivers/staging/vc04_services/interface/TODO | 5 -
.../interface/vchiq_arm/vchiq_arm.c | 124 +++++-----
.../interface/vchiq_arm/vchiq_arm.h | 12 +-
.../interface/vchiq_arm/vchiq_core.c | 216 +++++++++---------
.../interface/vchiq_arm/vchiq_core.h | 18 +-
.../interface/vchiq_arm/vchiq_dev.c | 36 +--
.../interface/vchiq_arm/vchiq_ioctl.h | 8 +-
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 11 +-
10 files changed, 246 insertions(+), 261 deletions(-)

--
2.38.1


2022-12-19 12:08:20

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect

Drop the usage of VCHIQ_RETRY when the vchiq has connection status
VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.

This patch removes the usage of vCHIQ_RETRY completely and act as
intermediatory to address the TODO item:
* Get rid of custom function return values
for vc04_services/interface.

Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
Signed-off-by: Umang Jain <[email protected]>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 9c64d5de810e..ddb6d0f4daed 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void)
int vchiq_send_remote_use(struct vchiq_state *state)
{
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
- return VCHIQ_RETRY;
+ return -ENOTCONN;

return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0);
}
@@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state *state)
int vchiq_send_remote_use_active(struct vchiq_state *state)
{
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
- return VCHIQ_RETRY;
+ return -ENOTCONN;

return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE,
NULL, NULL, 0, 0);
--
2.38.1

2022-12-19 12:08:42

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 6/7] staging: vc04_services: Drop enum vchiq_status remnants

Drop all references to enum vchiq_status as they are no longer in
use.

Signed-off-by: Umang Jain <[email protected]>
---
.../include/linux/raspberrypi/vchiq.h | 4 ----
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 +++++++-------
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 6 +++---
3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 2ca4461d26ee..66965da11443 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -17,10 +17,6 @@ enum vchiq_reason {
VCHIQ_BULK_RECEIVE_ABORTED /* service, -, bulk_userdata */
};

-enum vchiq_status {
- VCHIQ_RETRY = 1
-};
-
enum vchiq_bulk_mode {
VCHIQ_BULK_MODE_CALLBACK,
VCHIQ_BULK_MODE_BLOCKING,
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 5cfad840bd18..22de23f3af02 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -751,7 +751,7 @@ static int vchiq_is_connected(struct vchiq_instance *instance)

int vchiq_connect(struct vchiq_instance *instance)
{
- enum vchiq_status status;
+ int status;
struct vchiq_state *state = instance->state;

if (mutex_lock_killable(&state->mutex)) {
@@ -778,7 +778,7 @@ vchiq_add_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
{
- enum vchiq_status status;
+ int status;
struct vchiq_state *state = instance->state;
struct vchiq_service *service = NULL;
int srvstate;
@@ -839,7 +839,7 @@ int
vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const void *data,
unsigned int size, void *userdata, enum vchiq_bulk_mode mode)
{
- enum vchiq_status status;
+ int status;

while (1) {
switch (mode) {
@@ -877,7 +877,7 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
void *data, unsigned int size, void *userdata,
enum vchiq_bulk_mode mode)
{
- enum vchiq_status status;
+ int status;

while (1) {
switch (mode) {
@@ -915,7 +915,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
unsigned int size, enum vchiq_bulk_dir dir)
{
struct vchiq_service *service;
- enum vchiq_status status;
+ int status;
struct bulk_waiter_node *waiter = NULL, *iter;

service = find_service_by_handle(instance, handle);
@@ -1103,7 +1103,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
*/
if ((user_service->message_available_pos -
instance->completion_remove) < 0) {
- enum vchiq_status status;
+ int status;

vchiq_log_info(vchiq_arm_log_level,
"Inserting extra MESSAGE_AVAILABLE");
@@ -1330,7 +1330,7 @@ vchiq_keepalive_thread_func(void *v)
struct vchiq_state *state = (struct vchiq_state *)v;
struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);

- enum vchiq_status status;
+ int status;
struct vchiq_instance *instance;
unsigned int ka_handle;
int ret;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index ddb6d0f4daed..4e705a447a62 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2591,7 +2591,7 @@ release_service_messages(struct vchiq_service *service)
static int
do_abort_bulks(struct vchiq_service *service)
{
- enum vchiq_status status;
+ int status;

/* Abort any outstanding bulk transfers */
if (mutex_lock_killable(&service->bulk_mutex))
@@ -2611,7 +2611,7 @@ do_abort_bulks(struct vchiq_service *service)
static int
close_service_complete(struct vchiq_service *service, int failstate)
{
- enum vchiq_status status;
+ int status;
int is_server = (service->public_fourcc != VCHIQ_FOURCC_INVALID);
int newstate;

@@ -3212,7 +3212,7 @@ vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
int vchiq_queue_kernel_message(struct vchiq_instance *instance, unsigned int handle, void *data,
unsigned int size)
{
- enum vchiq_status status;
+ int status;

while (1) {
status = vchiq_queue_message(instance, handle, memcpy_copy_callback,
--
2.38.1

2022-12-19 12:10:11

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 7/7] staging: vc04_services: vchiq: Drop custom return values from TODO

The custom return values (enum vchiq_status) has been dropped.
Remove the TODO entry for the same.

Signed-off-by: Umang Jain <[email protected]>
---
drivers/staging/vc04_services/interface/TODO | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 97085a0b3223..6d9d4a800aa7 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -40,11 +40,6 @@ 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.

-* 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.
-
* Reformat core code with more sane indentations

The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
--
2.38.1

2022-12-19 12:20:14

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 4/7] staging: vc04_services: Drop VCHIQ_RETRY usage

Drop the usage of VCHIQ_RETRY vchiq_status enum type in most of the
places and replace it with -EAGAIN. The exception to this replacement
is vchiq_send_remote_use() and vchiq_send_remote_use_active() which will
be addressed in the subsequent commit.

This patch acts as intermediatory to address the TODO item:
* Get rid of custom function return values
for vc04_services/interface.

Signed-off-by: Umang Jain <[email protected]>
---
.../interface/vchiq_arm/vchiq_arm.c | 18 ++---
.../interface/vchiq_arm/vchiq_core.c | 76 +++++++++----------
.../interface/vchiq_arm/vchiq_dev.c | 12 +--
3 files changed, 53 insertions(+), 53 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 2415baeccc9c..5cfad840bd18 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -728,7 +728,7 @@ int vchiq_shutdown(struct vchiq_instance *instance)
struct vchiq_state *state = instance->state;

if (mutex_lock_killable(&state->mutex))
- return VCHIQ_RETRY;
+ return -EAGAIN;

/* Remove all services */
vchiq_shutdown_internal(state, instance);
@@ -756,7 +756,7 @@ int vchiq_connect(struct vchiq_instance *instance)

if (mutex_lock_killable(&state->mutex)) {
vchiq_log_trace(vchiq_core_log_level, "%s: call to mutex_lock failed", __func__);
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
goto failed;
}
status = vchiq_connect_internal(state, instance);
@@ -859,11 +859,11 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
}

/*
- * vchiq_*_bulk_transfer() may return VCHIQ_RETRY, so we need
+ * vchiq_*_bulk_transfer() may return -EAGAIN, so we need
* to implement a retry mechanism since this function is
* supposed to block until queued
*/
- if (status != VCHIQ_RETRY)
+ if (status != -EAGAIN)
break;

msleep(1);
@@ -896,11 +896,11 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
}

/*
- * vchiq_*_bulk_transfer() may return VCHIQ_RETRY, so we need
+ * vchiq_*_bulk_transfer() may return -EAGAIN, so we need
* to implement a retry mechanism since this function is
* supposed to block until queued
*/
- if (status != VCHIQ_RETRY)
+ if (status != -EAGAIN)
break;

msleep(1);
@@ -961,7 +961,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
status = vchiq_bulk_transfer(instance, handle, data, NULL, size,
&waiter->bulk_waiter,
VCHIQ_BULK_MODE_BLOCKING, dir);
- if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
+ if ((status != -EAGAIN) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;

if (bulk) {
@@ -1001,7 +1001,7 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
if (wait_for_completion_interruptible(&instance->remove_event)) {
vchiq_log_info(vchiq_arm_log_level, "service_callback interrupted");
- return VCHIQ_RETRY;
+ return -EAGAIN;
} else if (instance->closing) {
vchiq_log_info(vchiq_arm_log_level, "service_callback closing");
return 0;
@@ -1122,7 +1122,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
vchiq_log_info(vchiq_arm_log_level, "%s interrupted", __func__);
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
- return VCHIQ_RETRY;
+ return -EAGAIN;
} else if (instance->closing) {
vchiq_log_info(vchiq_arm_log_level, "%s closing", __func__);
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
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 640fdb28d1c0..9c64d5de810e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -474,7 +474,7 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
header, bulk_userdata);
status = service->base.callback(service->instance, reason, header, service->handle,
bulk_userdata);
- if (status && (status != VCHIQ_RETRY)) {
+ if (status && (status != -EAGAIN)) {
vchiq_log_warning(vchiq_core_log_level,
"%d: ignoring ERROR from callback to service %x",
service->state->id, service->handle);
@@ -922,7 +922,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,

if (!(flags & QMFLAGS_NO_MUTEX_LOCK) &&
mutex_lock_killable(&state->slot_mutex))
- return VCHIQ_RETRY;
+ return -EAGAIN;

if (type == VCHIQ_MSG_DATA) {
int tx_end_index;
@@ -963,7 +963,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
mutex_unlock(&state->slot_mutex);

if (wait_for_completion_interruptible(&state->data_quota_event))
- return VCHIQ_RETRY;
+ return -EAGAIN;

mutex_lock(&state->slot_mutex);
spin_lock(&quota_spinlock);
@@ -987,11 +987,11 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
VCHIQ_SERVICE_STATS_INC(service, quota_stalls);
mutex_unlock(&state->slot_mutex);
if (wait_for_completion_interruptible(&quota->quota_event))
- return VCHIQ_RETRY;
+ return -EAGAIN;
if (service->closing)
return -EHOSTDOWN;
if (mutex_lock_killable(&state->slot_mutex))
- return VCHIQ_RETRY;
+ return -EAGAIN;
if (service->srvstate != VCHIQ_SRVSTATE_OPEN) {
/* The service has been closed */
mutex_unlock(&state->slot_mutex);
@@ -1015,7 +1015,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
*/
if (!(flags & QMFLAGS_NO_MUTEX_LOCK))
mutex_unlock(&state->slot_mutex);
- return VCHIQ_RETRY;
+ return -EAGAIN;
}

if (type == VCHIQ_MSG_DATA) {
@@ -1154,7 +1154,7 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,

if (VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_RESUME &&
mutex_lock_killable(&state->sync_mutex))
- return VCHIQ_RETRY;
+ return -EAGAIN;

remote_event_wait(&state->sync_release_event, &local->sync_release);

@@ -1348,7 +1348,7 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
get_bulk_reason(bulk);
status = make_service_callback(service, reason, NULL,
bulk->userdata);
- if (status == VCHIQ_RETRY)
+ if (status == -EAGAIN)
break;
}
}
@@ -1359,7 +1359,7 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
if (!retry_poll)
status = 0;

- if (status == VCHIQ_RETRY)
+ if (status == -EAGAIN)
request_poll(service->state, service, (queue == &service->bulk_tx) ?
VCHIQ_POLL_TXNOTIFY : VCHIQ_POLL_RXNOTIFY);

@@ -1526,14 +1526,14 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
/* Acknowledge the OPEN */
if (service->sync) {
if (queue_message_sync(state, NULL, openack_id, memcpy_copy_callback,
- &ack_payload, sizeof(ack_payload), 0) == VCHIQ_RETRY)
+ &ack_payload, sizeof(ack_payload), 0) == -EAGAIN)
goto bail_not_ready;

/* The service is now open */
set_service_state(service, VCHIQ_SRVSTATE_OPENSYNC);
} else {
if (queue_message(state, NULL, openack_id, memcpy_copy_callback,
- &ack_payload, sizeof(ack_payload), 0) == VCHIQ_RETRY)
+ &ack_payload, sizeof(ack_payload), 0) == -EAGAIN)
goto bail_not_ready;

/* The service is now open */
@@ -1548,7 +1548,7 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
fail_open:
/* No available service, or an invalid request - send a CLOSE */
if (queue_message(state, NULL, MAKE_CLOSE(0, VCHIQ_MSG_SRCPORT(msgid)),
- NULL, NULL, 0, 0) == VCHIQ_RETRY)
+ NULL, NULL, 0, 0) == -EAGAIN)
goto bail_not_ready;

return 1;
@@ -1687,7 +1687,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)

mark_service_closing_internal(service, 1);

- if (vchiq_close_service_internal(service, CLOSE_RECVD) == VCHIQ_RETRY)
+ if (vchiq_close_service_internal(service, CLOSE_RECVD) == -EAGAIN)
goto bail_not_ready;

vchiq_log_info(vchiq_core_log_level, "Close Service %c%c%c%c s:%u d:%d",
@@ -1704,7 +1704,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
claim_slot(state->rx_info);
DEBUG_TRACE(PARSE_LINE);
if (make_service_callback(service, VCHIQ_MESSAGE_AVAILABLE, header,
- NULL) == VCHIQ_RETRY) {
+ NULL) == -EAGAIN) {
DEBUG_TRACE(PARSE_LINE);
goto bail_not_ready;
}
@@ -1802,7 +1802,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
/* Send a PAUSE in response */
if (queue_message(state, NULL, MAKE_PAUSE, NULL, NULL, 0,
- QMFLAGS_NO_MUTEX_UNLOCK) == VCHIQ_RETRY)
+ QMFLAGS_NO_MUTEX_UNLOCK) == -EAGAIN)
goto bail_not_ready;
}
/* At this point slot_mutex is held */
@@ -1919,7 +1919,7 @@ handle_poll(struct vchiq_state *state)

case VCHIQ_CONNSTATE_PAUSING:
if (queue_message(state, NULL, MAKE_PAUSE, NULL, NULL, 0,
- QMFLAGS_NO_MUTEX_UNLOCK) != VCHIQ_RETRY) {
+ QMFLAGS_NO_MUTEX_UNLOCK) != -EAGAIN) {
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSE_SENT);
} else {
/* Retry later */
@@ -1929,7 +1929,7 @@ handle_poll(struct vchiq_state *state)

case VCHIQ_CONNSTATE_RESUMING:
if (queue_message(state, NULL, MAKE_RESUME, NULL, NULL, 0,
- QMFLAGS_NO_MUTEX_LOCK) != VCHIQ_RETRY) {
+ QMFLAGS_NO_MUTEX_LOCK) != -EAGAIN) {
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
} else {
/*
@@ -2085,9 +2085,9 @@ sync_func(void *v)
if ((service->remoteport == remoteport) &&
(service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
if (make_service_callback(service, VCHIQ_MESSAGE_AVAILABLE, header,
- NULL) == VCHIQ_RETRY)
+ NULL) == -EAGAIN)
vchiq_log_error(vchiq_sync_log_level,
- "synchronous callback to service %d returns VCHIQ_RETRY",
+ "synchronous callback to service %d returns -EAGAIN",
localport);
}
break;
@@ -2510,7 +2510,7 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)

/* Wait for the ACK/NAK */
if (wait_for_completion_interruptible(&service->remove_event)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
vchiq_release_service_internal(service);
} else if ((service->srvstate != VCHIQ_SRVSTATE_OPEN) &&
(service->srvstate != VCHIQ_SRVSTATE_OPENSYNC)) {
@@ -2643,7 +2643,7 @@ close_service_complete(struct vchiq_service *service, int failstate)

status = make_service_callback(service, VCHIQ_SERVICE_CLOSED, NULL, NULL);

- if (status != VCHIQ_RETRY) {
+ if (status != -EAGAIN) {
int uc = service->service_use_count;
int i;
/* Complete the close process */
@@ -2724,7 +2724,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
case VCHIQ_SRVSTATE_OPEN:
if (close_recvd) {
if (!do_abort_bulks(service))
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
}

release_service_messages(service);
@@ -2763,7 +2763,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
break;

if (!do_abort_bulks(service)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
break;
}

@@ -2847,15 +2847,15 @@ vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instanc

if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) {
if (queue_message(state, NULL, MAKE_CONNECT, NULL, NULL, 0,
- QMFLAGS_IS_BLOCKING) == VCHIQ_RETRY)
- return VCHIQ_RETRY;
+ QMFLAGS_IS_BLOCKING) == -EAGAIN)
+ return -EAGAIN;

vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTING);
}

if (state->conn_state == VCHIQ_CONNSTATE_CONNECTING) {
if (wait_for_completion_interruptible(&state->connect))
- return VCHIQ_RETRY;
+ return -EAGAIN;

vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
complete(&state->connect);
@@ -2902,7 +2902,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)

if (current == service->state->slot_handler_thread) {
status = vchiq_close_service_internal(service, NO_CLOSE_RECVD);
- WARN_ON(status == VCHIQ_RETRY);
+ WARN_ON(status == -EAGAIN);
} else {
/* Mark the service for termination by the slot handler */
request_poll(service->state, service, VCHIQ_POLL_TERMINATE);
@@ -2910,7 +2910,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)

while (1) {
if (wait_for_completion_interruptible(&service->remove_event)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
break;
}

@@ -2965,14 +2965,14 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
service->public_fourcc = VCHIQ_FOURCC_INVALID;

status = vchiq_close_service_internal(service, NO_CLOSE_RECVD);
- WARN_ON(status == VCHIQ_RETRY);
+ WARN_ON(status == -EAGAIN);
} else {
/* Mark the service for removal by the slot handler */
request_poll(service->state, service, VCHIQ_POLL_REMOVE);
}
while (1) {
if (wait_for_completion_interruptible(&service->remove_event)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
break;
}

@@ -2996,7 +2996,7 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)

/*
* This function may be called by kernel threads or user threads.
- * User threads may receive VCHIQ_RETRY to indicate that a signal has been
+ * User threads may receive -EAGAIN to indicate that a signal has been
* received and the call should be retried after being returned to user
* context.
* When called in blocking mode, the userdata field points to a bulk_waiter
@@ -3053,7 +3053,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
&service->bulk_tx : &service->bulk_rx;

if (mutex_lock_killable(&service->bulk_mutex)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
goto error_exit;
}

@@ -3062,11 +3062,11 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
do {
mutex_unlock(&service->bulk_mutex);
if (wait_for_completion_interruptible(&service->bulk_remove_event)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
goto error_exit;
}
if (mutex_lock_killable(&service->bulk_mutex)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
goto error_exit;
}
} while (queue->local_insert == queue->remove +
@@ -3099,7 +3099,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
* claim it here to ensure that isn't happening
*/
if (mutex_lock_killable(&state->slot_mutex)) {
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
goto cancel_bulk_error_exit;
}

@@ -3139,7 +3139,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
if (bulk_waiter) {
bulk_waiter->bulk = bulk;
if (wait_for_completion_interruptible(&bulk_waiter->event))
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
else if (bulk_waiter->actual == VCHIQ_BULK_ACTUAL_ABORTED)
status = -EINVAL;
}
@@ -3219,11 +3219,11 @@ int vchiq_queue_kernel_message(struct vchiq_instance *instance, unsigned int han
data, size);

/*
- * vchiq_queue_message() may return VCHIQ_RETRY, so we need to
+ * vchiq_queue_message() may return -EAGAIN, so we need to
* implement a retry mechanism since this function is supposed
* to block until queued
*/
- if (status != VCHIQ_RETRY)
+ if (status != -EAGAIN)
break;

msleep(1);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index df274192937e..841e1a535642 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -132,7 +132,7 @@ vchiq_ioc_queue_message(struct vchiq_instance *instance, unsigned int handle,

if (status == -EINVAL)
return -EIO;
- else if (status == VCHIQ_RETRY)
+ else if (status == -EAGAIN)
return -EINTR;
return 0;
}
@@ -192,7 +192,7 @@ static int vchiq_ioc_create_service(struct vchiq_instance *instance,
status = vchiq_open_service_internal(service, instance->pid);
if (status) {
vchiq_remove_service(instance, service->handle);
- return (status == VCHIQ_RETRY) ?
+ return (status == -EAGAIN) ?
-EINTR : -EIO;
}
}
@@ -338,7 +338,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
goto out;
}

- if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
+ if ((status != -EAGAIN) || fatal_signal_pending(current) ||
!waiter->bulk_waiter.bulk) {
if (waiter->bulk_waiter.bulk) {
/* Cancel the signal when the transfer completes. */
@@ -366,7 +366,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
return ret;
else if (status == -EINVAL)
return -EIO;
- else if (status == VCHIQ_RETRY)
+ else if (status == -EAGAIN)
return -EINTR;
return 0;
}
@@ -686,7 +686,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
*/
if (user_service->close_pending &&
wait_for_completion_interruptible(&user_service->close_event))
- status = VCHIQ_RETRY;
+ status = -EAGAIN;
break;
}

@@ -864,7 +864,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (ret == 0) {
if (status == -EINVAL)
ret = -EIO;
- else if (status == VCHIQ_RETRY)
+ else if (status == -EAGAIN)
ret = -EINTR;
}

--
2.38.1

2022-12-19 12:20:47

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 3/7] staging: vc04_services: Drop VCHIQ_ERROR usage

Drop the usage of VCHIQ_ERROR vchiq_status enum type. Replace it with
-EINVAL to report the error in most cases, -ENOMEM for out-of-memory
errors and -EHOSTDOWN for service shutdown.

This patch acts as intermediatory to address the TODO item:
* Get rid of custom function return values
for vc04_services/interface.

Signed-off-by: Umang Jain <[email protected]>
---
.../include/linux/raspberrypi/vchiq.h | 1 -
.../interface/vchiq_arm/vchiq_arm.c | 24 +++++-----
.../interface/vchiq_arm/vchiq_core.c | 44 +++++++++----------
.../interface/vchiq_arm/vchiq_dev.c | 6 +--
4 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 52c513e5cf3b..2ca4461d26ee 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -18,7 +18,6 @@ enum vchiq_reason {
};

enum vchiq_status {
- VCHIQ_ERROR = -1,
VCHIQ_RETRY = 1
};

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 6e4e17272dad..2415baeccc9c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -501,7 +501,7 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)

vchiq_slot_zero = vchiq_init_slots(slot_mem, slot_mem_size);
if (!vchiq_slot_zero)
- return -EINVAL;
+ return -ENOMEM;

vchiq_slot_zero->platform_data[VCHIQ_PLATFORM_FRAGMENTS_OFFSET_IDX] =
(int)slot_phys + slot_mem_size;
@@ -795,7 +795,7 @@ vchiq_add_service(struct vchiq_instance *instance,
*phandle = service->handle;
status = 0;
} else {
- status = VCHIQ_ERROR;
+ status = -EINVAL;
}

vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, status);
@@ -808,7 +808,7 @@ vchiq_open_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
{
- enum vchiq_status status = VCHIQ_ERROR;
+ int status = -EINVAL;
struct vchiq_state *state = instance->state;
struct vchiq_service *service = NULL;

@@ -855,7 +855,7 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
VCHIQ_BULK_TRANSMIT);
break;
default:
- return VCHIQ_ERROR;
+ return -EINVAL;
}

/*
@@ -892,7 +892,7 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
VCHIQ_BULK_RECEIVE);
break;
default:
- return VCHIQ_ERROR;
+ return -EINVAL;
}

/*
@@ -920,7 +920,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl

service = find_service_by_handle(instance, handle);
if (!service)
- return VCHIQ_ERROR;
+ return -EINVAL;

vchiq_service_put(service);

@@ -954,7 +954,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
if (!waiter) {
vchiq_log_error(vchiq_core_log_level, "%s - out of memory", __func__);
- return VCHIQ_ERROR;
+ return -ENOMEM;
}
}

@@ -1127,7 +1127,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
vchiq_log_info(vchiq_arm_log_level, "%s closing", __func__);
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
- return VCHIQ_ERROR;
+ return -EINVAL;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
spin_lock(&msg_queue_spinlock);
@@ -1590,7 +1590,7 @@ vchiq_instance_set_trace(struct vchiq_instance *instance, int trace)
int
vchiq_use_service(struct vchiq_instance *instance, unsigned int handle)
{
- enum vchiq_status ret = VCHIQ_ERROR;
+ int ret = -EINVAL;
struct vchiq_service *service = find_service_by_handle(instance, handle);

if (service) {
@@ -1604,7 +1604,7 @@ EXPORT_SYMBOL(vchiq_use_service);
int
vchiq_release_service(struct vchiq_instance *instance, unsigned int handle)
{
- enum vchiq_status ret = VCHIQ_ERROR;
+ int ret = -EINVAL;
struct vchiq_service *service = find_service_by_handle(instance, handle);

if (service) {
@@ -1699,7 +1699,7 @@ int
vchiq_check_service(struct vchiq_service *service)
{
struct vchiq_arm_state *arm_state;
- enum vchiq_status ret = VCHIQ_ERROR;
+ int ret = -EINVAL;

if (!service || !service->state)
goto out;
@@ -1711,7 +1711,7 @@ vchiq_check_service(struct vchiq_service *service)
ret = 0;
read_unlock_bh(&arm_state->susp_res_lock);

- if (ret == VCHIQ_ERROR) {
+ if (ret) {
vchiq_log_error(vchiq_susp_log_level,
"%s ERROR - %c%c%c%c:%d service count %d, state count %d", __func__,
VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), service->client_id,
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 8b8f9e56d924..640fdb28d1c0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -467,14 +467,14 @@ static inline int
make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
struct vchiq_header *header, void *bulk_userdata)
{
- enum vchiq_status status;
+ int status;

vchiq_log_trace(vchiq_core_log_level, "%d: callback:%d (%s, %pK, %pK)",
service->state->id, service->localport, reason_names[reason],
header, bulk_userdata);
status = service->base.callback(service->instance, reason, header, service->handle,
bulk_userdata);
- if (status == VCHIQ_ERROR) {
+ if (status && (status != VCHIQ_RETRY)) {
vchiq_log_warning(vchiq_core_log_level,
"%d: ignoring ERROR from callback to service %x",
service->state->id, service->handle);
@@ -930,7 +930,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
if (!service) {
WARN(1, "%s: service is NULL\n", __func__);
mutex_unlock(&state->slot_mutex);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

WARN_ON(flags & (QMFLAGS_NO_MUTEX_LOCK |
@@ -939,7 +939,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
if (service->closing) {
/* The service has been closed */
mutex_unlock(&state->slot_mutex);
- return VCHIQ_ERROR;
+ return -EHOSTDOWN;
}

quota = &state->service_quotas[service->localport];
@@ -989,13 +989,13 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
if (wait_for_completion_interruptible(&quota->quota_event))
return VCHIQ_RETRY;
if (service->closing)
- return VCHIQ_ERROR;
+ return -EHOSTDOWN;
if (mutex_lock_killable(&state->slot_mutex))
return VCHIQ_RETRY;
if (service->srvstate != VCHIQ_SRVSTATE_OPEN) {
/* The service has been closed */
mutex_unlock(&state->slot_mutex);
- return VCHIQ_ERROR;
+ return -EHOSTDOWN;
}
spin_lock(&quota_spinlock);
tx_end_index = SLOT_QUEUE_INDEX_FROM_POS(state->local_tx_pos + stride - 1);
@@ -1037,7 +1037,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
if (callback_result < 0) {
mutex_unlock(&state->slot_mutex);
VCHIQ_SERVICE_STATS_INC(service, error_count);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

if (SRVTRACE_ENABLED(service,
@@ -1185,7 +1185,7 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
if (callback_result < 0) {
mutex_unlock(&state->slot_mutex);
VCHIQ_SERVICE_STATS_INC(service, error_count);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

if (service) {
@@ -2520,7 +2520,7 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
service->state->id,
srvstate_names[service->srvstate],
kref_read(&service->ref_count));
- status = VCHIQ_ERROR;
+ status = -EINVAL;
VCHIQ_SERVICE_STATS_INC(service, error_count);
vchiq_release_service_internal(service);
}
@@ -2638,7 +2638,7 @@ close_service_complete(struct vchiq_service *service, int failstate)
vchiq_log_error(vchiq_core_log_level, "%s(%x) called in state %s", __func__,
service->handle, srvstate_names[service->srvstate]);
WARN(1, "%s in unexpected state\n", __func__);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

status = make_service_callback(service, VCHIQ_SERVICE_CLOSED, NULL, NULL);
@@ -2695,7 +2695,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
__func__, srvstate_names[service->srvstate]);
} else if (is_server) {
if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
- status = VCHIQ_ERROR;
+ status = -EINVAL;
} else {
service->client_id = 0;
service->remoteport = VCHIQ_PORT_FREE;
@@ -2886,7 +2886,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
int status = 0;

if (!service)
- return VCHIQ_ERROR;
+ return -EINVAL;

vchiq_log_info(vchiq_core_log_level, "%d: close_service:%d",
service->state->id, service->localport);
@@ -2895,7 +2895,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
(service->srvstate == VCHIQ_SRVSTATE_LISTENING) ||
(service->srvstate == VCHIQ_SRVSTATE_HIDDEN)) {
vchiq_service_put(service);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

mark_service_closing(service);
@@ -2928,7 +2928,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
if (!status &&
(service->srvstate != VCHIQ_SRVSTATE_FREE) &&
(service->srvstate != VCHIQ_SRVSTATE_LISTENING))
- status = VCHIQ_ERROR;
+ status = -EINVAL;

vchiq_service_put(service);

@@ -2944,14 +2944,14 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
int status = 0;

if (!service)
- return VCHIQ_ERROR;
+ return -EINVAL;

vchiq_log_info(vchiq_core_log_level, "%d: remove_service:%d",
service->state->id, service->localport);

if (service->srvstate == VCHIQ_SRVSTATE_FREE) {
vchiq_service_put(service);
- return VCHIQ_ERROR;
+ return -EINVAL;
}

mark_service_closing(service);
@@ -2987,7 +2987,7 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
}

if (!status && (service->srvstate != VCHIQ_SRVSTATE_FREE))
- status = VCHIQ_ERROR;
+ status = -EINVAL;

vchiq_service_put(service);

@@ -3014,7 +3014,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
const char dir_char = (dir == VCHIQ_BULK_TRANSMIT) ? 't' : 'r';
const int dir_msgtype = (dir == VCHIQ_BULK_TRANSMIT) ?
VCHIQ_MSG_BULK_TX : VCHIQ_MSG_BULK_RX;
- enum vchiq_status status = VCHIQ_ERROR;
+ int status = -EINVAL;
int payload[2];

if (!service)
@@ -3141,7 +3141,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
if (wait_for_completion_interruptible(&bulk_waiter->event))
status = VCHIQ_RETRY;
else if (bulk_waiter->actual == VCHIQ_BULK_ACTUAL_ABORTED)
- status = VCHIQ_ERROR;
+ status = -EINVAL;
}

return status;
@@ -3167,7 +3167,7 @@ vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
size_t size)
{
struct vchiq_service *service = find_service_by_handle(instance, handle);
- enum vchiq_status status = VCHIQ_ERROR;
+ int status = -EINVAL;
int data_id;

if (!service)
@@ -3198,7 +3198,7 @@ vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
copy_callback, context, size, 1);
break;
default:
- status = VCHIQ_ERROR;
+ status = -EINVAL;
break;
}

@@ -3278,7 +3278,7 @@ release_message_sync(struct vchiq_state *state, struct vchiq_header *header)
int
vchiq_get_peer_version(struct vchiq_instance *instance, unsigned int handle, short *peer_version)
{
- enum vchiq_status status = VCHIQ_ERROR;
+ int status = -EINVAL;
struct vchiq_service *service = find_service_by_handle(instance, handle);

if (!service)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index d9c4d550412e..df274192937e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -130,7 +130,7 @@ vchiq_ioc_queue_message(struct vchiq_instance *instance, unsigned int handle,
status = vchiq_queue_message(instance, handle, vchiq_ioc_copy_element_data,
&context, total_size);

- if (status == VCHIQ_ERROR)
+ if (status == -EINVAL)
return -EIO;
else if (status == VCHIQ_RETRY)
return -EINTR;
@@ -364,7 +364,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
vchiq_service_put(service);
if (ret)
return ret;
- else if (status == VCHIQ_ERROR)
+ else if (status == -EINVAL)
return -EIO;
else if (status == VCHIQ_RETRY)
return -EINTR;
@@ -862,7 +862,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
vchiq_service_put(service);

if (ret == 0) {
- if (status == VCHIQ_ERROR)
+ if (status == -EINVAL)
ret = -EIO;
else if (status == VCHIQ_RETRY)
ret = -EINTR;
--
2.38.1

2022-12-19 12:27:19

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 1/7] staging: vc04_services: Replace vchiq_status return type to int

Modify the functions' signature using enum vchiq_status to return int.
Currently, this patch only touches the function signatures and
in subsequent patches each vchiq_status enumerated value will be
replaced by a integer value.

This patch acts as an initial point to address the TODO item:
* Get rid of custom function return values
for vc04_services/interface.

Signed-off-by: Umang Jain <[email protected]>
---
.../bcm2835-audio/bcm2835-vchiq.c | 8 +--
.../include/linux/raspberrypi/vchiq.h | 59 +++++++++----------
.../interface/vchiq_arm/vchiq_arm.c | 32 +++++-----
.../interface/vchiq_arm/vchiq_arm.h | 12 ++--
.../interface/vchiq_arm/vchiq_core.c | 34 +++++------
.../interface/vchiq_arm/vchiq_core.h | 18 +++---
.../interface/vchiq_arm/vchiq_ioctl.h | 8 +--
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 7 +--
8 files changed, 88 insertions(+), 90 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index f4c2c9506d86..0fd92affc264 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -91,10 +91,10 @@ static int bcm2835_audio_send_simple(struct bcm2835_audio_instance *instance,
return bcm2835_audio_send_msg(instance, &m, wait);
}

-static enum vchiq_status audio_vchi_callback(struct vchiq_instance *vchiq_instance,
- enum vchiq_reason reason,
- struct vchiq_header *header,
- unsigned int handle, void *userdata)
+static int audio_vchi_callback(struct vchiq_instance *vchiq_instance,
+ enum vchiq_reason reason,
+ struct vchiq_header *header,
+ unsigned int handle, void *userdata)
{
struct bcm2835_audio_instance *instance = vchiq_get_service_userdata(vchiq_instance,
handle);
diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index db1441c0cc66..71f4cb5d5cfd 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -57,11 +57,11 @@ struct vchiq_instance;

struct vchiq_service_base {
int fourcc;
- enum vchiq_status (*callback)(struct vchiq_instance *instance,
- enum vchiq_reason reason,
- struct vchiq_header *header,
- unsigned int handle,
- void *bulk_userdata);
+ int (*callback)(struct vchiq_instance *instance,
+ enum vchiq_reason reason,
+ struct vchiq_header *header,
+ unsigned int handle,
+ void *bulk_userdata);
void *userdata;
};

@@ -74,11 +74,11 @@ struct vchiq_completion_data_kernel {

struct vchiq_service_params_kernel {
int fourcc;
- enum vchiq_status (*callback)(struct vchiq_instance *instance,
- enum vchiq_reason reason,
- struct vchiq_header *header,
- unsigned int handle,
- void *bulk_userdata);
+ int (*callback)(struct vchiq_instance *instance,
+ enum vchiq_reason reason,
+ struct vchiq_header *header,
+ unsigned int handle,
+ void *bulk_userdata);
void *userdata;
short version; /* Increment for non-trivial changes */
short version_min; /* Update for incompatible changes */
@@ -86,33 +86,32 @@ struct vchiq_service_params_kernel {

struct vchiq_instance;

-extern enum vchiq_status vchiq_initialise(struct vchiq_instance **pinstance);
-extern enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance);
-extern enum vchiq_status vchiq_connect(struct vchiq_instance *instance);
-extern enum vchiq_status vchiq_open_service(struct vchiq_instance *instance,
- const struct vchiq_service_params_kernel *params,
- unsigned int *pservice);
-extern enum vchiq_status vchiq_close_service(struct vchiq_instance *instance,
- unsigned int service);
-extern enum vchiq_status vchiq_use_service(struct vchiq_instance *instance, unsigned int service);
-extern enum vchiq_status vchiq_release_service(struct vchiq_instance *instance,
- unsigned int service);
+extern int vchiq_initialise(struct vchiq_instance **pinstance);
+extern int vchiq_shutdown(struct vchiq_instance *instance);
+extern int vchiq_connect(struct vchiq_instance *instance);
+extern int vchiq_open_service(struct vchiq_instance *instance,
+ const struct vchiq_service_params_kernel *params,
+ unsigned int *pservice);
+extern int vchiq_close_service(struct vchiq_instance *instance,
+ unsigned int service);
+extern int vchiq_use_service(struct vchiq_instance *instance, unsigned int service);
+extern int vchiq_release_service(struct vchiq_instance *instance,
+ unsigned int service);
extern void vchiq_msg_queue_push(struct vchiq_instance *instance, unsigned int handle,
struct vchiq_header *header);
extern void vchiq_release_message(struct vchiq_instance *instance, unsigned int service,
struct vchiq_header *header);
extern int vchiq_queue_kernel_message(struct vchiq_instance *instance, unsigned int handle,
void *data, unsigned int size);
-extern enum vchiq_status vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int service,
- const void *data, unsigned int size, void *userdata,
- enum vchiq_bulk_mode mode);
-extern enum vchiq_status vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int service,
- void *data, unsigned int size, void *userdata,
- enum vchiq_bulk_mode mode);
+extern int vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int service,
+ const void *data, unsigned int size, void *userdata,
+ enum vchiq_bulk_mode mode);
+extern int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int service,
+ void *data, unsigned int size, void *userdata,
+ enum vchiq_bulk_mode mode);
extern void *vchiq_get_service_userdata(struct vchiq_instance *instance, unsigned int service);
-extern enum vchiq_status vchiq_get_peer_version(struct vchiq_instance *instance,
- unsigned int handle,
- short *peer_version);
+extern int vchiq_get_peer_version(struct vchiq_instance *instance, unsigned int handle,
+ short *peer_version);
extern struct vchiq_header *vchiq_msg_hold(struct vchiq_instance *instance, unsigned int handle);

#endif /* VCHIQ_H */
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 dc33490ba7fb..fa92c34890e0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -151,7 +151,7 @@ static struct semaphore g_free_fragments_sema;

static DEFINE_SEMAPHORE(g_free_fragments_mutex);

-static enum vchiq_status
+static int
vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *data,
unsigned int size, enum vchiq_bulk_dir dir);

@@ -722,7 +722,7 @@ void free_bulk_waiter(struct vchiq_instance *instance)
}
}

-enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
+int vchiq_shutdown(struct vchiq_instance *instance)
{
enum vchiq_status status = VCHIQ_SUCCESS;
struct vchiq_state *state = instance->state;
@@ -749,7 +749,7 @@ static int vchiq_is_connected(struct vchiq_instance *instance)
return instance->connected;
}

-enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
+int vchiq_connect(struct vchiq_instance *instance)
{
enum vchiq_status status;
struct vchiq_state *state = instance->state;
@@ -773,7 +773,7 @@ enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
}
EXPORT_SYMBOL(vchiq_connect);

-static enum vchiq_status
+static int
vchiq_add_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
@@ -803,7 +803,7 @@ vchiq_add_service(struct vchiq_instance *instance,
return status;
}

-enum vchiq_status
+int
vchiq_open_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
@@ -835,7 +835,7 @@ vchiq_open_service(struct vchiq_instance *instance,
}
EXPORT_SYMBOL(vchiq_open_service);

-enum vchiq_status
+int
vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const void *data,
unsigned int size, void *userdata, enum vchiq_bulk_mode mode)
{
@@ -873,9 +873,9 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
}
EXPORT_SYMBOL(vchiq_bulk_transmit);

-enum vchiq_status vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
- void *data, unsigned int size, void *userdata,
- enum vchiq_bulk_mode mode)
+int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
+ void *data, unsigned int size, void *userdata,
+ enum vchiq_bulk_mode mode)
{
enum vchiq_status status;

@@ -910,7 +910,7 @@ enum vchiq_status vchiq_bulk_receive(struct vchiq_instance *instance, unsigned i
}
EXPORT_SYMBOL(vchiq_bulk_receive);

-static enum vchiq_status
+static int
vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *data,
unsigned int size, enum vchiq_bulk_dir dir)
{
@@ -983,7 +983,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
return status;
}

-static enum vchiq_status
+static int
add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
struct vchiq_header *header, struct user_service *user_service,
void *bulk_userdata)
@@ -1044,7 +1044,7 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
return VCHIQ_SUCCESS;
}

-enum vchiq_status
+int
service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
struct vchiq_header *header, unsigned int handle, void *bulk_userdata)
{
@@ -1314,7 +1314,7 @@ vchiq_get_state(void)
* Autosuspend related functionality
*/

-static enum vchiq_status
+static int
vchiq_keepalive_vchiq_callback(struct vchiq_instance *instance,
enum vchiq_reason reason,
struct vchiq_header *header,
@@ -1587,7 +1587,7 @@ vchiq_instance_set_trace(struct vchiq_instance *instance, int trace)
instance->trace = (trace != 0);
}

-enum vchiq_status
+int
vchiq_use_service(struct vchiq_instance *instance, unsigned int handle)
{
enum vchiq_status ret = VCHIQ_ERROR;
@@ -1601,7 +1601,7 @@ vchiq_use_service(struct vchiq_instance *instance, unsigned int handle)
}
EXPORT_SYMBOL(vchiq_use_service);

-enum vchiq_status
+int
vchiq_release_service(struct vchiq_instance *instance, unsigned int handle)
{
enum vchiq_status ret = VCHIQ_ERROR;
@@ -1695,7 +1695,7 @@ vchiq_dump_service_use_state(struct vchiq_state *state)
kfree(service_data);
}

-enum vchiq_status
+int
vchiq_check_service(struct vchiq_service *service)
{
struct vchiq_arm_state *arm_state;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 2851ef6b9cd0..2fb31f9b527f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -85,13 +85,13 @@ extern struct vchiq_state g_state;
extern struct vchiq_state *
vchiq_get_state(void);

-enum vchiq_status
+int
vchiq_use_service(struct vchiq_instance *instance, unsigned int handle);

-extern enum vchiq_status
+extern int
vchiq_release_service(struct vchiq_instance *instance, unsigned int handle);

-extern enum vchiq_status
+extern int
vchiq_check_service(struct vchiq_service *service);

extern void
@@ -100,10 +100,10 @@ vchiq_dump_platform_use_state(struct vchiq_state *state);
extern void
vchiq_dump_service_use_state(struct vchiq_state *state);

-extern enum vchiq_status
+extern int
vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
enum USE_TYPE_E use_type);
-extern enum vchiq_status
+extern int
vchiq_release_internal(struct vchiq_state *state,
struct vchiq_service *service);

@@ -137,7 +137,7 @@ static inline int vchiq_register_chrdev(struct device *parent) { return 0; }

#endif /* IS_ENABLED(CONFIG_VCHIQ_CDEV) */

-extern enum vchiq_status
+extern int
service_callback(struct vchiq_instance *vchiq_instance, enum vchiq_reason reason,
struct vchiq_header *header, unsigned int handle, void *bulk_userdata);

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 45ed30bfdbf5..a543c29b1598 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -463,7 +463,7 @@ mark_service_closing(struct vchiq_service *service)
mark_service_closing_internal(service, 0);
}

-static inline enum vchiq_status
+static inline int
make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
struct vchiq_header *header, void *bulk_userdata)
{
@@ -900,7 +900,7 @@ copy_message_data(ssize_t (*copy_callback)(void *context, void *dest, size_t off
}

/* Called by the slot handler and application threads */
-static enum vchiq_status
+static int
queue_message(struct vchiq_state *state, struct vchiq_service *service,
int msgid,
ssize_t (*copy_callback)(void *context, void *dest,
@@ -1139,7 +1139,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
}

/* Called by the slot handler and application threads */
-static enum vchiq_status
+static int
queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
int msgid,
ssize_t (*copy_callback)(void *context, void *dest,
@@ -1299,7 +1299,7 @@ get_bulk_reason(struct vchiq_bulk *bulk)
}

/* Called by the slot handler - don't hold the bulk mutex */
-static enum vchiq_status
+static int
notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
int retry_poll)
{
@@ -2486,7 +2486,7 @@ vchiq_add_service_internal(struct vchiq_state *state,
return service;
}

-enum vchiq_status
+int
vchiq_open_service_internal(struct vchiq_service *service, int client_id)
{
struct vchiq_open_payload payload = {
@@ -2609,7 +2609,7 @@ do_abort_bulks(struct vchiq_service *service)
return (status == VCHIQ_SUCCESS);
}

-static enum vchiq_status
+static int
close_service_complete(struct vchiq_service *service, int failstate)
{
enum vchiq_status status;
@@ -2674,7 +2674,7 @@ close_service_complete(struct vchiq_service *service, int failstate)
}

/* Called by the slot handler */
-enum vchiq_status
+int
vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
{
struct vchiq_state *state = service->state;
@@ -2832,7 +2832,7 @@ vchiq_free_service_internal(struct vchiq_service *service)
vchiq_service_put(service);
}

-enum vchiq_status
+int
vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instance)
{
struct vchiq_service *service;
@@ -2879,7 +2879,7 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
}
}

-enum vchiq_status
+int
vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
{
/* Unregister the service */
@@ -2937,7 +2937,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
}
EXPORT_SYMBOL(vchiq_close_service);

-enum vchiq_status
+int
vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
{
/* Unregister the service */
@@ -3004,9 +3004,9 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
* When called in blocking mode, the userdata field points to a bulk_waiter
* structure.
*/
-enum vchiq_status vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
- void *offset, void __user *uoffset, int size, void *userdata,
- enum vchiq_bulk_mode mode, enum vchiq_bulk_dir dir)
+int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
+ void *offset, void __user *uoffset, int size, void *userdata,
+ enum vchiq_bulk_mode mode, enum vchiq_bulk_dir dir)
{
struct vchiq_service *service = find_service_by_handle(instance, handle);
struct vchiq_bulk_queue *queue;
@@ -3161,7 +3161,7 @@ enum vchiq_status vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned
return status;
}

-enum vchiq_status
+int
vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
ssize_t (*copy_callback)(void *context, void *dest,
size_t offset, size_t maxsize),
@@ -3277,7 +3277,7 @@ release_message_sync(struct vchiq_state *state, struct vchiq_header *header)
remote_event_signal(&state->remote->sync_release);
}

-enum vchiq_status
+int
vchiq_get_peer_version(struct vchiq_instance *instance, unsigned int handle, short *peer_version)
{
enum vchiq_status status = VCHIQ_ERROR;
@@ -3640,7 +3640,7 @@ vchiq_loud_error_footer(void)
"============================================================================");
}

-enum vchiq_status vchiq_send_remote_use(struct vchiq_state *state)
+int vchiq_send_remote_use(struct vchiq_state *state)
{
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
return VCHIQ_RETRY;
@@ -3648,7 +3648,7 @@ enum vchiq_status vchiq_send_remote_use(struct vchiq_state *state)
return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0);
}

-enum vchiq_status vchiq_send_remote_use_active(struct vchiq_state *state)
+int vchiq_send_remote_use_active(struct vchiq_state *state)
{
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
return VCHIQ_RETRY;
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 8b4a38f5b3f2..ec3505424718 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -458,7 +458,7 @@ vchiq_init_slots(void *mem_base, int mem_size);
extern int
vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, struct device *dev);

-extern enum vchiq_status
+extern int
vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instance);

struct vchiq_service *
@@ -467,10 +467,10 @@ vchiq_add_service_internal(struct vchiq_state *state,
int srvstate, struct vchiq_instance *instance,
void (*userdata_term)(void *userdata));

-extern enum vchiq_status
+extern int
vchiq_open_service_internal(struct vchiq_service *service, int client_id);

-extern enum vchiq_status
+extern int
vchiq_close_service_internal(struct vchiq_service *service, int close_recvd);

extern void
@@ -485,7 +485,7 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
extern void
remote_event_pollall(struct vchiq_state *state);

-extern enum vchiq_status
+extern int
vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *offset,
void __user *uoffset, int size, void *userdata, enum vchiq_bulk_mode mode,
enum vchiq_bulk_dir dir);
@@ -536,7 +536,7 @@ vchiq_service_get(struct vchiq_service *service);
extern void
vchiq_service_put(struct vchiq_service *service);

-extern enum vchiq_status
+extern int
vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
ssize_t (*copy_callback)(void *context, void *dest,
size_t offset, size_t maxsize),
@@ -568,13 +568,13 @@ void vchiq_on_remote_release(struct vchiq_state *state);

int vchiq_platform_init_state(struct vchiq_state *state);

-enum vchiq_status vchiq_check_service(struct vchiq_service *service);
+int vchiq_check_service(struct vchiq_service *service);

void vchiq_on_remote_use_active(struct vchiq_state *state);

-enum vchiq_status vchiq_send_remote_use(struct vchiq_state *state);
+int vchiq_send_remote_use(struct vchiq_state *state);

-enum vchiq_status vchiq_send_remote_use_active(struct vchiq_state *state);
+int vchiq_send_remote_use_active(struct vchiq_state *state);

void vchiq_platform_conn_state_changed(struct vchiq_state *state,
enum vchiq_connstate oldstate,
@@ -584,7 +584,7 @@ void vchiq_set_conn_state(struct vchiq_state *state, enum vchiq_connstate newsta

void vchiq_log_dump_mem(const char *label, u32 addr, const void *void_mem, size_t num_bytes);

-enum vchiq_status vchiq_remove_service(struct vchiq_instance *instance, unsigned int service);
+int vchiq_remove_service(struct vchiq_instance *instance, unsigned int service);

int vchiq_get_client_id(struct vchiq_instance *instance, unsigned int service);

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
index 86d77f2eeea5..96f50beace44 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
@@ -12,10 +12,10 @@

struct vchiq_service_params {
int fourcc;
- enum vchiq_status __user (*callback)(enum vchiq_reason reason,
- struct vchiq_header *header,
- unsigned int handle,
- void *bulk_userdata);
+ int __user (*callback)(enum vchiq_reason reason,
+ struct vchiq_header *header,
+ unsigned int handle,
+ void *bulk_userdata);
void __user *userdata;
short version; /* Increment for non-trivial changes */
short version_min; /* Update for incompatible changes */
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index cb921c94996a..48a983f497f0 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -548,10 +548,9 @@ static void bulk_abort_cb(struct vchiq_mmal_instance *instance,
}

/* incoming event service callback */
-static enum vchiq_status service_callback(struct vchiq_instance *vchiq_instance,
- enum vchiq_reason reason,
- struct vchiq_header *header,
- unsigned int handle, void *bulk_ctx)
+static int service_callback(struct vchiq_instance *vchiq_instance,
+ enum vchiq_reason reason, struct vchiq_header *header,
+ unsigned int handle, void *bulk_ctx)
{
struct vchiq_mmal_instance *instance = vchiq_get_service_userdata(vchiq_instance, handle);
u32 msg_len;
--
2.38.1

2022-12-19 12:27:51

by Umang Jain

[permalink] [raw]
Subject: [PATCH v2 2/7] staging: vc04_services: Drop VCHIQ_SUCCESS usage

Drop the usage of VCHIQ_SUCCESS vchiq_status enum type. Replace it with
0 to report the success status.

This patch acts as intermediatory to address the TODO item:
* Get rid of custom function return values
for vc04_services/interface.

Signed-off-by: Umang Jain <[email protected]>
---
.../bcm2835-audio/bcm2835-vchiq.c | 4 +-
.../include/linux/raspberrypi/vchiq.h | 1 -
.../interface/vchiq_arm/vchiq_arm.c | 36 ++++++-------
.../interface/vchiq_arm/vchiq_core.c | 54 +++++++++----------
.../interface/vchiq_arm/vchiq_dev.c | 18 +++----
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 4 +-
6 files changed, 57 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 0fd92affc264..d74110ca17ab 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -101,7 +101,7 @@ static int audio_vchi_callback(struct vchiq_instance *vchiq_instance,
struct vc_audio_msg *m;

if (reason != VCHIQ_MESSAGE_AVAILABLE)
- return VCHIQ_SUCCESS;
+ return 0;

m = (void *)header->data;
if (m->type == VC_AUDIO_MSG_TYPE_RESULT) {
@@ -119,7 +119,7 @@ static int audio_vchi_callback(struct vchiq_instance *vchiq_instance,
}

vchiq_release_message(vchiq_instance, instance->service_handle, header);
- return VCHIQ_SUCCESS;
+ return 0;
}

static int
diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 71f4cb5d5cfd..52c513e5cf3b 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -19,7 +19,6 @@ enum vchiq_reason {

enum vchiq_status {
VCHIQ_ERROR = -1,
- VCHIQ_SUCCESS = 0,
VCHIQ_RETRY = 1
};

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 fa92c34890e0..6e4e17272dad 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -724,7 +724,7 @@ void free_bulk_waiter(struct vchiq_instance *instance)

int vchiq_shutdown(struct vchiq_instance *instance)
{
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
struct vchiq_state *state = instance->state;

if (mutex_lock_killable(&state->mutex))
@@ -761,7 +761,7 @@ int vchiq_connect(struct vchiq_instance *instance)
}
status = vchiq_connect_internal(state, instance);

- if (status == VCHIQ_SUCCESS)
+ if (!status)
instance->connected = 1;

mutex_unlock(&state->mutex);
@@ -793,7 +793,7 @@ vchiq_add_service(struct vchiq_instance *instance,

if (service) {
*phandle = service->handle;
- status = VCHIQ_SUCCESS;
+ status = 0;
} else {
status = VCHIQ_ERROR;
}
@@ -822,7 +822,7 @@ vchiq_open_service(struct vchiq_instance *instance,
if (service) {
*phandle = service->handle;
status = vchiq_open_service_internal(service, current->pid);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_remove_service(instance, service->handle);
*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
}
@@ -1004,7 +1004,7 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
return VCHIQ_RETRY;
} else if (instance->closing) {
vchiq_log_info(vchiq_arm_log_level, "service_callback closing");
- return VCHIQ_SUCCESS;
+ return 0;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
}
@@ -1041,7 +1041,7 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,

complete(&instance->insert_event);

- return VCHIQ_SUCCESS;
+ return 0;
}

int
@@ -1066,14 +1066,14 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
service = handle_to_service(instance, handle);
if (WARN_ON(!service)) {
rcu_read_unlock();
- return VCHIQ_SUCCESS;
+ return 0;
}

user_service = (struct user_service *)service->base.userdata;

if (!instance || instance->closing) {
rcu_read_unlock();
- return VCHIQ_SUCCESS;
+ return 0;
}

/*
@@ -1110,7 +1110,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
status = add_completion(instance, reason, NULL, user_service,
bulk_userdata);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
return status;
@@ -1158,7 +1158,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
vchiq_service_put(service);

if (skip_completion)
- return VCHIQ_SUCCESS;
+ return 0;

return add_completion(instance, reason, header, user_service,
bulk_userdata);
@@ -1350,14 +1350,14 @@ vchiq_keepalive_thread_func(void *v)
}

status = vchiq_connect(instance);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_log_error(vchiq_susp_log_level, "%s vchiq_connect failed %d", __func__,
status);
goto shutdown;
}

status = vchiq_add_service(instance, &params, &ka_handle);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_log_error(vchiq_susp_log_level, "%s vchiq_open_service failed %d", __func__,
status);
goto shutdown;
@@ -1386,14 +1386,14 @@ vchiq_keepalive_thread_func(void *v)
while (uc--) {
atomic_inc(&arm_state->ka_use_ack_count);
status = vchiq_use_service(instance, ka_handle);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_log_error(vchiq_susp_log_level,
"%s vchiq_use_service error %d", __func__, status);
}
}
while (rc--) {
status = vchiq_release_service(instance, ka_handle);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_log_error(vchiq_susp_log_level,
"%s vchiq_release_service error %d", __func__,
status);
@@ -1446,13 +1446,13 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
write_unlock_bh(&arm_state->susp_res_lock);

if (!ret) {
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
long ack_cnt = atomic_xchg(&arm_state->ka_use_ack_count, 0);

- while (ack_cnt && (status == VCHIQ_SUCCESS)) {
+ while (ack_cnt && !status) {
/* Send the use notify to videocore */
status = vchiq_send_remote_use_active(state);
- if (status == VCHIQ_SUCCESS)
+ if (!status)
ack_cnt--;
else
atomic_add(ack_cnt, &arm_state->ka_use_ack_count);
@@ -1708,7 +1708,7 @@ vchiq_check_service(struct vchiq_service *service)

read_lock_bh(&arm_state->susp_res_lock);
if (service->service_use_count)
- ret = VCHIQ_SUCCESS;
+ ret = 0;
read_unlock_bh(&arm_state->susp_res_lock);

if (ret == VCHIQ_ERROR) {
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 a543c29b1598..8b8f9e56d924 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -478,7 +478,7 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
vchiq_log_warning(vchiq_core_log_level,
"%d: ignoring ERROR from callback to service %x",
service->state->id, service->handle);
- status = VCHIQ_SUCCESS;
+ status = 0;
}

if (reason != VCHIQ_MESSAGE_AVAILABLE)
@@ -1135,7 +1135,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,

remote_event_signal(&state->remote->trigger);

- return VCHIQ_SUCCESS;
+ return 0;
}

/* Called by the slot handler and application threads */
@@ -1223,7 +1223,7 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
if (VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_PAUSE)
mutex_unlock(&state->sync_mutex);

- return VCHIQ_SUCCESS;
+ return 0;
}

static inline void
@@ -1303,7 +1303,7 @@ static int
notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
int retry_poll)
{
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;

vchiq_log_trace(vchiq_core_log_level, "%d: nb:%d %cx - p=%x rn=%x r=%x", service->state->id,
service->localport, (queue == &service->bulk_tx) ? 't' : 'r',
@@ -1357,7 +1357,7 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
complete(&service->bulk_remove_event);
}
if (!retry_poll)
- status = VCHIQ_SUCCESS;
+ status = 0;

if (status == VCHIQ_RETRY)
request_poll(service->state, service, (queue == &service->bulk_tx) ?
@@ -1398,13 +1398,12 @@ poll_services_of_group(struct vchiq_state *state, int group)
*/
service->public_fourcc = VCHIQ_FOURCC_INVALID;

- if (vchiq_close_service_internal(service, NO_CLOSE_RECVD) !=
- VCHIQ_SUCCESS)
+ if (vchiq_close_service_internal(service, NO_CLOSE_RECVD))
request_poll(state, service, VCHIQ_POLL_REMOVE);
} else if (service_flags & BIT(VCHIQ_POLL_TERMINATE)) {
vchiq_log_info(vchiq_core_log_level, "%d: ps - terminate %d<->%d",
state->id, service->localport, service->remoteport);
- if (vchiq_close_service_internal(service, NO_CLOSE_RECVD) != VCHIQ_SUCCESS)
+ if (vchiq_close_service_internal(service, NO_CLOSE_RECVD))
request_poll(state, service, VCHIQ_POLL_TERMINATE);
}
if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
@@ -2495,7 +2494,7 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
service->version,
service->version_min
};
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;

service->client_id = client_id;
vchiq_use_service_internal(service);
@@ -2506,7 +2505,7 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
sizeof(payload),
QMFLAGS_IS_BLOCKING);

- if (status != VCHIQ_SUCCESS)
+ if (status)
return status;

/* Wait for the ACK/NAK */
@@ -2602,11 +2601,11 @@ do_abort_bulks(struct vchiq_service *service)
mutex_unlock(&service->bulk_mutex);

status = notify_bulks(service, &service->bulk_tx, NO_RETRY_POLL);
- if (status != VCHIQ_SUCCESS)
+ if (status)
return 0;

status = notify_bulks(service, &service->bulk_rx, NO_RETRY_POLL);
- return (status == VCHIQ_SUCCESS);
+ return !status;
}

static int
@@ -2678,7 +2677,7 @@ int
vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
{
struct vchiq_state *state = service->state;
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
int is_server = (service->public_fourcc != VCHIQ_FOURCC_INVALID);
int close_id = MAKE_CLOSE(service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport));
@@ -2730,11 +2729,11 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)

release_service_messages(service);

- if (status == VCHIQ_SUCCESS)
+ if (!status)
status = queue_message(state, service, close_id, NULL,
NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK);

- if (status != VCHIQ_SUCCESS) {
+ if (status) {
if (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)
mutex_unlock(&state->sync_mutex);
break;
@@ -2768,7 +2767,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
break;
}

- if (status == VCHIQ_SUCCESS)
+ if (!status)
status = close_service_complete(service, VCHIQ_SRVSTATE_CLOSERECVD);
break;

@@ -2862,7 +2861,7 @@ vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instanc
complete(&state->connect);
}

- return VCHIQ_SUCCESS;
+ return 0;
}

void
@@ -2884,7 +2883,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
{
/* Unregister the service */
struct vchiq_service *service = find_service_by_handle(instance, handle);
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;

if (!service)
return VCHIQ_ERROR;
@@ -2926,7 +2925,7 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
srvstate_names[service->srvstate]);
}

- if ((status == VCHIQ_SUCCESS) &&
+ if (!status &&
(service->srvstate != VCHIQ_SRVSTATE_FREE) &&
(service->srvstate != VCHIQ_SRVSTATE_LISTENING))
status = VCHIQ_ERROR;
@@ -2942,7 +2941,7 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
{
/* Unregister the service */
struct vchiq_service *service = find_service_by_handle(instance, handle);
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;

if (!service)
return VCHIQ_ERROR;
@@ -2987,8 +2986,7 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
srvstate_names[service->srvstate]);
}

- if ((status == VCHIQ_SUCCESS) &&
- (service->srvstate != VCHIQ_SRVSTATE_FREE))
+ if (!status && (service->srvstate != VCHIQ_SRVSTATE_FREE))
status = VCHIQ_ERROR;

vchiq_service_put(service);
@@ -3028,7 +3026,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
if (!offset && !uoffset)
goto error_exit;

- if (vchiq_check_service(service) != VCHIQ_SUCCESS)
+ if (vchiq_check_service(service))
goto error_exit;

switch (mode) {
@@ -3121,7 +3119,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
QMFLAGS_IS_BLOCKING |
QMFLAGS_NO_MUTEX_LOCK |
QMFLAGS_NO_MUTEX_UNLOCK);
- if (status != VCHIQ_SUCCESS)
+ if (status)
goto unlock_both_error_exit;

queue->local_insert++;
@@ -3136,7 +3134,7 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
waiting:
vchiq_service_put(service);

- status = VCHIQ_SUCCESS;
+ status = 0;

if (bulk_waiter) {
bulk_waiter->bulk = bulk;
@@ -3175,7 +3173,7 @@ vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
if (!service)
goto error_exit;

- if (vchiq_check_service(service) != VCHIQ_SUCCESS)
+ if (vchiq_check_service(service))
goto error_exit;

if (!size) {
@@ -3286,14 +3284,14 @@ vchiq_get_peer_version(struct vchiq_instance *instance, unsigned int handle, sho
if (!service)
goto exit;

- if (vchiq_check_service(service) != VCHIQ_SUCCESS)
+ if (vchiq_check_service(service))
goto exit;

if (!peer_version)
goto exit;

*peer_version = service->peer_version;
- status = VCHIQ_SUCCESS;
+ status = 0;

exit:
if (service)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 7e297494437e..d9c4d550412e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -112,7 +112,7 @@ vchiq_ioc_queue_message(struct vchiq_instance *instance, unsigned int handle,
struct vchiq_element *elements, unsigned long count)
{
struct vchiq_io_copy_callback_context context;
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
unsigned long i;
size_t total_size = 0;

@@ -142,7 +142,7 @@ static int vchiq_ioc_create_service(struct vchiq_instance *instance,
{
struct user_service *user_service = NULL;
struct vchiq_service *service;
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
struct vchiq_service_params_kernel params;
int srvstate;

@@ -190,7 +190,7 @@ static int vchiq_ioc_create_service(struct vchiq_instance *instance,

if (args->is_open) {
status = vchiq_open_service_internal(service, instance->pid);
- if (status != VCHIQ_SUCCESS) {
+ if (status) {
vchiq_remove_service(instance, service->handle);
return (status == VCHIQ_RETRY) ?
-EINTR : -EIO;
@@ -577,7 +577,7 @@ static long
vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
struct vchiq_instance *instance = file->private_data;
- enum vchiq_status status = VCHIQ_SUCCESS;
+ int status = 0;
struct vchiq_service *service = NULL;
long ret = 0;
int i, rc;
@@ -598,12 +598,12 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
instance, &i))) {
status = vchiq_remove_service(instance, service->handle);
vchiq_service_put(service);
- if (status != VCHIQ_SUCCESS)
+ if (status)
break;
}
service = NULL;

- if (status == VCHIQ_SUCCESS) {
+ if (!status) {
/* Wake the completion thread and ask it to exit */
instance->closing = 1;
complete(&instance->insert_event);
@@ -627,7 +627,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
status = vchiq_connect_internal(instance->state, instance);
mutex_unlock(&instance->state->mutex);

- if (status == VCHIQ_SUCCESS)
+ if (!status)
instance->connected = 1;
else
vchiq_log_error(vchiq_arm_log_level,
@@ -675,7 +675,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
status = (cmd == VCHIQ_IOC_CLOSE_SERVICE) ?
vchiq_close_service(instance, service->handle) :
vchiq_remove_service(instance, service->handle);
- if (status != VCHIQ_SUCCESS)
+ if (status)
break;
}

@@ -868,7 +868,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
ret = -EINTR;
}

- if ((status == VCHIQ_SUCCESS) && (ret < 0) && (ret != -EINTR) && (ret != -EWOULDBLOCK))
+ if (!status && (ret < 0) && (ret != -EINTR) && (ret != -EWOULDBLOCK))
vchiq_log_info(vchiq_arm_log_level,
" ioctl instance %pK, cmd %s -> status %d, %ld",
instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index 48a983f497f0..cda46497977c 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -559,7 +559,7 @@ static int service_callback(struct vchiq_instance *vchiq_instance,

if (!instance) {
pr_err("Message callback passed NULL instance\n");
- return VCHIQ_SUCCESS;
+ return 0;
}

switch (reason) {
@@ -643,7 +643,7 @@ static int service_callback(struct vchiq_instance *vchiq_instance,
break;
}

- return VCHIQ_SUCCESS;
+ return 0;
}

static int send_synchronous_mmal_msg(struct vchiq_mmal_instance *instance,
--
2.38.1

2022-12-22 11:30:35

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] staging: vc04_services: Remove custom return values

Am 19.12.22 um 12:57 schrieb Umang Jain:
> The series removes the custom return values from functions
> and replaces them with linux error codes. This address the TODO
> vchiq interface:
> * Get rid of custom function return values
>
> Changes in V2:
> - Patch 3/7 now reports for specific errors like -ENOMEM, -EHOSTDOWN
> - Patch 5/7 reports -ENOTCONN instead of -EINVAL and adds a "Fixes" tag
>
> Umang Jain (7):
> staging: vc04_services: Replace vchiq_status return type to int
> staging: vc04_services: Drop VCHIQ_SUCCESS usage
> staging: vc04_services: Drop VCHIQ_ERROR usage
> staging: vc04_services: Drop VCHIQ_RETRY usage
> vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect
> staging: vc04_services: Drop enum vchiq_status remnants
> staging: vc04_services: vchiq: Drop custom return values from TODO

The whole series is:

Tested-by: Stefan Wahren <[email protected]>

2022-12-22 12:45:07

by Umang Jain

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect

Hi Stefan,

On 12/22/22 4:37 PM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 19.12.22 um 12:57 schrieb Umang Jain:
>> Drop the usage of VCHIQ_RETRY when the vchiq has connection status
>> VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
>> carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.
>>
>> This patch removes the usage of vCHIQ_RETRY completely and act as
>> intermediatory to address the TODO item:
>>     * Get rid of custom function return values
>> for vc04_services/interface.
>>
>> Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
> please drop this fixes tag since this commit doesn't fix a real issue
> and also shouldn't be applied to stable.

Should I send a v3 of the series with updated commit message or can you
drop the tag while applying?

Other option would be to send v2.1  --in-reply-to this patch. I am fine
with anything as long as it aligns with the merging workflow.

Thanks,
Umang
>> Signed-off-by: Umang Jain <[email protected]>
>> ---
>>   .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> index 9c64d5de810e..ddb6d0f4daed 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void)
>>   int vchiq_send_remote_use(struct vchiq_state *state)
>>   {
>>       if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
>> -        return VCHIQ_RETRY;
>> +        return -ENOTCONN;
>>         return queue_message(state, NULL, MAKE_REMOTE_USE, NULL,
>> NULL, 0, 0);
>>   }
>> @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state
>> *state)
>>   int vchiq_send_remote_use_active(struct vchiq_state *state)
>>   {
>>       if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
>> -        return VCHIQ_RETRY;
>> +        return -ENOTCONN;
>>         return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE,
>>                    NULL, NULL, 0, 0);

2022-12-22 12:53:32

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect

Hi Umang,

Am 19.12.22 um 12:57 schrieb Umang Jain:
> Drop the usage of VCHIQ_RETRY when the vchiq has connection status
> VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
> carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.
>
> This patch removes the usage of vCHIQ_RETRY completely and act as
> intermediatory to address the TODO item:
> * Get rid of custom function return values
> for vc04_services/interface.
>
> Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
please drop this fixes tag since this commit doesn't fix a real issue
and also shouldn't be applied to stable.
> Signed-off-by: Umang Jain <[email protected]>
> ---
> .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 9c64d5de810e..ddb6d0f4daed 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void)
> int vchiq_send_remote_use(struct vchiq_state *state)
> {
> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
> - return VCHIQ_RETRY;
> + return -ENOTCONN;
>
> return queue_message(state, NULL, MAKE_REMOTE_USE, NULL, NULL, 0, 0);
> }
> @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state *state)
> int vchiq_send_remote_use_active(struct vchiq_state *state)
> {
> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
> - return VCHIQ_RETRY;
> + return -ENOTCONN;
>
> return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE,
> NULL, NULL, 0, 0);

2022-12-22 13:19:39

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect

Hi Umang,

Am 22.12.22 um 13:04 schrieb Umang Jain:
> Hi Stefan,
>
> On 12/22/22 4:37 PM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 19.12.22 um 12:57 schrieb Umang Jain:
>>> Drop the usage of VCHIQ_RETRY when the vchiq has connection status
>>> VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
>>> carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.
>>>
>>> This patch removes the usage of vCHIQ_RETRY completely and act as
>>> intermediatory to address the TODO item:
>>>     * Get rid of custom function return values
>>> for vc04_services/interface.
>>>
>>> Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
>> please drop this fixes tag since this commit doesn't fix a real issue
>> and also shouldn't be applied to stable.
>
> Should I send a v3 of the series with updated commit message or can
> you drop the tag while applying?

Greg is the maintainer of staging, so i cannot decide. But i would
prefer a v3 with my tested tag added and this fixes tag dropped.

Best regards

>
> Other option would be to send v2.1  --in-reply-to this patch. I am
> fine with anything as long as it aligns with the merging workflow.
>
> Thanks,
> Umang
>>> Signed-off-by: Umang Jain <[email protected]>
>>> ---
>>>   .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>>> index 9c64d5de810e..ddb6d0f4daed 100644
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>>> @@ -3641,7 +3641,7 @@ vchiq_loud_error_footer(void)
>>>   int vchiq_send_remote_use(struct vchiq_state *state)
>>>   {
>>>       if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
>>> -        return VCHIQ_RETRY;
>>> +        return -ENOTCONN;
>>>         return queue_message(state, NULL, MAKE_REMOTE_USE, NULL,
>>> NULL, 0, 0);
>>>   }
>>> @@ -3649,7 +3649,7 @@ int vchiq_send_remote_use(struct vchiq_state
>>> *state)
>>>   int vchiq_send_remote_use_active(struct vchiq_state *state)
>>>   {
>>>       if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
>>> -        return VCHIQ_RETRY;
>>> +        return -ENOTCONN;
>>>         return queue_message(state, NULL, MAKE_REMOTE_USE_ACTIVE,
>>>                    NULL, NULL, 0, 0);
>

2022-12-23 15:33:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] vc04_services: vchiq_arm: Drop VCHIQ_RETRY usage on disconnect

On Thu, Dec 22, 2022 at 12:07:56PM +0100, Stefan Wahren wrote:
> Hi Umang,
>
> Am 19.12.22 um 12:57 schrieb Umang Jain:
> > Drop the usage of VCHIQ_RETRY when the vchiq has connection status
> > VCHIQ_CONNSTATE_DISCONNECTED. Disconnected status will not be valid to
> > carry on a retry, replace the VCHIQ_RETRY with -ENOTCONN.
> >
> > This patch removes the usage of vCHIQ_RETRY completely and act as
> > intermediatory to address the TODO item:
> > * Get rid of custom function return values
> > for vc04_services/interface.
> >
> > Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
> please drop this fixes tag since this commit doesn't fix a real issue and
> also shouldn't be applied to stable.

I asked Umang to add the Fixes tag based on the patch description and
based on that it seemed like a behavior change just from looking at
the patch.

But actually you are right that the fixes tag is not required.

The vchiq_send_remote_use() function is never called (dead code). The
vchiq_send_remote_use_active() function now returns a mix of custom
error codes and negative error codes. (Ugly). The caller only tests
for VCHIQ_SUCCESS so this code does not change behavior. I really feel
like the commit description needs to be clearer on this.

The fixes tag is not really about stable. Stable uses the tag, but the
fixes tag is appropriate whenever there is a bug fix.

regards,
dan carpenter