Hi, On 11/25/22 12:42, Dan Scally wrote: > Hi > > On 25/11/2022 11:23, Hans de Goede wrote: >> Hi, >> >> On 11/25/22 12:11, Dan Scally wrote: >>> On 25/11/2022 11:06, Andy Shevchenko wrote: >>>> On Fri, Nov 25, 2022 at 12:58:46PM +0200, Laurent Pinchart wrote: >>>>> On Fri, Nov 25, 2022 at 10:17:17AM +0000, Dan Scally wrote: >>>> ... >>>> >>>>> Can the LED framework be used without having the LED exposed to >>>>> userspace ? >>>> I believe the correct question here is "can the states of some leds be >>>> read-only from user perspective" (this way any changes into led subsystems >>>> looks less intrusive, esp. taking into account that subsystem is de facto >>>> unmaintained). >>>> >>> I think the answer to that is yes: >>> >>> >>> https://elixir.bootlin.com/linux/latest/source/drivers/leds/led-class.c#L47 >> Interesting, I did not know that. But what is the added value of >> using the LED subsytem then for a simple only on/off LED driven >> by a GPIO? > > > Well I suppose it depends on the LED. In the flash case the v4l2 framework disables the sysfs interface for the LED whilst it holds the flash subdev open, which should mean that no nefarious program could turn off the LED whilst it was running the camera but because the sysfs is enabled whilst the v4l2 subdev is closed [1] you could still use that LED as a torch outside of camera streaming. That's probably not a situation that's likely to occur with a privacy LED given they're likely to be much less bright though I suppose, and maybe it's right to treat them differently. > > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-flash-led-class.c#L632 If we only disable the sysfs access temporarily then spy-ware in userspace can still just clear the trigger, so we would need to permanently disable userspace access (or decide to not disable userspace access at all). >> One of the challenges with using LED triggers for the privacy led, >> is that we need at least 2 triggers: "camera-front" and "camera-back" >> and then somehow to let what ever code sets the triggers know if >> it is dealing with the front or back sensor. > > > Yes, that is a problem, my plan was to connect them with fwnode and ancillary links, in the same way for example we connected the VCM to the cameras. I think that the int3472-discrete driver would have to do that. Which would involve a bunch of non trivial code. Where as if we just model the LED as a GPIO for the sensor-driver to consume we get this for free. My conclusion here is: 1. We don't want userspace access because we don't want to make things easier for spy-ware. 2. Without userspace access there is no added value in using the LED subsystem and just modelling this as a GPIO is easier / more KISS. >> Where as with GPIO-s we *bind* them to the sensor i2c_client so if >> we just have the sensor-driver look for an optional GPIO called >> "privacy-led" then we don't have this how to we bind the LED to >> the sensor problem; and if we drop the sysfs interface I fail to >> see the value in using the LED subsystem for GPIO a driven LED. >> >> Also see my other reply for a proposal to be able to share the >> code dealing with this between sensor drivers (and also remove >> some other gpio/clk/regulator boilerplate from sensor drivers). > > > Yes I certainly find that idea appealing, there is of lot of boilerplate that could be reduced with that idea. I glad you like the idea. Any suggestions for a better name for the helper lib / namespace ? Regards, Hans
