Hi Umang,
Thank you for the patch.
On Fri, Aug 23, 2024 at 08:44:24PM +0530, Umang Jain wrote:
> Factor out bulk transfer for VCHIQ_BULK_MODE_NOCALLBACK and
> VCHIQ_BULK_MODE_CALLBACK mode into a separate dedicated function
> bulk_xfer_callback_interruptible(). It is suffixed by "_interruptible"
> to denote that it can be interrupted and -EAGAIN can be returned. It
> would be up to the users of the function to retry the call in those cases.
>
> bulk_xfer_callback_interruptible() also takes in 'mode' parameter to
> differentiate between VCHIQ_BULK_MODE_NOCALLBACK and
> VCHIQ_BULK_MODE_CALLBACK, which then is directly passed to
> vchiq_bulk_xfer_queue_msg_interruptible() inside the function.
>
> Adjust the calls to vchiq-dev.c ioctl interface and vchiq_arm.c
> for the respective bulk transfers.
>
> Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 15 ++++----
> .../vc04_services/interface/vchiq_arm/vchiq_core.c | 40 ++++++++++++++++++++++
> .../vc04_services/interface/vchiq_arm/vchiq_core.h | 6 ++++
> .../vc04_services/interface/vchiq_arm/vchiq_dev.c | 6 ++++
> 4 files changed, 60 insertions(+), 7 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 e9b9c76381dc..5210ce8d269e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -857,10 +857,10 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
> switch (mode) {
> case VCHIQ_BULK_MODE_NOCALLBACK:
> case VCHIQ_BULK_MODE_CALLBACK:
> - ret = vchiq_bulk_transfer(instance, handle,
> - (void *)data, NULL,
> - size, userdata, mode,
> - VCHIQ_BULK_TRANSMIT);
> + ret = bulk_xfer_callback_interruptible(instance, handle,
> + (void *)data, NULL,
> + size, mode, userdata,
> + VCHIQ_BULK_TRANSMIT);
> break;
> case VCHIQ_BULK_MODE_BLOCKING:
> ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> @@ -895,9 +895,10 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
> switch (mode) {
> case VCHIQ_BULK_MODE_NOCALLBACK:
> case VCHIQ_BULK_MODE_CALLBACK:
> - ret = vchiq_bulk_transfer(instance, handle, data, NULL,
> - size, userdata,
> - mode, VCHIQ_BULK_RECEIVE);
> + ret = bulk_xfer_callback_interruptible(instance, handle,
> + (void *)data, NULL,
> + size, mode, userdata,
> + VCHIQ_BULK_RECEIVE);
> break;
> case VCHIQ_BULK_MODE_BLOCKING:
> ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> 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 341a06997848..0606561fd3d0 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3027,6 +3027,46 @@ bulk_xfer_blocking_interruptible(struct vchiq_instance *instance, unsigned int h
> return status;
> }
>
> +int
> +bulk_xfer_callback_interruptible(struct vchiq_instance *instance, unsigned int handle,
Please prefix the name of all functions with an appropriate driver
prefix, especially for functions that are not static.
> + void *offset, void __user *uoffset, int size,
> + enum vchiq_bulk_mode mode, void *userdata,
> + enum vchiq_bulk_dir dir)
> +{
> + struct vchiq_service *service = find_service_by_handle(instance, handle);
> + int status = -EINVAL;
> +
> + if (!service)
> + goto error_exit;
> +
> + if (mode != VCHIQ_BULK_MODE_CALLBACK &&
> + mode != VCHIQ_BULK_MODE_NOCALLBACK)
> + goto error_exit;
> +
> + if (service->srvstate != VCHIQ_SRVSTATE_OPEN)
> + goto error_exit;
> +
> + if (!offset && !uoffset)
> + goto error_exit;
> +
> + if (vchiq_check_service(service))
> + goto error_exit;
> +
> + status = vchiq_bulk_xfer_queue_msg_interruptible(service, offset, uoffset,
> + size, userdata, mode, dir);
> + if (status)
> + goto error_exit;
> +
> + vchiq_service_put(service);
> +
> + return 0;
> +
> +error_exit:
> + if (service)
> + vchiq_service_put(service);
> + return status;
> +}
> +
> /*
> * This function may be called by kernel threads or user threads.
> * User threads may receive -EAGAIN to indicate that a signal has been
> 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 ff3559c3d1ba..8aaf3c9d3dbe 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -479,6 +479,12 @@ bulk_xfer_blocking_interruptible(struct vchiq_instance *instance, unsigned int h
> void *offset, void __user *uoffset, int size,
> void __user *userdata, enum vchiq_bulk_dir dir);
>
> +extern int
> +bulk_xfer_callback_interruptible(struct vchiq_instance *instance, unsigned int handle,
> + void *offset, void __user *uoffset, int size,
> + enum vchiq_bulk_mode mode, void *userdata,
> + enum vchiq_bulk_dir dir);
> +
> 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,
> 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 1bff97ad28fa..7ecfcaa85569 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> @@ -336,6 +336,12 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
> goto bulk_transfer_handled;
> } else {
> userdata = args->userdata;
> +
> + status = bulk_xfer_callback_interruptible(instance, args->handle,
> + NULL, args->data, args->size,
> + args->mode, userdata, dir);
> +
> + goto bulk_transfer_handled;
> }
>
> status = vchiq_bulk_transfer(instance, args->handle, NULL, args->data, args->size,
>
--
Regards,
Laurent Pinchart
