Message ID | 20241217215445.901459-7-sakari.ailus@linux.intel.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Use V4L2 mbus config for conveying link frequency | expand |
On 17/12/2024 23:54, Sakari Ailus wrote: > Document that the implementations of the V4L2 sub-device pad operation > get_mbus_config shall set all fields of the struct to defined values. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/media/v4l2-subdev.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 2f2200875b03..abcc879fabb2 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -822,7 +822,10 @@ struct v4l2_subdev_state { > * possible configuration from the remote end, likely calling > * this operation as close as possible to stream on time. The > * operation shall fail if the pad index it has been called on > - * is not valid or in case of unrecoverable failures. > + * is not valid or in case of unrecoverable failures. The > + * implementations of the get_mbus_config operation shall set > + * all the values of struct v4l2_mbus_config to defined > + * values. The drivers can't set all fields, as we can add new fields (like you do in this series). So I assume this means that the drivers should memset() the struct to 0, and only then set the values it wants? I think we should have a wrapper for get_mbus_config, which would clear the struct before calling the op. And the drivers should always use that wrapper. Tomi
On Fri, Dec 20, 2024 at 03:29:27PM +0200, Tomi Valkeinen wrote: > > On 17/12/2024 23:54, Sakari Ailus wrote: > > Document that the implementations of the V4L2 sub-device pad operation > > get_mbus_config shall set all fields of the struct to defined values. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > include/media/v4l2-subdev.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index 2f2200875b03..abcc879fabb2 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -822,7 +822,10 @@ struct v4l2_subdev_state { > > * possible configuration from the remote end, likely calling > > * this operation as close as possible to stream on time. The > > * operation shall fail if the pad index it has been called on > > - * is not valid or in case of unrecoverable failures. > > + * is not valid or in case of unrecoverable failures. The > > + * implementations of the get_mbus_config operation shall set > > + * all the values of struct v4l2_mbus_config to defined > > + * values. > > The drivers can't set all fields, as we can add new fields (like you do in > this series). So I assume this means that the drivers should memset() the > struct to 0, and only then set the values it wants? > > I think we should have a wrapper for get_mbus_config, which would clear the > struct before calling the op. And the drivers should always use that > wrapper. It seems we already have a wrapper actually, one that checks the pad argument is valid. I'll add the memset().
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 2f2200875b03..abcc879fabb2 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -822,7 +822,10 @@ struct v4l2_subdev_state { * possible configuration from the remote end, likely calling * this operation as close as possible to stream on time. The * operation shall fail if the pad index it has been called on - * is not valid or in case of unrecoverable failures. + * is not valid or in case of unrecoverable failures. The + * implementations of the get_mbus_config operation shall set + * all the values of struct v4l2_mbus_config to defined + * values. * * @set_routing: Enable or disable data connection routes described in the * subdevice routing table. Subdevs that implement this operation
Document that the implementations of the V4L2 sub-device pad operation get_mbus_config shall set all fields of the struct to defined values. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- include/media/v4l2-subdev.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)