Hi Niklas, Thank you for the patch. On Wednesday 22 Jun 2016 02:19:24 Niklas Söderlund wrote: > This is done in preparation to move s_stream from v4l2_subdev_video_ops > to v4l2_subdev_pad_ops. Only wpf implements s_stream so it will no > longer be possible to share the v4l2_subdev_pad_ops once s_stream is > moved. > > Suggested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > --- > drivers/media/platform/vsp1/vsp1_rpf.c | 12 +++++++++- > drivers/media/platform/vsp1/vsp1_rwpf.c | 40 +++++++++++------------------ > drivers/media/platform/vsp1/vsp1_rwpf.h | 20 +++++++++++++++++ > drivers/media/platform/vsp1/vsp1_wpf.c | 12 +++++++++- > 4 files changed, 57 insertions(+), 27 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c > b/drivers/media/platform/vsp1/vsp1_rpf.c index 49168db..fabe8b2 100644 > --- a/drivers/media/platform/vsp1/vsp1_rpf.c > +++ b/drivers/media/platform/vsp1/vsp1_rpf.c > @@ -38,8 +38,18 @@ static inline void vsp1_rpf_write(struct vsp1_rwpf *rpf, > * V4L2 Subdevice Operations > */ > > +const struct v4l2_subdev_pad_ops vsp1_rpf_pad_ops = { This should be static const. > + .init_cfg = vsp1_entity_init_cfg, > + .enum_mbus_code = vsp1_rwpf_enum_mbus_code, > + .enum_frame_size = vsp1_rwpf_enum_frame_size, > + .get_fmt = vsp1_subdev_get_pad_format, > + .set_fmt = vsp1_rwpf_set_format, > + .get_selection = vsp1_rwpf_get_selection, > + .set_selection = vsp1_rwpf_set_selection, > +}; > + > static struct v4l2_subdev_ops rpf_ops = { > - .pad = &vsp1_rwpf_pad_ops, > + .pad = &vsp1_rpf_pad_ops, > }; > > /* > --------------------------------------------------------------------------- > -- diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c > b/drivers/media/platform/vsp1/vsp1_rwpf.c index 3b6e032..ff03b9c 100644 > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c > @@ -31,9 +31,9 @@ struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf > *rwpf, * V4L2 Subdevice Pad Operations > */ > > -static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_mbus_code_enum *code) > +int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_mbus_code_enum *code) > { > static const unsigned int codes[] = { > MEDIA_BUS_FMT_ARGB8888_1X32, > @@ -48,9 +48,9 @@ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev > *subdev, return 0; > } > > -static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_frame_size_enum *fse) > +int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_frame_size_enum *fse) > { > struct vsp1_rwpf *rwpf = to_rwpf(subdev); > > @@ -59,9 +59,9 @@ static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev > *subdev, rwpf->max_height); > } > > -static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_format *fmt) > +int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > { > struct vsp1_rwpf *rwpf = to_rwpf(subdev); > struct v4l2_subdev_pad_config *config; > @@ -113,9 +113,9 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev > *subdev, return 0; > } > > -static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_selection *sel) > +int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_selection *sel) > { > struct vsp1_rwpf *rwpf = to_rwpf(subdev); > struct v4l2_subdev_pad_config *config; > @@ -150,9 +150,9 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev > *subdev, return 0; > } > > -static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_selection *sel) > +int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_selection *sel) > { > struct vsp1_rwpf *rwpf = to_rwpf(subdev); > struct v4l2_subdev_pad_config *config; > @@ -209,16 +209,6 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev > *subdev, return 0; > } > > -const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = { You can also remove the declaration of the structure from vsp1_rwpf.h. > - .init_cfg = vsp1_entity_init_cfg, > - .enum_mbus_code = vsp1_rwpf_enum_mbus_code, > - .enum_frame_size = vsp1_rwpf_enum_frame_size, > - .get_fmt = vsp1_subdev_get_pad_format, > - .set_fmt = vsp1_rwpf_set_format, > - .get_selection = vsp1_rwpf_get_selection, > - .set_selection = vsp1_rwpf_set_selection, > -}; > - > /* > --------------------------------------------------------------------------- > -- * Controls > */ > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h > b/drivers/media/platform/vsp1/vsp1_rwpf.h index 9ff7c78..5ed4be5 100644 > --- a/drivers/media/platform/vsp1/vsp1_rwpf.h > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h > @@ -74,6 +74,26 @@ extern const struct v4l2_subdev_pad_ops > vsp1_rwpf_pad_ops; > > struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf, > struct v4l2_subdev_pad_config *config); > + > +int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_mbus_code_enum *code); > + > +int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_frame_size_enum *fse); > + > +int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt); > + > +int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_selection *sel); > + > +int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_selection *sel); You can remove the white lines between the functions as an indication they belong to the same group, and I'd add a white line here. > /** > * vsp1_rwpf_set_memory - Configure DMA addresses for a [RW]PF > * @rwpf: the [RW]PF instance > diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c > b/drivers/media/platform/vsp1/vsp1_wpf.c index 6c91eaa..75fe7de 100644 > --- a/drivers/media/platform/vsp1/vsp1_wpf.c > +++ b/drivers/media/platform/vsp1/vsp1_wpf.c > @@ -62,13 +62,23 @@ static int wpf_s_stream(struct v4l2_subdev *subdev, int > enable) * V4L2 Subdevice Operations > */ > > +const struct v4l2_subdev_pad_ops vsp1_wpf_pad_ops = { This should be static const. > + .init_cfg = vsp1_entity_init_cfg, > + .enum_mbus_code = vsp1_rwpf_enum_mbus_code, > + .enum_frame_size = vsp1_rwpf_enum_frame_size, > + .get_fmt = vsp1_subdev_get_pad_format, > + .set_fmt = vsp1_rwpf_set_format, > + .get_selection = vsp1_rwpf_get_selection, > + .set_selection = vsp1_rwpf_set_selection, > +}; > + > static struct v4l2_subdev_video_ops wpf_video_ops = { > .s_stream = wpf_s_stream, > }; > > static struct v4l2_subdev_ops wpf_ops = { > .video = &wpf_video_ops, > - .pad = &vsp1_rwpf_pad_ops, > + .pad = &vsp1_wpf_pad_ops, > }; > > /* > --------------------------------------------------------------------------- > -- -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html