| Message ID | 20240410-enable-streams-impro-v3-1-e5e7a5da7420@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: subdev: Improve stream enable/disable machinery | expand |
Hi Tomi, Thank you for the patch. On 10/04/24 6:05 pm, Tomi Valkeinen wrote: > Add helper functions to enable and disable the privacy led. This moves > the #if from the call site to the privacy led functions, and makes > adding privacy led support to v4l2_subdev_enable/disable_streams() > cleaner. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 012b757eac9f..13957543d153 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -148,6 +148,23 @@ static int subdev_close(struct file *file) > } > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > +static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) > +{ > +#if IS_REACHABLE(CONFIG_LEDS_CLASS) > + if (!IS_ERR_OR_NULL(sd->privacy_led)) > + led_set_brightness(sd->privacy_led, > + sd->privacy_led->max_brightness); > +#endif > +} > + > +static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) > +{ > +#if IS_REACHABLE(CONFIG_LEDS_CLASS) > + if (!IS_ERR_OR_NULL(sd->privacy_led)) > + led_set_brightness(sd->privacy_led, 0); > +#endif > +} > + > static inline int check_which(u32 which) > { > if (which != V4L2_SUBDEV_FORMAT_TRY && > @@ -422,15 +439,10 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) > if (!ret) { > sd->enabled_streams = enable ? BIT(0) : 0; > > -#if IS_REACHABLE(CONFIG_LEDS_CLASS) > - if (!IS_ERR_OR_NULL(sd->privacy_led)) { > - if (enable) > - led_set_brightness(sd->privacy_led, > - sd->privacy_led->max_brightness); > - else > - led_set_brightness(sd->privacy_led, 0); > - } > -#endif > + if (enable) > + v4l2_subdev_enable_privacy_led(sd); > + else > + v4l2_subdev_disable_privacy_led(sd); > } > > return ret; >
Hi Tomi, Thank you for the patch. On Wed, Apr 10, 2024 at 03:35:48PM +0300, Tomi Valkeinen wrote: > Add helper functions to enable and disable the privacy led. This moves > the #if from the call site to the privacy led functions, and makes > adding privacy led support to v4l2_subdev_enable/disable_streams() > cleaner. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 012b757eac9f..13957543d153 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -148,6 +148,23 @@ static int subdev_close(struct file *file) > } > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > +static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) > +{ > +#if IS_REACHABLE(CONFIG_LEDS_CLASS) > + if (!IS_ERR_OR_NULL(sd->privacy_led)) > + led_set_brightness(sd->privacy_led, > + sd->privacy_led->max_brightness); > +#endif > +} > + > +static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) > +{ > +#if IS_REACHABLE(CONFIG_LEDS_CLASS) > + if (!IS_ERR_OR_NULL(sd->privacy_led)) > + led_set_brightness(sd->privacy_led, 0); > +#endif > +} I would have written this as #if IS_REACHABLE(CONFIG_LEDS_CLASS) static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) { if (!IS_ERR_OR_NULL(sd->privacy_led)) led_set_brightness(sd->privacy_led, sd->privacy_led->max_brightness); } static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) { if (!IS_ERR_OR_NULL(sd->privacy_led)) led_set_brightness(sd->privacy_led, 0); } #else static inline void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) { } static inline void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) { } #endif /* CONFIG_LEDS_CLASS */ to avoid multipe #if but that likely makes no difference in the generated code. Either way, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > static inline int check_which(u32 which) > { > if (which != V4L2_SUBDEV_FORMAT_TRY && > @@ -422,15 +439,10 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) > if (!ret) { > sd->enabled_streams = enable ? BIT(0) : 0; > > -#if IS_REACHABLE(CONFIG_LEDS_CLASS) > - if (!IS_ERR_OR_NULL(sd->privacy_led)) { > - if (enable) > - led_set_brightness(sd->privacy_led, > - sd->privacy_led->max_brightness); > - else > - led_set_brightness(sd->privacy_led, 0); > - } > -#endif > + if (enable) > + v4l2_subdev_enable_privacy_led(sd); > + else > + v4l2_subdev_disable_privacy_led(sd); > } > > return ret; >
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 012b757eac9f..13957543d153 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -148,6 +148,23 @@ static int subdev_close(struct file *file) } #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ +static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) +{ +#if IS_REACHABLE(CONFIG_LEDS_CLASS) + if (!IS_ERR_OR_NULL(sd->privacy_led)) + led_set_brightness(sd->privacy_led, + sd->privacy_led->max_brightness); +#endif +} + +static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) +{ +#if IS_REACHABLE(CONFIG_LEDS_CLASS) + if (!IS_ERR_OR_NULL(sd->privacy_led)) + led_set_brightness(sd->privacy_led, 0); +#endif +} + static inline int check_which(u32 which) { if (which != V4L2_SUBDEV_FORMAT_TRY && @@ -422,15 +439,10 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) if (!ret) { sd->enabled_streams = enable ? BIT(0) : 0; -#if IS_REACHABLE(CONFIG_LEDS_CLASS) - if (!IS_ERR_OR_NULL(sd->privacy_led)) { - if (enable) - led_set_brightness(sd->privacy_led, - sd->privacy_led->max_brightness); - else - led_set_brightness(sd->privacy_led, 0); - } -#endif + if (enable) + v4l2_subdev_enable_privacy_led(sd); + else + v4l2_subdev_disable_privacy_led(sd); } return ret;
Add helper functions to enable and disable the privacy led. This moves the #if from the call site to the privacy led functions, and makes adding privacy led support to v4l2_subdev_enable/disable_streams() cleaner. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/media/v4l2-core/v4l2-subdev.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)