Hi Mani, On 14/05/2020 10:59, Manivannan Sadhasivam wrote: > On Thu, May 14, 2020 at 10:53:00AM +0100, Kieran Bingham wrote: >> Hi Mani, >>>> +static int max9286_set_fmt(struct v4l2_subdev *sd, >>>> + struct v4l2_subdev_pad_config *cfg, >>>> + struct v4l2_subdev_format *format) >>>> +{ >>>> + struct max9286_priv *priv = sd_to_max9286(sd); >>>> + struct v4l2_mbus_framefmt *cfg_fmt; >>>> + >>>> + if (format->pad >= MAX9286_SRC_PAD) >>>> + return -EINVAL; >>>> + >>>> + /* Refuse non YUV422 formats as we hardcode DT to 8 bit YUV422 */ >>>> + switch (format->format.code) { >>>> + case MEDIA_BUS_FMT_UYVY8_2X8: >>>> + case MEDIA_BUS_FMT_VYUY8_2X8: >>>> + case MEDIA_BUS_FMT_YUYV8_2X8: >>>> + case MEDIA_BUS_FMT_YVYU8_2X8: >>>> + break; >>>> + default: >>>> + format->format.code = MEDIA_BUS_FMT_YUYV8_2X8; >>> >>> Is there any reason for not setting default format to MEDIA_BUS_FMT_UYVY8_2X8? >>> >> >> >> No good reason no, and I see that in max9286_enum_mbus_code(), we >> currently code to MEDIA_BUS_FMT_YUYV8_2X8, and that's the value that we >> init to, so that would be a better default indeed. >> > > max9286_enum_mbus_code() returns MEDIA_BUS_FMT_UYVY8_2X8, no? Argh, sorry - yet anther typo or copy/paste error. Indeed, I have changed this locally to ... <double checks> MEDIA_BUS_FMT_UYVY8_2X8 </double checks> as suggested. ;-) > > Thanks, > Mani