mbox series

[0/6] Avoid v4l2_subdev_get_try_ ifdef dance

Message ID 20190404074002.18730-1-m.felsch@pengutronix.de (mailing list archive)
Headers show
Series Avoid v4l2_subdev_get_try_ ifdef dance | expand

Message

Marco Felsch April 4, 2019, 7:39 a.m. UTC
Hi,

during my work on [1] I prepared a patch to avoid driver interal ifdef
dances for:
 - v4l2_subdev_get_try_format
 - v4l2_subdev_get_try_crop
 - v4l2_subdev_get_try_compose
helper functions. Jacopo did some comments on it so I picked Lubomir's
series [2] as base and prepared a new one since this series didn't got
merged.

During discussion on [2] Sakari mentioned Hans RFC Patch [3] which didn't
got merged too due to Mauro's concerns.

The driver changes are only compile tested due to the lack of missing
hardware. It would be cool if someone can verify my changes.

[1] https://patchwork.kernel.org/cover/10786553/
[2] https://patchwork.kernel.org/patch/10703029/
[3] https://patchwork.linuxtv.org/patch/53370/

Marco Felsch (6):
  media: v4l2-subdev: add stubs for v4l2_subdev_get_try_*
  media: ov2659: get rid of extra ifdefs
  media: ov2680: get rid of extra ifdefs
  media: ov5695: get rid of extra ifdefs
  media: ov7670: get rid of extra ifdefs
  media: ov7740: get rid of extra ifdefs

 drivers/media/i2c/ov2659.c  | 15 +++++++--------
 drivers/media/i2c/ov2680.c  | 20 +++++++++-----------
 drivers/media/i2c/ov5695.c  | 26 ++++++++++++++------------
 drivers/media/i2c/ov7670.c  | 19 +++++++------------
 drivers/media/i2c/ov7740.c  | 27 +++++++++++++--------------
 include/media/v4l2-subdev.h | 29 ++++++++++++++++++++++++++++-
 6 files changed, 78 insertions(+), 58 deletions(-)

Comments

Sakari Ailus May 31, 2019, 11:51 a.m. UTC | #1
Hi Marco,

On Thu, Apr 04, 2019 at 09:39:56AM +0200, Marco Felsch wrote:
> Hi,
> 
> during my work on [1] I prepared a patch to avoid driver interal ifdef
> dances for:
>  - v4l2_subdev_get_try_format
>  - v4l2_subdev_get_try_crop
>  - v4l2_subdev_get_try_compose
> helper functions. Jacopo did some comments on it so I picked Lubomir's
> series [2] as base and prepared a new one since this series didn't got
> merged.
> 
> During discussion on [2] Sakari mentioned Hans RFC Patch [3] which didn't
> got merged too due to Mauro's concerns.
> 
> The driver changes are only compile tested due to the lack of missing
> hardware. It would be cool if someone can verify my changes.
> 
> [1] https://patchwork.kernel.org/cover/10786553/
> [2] https://patchwork.kernel.org/patch/10703029/
> [3] https://patchwork.linuxtv.org/patch/53370/
> 
> Marco Felsch (6):
>   media: v4l2-subdev: add stubs for v4l2_subdev_get_try_*
>   media: ov2659: get rid of extra ifdefs
>   media: ov2680: get rid of extra ifdefs
>   media: ov5695: get rid of extra ifdefs
>   media: ov7670: get rid of extra ifdefs
>   media: ov7740: get rid of extra ifdefs

Assuming that this patch gets merged:

<URL:https://patchwork.linuxtv.org/patch/56482/>

With the above patch, calling these functions when subdev API is disabled
becomes clearly a driver bug. Instead of returning an error from the
functions, I'd suggest adding dummy format and selection configurations
(which are memset to zero by the call to obtain the said configuration)
that would be returned by these functions. Calling these functions should
also emit a warning.

That would be much safer and also would make it easier to catch API misuse.