Messages in this thread | | | Subject | Re: [PATCH] media: i2c: imx274: implement enum_mbus_code | From | Luca Ceresoli <> | Date | Thu, 18 Nov 2021 18:11:35 +0100 |
| |
Hi Eugen,
On 18/11/21 16:40, Eugen Hristev wrote: > Current driver supports only SRGGB 10 bit RAW bayer format. > Add the enum_mbus_code implementation to report this format supported. > > # v4l2-ctl -d /dev/v4l-subdev3 --list-subdev-mbus-codes > ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 > # > > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Generally OK, but I have a few minor comments.
> --- > drivers/media/i2c/imx274.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > index 2e804e3b70c4..25a4ef8f6187 100644 > --- a/drivers/media/i2c/imx274.c > +++ b/drivers/media/i2c/imx274.c > @@ -1909,7 +1909,21 @@ static int imx274_set_frame_interval(struct stimx274 *priv, > return err; > } > > +static int imx274_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + if (code->index > 0) > + return -EINVAL;
Many driver do check code->pad too, so you might want to do
if (code->pad > 0 || code->index > 0) return -EINVAL;
However I don't think it is strictly necessary, thus
> + > + /* only supported format in the driver is Raw 10 bits SRGGB */ > + code->code = MEDIA_BUS_FMT_SRGGB10_1X10;
Maybe better:
code->code = to_imx274(sd)->format.code
just as a little guard for future format changes.
With or without these I'm OK anyway with the patch, so:
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
-- Luca
| |