v4l2-subdev: Rework subdev format and selection macros to work without MC
RFC 1/1 v4l2-subdev: Rework subdev format and selection macros to work without MC
Rework the macros for accessing subdev try formats to work meaningfully and relatively safely without V4L2 sub-device uAPI (and without MC). This is done by simply reverting to accessing the pad number zero if CONFIG_VIDEO_V4L2_SUBDEV_API isn't enabled, and emitting a kernel warning if the pad is non-zero in that case.
The functions are seen useful if subdev uAPI is disabled for drivers that can work without the Kconfig option but benefit from the option if it's enabled.
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality to a common macro.
---
Hi guys,
This might not be pretty but should provide some comfort for drivers working with different Kconfig options. Comments are welcome...
include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
include/media/v4l2-subdev.h +14 -10
@@ -930,6 +930,17 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ? \ + (WARN_ON((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) : \ + &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */ /** * v4l2_subdev_get_try_format - ancillary routine to call
@@ -944,9 +955,7 @@ static inline struct v4l2_mbus_framefmtstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_fmt; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_fmt); }/**
@@ -962,9 +971,7 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_crop; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_crop); }/**
@@ -980,11 +987,8 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_compose; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_compose); } -#endif extern const struct v4l2_file_operations v4l2_subdev_fops;
Signature · 1 line
2.11.0
Hi Sakari,
Quoted 9 lines · CEST Sakari Ailus
Rework the macros for accessing subdev try formats to work meaningfully and relatively safely without V4L2 sub-device uAPI (and without MC). This is done by simply reverting to accessing the pad number zero if
CONFIG_VIDEO_V4L2_SUBDEV_APIisn't enabled, and emitting a kernel warning if the pad is non-zero in that case.The functions are seen useful if subdev uAPI is disabled for drivers that can work without the
Kconfigoption but benefit from the option if it's enabled.
I'm not sure which drivers you (we) consider valid users of those functions. Subdevice drivers only? Or other drivers which interact with subdevices as well? An answer to that question determines my possible other comments.
Thanks,
Janusz
Quoted 19 lines
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality to a common macro.
Signed-off-by Sakari Ailus <[email protected]>---
Hi guys,This might not be pretty but should provide some comfort for drivers working with different
Kconfigoptions. Comments are welcome...include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)include/media/v4l2-subdev.h +0
-0
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 935
Quoted 6 lines
include/media/v4l2-subdev.h +3
-0@@ -930,6 +930,17 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ?
\
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 937
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON ((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) :
\
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 4 lines
include/media/v4l2-subdev.h +4
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL :
\
Quoted 41 lines
include/media/v4l2-subdev.h +5
-10+ &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */ /** * v4l2_subdev_get_try_format - ancillary routine to call @@ -944,9 +955,7 @@ static inline struct v4l2_mbus_framefmtstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_fmt; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_fmt); }/** @@ -962,9 +971,7 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_crop; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_crop); }/** @@ -980,11 +987,8 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_compose; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_compose); } -#endif extern const struct v4l2_file_operations v4l2_subdev_fops;
Hi Janusz,
Quoted 9 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 6 lines · CEST Sakari Ailus
Rework the macros for accessing subdev try formats to work meaningfully and relatively safely without V4L2 sub-device uAPI (and without MC). This is done by simply reverting to accessing the pad number zero if
CONFIG_VIDEO_V4L2_SUBDEV_APIisn't enabled, and emitting a kernel warning if the pad is non-zero in that case.
Quoted 7 lines
Quoted 3 lines
The functions are seen useful if subdev uAPI is disabled for drivers that can work without the
Kconfigoption but benefit from the option if it's enabled.I'm not sure which drivers you (we) consider valid users of those functions. Subdevice drivers only? Or other drivers which interact with subdevices as well? An answer to that question determines my possible other comments.
These functions are only by drivers for the devices they control directly only.
Quoted 83 lines
Thanks,
JanuszQuoted 19 lines
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality to a common macro.
Signed-off-by Sakari Ailus <[email protected]>---
Hi guys,This might not be pretty but should provide some comfort for drivers working with different
Kconfigoptions. Comments are welcome...include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)include/media/v4l2-subdev.h +0
-0include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 935
Quoted 6 lines
include/media/v4l2-subdev.h +3
-0@@ -930,6 +930,17 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ?\
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 937
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON ((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) :\
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 4 lines
include/media/v4l2-subdev.h +4
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL :\Quoted 41 lines
include/media/v4l2-subdev.h +5
-10+ &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */ /** * v4l2_subdev_get_try_format - ancillary routine to call @@ -944,9 +955,7 @@ static inline struct v4l2_mbus_framefmtstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_fmt; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_fmt); }/** @@ -962,9 +971,7 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_crop; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_crop); }/** @@ -980,11 +987,8 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_compose; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_compose); } -#endif extern const struct v4l2_file_operations v4l2_subdev_fops;
Signature · 2 lines
Sakari Ailus [email protected]
Hi Sakari,
Quoted 16 lines · CEST Sakari Ailus
Hi Janusz,
Quoted 13 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 9 lines · CEST Sakari Ailus
Rework the macros for accessing subdev try formats to work meaningfully and relatively safely without V4L2 sub-device uAPI (and without MC). This is done by simply reverting to accessing the pad number zero if
CONFIG_VIDEO_V4L2_SUBDEV_APIisn't enabled, and emitting a kernel warning if the pad is non-zero in that case.The functions are seen useful if subdev uAPI is disabled for drivers that can work without the
Kconfigoption but benefit from the option if it's enabled.
Quoted 6 lines
Quoted 3 lines
I'm not sure which drivers you (we) consider valid users of those functions. Subdevice drivers only? Or other drivers which interact with subdevices as well? An answer to that question determines my possible other comments.
These functions are only by drivers for the devices they control directly only.
That's what I expected.
Exposing those functions to drivers not supporting V4L2 subdev uAPI is not a bad idea but it would make more sense to me if we also updated potential users. Otherwise I just don't believe anyone will care for as long as a driver is not refreshed to support MC and V4L2 subdev uAPI.
Quoted 30 lines
Quoted 30 lines
...
Quoted 19 lines
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality to a common macro.
Signed-off-by Sakari Ailus <[email protected]>---
Hi guys,This might not be pretty but should provide some comfort for drivers working with different
Kconfigoptions. Comments are welcome...include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)include/media/v4l2-subdev.h +0
-0include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 935
Quoted 6 lines
include/media/v4l2-subdev.h +3
-0@@ -930,6 +930,17 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ?\strange
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 937
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON ((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) :\
Quoted 7 lines
Quoted 7 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 4 lines
include/media/v4l2-subdev.h +4
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL :\include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 943
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */
I think that as long as we already have in place the same checks performed by v4l2_subdev_call() which seems the only user entry point to a subdevice driver, invalid cfg or pad values you are trying to catch here will never be passed unless the driver performs unusual operations and, moreover, is internally broken.
Quoted 9 lines
Quoted 9 lines
include/media/v4l2-subdev.h static inline struct v4l2_mbus_framefmt · line 948
Quoted 9 lines
include/media/v4l2-subdev.h +0
-2/** * v4l2_subdev_get_try_format - ancillary routine to call @@ -944,9 +955,7 @@ static inline struct v4l2_mbus_framefmtstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ;
Dropping this check from here and below makes sense to me anyway, for the same reason as explained above.
Quoted 2 lines
Quoted 2 lines
include/media/v4l2-subdev.h static inline struct v4l2_mbus_framefmt · line 958
Quoted 2 lines
include/media/v4l2-subdev.h +1
-1-return &cfg[pad].try_fmt; +return __v4l2_subdev_get_try_field (sd, cfg, pad, try_fmt);
If we agree that correctness of cfg and pad has already been verified, this change and similar below, perfectly correct otherwise, seem of limited value to me.
Thanks,
Janusz
Quoted 34 lines
Quoted 32 lines
Quoted 28 lines
include/media/v4l2-subdev.h +2
-7}/** @@ -962,9 +971,7 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_crop; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_crop); }/** @@ -980,11 +987,8 @@ static inline struct v4l2_rectstruct v4l2_subdev_pad_config *cfg, unsigned int pad) { - if (WARN_ON(pad >= sd->entity.num_pads)) - pad =0 ; -return &cfg[pad].try_compose; + return __v4l2_subdev_get_try_field(sd, cfg, pad, try_compose); } -#endif extern const struct v4l2_file_operations v4l2_subdev_fops;
Hi Janusz,
Quoted 26 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 16 lines · CEST Sakari Ailus
Hi Janusz,
Quoted 13 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 9 lines · CEST Sakari Ailus
Rework the macros for accessing subdev try formats to work meaningfully and relatively safely without V4L2 sub-device uAPI (and without MC). This is done by simply reverting to accessing the pad number zero if
CONFIG_VIDEO_V4L2_SUBDEV_APIisn't enabled, and emitting a kernel warning if the pad is non-zero in that case.The functions are seen useful if subdev uAPI is disabled for drivers that can work without the
Kconfigoption but benefit from the option if it's enabled.Quoted 6 lines
Quoted 3 lines
I'm not sure which drivers you (we) consider valid users of those functions. Subdevice drivers only? Or other drivers which interact with subdevices as well? An answer to that question determines my possible other comments.
These functions are only by drivers for the devices they control directly only.
Quoted 6 lines
That's what I expected.
Exposing those functions to drivers not supporting V4L2 subdev uAPI is not a bad idea but it would make more sense to me if we also updated potential users. Otherwise I just don't believe anyone will care for as long as a driver is not refreshed to support MC and V4L2 subdev uAPI.
The primary users of these functions are drivers that do support subdev uAPI. Some drivers can function that disabled, so making these functions usable to those drivers in all cases simplifies those drivers.
Quoted 36 lines
Quoted 36 lines
Quoted 36 lines
...
Quoted 19 lines
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality to a common macro.
Signed-off-by Sakari Ailus <[email protected]>---
Hi guys,This might not be pretty but should provide some comfort for drivers working with different
Kconfigoptions. Comments are welcome...include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)include/media/v4l2-subdev.h +0
-0include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 935
Quoted 6 lines
include/media/v4l2-subdev.h +3
-0@@ -930,6 +930,17 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ?\strange
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 937
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON ((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) :\
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 4 lines
include/media/v4l2-subdev.h +4
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field) \ + (WARN_ON(!(__cfg)) ? NULL :\
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 943
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */ I think that as long as we already have in place the same checks performed by
v4l2_subdev_call()which seems the only user entry point to a subdevice driver, invalid cfg or pad values you are trying to catch here will never be passed unless the driver performs unusual operations and, moreover, is internally broken.
You can't rely on checks done by the v4l2_subdev_call macro as these parameters do not come from the caller; they are set by the driver itself.
Signature · 4 lines
Regards, Sakari Ailus [email protected]
Hi Sakari,
Quoted 37 lines · CEST Sakari Ailus
Hi Janusz,
Quoted 34 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 21 lines · CEST Sakari Ailus
Hi Janusz,
Quoted 18 lines · Janusz Krzysztofik
Hi Sakari,
Quoted 14 lines · CEST Sakari Ailus
Rework the macros for accessing subdev try formats to work meaningfully
and relatively safely without V4L2 sub-device uAPI (and without MC). This
is done by simply reverting to accessing the pad number zero ifCONFIG_VIDEO_V4L2_SUBDEV_APIisn't enabled, and emitting a kernel warning
if the pad is non-zero in that case.The functions are seen useful if subdev uAPI is disabled for drivers that
can work without theKconfigoption but benefit from the option if it's
enabled.Quoted 7 lines
Quoted 4 lines
I'm not sure which drivers you (we) consider valid users of those functions. Subdevice drivers only? Or other drivers which interact with subdevices as well? An answer to that question determines my possible other comments.> >
These functions are only by drivers for the devices they control directly
only.That's what I expected.
Quoted 8 lines
Quoted 4 lines
Exposing those functions to drivers not supporting V4L2 subdev uAPI is not a bad idea but it would make more sense to me if we also updated potential users. Otherwise I just don't believe anyone will care for as long as a driver is not refreshed to support MC and V4L2 subdev uAPI.
The primary users of these functions are drivers that do support subdev uAPI. Some drivers can function that disabled, so making these functions usable to those drivers in all cases simplifies those drivers.
Indeed. I didn't take that group of drivers into account.
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
...
Quoted 22 lines
As a by-product, the patch simplifies individual inline functions by removing two lines of code from each of them and moving the functionality
to a common macro.Signed-off-by Sakari Ailus <[email protected]>---
Hi guys,This might not be pretty but should provide some comfort for drivers working with different
Kconfigoptions. Comments are welcome...include/media/v4l2-subdev.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)include/media/v4l2-subdev.h +0
-0@@ -930,6 +930,17 @@ struct v4l2_subdev_fh {
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 5 lines
include/media/v4l2-subdev.h +1
-0container_of(fh, struct v4l2_subdev_fh, vfh)#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API ) +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field)
\
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 937
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON(!(__cfg)) ? NULL : \ + ((__sd)->entity.graph_obj.mdev ?\strange
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 3 lines
include/media/v4l2-subdev.h +2
-0+ (WARN_ON ((__pad) >= (__sd)->entity.num_pads) ? \ + NULL : &(__cfg)[__pad].__field) :\
include/media/v4l2-subdev.h struct v4l2_subdev_fh {
Quoted 3 lines
include/media/v4l2-subdev.h +3
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field)) +#else /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +#define __v4l2_subdev_get_try_field (__sd, __cfg, __pad, __field)
\
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 944
Quoted 2 lines
include/media/v4l2-subdev.h +1
-0+ (WARN_ON(!(__cfg)) ? NULL :\Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
include/media/v4l2-subdev.h struct v4l2_subdev_fh { · line 943
Quoted 2 lines
include/media/v4l2-subdev.h +2
-0+ &(__cfg)[WARN_ON(__pad) && 0].__field) +#endif /* !CONFIG_VIDEO_V4L2_SUBDEV_API */ I think that as long as we already have in place the same checks performed by
v4l2_subdev_call()which seems the only user entry point to a subdevice driver, invalid cfg or pad values you are trying to catch here will never be passed unless the driver performs unusual operations and, moreover, is internally broken.
Quoted 2 lines
You can't rely on checks done by the
v4l2_subdev_callmacro as these parameters do not come from the caller; they are set by the driver itself.
If that' the case, you are right.
It looks like I'm missing some knowledge on V4L2 framework needed to provide a valuable review of your change, please ignore my comments.
Thanks,
Janusz