diff mbox series

[07/11] staging: vchiq_arm: Reduce indentation of service_callback

Message ID 20240604172904.61613-8-wahrenst@gmx.net (mailing list archive)
State New, archived
Headers show
Series staging: vc04_services: Random cleanups | expand

Commit Message

Stefan Wahren June 4, 2024, 5:29 p.m. UTC 
The service_callback has 5 levels of indentation, which makes it
hard to read. Reduce this by using a goto for the corner cases
(no header or VCHI service).

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 111 +++++++++---------
 1 file changed, 57 insertions(+), 54 deletions(-)

--
2.34.1

Comments

Laurent Pinchart June 5, 2024, 7:07 a.m. UTC | #1 
Hi Stefan,

Thank you for the patch.

On Tue, Jun 04, 2024 at 07:29:00PM +0200, Stefan Wahren wrote:
> The service_callback has 5 levels of indentation, which makes it
> hard to read. Reduce this by using a goto for the corner cases
> (no header or VCHI service).

I think the goto isn't very nice. Could you instead try to split code to
a separate function ? That should do a better job at improve
readability.

> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>  .../interface/vchiq_arm/vchiq_arm.c           | 111 +++++++++---------
>  1 file changed, 57 insertions(+), 54 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 45acca670bbd..0055c7d7e617 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1101,71 +1101,74 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>  		user_service, service->localport, user_service->userdata,
>  		reason, header, instance, bulk_userdata);
> 
> -	if (header && user_service->is_vchi) {
> -		spin_lock(&service->state->msg_queue_spinlock);
> -		while (user_service->msg_insert ==
> -			(user_service->msg_remove + MSG_QUEUE_SIZE)) {
> -			spin_unlock(&service->state->msg_queue_spinlock);
> -			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -			DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
> -			dev_dbg(service->state->dev, "arm: msg queue full\n");
> -			/*
> -			 * If there is no MESSAGE_AVAILABLE in the completion
> -			 * queue, add one
> -			 */
> -			if ((user_service->message_available_pos -
> -				instance->completion_remove) < 0) {
> -				int status;
> -
> -				dev_dbg(instance->state->dev,
> -					"arm: Inserting extra MESSAGE_AVAILABLE\n");
> -				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -				status = add_completion(instance, reason, NULL, user_service,
> -							bulk_userdata);
> -				if (status) {
> -					DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -					vchiq_service_put(service);
> -					return status;
> -				}
> -			}
> +	if (!header || !user_service->is_vchi)
> +		goto service_put;
> +
> +	spin_lock(&service->state->msg_queue_spinlock);
> +	while (user_service->msg_insert ==
> +		(user_service->msg_remove + MSG_QUEUE_SIZE)) {
> +		spin_unlock(&service->state->msg_queue_spinlock);
> +		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> +		DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
> +		dev_dbg(service->state->dev, "arm: msg queue full\n");
> +		/*
> +		 * If there is no MESSAGE_AVAILABLE in the completion
> +		 * queue, add one
> +		 */
> +		if ((user_service->message_available_pos -
> +			instance->completion_remove) < 0) {
> +			int status;
> 
> +			dev_dbg(instance->state->dev,
> +				"arm: Inserting extra MESSAGE_AVAILABLE\n");
>  			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -			if (wait_for_completion_interruptible(&user_service->remove_event)) {
> -				dev_dbg(instance->state->dev, "arm: interrupted\n");
> -				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -				vchiq_service_put(service);
> -				return -EAGAIN;
> -			} else if (instance->closing) {
> -				dev_dbg(instance->state->dev, "arm: closing\n");
> +			status = add_completion(instance, reason, NULL, user_service,
> +						bulk_userdata);
> +			if (status) {
>  				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>  				vchiq_service_put(service);
> -				return -EINVAL;
> +				return status;
>  			}
> -			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -			spin_lock(&service->state->msg_queue_spinlock);
>  		}
> 
> -		user_service->msg_queue[user_service->msg_insert &
> -			(MSG_QUEUE_SIZE - 1)] = header;
> -		user_service->msg_insert++;
> -
> -		/*
> -		 * If there is a thread waiting in DEQUEUE_MESSAGE, or if
> -		 * there is a MESSAGE_AVAILABLE in the completion queue then
> -		 * bypass the completion queue.
> -		 */
> -		if (((user_service->message_available_pos -
> -			instance->completion_remove) >= 0) ||
> -			user_service->dequeue_pending) {
> -			user_service->dequeue_pending = 0;
> -			skip_completion = true;
> +		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> +		if (wait_for_completion_interruptible(&user_service->remove_event)) {
> +			dev_dbg(instance->state->dev, "arm: interrupted\n");
> +			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> +			vchiq_service_put(service);
> +			return -EAGAIN;
> +		} else if (instance->closing) {
> +			dev_dbg(instance->state->dev, "arm: closing\n");
> +			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> +			vchiq_service_put(service);
> +			return -EINVAL;
>  		}
> +		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> +		spin_lock(&service->state->msg_queue_spinlock);
> +	}
> 
> -		spin_unlock(&service->state->msg_queue_spinlock);
> -		complete(&user_service->insert_event);
> +	user_service->msg_queue[user_service->msg_insert &
> +		(MSG_QUEUE_SIZE - 1)] = header;
> +	user_service->msg_insert++;
> 
> -		header = NULL;
> +	/*
> +	 * If there is a thread waiting in DEQUEUE_MESSAGE, or if
> +	 * there is a MESSAGE_AVAILABLE in the completion queue then
> +	 * bypass the completion queue.
> +	 */
> +	if (((user_service->message_available_pos -
> +		instance->completion_remove) >= 0) ||
> +		user_service->dequeue_pending) {
> +		user_service->dequeue_pending = 0;
> +		skip_completion = true;
>  	}
> +
> +	spin_unlock(&service->state->msg_queue_spinlock);
> +	complete(&user_service->insert_event);
> +
> +	header = NULL;
> +
> +service_put:
>  	DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>  	vchiq_service_put(service);
>
diff mbox series

Patch

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 45acca670bbd..0055c7d7e617 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1101,71 +1101,74 @@  service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
 		user_service, service->localport, user_service->userdata,
 		reason, header, instance, bulk_userdata);

-	if (header && user_service->is_vchi) {
-		spin_lock(&service->state->msg_queue_spinlock);
-		while (user_service->msg_insert ==
-			(user_service->msg_remove + MSG_QUEUE_SIZE)) {
-			spin_unlock(&service->state->msg_queue_spinlock);
-			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-			DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
-			dev_dbg(service->state->dev, "arm: msg queue full\n");
-			/*
-			 * If there is no MESSAGE_AVAILABLE in the completion
-			 * queue, add one
-			 */
-			if ((user_service->message_available_pos -
-				instance->completion_remove) < 0) {
-				int status;
-
-				dev_dbg(instance->state->dev,
-					"arm: Inserting extra MESSAGE_AVAILABLE\n");
-				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-				status = add_completion(instance, reason, NULL, user_service,
-							bulk_userdata);
-				if (status) {
-					DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-					vchiq_service_put(service);
-					return status;
-				}
-			}
+	if (!header || !user_service->is_vchi)
+		goto service_put;
+
+	spin_lock(&service->state->msg_queue_spinlock);
+	while (user_service->msg_insert ==
+		(user_service->msg_remove + MSG_QUEUE_SIZE)) {
+		spin_unlock(&service->state->msg_queue_spinlock);
+		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
+		DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
+		dev_dbg(service->state->dev, "arm: msg queue full\n");
+		/*
+		 * If there is no MESSAGE_AVAILABLE in the completion
+		 * queue, add one
+		 */
+		if ((user_service->message_available_pos -
+			instance->completion_remove) < 0) {
+			int status;

+			dev_dbg(instance->state->dev,
+				"arm: Inserting extra MESSAGE_AVAILABLE\n");
 			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-			if (wait_for_completion_interruptible(&user_service->remove_event)) {
-				dev_dbg(instance->state->dev, "arm: interrupted\n");
-				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-				vchiq_service_put(service);
-				return -EAGAIN;
-			} else if (instance->closing) {
-				dev_dbg(instance->state->dev, "arm: closing\n");
+			status = add_completion(instance, reason, NULL, user_service,
+						bulk_userdata);
+			if (status) {
 				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 				vchiq_service_put(service);
-				return -EINVAL;
+				return status;
 			}
-			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-			spin_lock(&service->state->msg_queue_spinlock);
 		}

-		user_service->msg_queue[user_service->msg_insert &
-			(MSG_QUEUE_SIZE - 1)] = header;
-		user_service->msg_insert++;
-
-		/*
-		 * If there is a thread waiting in DEQUEUE_MESSAGE, or if
-		 * there is a MESSAGE_AVAILABLE in the completion queue then
-		 * bypass the completion queue.
-		 */
-		if (((user_service->message_available_pos -
-			instance->completion_remove) >= 0) ||
-			user_service->dequeue_pending) {
-			user_service->dequeue_pending = 0;
-			skip_completion = true;
+		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
+		if (wait_for_completion_interruptible(&user_service->remove_event)) {
+			dev_dbg(instance->state->dev, "arm: interrupted\n");
+			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
+			vchiq_service_put(service);
+			return -EAGAIN;
+		} else if (instance->closing) {
+			dev_dbg(instance->state->dev, "arm: closing\n");
+			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
+			vchiq_service_put(service);
+			return -EINVAL;
 		}
+		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
+		spin_lock(&service->state->msg_queue_spinlock);
+	}

-		spin_unlock(&service->state->msg_queue_spinlock);
-		complete(&user_service->insert_event);
+	user_service->msg_queue[user_service->msg_insert &
+		(MSG_QUEUE_SIZE - 1)] = header;
+	user_service->msg_insert++;

-		header = NULL;
+	/*
+	 * If there is a thread waiting in DEQUEUE_MESSAGE, or if
+	 * there is a MESSAGE_AVAILABLE in the completion queue then
+	 * bypass the completion queue.
+	 */
+	if (((user_service->message_available_pos -
+		instance->completion_remove) >= 0) ||
+		user_service->dequeue_pending) {
+		user_service->dequeue_pending = 0;
+		skip_completion = true;
 	}
+
+	spin_unlock(&service->state->msg_queue_spinlock);
+	complete(&user_service->insert_event);
+
+	header = NULL;
+
+service_put:
 	DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 	vchiq_service_put(service);