Re: [PATCH v2 10/17] media: atomisp: Convert to videobuf2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 20, 2022 at 09:55:26PM +0200, Hans de Goede wrote:
> Convert atomisp to use videobuf2.
> 
> This fixes mmap not working and in general moving over to
> the more modern videobuf2 is a good plan.

...

> -				/* The is the end, stop further loop */
> +			if (list_entry_is_head(param, &pipe->per_frame_params, list)) {
> +				/* The is the end, stop outer loop */
>  				break;
>  			}

You can drop {} by writing this as

			/* If this is the end, stop further loop */
			if (list_entry_is_head(param, &pipe->per_frame_params, list))
				break;

...

> +		if (!list_empty(&pipe->buffers_waiting_for_param))

Why not positive conditional?

> +			atomisp_handle_parameter_and_buffer(pipe);
> +		else
> +			atomisp_qbuffers_to_css(asd);

...

> +	/*
> +	 * Workaround: Due to the design of HALv3,
> +	 * sometimes in ZSL or SDV mode HAL needs to
> +	 * capture multiple images within one streaming cycle.
> +	 * But the capture number cannot be determined by HAL.
> +	 * So HAL only sets the capture number to be 1 and queue multiple
> +	 * buffers. Atomisp driver needs to check this case and re-trigger
> +	 * CSS to do capture when new buffer is queued.

Indentation of the above seems arbitrary.

> +	 */

...

> +	/*
> +	 * FIXME This if is copied from _vb2_fop_release, this cannot use that

 _vb2_fop_release() ?

> +	 * because that calls v4l2_fh_release() earlier then this function.
> +	 * Maybe we can release the fh earlier though, it does not look like
> +	 * anything needs it after this.
> +	 */

...

> +out:

In some cases you use 'unlock' in some 'out' for the same, I would suggest to
unify as

out_unlock:

in all affected locations.

> +	mutex_unlock(&isp->mutex);
> +	return ret;

-- 
With Best Regards,
Andy Shevchenko






[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux