Hi Laurent,
On Tue, Sep 17, 2024 at 05:57:35PM +0300, Laurent Pinchart wrote:
> 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@xxxxxxxxxxxxxxx>
> > >> ---
> > >> 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.
Sounds good. I'll address these in v3, after waiting a bit for further
comments.
>
> > >> + *
> > >> * @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,
>
--
Kind regards,
Sakari Ailus
