[PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback

Dan Carpenter dan.carpenter at linaro.org
Mon Jun 10 23:01:31 PDT 2024


On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> The service_callback has 5 levels of indentation, which makes it
> hard to read. Reduce this by splitting the code in a new function
> service_single_message() as suggested by Laurent Pinchart.
> 
> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
> ---
>  .../interface/vchiq_arm/vchiq_arm.c           | 68 +++++++++++--------
>  1 file changed, 39 insertions(+), 29 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 0ba52c9d8bc3..706bfc7a0b90 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1055,6 +1055,39 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
>  	return 0;
>  }
> 
> +static int
> +service_single_message(struct vchiq_instance *instance,
> +		       enum vchiq_reason reason,
> +		       struct vchiq_service *service, void *bulk_userdata)
> +{
> +	struct user_service *user_service;
> +
> +	user_service = (struct user_service *)service->base.userdata;
> +
> +	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) {
> +		dev_dbg(instance->state->dev,
> +			"arm: Inserting extra MESSAGE_AVAILABLE\n");
> +		return add_completion(instance, reason, NULL, user_service,
> +				      bulk_userdata);

In the original code we added a completion and

> +	}
> +
> +	if (wait_for_completion_interruptible(&user_service->remove_event)) {

then waited for it here...  Why did this change?

regards,
dan carpenter

> +		dev_dbg(instance->state->dev, "arm: interrupted\n");
> +		return -EAGAIN;
> +	} else if (instance->closing) {
> +		dev_dbg(instance->state->dev, "arm: closing\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  int
>  service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>  		 struct vchiq_header *header, unsigned int handle, void *bulk_userdata)
> @@ -1104,41 +1137,18 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>  		spin_lock(&service->state->msg_queue_spinlock);
>  		while (user_service->msg_insert ==
>  			(user_service->msg_remove + MSG_QUEUE_SIZE)) {
> +			int ret;
> +
>  			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 ret;
> -
> -				dev_dbg(instance->state->dev,
> -					"arm: Inserting extra MESSAGE_AVAILABLE\n");
> -				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -				ret = add_completion(instance, reason, NULL, user_service,
> -						     bulk_userdata);
> -				if (ret) {
> -					DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> -					vchiq_service_put(service);
> -					return ret;
> -				}
> -			}
> 
> -			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");
> +			ret = service_single_message(instance, reason,
> +						     service, bulk_userdata);
> +			if (ret) {
>  				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>  				vchiq_service_put(service);
> -				return -EINVAL;
> +				return ret;
>  			}
>  			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>  			spin_lock(&service->state->msg_queue_spinlock);
> --
> 2.34.1
> 



More information about the linux-arm-kernel mailing list