Re: [PATCH 1/7] staging: vchiq: Factor out bulk transfer for VCHIQ_BULK_MODE_WAITING

["Arnd Bergmann" <arnd@arndb.de>]

On Fri, Aug 23, 2024, at 15:14, Umang Jain wrote:
> The bulk transfer is VCHIQ_BULK_MODE_WAITING is used by VCHIQ ioctl
> interface. It is factored out to a separate function from
> vchiq_bulk_transfer() to bulk_xfer_waiting_interruptible().
>
> This is a part of vchiq_bulk_transfer refactoring. Each bulk mode
> will have their dedicated functions to execute bulk transfers.
> Each mode will be handled separately in subsequent patches.
>
> bulk_xfer_waiting_interruptible() is suffixed with "_interruptible"
> to denote that it can be interrupted when a signal is received.
> -EAGAIN maybe returned in those cases, similar to what
> vchiq_bulk_transfer() does.
>
> Adjust the vchiq_irq_queue_bulk_tx_rx() in the vchiq-dev.c to call
> bulk_xfer_waiting_interruptible() for waiting mode. A temporary
> goto label has been introduced to jump the call execution over
> vchiq_bulk_transfer() for waiting mode only. When all dedicated bulk
> transfer calls are introduced, this label shall be dropped.
>
> No function changes intended in this patch.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

This looks reasonable, just one think I would change:

> +int
> +bulk_xfer_waiting_interruptible(struct vchiq_instance *instance, 
> unsigned int handle,
> +				void *userdata)
> +{
> +	struct vchiq_service *service = find_service_by_handle(instance, 
> handle);
> +	struct bulk_waiter *bulk_waiter = NULL;

> +
> +	bulk_waiter = userdata;

Since you always pass a 'struct bulk_waiter' here, please
replace the 'void *userdata' with the typed argument directly.

       Arnd