| Message ID | 20230120114524.408368-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | leds: lookup-table support + int3472/media privacy LED support | expand |
Hi Hans, Many thanks for working on this. On Fri, Jan 20, 2023 at 12:45:19PM +0100, Hans de Goede wrote: > Currently the videodev.ko code may be builtin while e.g. v4l2-fwnode.ko > is build as a module. > > This makes it hard to add code depending on other subsystems spanning > both videodev.ko and v4l2-fwnode.ko. Specifically this block adding code > depending on the LED subsystem. > > This is made even harder because CONFIG_V4L2_FWNODE is selected, > not depended on so it itself cannot depend on another subsystem without > editing all the Kconfig symbols selecting it to also list the dependency > and there are many of such symbols. > > Adding a "select LED_CLASS if NEW_LEDS" to CONFIG_V4L2_FWNODE leads > to Kconfig erroring out with "error: recursive dependency detected!". > > To fix this dependency mess, change the V4L2_FWNODE and V4L2_ASYNC > (which V4L2_FWNODE selects) Kconfig symbols from tristate to bools and > link their code into videodev.ko instead of making them separate modules. > > This will allow using IS_REACHABLE(LED_CLASS) for the new LED integration > code without needing to worry that it expands to 0 in some places and > 1 in other places because some of the code being builtin vs modular. > > On x86_64 this leads to the following size changes for videodev.ko > > [hans@shalem linux]$ size drivers/media/v4l2-core/videodev.ko > > Before: > text data bss dec hex filename > 218206 14395 2448 235049 39629 drivers/media/v4l2-core/videodev.ko > After: > text data bss dec hex filename > 243213 17615 2456 263284 40474 drivers/media/v4l2-core/videodev.ko > > So (as expected) there is some increase in size here, but it > really is not that much. > > And the uncompressed no-debuginfo .ko file disk-usage actually shrinks > by 17 KiB (comparing the slightly larger videodev.ko against the > 3 original modules) and loading time will also be better. > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v5: > - Add a new v4l2-dev-priv.h for the async debugfs prototypes and add > static inline wrappers there when CONFIG_V4L2_ASYNC is not enabled > > Changes in v4: > - New patch in v4 of this patch-set > --- > drivers/media/v4l2-core/Kconfig | 4 ++-- > drivers/media/v4l2-core/Makefile | 4 ++-- > drivers/media/v4l2-core/v4l2-async.c | 17 ++++------------- > drivers/media/v4l2-core/v4l2-dev-priv.h | 19 +++++++++++++++++++ > drivers/media/v4l2-core/v4l2-dev.c | 8 ++++++++ > drivers/media/v4l2-core/v4l2-fwnode.c | 6 ------ > 6 files changed, 35 insertions(+), 23 deletions(-) > create mode 100644 drivers/media/v4l2-core/v4l2-dev-priv.h > > diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig > index 348559bc2468..73574d946010 100644 > --- a/drivers/media/v4l2-core/Kconfig > +++ b/drivers/media/v4l2-core/Kconfig > @@ -68,11 +68,11 @@ config V4L2_FLASH_LED_CLASS > When in doubt, say N. > > config V4L2_FWNODE > - tristate > + bool > select V4L2_ASYNC > > config V4L2_ASYNC > - tristate > + bool > > # Used by drivers that need Videobuf modules > config VIDEOBUF_GEN > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile > index 41d91bd10cf2..8c5a1ab8d939 100644 > --- a/drivers/media/v4l2-core/Makefile > +++ b/drivers/media/v4l2-core/Makefile > @@ -15,7 +15,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ > > # Please keep it alphabetically sorted by Kconfig name > # (e. g. LC_ALL=C sort Makefile) > +videodev-$(CONFIG_V4L2_ASYNC) += v4l2-async.o > videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o > +videodev-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o > videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o > videodev-$(CONFIG_SPI) += v4l2-spi.o > videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o > @@ -24,9 +26,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o > # Please keep it alphabetically sorted by Kconfig name > # (e. g. LC_ALL=C sort Makefile) > > -obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o > obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o > -obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o > obj-$(CONFIG_V4L2_H264) += v4l2-h264.o > obj-$(CONFIG_V4L2_JPEG_HELPER) += v4l2-jpeg.o > obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index 2f1b718a9189..024d6b82b50a 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -11,7 +11,6 @@ > #include <linux/i2c.h> > #include <linux/list.h> > #include <linux/mm.h> > -#include <linux/module.h> > #include <linux/mutex.h> > #include <linux/of.h> > #include <linux/platform_device.h> > @@ -24,6 +23,8 @@ > #include <media/v4l2-fwnode.h> > #include <media/v4l2-subdev.h> > > +#include "v4l2-dev-priv.h" > + > static int v4l2_async_nf_call_bound(struct v4l2_async_notifier *n, > struct v4l2_subdev *subdev, > struct v4l2_async_subdev *asd) > @@ -900,25 +901,15 @@ DEFINE_SHOW_ATTRIBUTE(pending_subdevs); > > static struct dentry *v4l2_async_debugfs_dir; > > -static int __init v4l2_async_init(void) > +void __init v4l2_async_debugfs_init(void) > { > v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL); > debugfs_create_file("pending_async_subdevices", 0444, > v4l2_async_debugfs_dir, NULL, > &pending_subdevs_fops); > - > - return 0; > } > > -static void __exit v4l2_async_exit(void) > +void __exit v4l2_async_debugfs_exit(void) > { > debugfs_remove_recursive(v4l2_async_debugfs_dir); > } > - > -subsys_initcall(v4l2_async_init); > -module_exit(v4l2_async_exit); > - > -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); > -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); > -MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); > -MODULE_LICENSE("GPL"); > diff --git a/drivers/media/v4l2-core/v4l2-dev-priv.h b/drivers/media/v4l2-core/v4l2-dev-priv.h > new file mode 100644 > index 000000000000..b5b1ee78be20 > --- /dev/null > +++ b/drivers/media/v4l2-core/v4l2-dev-priv.h Could we call this v4l2-async-debugfs.h? I don't necessarily expect more material here. > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Video capture interface for Linux version 2 private header. > + * > + * Copyright (C) 2023 Hans de Goede <hdegoede@redhat.com> > + */ > + > +#ifndef _V4L2_DEV_PRIV_H_ > +#define _V4L2_DEV_PRIV_H_ > + > +#if IS_ENABLED(CONFIG_V4L2_ASYNC) > +void v4l2_async_debugfs_init(void); > +void v4l2_async_debugfs_exit(void); > +#else > +static inline void v4l2_async_debugfs_init(void) {} > +static inline void v4l2_async_debugfs_exit(void) {} > +#endif > + > +#endif > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index 397d553177fa..10ba2e4196a6 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -31,6 +31,8 @@ > #include <media/v4l2-ioctl.h> > #include <media/v4l2-event.h> > > +#include "v4l2-dev-priv.h" > + > #define VIDEO_NUM_DEVICES 256 > #define VIDEO_NAME "video4linux" > > @@ -1190,6 +1192,7 @@ static int __init videodev_init(void) > return -EIO; > } > > + v4l2_async_debugfs_init(); > return 0; > } > > @@ -1197,6 +1200,7 @@ static void __exit videodev_exit(void) > { > dev_t dev = MKDEV(VIDEO_MAJOR, 0); > > + v4l2_async_debugfs_exit(); > class_unregister(&video_class); > unregister_chrdev_region(dev, VIDEO_NUM_DEVICES); > } > @@ -1205,6 +1209,10 @@ subsys_initcall(videodev_init); > module_exit(videodev_exit) > > MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <mchehab@kernel.org>, Bill Dirks, Justin Schoeman, Gerd Knorr"); > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); > +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); > +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); > +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); > MODULE_DESCRIPTION("Video4Linux2 core driver"); > MODULE_LICENSE("GPL"); > MODULE_ALIAS_CHARDEV_MAJOR(VIDEO_MAJOR); > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > index 3d9533c1b202..c8a2264262bc 100644 > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > @@ -17,7 +17,6 @@ > #include <linux/acpi.h> > #include <linux/kernel.h> > #include <linux/mm.h> > -#include <linux/module.h> > #include <linux/of.h> > #include <linux/property.h> > #include <linux/slab.h> > @@ -1328,8 +1327,3 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd) > return ret; > } > EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor); > - > -MODULE_LICENSE("GPL"); > -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); > -MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); > -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); > -- > 2.39.0 >
Hi, On 1/20/23 13:47, Sakari Ailus wrote: > Hi Hans, > > Many thanks for working on this. You are welcome. > On Fri, Jan 20, 2023 at 12:45:19PM +0100, Hans de Goede wrote: >> Currently the videodev.ko code may be builtin while e.g. v4l2-fwnode.ko >> is build as a module. >> >> This makes it hard to add code depending on other subsystems spanning >> both videodev.ko and v4l2-fwnode.ko. Specifically this block adding code >> depending on the LED subsystem. >> >> This is made even harder because CONFIG_V4L2_FWNODE is selected, >> not depended on so it itself cannot depend on another subsystem without >> editing all the Kconfig symbols selecting it to also list the dependency >> and there are many of such symbols. >> >> Adding a "select LED_CLASS if NEW_LEDS" to CONFIG_V4L2_FWNODE leads >> to Kconfig erroring out with "error: recursive dependency detected!". >> >> To fix this dependency mess, change the V4L2_FWNODE and V4L2_ASYNC >> (which V4L2_FWNODE selects) Kconfig symbols from tristate to bools and >> link their code into videodev.ko instead of making them separate modules. >> >> This will allow using IS_REACHABLE(LED_CLASS) for the new LED integration >> code without needing to worry that it expands to 0 in some places and >> 1 in other places because some of the code being builtin vs modular. >> >> On x86_64 this leads to the following size changes for videodev.ko >> >> [hans@shalem linux]$ size drivers/media/v4l2-core/videodev.ko >> >> Before: >> text data bss dec hex filename >> 218206 14395 2448 235049 39629 drivers/media/v4l2-core/videodev.ko >> After: >> text data bss dec hex filename >> 243213 17615 2456 263284 40474 drivers/media/v4l2-core/videodev.ko >> >> So (as expected) there is some increase in size here, but it >> really is not that much. >> >> And the uncompressed no-debuginfo .ko file disk-usage actually shrinks >> by 17 KiB (comparing the slightly larger videodev.ko against the >> 3 original modules) and loading time will also be better. >> >> Acked-by: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v5: >> - Add a new v4l2-dev-priv.h for the async debugfs prototypes and add >> static inline wrappers there when CONFIG_V4L2_ASYNC is not enabled >> >> Changes in v4: >> - New patch in v4 of this patch-set >> --- >> drivers/media/v4l2-core/Kconfig | 4 ++-- >> drivers/media/v4l2-core/Makefile | 4 ++-- >> drivers/media/v4l2-core/v4l2-async.c | 17 ++++------------- >> drivers/media/v4l2-core/v4l2-dev-priv.h | 19 +++++++++++++++++++ >> drivers/media/v4l2-core/v4l2-dev.c | 8 ++++++++ >> drivers/media/v4l2-core/v4l2-fwnode.c | 6 ------ >> 6 files changed, 35 insertions(+), 23 deletions(-) >> create mode 100644 drivers/media/v4l2-core/v4l2-dev-priv.h >> >> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig >> index 348559bc2468..73574d946010 100644 >> --- a/drivers/media/v4l2-core/Kconfig >> +++ b/drivers/media/v4l2-core/Kconfig >> @@ -68,11 +68,11 @@ config V4L2_FLASH_LED_CLASS >> When in doubt, say N. >> >> config V4L2_FWNODE >> - tristate >> + bool >> select V4L2_ASYNC >> >> config V4L2_ASYNC >> - tristate >> + bool >> >> # Used by drivers that need Videobuf modules >> config VIDEOBUF_GEN >> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile >> index 41d91bd10cf2..8c5a1ab8d939 100644 >> --- a/drivers/media/v4l2-core/Makefile >> +++ b/drivers/media/v4l2-core/Makefile >> @@ -15,7 +15,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ >> >> # Please keep it alphabetically sorted by Kconfig name >> # (e. g. LC_ALL=C sort Makefile) >> +videodev-$(CONFIG_V4L2_ASYNC) += v4l2-async.o >> videodev-$(CONFIG_COMPAT) += v4l2-compat-ioctl32.o >> +videodev-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o >> videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o >> videodev-$(CONFIG_SPI) += v4l2-spi.o >> videodev-$(CONFIG_TRACEPOINTS) += v4l2-trace.o >> @@ -24,9 +26,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o >> # Please keep it alphabetically sorted by Kconfig name >> # (e. g. LC_ALL=C sort Makefile) >> >> -obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o >> obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o >> -obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o >> obj-$(CONFIG_V4L2_H264) += v4l2-h264.o >> obj-$(CONFIG_V4L2_JPEG_HELPER) += v4l2-jpeg.o >> obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o >> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c >> index 2f1b718a9189..024d6b82b50a 100644 >> --- a/drivers/media/v4l2-core/v4l2-async.c >> +++ b/drivers/media/v4l2-core/v4l2-async.c >> @@ -11,7 +11,6 @@ >> #include <linux/i2c.h> >> #include <linux/list.h> >> #include <linux/mm.h> >> -#include <linux/module.h> >> #include <linux/mutex.h> >> #include <linux/of.h> >> #include <linux/platform_device.h> >> @@ -24,6 +23,8 @@ >> #include <media/v4l2-fwnode.h> >> #include <media/v4l2-subdev.h> >> >> +#include "v4l2-dev-priv.h" >> + >> static int v4l2_async_nf_call_bound(struct v4l2_async_notifier *n, >> struct v4l2_subdev *subdev, >> struct v4l2_async_subdev *asd) >> @@ -900,25 +901,15 @@ DEFINE_SHOW_ATTRIBUTE(pending_subdevs); >> >> static struct dentry *v4l2_async_debugfs_dir; >> >> -static int __init v4l2_async_init(void) >> +void __init v4l2_async_debugfs_init(void) >> { >> v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL); >> debugfs_create_file("pending_async_subdevices", 0444, >> v4l2_async_debugfs_dir, NULL, >> &pending_subdevs_fops); >> - >> - return 0; >> } >> >> -static void __exit v4l2_async_exit(void) >> +void __exit v4l2_async_debugfs_exit(void) >> { >> debugfs_remove_recursive(v4l2_async_debugfs_dir); >> } >> - >> -subsys_initcall(v4l2_async_init); >> -module_exit(v4l2_async_exit); >> - >> -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> -MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); >> -MODULE_LICENSE("GPL"); >> diff --git a/drivers/media/v4l2-core/v4l2-dev-priv.h b/drivers/media/v4l2-core/v4l2-dev-priv.h >> new file mode 100644 >> index 000000000000..b5b1ee78be20 >> --- /dev/null >> +++ b/drivers/media/v4l2-core/v4l2-dev-priv.h > > Could we call this v4l2-async-debugfs.h? I don't necessarily expect more > material here. Renaming it is fine by my. I have renamed it to v4l2-async-debugfs.h for the upcoming v6 of this series. Regards, Hans > >> @@ -0,0 +1,19 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +/* >> + * Video capture interface for Linux version 2 private header. >> + * >> + * Copyright (C) 2023 Hans de Goede <hdegoede@redhat.com> >> + */ >> + >> +#ifndef _V4L2_DEV_PRIV_H_ >> +#define _V4L2_DEV_PRIV_H_ >> + >> +#if IS_ENABLED(CONFIG_V4L2_ASYNC) >> +void v4l2_async_debugfs_init(void); >> +void v4l2_async_debugfs_exit(void); >> +#else >> +static inline void v4l2_async_debugfs_init(void) {} >> +static inline void v4l2_async_debugfs_exit(void) {} >> +#endif >> + >> +#endif >> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c >> index 397d553177fa..10ba2e4196a6 100644 >> --- a/drivers/media/v4l2-core/v4l2-dev.c >> +++ b/drivers/media/v4l2-core/v4l2-dev.c >> @@ -31,6 +31,8 @@ >> #include <media/v4l2-ioctl.h> >> #include <media/v4l2-event.h> >> >> +#include "v4l2-dev-priv.h" >> + >> #define VIDEO_NUM_DEVICES 256 >> #define VIDEO_NAME "video4linux" >> >> @@ -1190,6 +1192,7 @@ static int __init videodev_init(void) >> return -EIO; >> } >> >> + v4l2_async_debugfs_init(); >> return 0; >> } >> >> @@ -1197,6 +1200,7 @@ static void __exit videodev_exit(void) >> { >> dev_t dev = MKDEV(VIDEO_MAJOR, 0); >> >> + v4l2_async_debugfs_exit(); >> class_unregister(&video_class); >> unregister_chrdev_region(dev, VIDEO_NUM_DEVICES); >> } >> @@ -1205,6 +1209,10 @@ subsys_initcall(videodev_init); >> module_exit(videodev_exit) >> >> MODULE_AUTHOR("Alan Cox, Mauro Carvalho Chehab <mchehab@kernel.org>, Bill Dirks, Justin Schoeman, Gerd Knorr"); >> +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>"); >> +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); >> MODULE_DESCRIPTION("Video4Linux2 core driver"); >> MODULE_LICENSE("GPL"); >> MODULE_ALIAS_CHARDEV_MAJOR(VIDEO_MAJOR); >> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c >> index 3d9533c1b202..c8a2264262bc 100644 >> --- a/drivers/media/v4l2-core/v4l2-fwnode.c >> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c >> @@ -17,7 +17,6 @@ >> #include <linux/acpi.h> >> #include <linux/kernel.h> >> #include <linux/mm.h> >> -#include <linux/module.h> >> #include <linux/of.h> >> #include <linux/property.h> >> #include <linux/slab.h> >> @@ -1328,8 +1327,3 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd) >> return ret; >> } >> EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor); >> - >> -MODULE_LICENSE("GPL"); >> -MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>"); >> -MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); >> -MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>"); >> -- >> 2.39.0 >> >
On Fri, 20 Jan 2023, Hans de Goede wrote: > Hi All, > > Here is version 5 of my series to adjust the INT3472 code's handling of > the privacy LED on x86 laptops with MIPI camera(s) so that it will also > work on devices which have a privacy-LED GPIO but not a clk-enable GPIO > (so that we cannot just tie the LED state to the clk-enable state). > > Changes in v5: > - Rename lookup-table names to match those from the gpio and reset lookups: > s/led_name/provider/ > s/consumer_dev_name/dev_id/ > s/consumer_function/con_id/ > - Add static inline wrappers for the v4l2_async debugfs init/exit funcs, > to fix build errors when CONFIG_V4L2_ASYNC is not enabled > > Changes in v4: > - Rename new __led_get() helper to led_module_get() > - Drop of/devicetree support from "led-class: Add generic [devm_]led_get()" > - Add RFC patch to re-add of/devicetree support to show that the new > led_get() can easily be extended with dt support when the need for this > arises (proof-of-concept dt code, not intended for merging) > - New patch to built async and fwnode code into videodev.ko, > to avoid issues with some of the new LED code getting builtin vs > other parts possibly being in a module > - Move the led_get() call to v4l2_async_register_subdev_sensor() > - Move the led_disable_sysfs() call to be done at led_get() time > - Address some other minor review comments > > Changes in v3: > - Due to popular request by multiple people this new version now models > the privacy LED as a LED class device. This requires being able to > "tie" the LED class device to a specific camera sensor (some devices > have multiple sensors + privacy-LEDs). > > Patches 1-5 are LED subsystem patches for this. 1 is a bug fix, 2-4 add > the new [devm_]led_get() functions. Patch 5 is the RFC patch adding dt > support to led_get() and is not intended for merging. > > Patch 6 + 7 add generic privacy-LED support to the v4l2-core/v4l2-subdev.c > code automatically enabling the privacy-LED when s_stream(subdev, 1) > is called. So that we don't need to add privacy-LED code to all the > camera sensor drivers separately (as requested by Sakari). > > Patches 8-11 are patches to the platform specific INT3472 code to register > privacy-LED class devices + lookup table entries for privacy-LEDs described > in the special INT3472 ACPI nodes found on x86 devices with MIPI cameras. > > Assuming at least the LED maintainers are happy with the approach suggested > here, the first step to merging this would be to merge patches 1-4 and then > provide an immutable branch with those to merge for the other subsystems > since the other changes depend on these. LEDs pull request to follow.
Hi, On 1/27/23 12:08, Lee Jones wrote: > On Fri, 20 Jan 2023, Hans de Goede wrote: > >> Hi All, >> >> Here is version 5 of my series to adjust the INT3472 code's handling of >> the privacy LED on x86 laptops with MIPI camera(s) so that it will also >> work on devices which have a privacy-LED GPIO but not a clk-enable GPIO >> (so that we cannot just tie the LED state to the clk-enable state). >> >> Changes in v5: >> - Rename lookup-table names to match those from the gpio and reset lookups: >> s/led_name/provider/ >> s/consumer_dev_name/dev_id/ >> s/consumer_function/con_id/ >> - Add static inline wrappers for the v4l2_async debugfs init/exit funcs, >> to fix build errors when CONFIG_V4L2_ASYNC is not enabled >> >> Changes in v4: >> - Rename new __led_get() helper to led_module_get() >> - Drop of/devicetree support from "led-class: Add generic [devm_]led_get()" >> - Add RFC patch to re-add of/devicetree support to show that the new >> led_get() can easily be extended with dt support when the need for this >> arises (proof-of-concept dt code, not intended for merging) >> - New patch to built async and fwnode code into videodev.ko, >> to avoid issues with some of the new LED code getting builtin vs >> other parts possibly being in a module >> - Move the led_get() call to v4l2_async_register_subdev_sensor() >> - Move the led_disable_sysfs() call to be done at led_get() time >> - Address some other minor review comments >> >> Changes in v3: >> - Due to popular request by multiple people this new version now models >> the privacy LED as a LED class device. This requires being able to >> "tie" the LED class device to a specific camera sensor (some devices >> have multiple sensors + privacy-LEDs). >> >> Patches 1-5 are LED subsystem patches for this. 1 is a bug fix, 2-4 add >> the new [devm_]led_get() functions. Patch 5 is the RFC patch adding dt >> support to led_get() and is not intended for merging. >> >> Patch 6 + 7 add generic privacy-LED support to the v4l2-core/v4l2-subdev.c >> code automatically enabling the privacy-LED when s_stream(subdev, 1) >> is called. So that we don't need to add privacy-LED code to all the >> camera sensor drivers separately (as requested by Sakari). >> >> Patches 8-11 are patches to the platform specific INT3472 code to register >> privacy-LED class devices + lookup table entries for privacy-LEDs described >> in the special INT3472 ACPI nodes found on x86 devices with MIPI cameras. >> >> Assuming at least the LED maintainers are happy with the approach suggested >> here, the first step to merging this would be to merge patches 1-4 and then >> provide an immutable branch with those to merge for the other subsystems >> since the other changes depend on these. > > LEDs pull request to follow. Great, thank you! Regards, Hans
Enjoy!
The following changes since commit 1b929c02afd37871d5afb9d498426f83432e71c2:
Linux 6.2-rc1 (2022-12-25 13:41:39 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git ib-leds-led_get-6.3
for you to fetch changes up to abc3100fcba6827444ef4bdb17065ac3b6619dff:
leds: led-class: Add generic [devm_]led_get() (2023-01-27 11:07:11 +0000)
----------------------------------------------------------------
Hans de Goede (4):
leds: led-class: Add missing put_device() to led_put()
leds: led-class: Add led_module_get() helper
leds: led-class: Add __devm_led_get() helper
leds: led-class: Add generic [devm_]led_get()
drivers/leds/led-class.c | 138 ++++++++++++++++++++++++++++++++++++++++-------
include/linux/leds.h | 21 ++++++++
2 files changed, 139 insertions(+), 20 deletions(-)
Hi All, Here is version 5 of my series to adjust the INT3472 code's handling of the privacy LED on x86 laptops with MIPI camera(s) so that it will also work on devices which have a privacy-LED GPIO but not a clk-enable GPIO (so that we cannot just tie the LED state to the clk-enable state). Changes in v5: - Rename lookup-table names to match those from the gpio and reset lookups: s/led_name/provider/ s/consumer_dev_name/dev_id/ s/consumer_function/con_id/ - Add static inline wrappers for the v4l2_async debugfs init/exit funcs, to fix build errors when CONFIG_V4L2_ASYNC is not enabled Changes in v4: - Rename new __led_get() helper to led_module_get() - Drop of/devicetree support from "led-class: Add generic [devm_]led_get()" - Add RFC patch to re-add of/devicetree support to show that the new led_get() can easily be extended with dt support when the need for this arises (proof-of-concept dt code, not intended for merging) - New patch to built async and fwnode code into videodev.ko, to avoid issues with some of the new LED code getting builtin vs other parts possibly being in a module - Move the led_get() call to v4l2_async_register_subdev_sensor() - Move the led_disable_sysfs() call to be done at led_get() time - Address some other minor review comments Changes in v3: - Due to popular request by multiple people this new version now models the privacy LED as a LED class device. This requires being able to "tie" the LED class device to a specific camera sensor (some devices have multiple sensors + privacy-LEDs). Patches 1-5 are LED subsystem patches for this. 1 is a bug fix, 2-4 add the new [devm_]led_get() functions. Patch 5 is the RFC patch adding dt support to led_get() and is not intended for merging. Patch 6 + 7 add generic privacy-LED support to the v4l2-core/v4l2-subdev.c code automatically enabling the privacy-LED when s_stream(subdev, 1) is called. So that we don't need to add privacy-LED code to all the camera sensor drivers separately (as requested by Sakari). Patches 8-11 are patches to the platform specific INT3472 code to register privacy-LED class devices + lookup table entries for privacy-LEDs described in the special INT3472 ACPI nodes found on x86 devices with MIPI cameras. Assuming at least the LED maintainers are happy with the approach suggested here, the first step to merging this would be to merge patches 1-4 and then provide an immutable branch with those to merge for the other subsystems since the other changes depend on these. If you are one of the folks who requested the new LED lookup table + led_get() approach I would appreciate a Reviewed-by or Acked-by for patches 1-4. This series has been tested on: - Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED - Dell Latitude 9420, IPU 6, front: ov01a1s with privacy LED - Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED back: ov8865 with privacy LED (pled not yet supported) Regards, Hans Hans de Goede (11): leds: led-class: Add missing put_device() to led_put() leds: led-class: Add led_module_get() helper leds: led-class: Add __devm_led_get() helper leds: led-class: Add generic [devm_]led_get() [RFC] leds: led-class: Add devicetree support to led_get() media: v4l2-core: Built async and fwnode code into videodev.ko media: v4l2-core: Make the v4l2-core code enable/disable the privacy LED if present platform/x86: int3472/discrete: Refactor GPIO to sensor mapping platform/x86: int3472/discrete: Create a LED class device for the privacy LED platform/x86: int3472/discrete: Move GPIO request to skl_int3472_register_clock() platform/x86: int3472/discrete: Get the polarity from the _DSM entry drivers/leds/led-class.c | 173 +++++++++++++++--- drivers/media/v4l2-core/Kconfig | 4 +- drivers/media/v4l2-core/Makefile | 4 +- drivers/media/v4l2-core/v4l2-async.c | 17 +- drivers/media/v4l2-core/v4l2-dev-priv.h | 19 ++ drivers/media/v4l2-core/v4l2-dev.c | 8 + drivers/media/v4l2-core/v4l2-fwnode.c | 21 ++- drivers/media/v4l2-core/v4l2-subdev.c | 18 ++ drivers/platform/x86/intel/int3472/Makefile | 2 +- .../x86/intel/int3472/clk_and_regulator.c | 34 +++- drivers/platform/x86/intel/int3472/common.h | 18 +- drivers/platform/x86/intel/int3472/discrete.c | 100 +++++----- drivers/platform/x86/intel/int3472/led.c | 74 ++++++++ include/linux/leds.h | 21 +++ include/media/v4l2-subdev.h | 3 + 15 files changed, 399 insertions(+), 117 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-dev-priv.h create mode 100644 drivers/platform/x86/intel/int3472/led.c