Re: [PATCH v3 2/5] media: atomisp: queue: move remote queue handling into helpers

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:03:37PM +0000, Rareș-Mihai Vladu wrote:
> Move the remote queue branch of every queue operation into its own
> static helper, written the way the function is meant to look once the
> unused local queue support is gone, and call it from the existing
> function after the local branch.
> 
> This prepares for removing the local queue support in the next patch
> and keeps that change easy to review. No functional change intended.

...

> +static int ia_css_queue_dequeue_remote(ia_css_queue_t *qhandle, uint32_t *item)
> +{
> +	ia_css_circbuf_desc_t cb_desc;
> +	ia_css_circbuf_elem_t cb_elem;
> +	u32 ignore_desc_flags = QUEUE_IGNORE_STEP_FLAG;
> +	int error;

> +	if (!qhandle || NULL == item)

While at it, do some small improvements on the moved lines, like

	if (!qhandle || !item)

> +		return -EINVAL;

> +	/* a. Load the queue from remote */
> +	QUEUE_CB_DESC_INIT(&cb_desc);
> +
> +	error = ia_css_queue_load(qhandle, &cb_desc, ignore_desc_flags);
> +	if (error != 0)

Drop all these ' != 0' parts where it appropriate.

> +		return error;
> +
> +	/* b. Operate on the queue */
> +	if (ia_css_circbuf_desc_is_empty(&cb_desc))
> +		return -ENODATA;
> +
> +	error = ia_css_queue_item_load(qhandle, cb_desc.start, &cb_elem);
> +	if (error != 0)
> +		return error;
> +
> +	*item = cb_elem.val;
> +
> +	cb_desc.start = (cb_desc.start + 1) % cb_desc.size;
> +
> +	/* c. Store the queue object */
> +	/* Set only fields requiring update with
> +	 * valid value. Avoids unnecessary calls
> +	 * to load/store functions
> +	 */
> +	ignore_desc_flags = QUEUE_IGNORE_SIZE_END_STEP_FLAGS;

> +	error = ia_css_queue_store(qhandle, &cb_desc, ignore_desc_flags);
> +	if (error != 0)
> +		return error;
> +
>  	return 0;

So it's simply

	return ia_css_queue_store(qhandle, &cb_desc, ignore_desc_flags);

>  }

...

> +static int ia_css_queue_is_full_remote(ia_css_queue_t *qhandle, bool *is_full)
> +{
> +	ia_css_circbuf_desc_t cb_desc;
> +	u32 ignore_desc_flags = QUEUE_IGNORE_STEP_FLAG;
> +	int error;
> +
> +	if ((!qhandle) || (!is_full))

Drop useless parentheses.

> +		return -EINVAL;
> +
> +	/* a. Load the queue from remote */
> +	QUEUE_CB_DESC_INIT(&cb_desc);
> +	error = ia_css_queue_load(qhandle, &cb_desc, ignore_desc_flags);
> +	if (error != 0)
> +		return error;
> +
> +	/* b. Operate on the queue */
> +	*is_full = ia_css_circbuf_desc_is_full(&cb_desc);
> +
>  	return 0;
>  }

...and so on...

With all above being addressed in the whole patch feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>

...

>  	if ((!qhandle) || (!is_full))
>  		return -EINVAL;

(But no need to touch the lines that are going to be removed!)

-- 
With Best Regards,
Andy Shevchenko