| Message ID | 20230131192016.3476937-4-dave.stevenson@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | imx290: Minor fixes, support for alternate INCK, and more ctrls | expand |
Hi Dave, Thank you for the patch. On Tue, Jan 31, 2023 at 07:20:08PM +0000, Dave Stevenson wrote: > Any V4L2 subdevice that implements controls and declares > V4L2_SUBDEV_FL_HAS_DEVNODE should also declare V4L2_SUBDEV_FL_HAS_EVENTS > and implement subscribe_event and unsubscribe_event hooks. > > This driver didn't and would therefore fail v4l2-compliance > testing. > > Add the relevant hooks. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx290.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index bf96fd914303..12946ca9d8d2 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -20,6 +20,7 @@ > #include <media/media-entity.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > +#include <media/v4l2-event.h> > #include <media/v4l2-fwnode.h> > #include <media/v4l2-subdev.h> > > @@ -977,6 +978,11 @@ static int imx290_entity_init_cfg(struct v4l2_subdev *subdev, > return 0; > } > > +static const struct v4l2_subdev_core_ops imx290_core_ops = { > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > +}; > + > static const struct v4l2_subdev_video_ops imx290_video_ops = { > .s_stream = imx290_set_stream, > }; > @@ -991,6 +997,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = { > }; > > static const struct v4l2_subdev_ops imx290_subdev_ops = { > + .core = &imx290_core_ops, > .video = &imx290_video_ops, > .pad = &imx290_pad_ops, > }; > @@ -1009,7 +1016,8 @@ static int imx290_subdev_init(struct imx290 *imx290) > imx290->current_mode = &imx290_modes_ptr(imx290)[0]; > > v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops); > - imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | > + V4L2_SUBDEV_FL_HAS_EVENTS; > imx290->sd.dev = imx290->dev; > imx290->sd.entity.ops = &imx290_subdev_entity_ops; > imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
Hi Dave, thanks for the patch. Am Dienstag, 31. Januar 2023, 20:20:08 CET schrieb Dave Stevenson: > Any V4L2 subdevice that implements controls and declares > V4L2_SUBDEV_FL_HAS_DEVNODE should also declare V4L2_SUBDEV_FL_HAS_EVENTS > and implement subscribe_event and unsubscribe_event hooks. > > This driver didn't and would therefore fail v4l2-compliance > testing. > > Add the relevant hooks. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > drivers/media/i2c/imx290.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index bf96fd914303..12946ca9d8d2 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -20,6 +20,7 @@ > #include <media/media-entity.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > +#include <media/v4l2-event.h> > #include <media/v4l2-fwnode.h> > #include <media/v4l2-subdev.h> > > @@ -977,6 +978,11 @@ static int imx290_entity_init_cfg(struct v4l2_subdev > *subdev, return 0; > } > > +static const struct v4l2_subdev_core_ops imx290_core_ops = { > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > +}; > + > static const struct v4l2_subdev_video_ops imx290_video_ops = { > .s_stream = imx290_set_stream, > }; > @@ -991,6 +997,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = > { }; > > static const struct v4l2_subdev_ops imx290_subdev_ops = { > + .core = &imx290_core_ops, > .video = &imx290_video_ops, > .pad = &imx290_pad_ops, > }; > @@ -1009,7 +1016,8 @@ static int imx290_subdev_init(struct imx290 *imx290) > imx290->current_mode = &imx290_modes_ptr(imx290)[0]; > > v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops); > - imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | > + V4L2_SUBDEV_FL_HAS_EVENTS; > imx290->sd.dev = imx290->dev; > imx290->sd.entity.ops = &imx290_subdev_entity_ops; > imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index bf96fd914303..12946ca9d8d2 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -20,6 +20,7 @@ #include <media/media-entity.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> +#include <media/v4l2-event.h> #include <media/v4l2-fwnode.h> #include <media/v4l2-subdev.h> @@ -977,6 +978,11 @@ static int imx290_entity_init_cfg(struct v4l2_subdev *subdev, return 0; } +static const struct v4l2_subdev_core_ops imx290_core_ops = { + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, + .unsubscribe_event = v4l2_event_subdev_unsubscribe, +}; + static const struct v4l2_subdev_video_ops imx290_video_ops = { .s_stream = imx290_set_stream, }; @@ -991,6 +997,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = { }; static const struct v4l2_subdev_ops imx290_subdev_ops = { + .core = &imx290_core_ops, .video = &imx290_video_ops, .pad = &imx290_pad_ops, }; @@ -1009,7 +1016,8 @@ static int imx290_subdev_init(struct imx290 *imx290) imx290->current_mode = &imx290_modes_ptr(imx290)[0]; v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops); - imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | + V4L2_SUBDEV_FL_HAS_EVENTS; imx290->sd.dev = imx290->dev; imx290->sd.entity.ops = &imx290_subdev_entity_ops; imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
Any V4L2 subdevice that implements controls and declares V4L2_SUBDEV_FL_HAS_DEVNODE should also declare V4L2_SUBDEV_FL_HAS_EVENTS and implement subscribe_event and unsubscribe_event hooks. This driver didn't and would therefore fail v4l2-compliance testing. Add the relevant hooks. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- drivers/media/i2c/imx290.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)