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@xxxxxxx>
> ---
> .../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
>
