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

[Stefan Wahren]

Hi Dan,

Am 11.06.24 um 08:01 schrieb Dan Carpenter:
> 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?
Oops, this wasn't intended. Thanks for catching this.
>
> 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
>>