| Message ID | 20240506175255.24394-2-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: v4l2-subdev: Support const-awareness in state accessors | expand |
Hi Laurent, Thanks for the patch. On Mon, May 06, 2024 at 08:52:54PM +0300, Laurent Pinchart wrote: > It would be useful to mark instances of v4l2_subdev_state structures as > const when code needs to access them read-only. This isn't currently > possible, as the v4l2_subdev_state_get_*() accessor functions take a > non-const pointer to the state. > > Use _Generic() to provide two different versions of the accessors, for > const and non-const states respectively. The former returns a const > pointer to the requested format, rectangle or interval, implementing > const-correctness. The latter returns a non-const pointer, preserving > the current behaviour for drivers. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > Bike-shedding about names and the 4 leading underscores is allowed (and > expected). > --- > drivers/media/v4l2-core/v4l2-subdev.c | 24 +++++----- > include/media/v4l2-subdev.h | 66 ++++++++++++++++++--------- > 2 files changed, 57 insertions(+), 33 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 83c6d1e36871..af1fbc28c8f0 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1616,8 +1616,8 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd) > EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); > > struct v4l2_mbus_framefmt * > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1647,11 +1647,11 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format); > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_format); > > struct v4l2_rect * > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > - u32 stream) > +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1681,11 +1681,11 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > return NULL; > } > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop); > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_crop); > > struct v4l2_rect * > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1715,11 +1715,11 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose); > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_compose); > > struct v4l2_fract * > -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream) > +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream) > { > struct v4l2_subdev_stream_configs *stream_configs; > unsigned int i; > @@ -1751,7 +1751,7 @@ __v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > return NULL; > } > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_interval); > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_interval); > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index e30c463d90e5..ba722ab501c1 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1340,21 +1340,27 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd); > */ > /* > * Wrap v4l2_subdev_state_get_format(), allowing the function to be called with > - * two or three arguments. The purpose of the __v4l2_subdev_state_get_format() > + * two or three arguments. The purpose of the __v4l2_subdev_state_gen_call() > * macro below is to come up with the name of the function or macro to call, > * using the last two arguments (_stream and _pad). The selected function or > * macro is then called using the arguments specified by the caller. A similar > * arrangement is used for v4l2_subdev_state_crop() and > * v4l2_subdev_state_compose() below. > */ > -#define v4l2_subdev_state_get_format(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > +#define v4l2_subdev_state_get_format(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_format_pad(state, pad) \ > +#define __v4l2_subdev_state_get_format_pad(state, pad) \ > __v4l2_subdev_state_get_format(state, pad, 0) > +#define __v4l2_subdev_state_get_format(state, pad, stream) \ > + _Generic(state, \ > + const struct v4l2_subdev_state *: ((const struct v4l2_mbus_framefmt *) \ > + ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream)), \ > + default: ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream) \ > + ) > struct v4l2_mbus_framefmt * > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream); > > /** > * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle > @@ -1368,14 +1374,20 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > * For stream-unaware drivers the crop rectangle for the corresponding pad is > * returned. If the pad does not exist, NULL is returned. > */ > -#define v4l2_subdev_state_get_crop(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > +#define v4l2_subdev_state_get_crop(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > +#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > __v4l2_subdev_state_get_crop(state, pad, 0) > +#define __v4l2_subdev_state_get_crop(state, pad, stream) \ > + _Generic(state, \ > + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ > + ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream)), \ > + default: ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream) \ > + ) > struct v4l2_rect * > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > - u32 stream); > +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > + u32 stream); > > /** > * v4l2_subdev_state_get_compose() - Get pointer to a stream compose rectangle > @@ -1389,14 +1401,20 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > * For stream-unaware drivers the compose rectangle for the corresponding pad is > * returned. If the pad does not exist, NULL is returned. > */ > -#define v4l2_subdev_state_get_compose(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > +#define v4l2_subdev_state_get_compose(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > +#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > __v4l2_subdev_state_get_compose(state, pad, 0) > +#define __v4l2_subdev_state_get_compose(state, pad, stream) \ > + _Generic(state, \ > + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ > + ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream)), \ > + default: ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream) \ > + ) > struct v4l2_rect * > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream); > > /** > * v4l2_subdev_state_get_interval() - Get pointer to a stream frame interval > @@ -1410,14 +1428,20 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > * For stream-unaware drivers the frame interval for the corresponding pad is > * returned. If the pad does not exist, NULL is returned. > */ > -#define v4l2_subdev_state_get_interval(state, pad, ...) \ > - __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ > +#define v4l2_subdev_state_get_interval(state, pad, ...) \ > + __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ > (state, pad, ##__VA_ARGS__) > -#define __v4l2_subdev_state_get_interval_pad(state, pad) \ > +#define __v4l2_subdev_state_get_interval_pad(state, pad) \ > __v4l2_subdev_state_get_interval(state, pad, 0) > +#define __v4l2_subdev_state_get_interval(state, pad, stream) \ > + _Generic(state, \ > + const struct v4l2_subdev_state *: ((const struct v4l2_frac *) \ > + ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream)), \ > + default: ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream) \ Is the cast necessary in the latter case? It would seem like we lose most of the type safety checks as a result. Could this be implemented using a similar arrangement than the gen_call macro above, so that we wouldn't have three very similar sets of macros here? > + ) > struct v4l2_fract * > -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > - unsigned int pad, u32 stream); > +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > + unsigned int pad, u32 stream); > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) >
On Mon, May 06, 2024 at 09:53:18PM +0000, Sakari Ailus wrote: > Hi Laurent, > > Thanks for the patch. > > On Mon, May 06, 2024 at 08:52:54PM +0300, Laurent Pinchart wrote: > > It would be useful to mark instances of v4l2_subdev_state structures as > > const when code needs to access them read-only. This isn't currently > > possible, as the v4l2_subdev_state_get_*() accessor functions take a > > non-const pointer to the state. > > > > Use _Generic() to provide two different versions of the accessors, for > > const and non-const states respectively. The former returns a const > > pointer to the requested format, rectangle or interval, implementing > > const-correctness. The latter returns a non-const pointer, preserving > > the current behaviour for drivers. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > Bike-shedding about names and the 4 leading underscores is allowed (and > > expected). > > --- > > drivers/media/v4l2-core/v4l2-subdev.c | 24 +++++----- > > include/media/v4l2-subdev.h | 66 ++++++++++++++++++--------- > > 2 files changed, 57 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > index 83c6d1e36871..af1fbc28c8f0 100644 > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > @@ -1616,8 +1616,8 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd) > > EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); > > > > struct v4l2_mbus_framefmt * > > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream) > > +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream) > > { > > struct v4l2_subdev_stream_configs *stream_configs; > > unsigned int i; > > @@ -1647,11 +1647,11 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > > > return NULL; > > } > > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format); > > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_format); > > > > struct v4l2_rect * > > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > - u32 stream) > > +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > + u32 stream) > > { > > struct v4l2_subdev_stream_configs *stream_configs; > > unsigned int i; > > @@ -1681,11 +1681,11 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > > > return NULL; > > } > > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop); > > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_crop); > > > > struct v4l2_rect * > > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream) > > +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream) > > { > > struct v4l2_subdev_stream_configs *stream_configs; > > unsigned int i; > > @@ -1715,11 +1715,11 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > > > return NULL; > > } > > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose); > > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_compose); > > > > struct v4l2_fract * > > -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream) > > +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream) > > { > > struct v4l2_subdev_stream_configs *stream_configs; > > unsigned int i; > > @@ -1751,7 +1751,7 @@ __v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > > > return NULL; > > } > > -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_interval); > > +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_interval); > > > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index e30c463d90e5..ba722ab501c1 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -1340,21 +1340,27 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd); > > */ > > /* > > * Wrap v4l2_subdev_state_get_format(), allowing the function to be called with > > - * two or three arguments. The purpose of the __v4l2_subdev_state_get_format() > > + * two or three arguments. The purpose of the __v4l2_subdev_state_gen_call() > > * macro below is to come up with the name of the function or macro to call, > > * using the last two arguments (_stream and _pad). The selected function or > > * macro is then called using the arguments specified by the caller. A similar > > * arrangement is used for v4l2_subdev_state_crop() and > > * v4l2_subdev_state_compose() below. > > */ > > -#define v4l2_subdev_state_get_format(state, pad, ...) \ > > - __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > > +#define v4l2_subdev_state_get_format(state, pad, ...) \ > > + __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ > > (state, pad, ##__VA_ARGS__) > > -#define __v4l2_subdev_state_get_format_pad(state, pad) \ > > +#define __v4l2_subdev_state_get_format_pad(state, pad) \ > > __v4l2_subdev_state_get_format(state, pad, 0) > > +#define __v4l2_subdev_state_get_format(state, pad, stream) \ > > + _Generic(state, \ > > + const struct v4l2_subdev_state *: ((const struct v4l2_mbus_framefmt *) \ > > + ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream)), \ > > + default: ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream) \ > > + ) > > struct v4l2_mbus_framefmt * > > -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream); > > +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream); > > > > /** > > * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle > > @@ -1368,14 +1374,20 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, > > * For stream-unaware drivers the crop rectangle for the corresponding pad is > > * returned. If the pad does not exist, NULL is returned. > > */ > > -#define v4l2_subdev_state_get_crop(state, pad, ...) \ > > - __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > > +#define v4l2_subdev_state_get_crop(state, pad, ...) \ > > + __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ > > (state, pad, ##__VA_ARGS__) > > -#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > > +#define __v4l2_subdev_state_get_crop_pad(state, pad) \ > > __v4l2_subdev_state_get_crop(state, pad, 0) > > +#define __v4l2_subdev_state_get_crop(state, pad, stream) \ > > + _Generic(state, \ > > + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ > > + ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream)), \ > > + default: ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream) \ > > + ) > > struct v4l2_rect * > > -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > - u32 stream); > > +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > + u32 stream); > > > > /** > > * v4l2_subdev_state_get_compose() - Get pointer to a stream compose rectangle > > @@ -1389,14 +1401,20 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, > > * For stream-unaware drivers the compose rectangle for the corresponding pad is > > * returned. If the pad does not exist, NULL is returned. > > */ > > -#define v4l2_subdev_state_get_compose(state, pad, ...) \ > > - __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > > +#define v4l2_subdev_state_get_compose(state, pad, ...) \ > > + __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ > > (state, pad, ##__VA_ARGS__) > > -#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > > +#define __v4l2_subdev_state_get_compose_pad(state, pad) \ > > __v4l2_subdev_state_get_compose(state, pad, 0) > > +#define __v4l2_subdev_state_get_compose(state, pad, stream) \ > > + _Generic(state, \ > > + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ > > + ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream)), \ > > + default: ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream) \ > > + ) > > struct v4l2_rect * > > -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream); > > +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream); > > > > /** > > * v4l2_subdev_state_get_interval() - Get pointer to a stream frame interval > > @@ -1410,14 +1428,20 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, > > * For stream-unaware drivers the frame interval for the corresponding pad is > > * returned. If the pad does not exist, NULL is returned. > > */ > > -#define v4l2_subdev_state_get_interval(state, pad, ...) \ > > - __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ > > +#define v4l2_subdev_state_get_interval(state, pad, ...) \ > > + __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ > > (state, pad, ##__VA_ARGS__) > > -#define __v4l2_subdev_state_get_interval_pad(state, pad) \ > > +#define __v4l2_subdev_state_get_interval_pad(state, pad) \ > > __v4l2_subdev_state_get_interval(state, pad, 0) > > +#define __v4l2_subdev_state_get_interval(state, pad, stream) \ > > + _Generic(state, \ > > + const struct v4l2_subdev_state *: ((const struct v4l2_frac *) \ > > + ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream)), \ > > + default: ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream) \ > > Is the cast necessary in the latter case? It would seem like we lose most > of the type safety checks as a result. It is, and I was surprised by it. The reason is that all branches of the _Generic statement must compile, even the ones that are not selected. When the macro is called with a const state, the first branch is taken, but the second branch must compile too. Without the cast, the compiler complains that the state argument passed to ____v4l2_subdev_state_get_interval() is const. > Could this be implemented using a similar arrangement than the gen_call > macro above, so that we wouldn't have three very similar sets of macros > here? I've sent a v2 that does that. > > + ) > > struct v4l2_fract * > > -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > - unsigned int pad, u32 stream); > > +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, > > + unsigned int pad, u32 stream); > > > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > >
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 83c6d1e36871..af1fbc28c8f0 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1616,8 +1616,8 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd) EXPORT_SYMBOL_GPL(v4l2_subdev_cleanup); struct v4l2_mbus_framefmt * -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1647,11 +1647,11 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_format); +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_format); struct v4l2_rect * -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, - u32 stream) +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1681,11 +1681,11 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, return NULL; } -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_crop); +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_crop); struct v4l2_rect * -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1715,11 +1715,11 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_compose); +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_compose); struct v4l2_fract * -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream) +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream) { struct v4l2_subdev_stream_configs *stream_configs; unsigned int i; @@ -1751,7 +1751,7 @@ __v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, return NULL; } -EXPORT_SYMBOL_GPL(__v4l2_subdev_state_get_interval); +EXPORT_SYMBOL_GPL(____v4l2_subdev_state_get_interval); #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index e30c463d90e5..ba722ab501c1 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1340,21 +1340,27 @@ void v4l2_subdev_cleanup(struct v4l2_subdev *sd); */ /* * Wrap v4l2_subdev_state_get_format(), allowing the function to be called with - * two or three arguments. The purpose of the __v4l2_subdev_state_get_format() + * two or three arguments. The purpose of the __v4l2_subdev_state_gen_call() * macro below is to come up with the name of the function or macro to call, * using the last two arguments (_stream and _pad). The selected function or * macro is then called using the arguments specified by the caller. A similar * arrangement is used for v4l2_subdev_state_crop() and * v4l2_subdev_state_compose() below. */ -#define v4l2_subdev_state_get_format(state, pad, ...) \ - __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ +#define v4l2_subdev_state_get_format(state, pad, ...) \ + __v4l2_subdev_state_gen_call(format, ##__VA_ARGS__, , _pad) \ (state, pad, ##__VA_ARGS__) -#define __v4l2_subdev_state_get_format_pad(state, pad) \ +#define __v4l2_subdev_state_get_format_pad(state, pad) \ __v4l2_subdev_state_get_format(state, pad, 0) +#define __v4l2_subdev_state_get_format(state, pad, stream) \ + _Generic(state, \ + const struct v4l2_subdev_state *: ((const struct v4l2_mbus_framefmt *) \ + ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream)), \ + default: ____v4l2_subdev_state_get_format((struct v4l2_subdev_state *)state, pad, stream) \ + ) struct v4l2_mbus_framefmt * -__v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +____v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream); /** * v4l2_subdev_state_get_crop() - Get pointer to a stream crop rectangle @@ -1368,14 +1374,20 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state, * For stream-unaware drivers the crop rectangle for the corresponding pad is * returned. If the pad does not exist, NULL is returned. */ -#define v4l2_subdev_state_get_crop(state, pad, ...) \ - __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ +#define v4l2_subdev_state_get_crop(state, pad, ...) \ + __v4l2_subdev_state_gen_call(crop, ##__VA_ARGS__, , _pad) \ (state, pad, ##__VA_ARGS__) -#define __v4l2_subdev_state_get_crop_pad(state, pad) \ +#define __v4l2_subdev_state_get_crop_pad(state, pad) \ __v4l2_subdev_state_get_crop(state, pad, 0) +#define __v4l2_subdev_state_get_crop(state, pad, stream) \ + _Generic(state, \ + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ + ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream)), \ + default: ____v4l2_subdev_state_get_crop((struct v4l2_subdev_state *)state, pad, stream) \ + ) struct v4l2_rect * -__v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, - u32 stream); +____v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, + u32 stream); /** * v4l2_subdev_state_get_compose() - Get pointer to a stream compose rectangle @@ -1389,14 +1401,20 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad, * For stream-unaware drivers the compose rectangle for the corresponding pad is * returned. If the pad does not exist, NULL is returned. */ -#define v4l2_subdev_state_get_compose(state, pad, ...) \ - __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ +#define v4l2_subdev_state_get_compose(state, pad, ...) \ + __v4l2_subdev_state_gen_call(compose, ##__VA_ARGS__, , _pad) \ (state, pad, ##__VA_ARGS__) -#define __v4l2_subdev_state_get_compose_pad(state, pad) \ +#define __v4l2_subdev_state_get_compose_pad(state, pad) \ __v4l2_subdev_state_get_compose(state, pad, 0) +#define __v4l2_subdev_state_get_compose(state, pad, stream) \ + _Generic(state, \ + const struct v4l2_subdev_state *: ((const struct v4l2_rect *) \ + ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream)), \ + default: ____v4l2_subdev_state_get_compose((struct v4l2_subdev_state *)state, pad, stream) \ + ) struct v4l2_rect * -__v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +____v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream); /** * v4l2_subdev_state_get_interval() - Get pointer to a stream frame interval @@ -1410,14 +1428,20 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state, * For stream-unaware drivers the frame interval for the corresponding pad is * returned. If the pad does not exist, NULL is returned. */ -#define v4l2_subdev_state_get_interval(state, pad, ...) \ - __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ +#define v4l2_subdev_state_get_interval(state, pad, ...) \ + __v4l2_subdev_state_gen_call(interval, ##__VA_ARGS__, , _pad) \ (state, pad, ##__VA_ARGS__) -#define __v4l2_subdev_state_get_interval_pad(state, pad) \ +#define __v4l2_subdev_state_get_interval_pad(state, pad) \ __v4l2_subdev_state_get_interval(state, pad, 0) +#define __v4l2_subdev_state_get_interval(state, pad, stream) \ + _Generic(state, \ + const struct v4l2_subdev_state *: ((const struct v4l2_frac *) \ + ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream)), \ + default: ____v4l2_subdev_state_get_interval((struct v4l2_subdev_state *)state, pad, stream) \ + ) struct v4l2_fract * -__v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, - unsigned int pad, u32 stream); +____v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state, + unsigned int pad, u32 stream); #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
It would be useful to mark instances of v4l2_subdev_state structures as const when code needs to access them read-only. This isn't currently possible, as the v4l2_subdev_state_get_*() accessor functions take a non-const pointer to the state. Use _Generic() to provide two different versions of the accessors, for const and non-const states respectively. The former returns a const pointer to the requested format, rectangle or interval, implementing const-correctness. The latter returns a non-const pointer, preserving the current behaviour for drivers. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- Bike-shedding about names and the 4 leading underscores is allowed (and expected). --- drivers/media/v4l2-core/v4l2-subdev.c | 24 +++++----- include/media/v4l2-subdev.h | 66 ++++++++++++++++++--------- 2 files changed, 57 insertions(+), 33 deletions(-)