Re: [PATCH 2/2] media: Documentation: Update {enable,disable}_streams documentation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Sep 17, 2024 at 05:16:25PM +0300, Tomi Valkeinen wrote:
> On 17/09/2024 16:00, Laurent Pinchart wrote:
> > On Tue, Sep 17, 2024 at 03:43:45PM +0300, Sakari Ailus wrote:
> >> Document the expected {enable,disable}_streams callback behaviour for
> >> drivers that are stream-unaware i.e. don't specify the
> >> V4L2_SUBDEV_CAP_STREAMS sub-device capability flat. In this specific case,
> >> the mask argument can be ignored.
> > 
> > Wouldn't it be better to use BIT(0) in that case to simplifiy
> > interoperability with stream-aware devices ?
> 
> The caller has to set BIT(0), but I think here the documentation is 
> about the callee.
> 
> If the driver is not stream aware and implements the callbacks, it will 
> get BIT(0) as the mask parameter (do we enforce this?), but as there's 
> nothing it can do with the parameter it "does not need to be concerned 
> with the mask argument".

Right. I had misunderstood the patch.

> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> ---
> >>   include/media/v4l2-subdev.h | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> >> index 3cc6b4a5935f..67a6e6ec58b8 100644
> >> --- a/include/media/v4l2-subdev.h
> >> +++ b/include/media/v4l2-subdev.h
> >> @@ -834,11 +834,19 @@ struct v4l2_subdev_state {
> >>    *	v4l2_subdev_init_finalize() at initialization time). Do not call
> >>    *	directly, use v4l2_subdev_enable_streams() instead.
> >>    *
> >> + *	Drivers that support only a single stream without setting the
> >> + *	V4L2_SUBDEV_CAP_STREAMS sub-device capatility flag do not need to

s/capatility/capability/

Same below.

> >> + *	be concerned with the mask argument.

How about "can ignore the mask argument" instead ? I interpreted as "not
need to be concerned with" from the point of view of the caller.

> >> + *
> >>    * @disable_streams: Disable the streams defined in streams_mask on the given
> >>    *	source pad. Subdevs that implement this operation must use the active
> >>    *	state management provided by the subdev core (enabled through a call to
> >>    *	v4l2_subdev_init_finalize() at initialization time). Do not call
> >>    *	directly, use v4l2_subdev_disable_streams() instead.
> >> + *
> >> + *	Drivers that support only a single stream without setting the
> >> + *	V4L2_SUBDEV_CAP_STREAMS sub-device capatility flag do not need to
> >> + *	be concerned with the mask argument.
> >>    */
> >>   struct v4l2_subdev_pad_ops {
> >>   	int (*enum_mbus_code)(struct v4l2_subdev *sd,

-- 
Regards,

Laurent Pinchart