Hi Andy On Tue, Jun 27, 2023 at 07:55:48PM +0300, Andy Shevchenko wrote: > On Tue, Jun 27, 2023 at 05:08:39PM +0200, Jacopo Mondi wrote: > > On Tue, Jun 27, 2023 at 03:18:08PM +0200, Hans de Goede wrote: > > ... > > > > mode = v4l2_find_nearest_size(ov2680_mode_data, > > > - ARRAY_SIZE(ov2680_mode_data), width, > > > - height, fmt->width, fmt->height); > > > + ARRAY_SIZE(ov2680_mode_data), > > > + width, height, > > > + format->format.width, > > > + format->format.height); > > > if (!mode) > > > return -EINVAL; > > > > Nit: only if you have to resend, could this be dropped? mode will be NULL > > only if ov2680_mode_data[] has no entries. > > We shouldn't rely on the implementation details of some API if it's not > advertised that way. Even if it is, the robustness of the code is better with > this check. I don't 100% agree here. the s_fmt subdev operation is not supposed to return -EINVAL if the requested configuration cannot be found. So when I saw -EINVAL here it throw me off a little, until I didn't look into v4l2_find_nearest_size() implementation and realized this can't actually happen. But as Hans said, this is going away in the next patches, something I didn't notice when I wrote this comment, so no need to bother :) > > -- > With Best Regards, > Andy Shevchenko > >
