| Message ID | 20230914181704.4811-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: Reduce cargo cult | expand |
Hi Laurent On Thu, Sep 14, 2023 at 8:16 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > The subdev .s_stream() operation must not be called to start an already > started subdev, or stop an already stopped one. This requirement has > never been formally documented. Fix it. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/media/v4l2-subdev.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index d9fca929c10b..282914ea9d45 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags { > * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the > * sub-device. Failure on stop will remove any resources acquired in > * streaming start, while the error code is still returned by the driver. > - * Also see call_s_stream wrapper in v4l2-subdev.c. > + * The caller shall track the subdev state, and refrain from starting or What about s/refrain/ shall not/ ? I refrain (and fail) from eating chocolate, but I shall not eat rat poison :P > + * stopping an already started or stopped subdev. AAlso see call_s_stream > + * wrapper in v4l2-subdev.c. s/AAlso/Also/ > * > * @g_pixelaspect: callback to return the pixelaspect ratio. > * > -- > Regards, > > Laurent Pinchart >
Hi Ricardo, On Mon, Sep 18, 2023 at 09:49:27AM +0200, Ricardo Ribalda Delgado wrote: > On Thu, Sep 14, 2023 at 8:16 PM Laurent Pinchart wrote: > > > > The subdev .s_stream() operation must not be called to start an already > > started subdev, or stop an already stopped one. This requirement has > > never been formally documented. Fix it. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/media/v4l2-subdev.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index d9fca929c10b..282914ea9d45 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags { > > * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the > > * sub-device. Failure on stop will remove any resources acquired in > > * streaming start, while the error code is still returned by the driver. > > - * Also see call_s_stream wrapper in v4l2-subdev.c. > > + * The caller shall track the subdev state, and refrain from starting or > > What about > > s/refrain/ shall not/ ? > > I refrain (and fail) from eating chocolate, but I shall not eat rat poison :P I know someone who would tell you chocolate is at least as bad as rat poison. That's a fairly extreme point of view, I agree with your suggestion. I'll update this in v2. > > + * stopping an already started or stopped subdev. AAlso see call_s_stream > > + * wrapper in v4l2-subdev.c. > > s/AAlso/Also/ > > > * > > * @g_pixelaspect: callback to return the pixelaspect ratio. > > *
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index d9fca929c10b..282914ea9d45 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags { * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the * sub-device. Failure on stop will remove any resources acquired in * streaming start, while the error code is still returned by the driver. - * Also see call_s_stream wrapper in v4l2-subdev.c. + * The caller shall track the subdev state, and refrain from starting or + * stopping an already started or stopped subdev. AAlso see call_s_stream + * wrapper in v4l2-subdev.c. * * @g_pixelaspect: callback to return the pixelaspect ratio. *
The subdev .s_stream() operation must not be called to start an already started subdev, or stop an already stopped one. This requirement has never been formally documented. Fix it. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/media/v4l2-subdev.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)