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
