| Message ID | 20221128214408.165726-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | gpio/media/int3472: Add support for tps68470 privacy-LED output | expand |
On Mon, Nov 28, 2022 at 10:44 PM Hans de Goede <hdegoede@redhat.com> wrote: > Patches 1-2: 2 small bugfixes to the gpio-tps68470 code Good, please merge this. > Patch3: Add support for the indicator LED outputs on the tps68470 as GPIOs > Patch4: Add support for a privacy LED to the ov8865 sensor driver > Patch5: Add gpio-lookup table entry for the privacy LED. OK so I have to call out the hippo in the room: these "gpios" are not called "gpios" anywhere else than in this patch. General purpose input/output, remember. These are special purpose LED control registers. So can you provide a good explanation why these registers aren't handled in the drivers/led subsystem instead? IIUC the leds subsystem has gained support for leds as resources. I don't mind a LED driver inside of the GPIO driver if that is what it takes as a compromise, just that it should be handled by the right subsystem. Given that flash leds which are used by cameras are already in drivers/leds/flash this should be no different and there will be more cameras with these privacy leds. Yours, Linus Walleij
Hi, On 12/3/22 10:32, Linus Walleij wrote: > On Mon, Nov 28, 2022 at 10:44 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> Patches 1-2: 2 small bugfixes to the gpio-tps68470 code > > Good, please merge this. > >> Patch3: Add support for the indicator LED outputs on the tps68470 as GPIOs >> Patch4: Add support for a privacy LED to the ov8865 sensor driver >> Patch5: Add gpio-lookup table entry for the privacy LED. > > OK so I have to call out the hippo in the room: > > these "gpios" are not called "gpios" anywhere else than in this > patch. General purpose input/output, remember. These are special > purpose LED control registers. > > So can you provide a good explanation why these registers aren't > handled in the drivers/led subsystem instead? This was discussed in another thread: https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/ There were 2 problems identified which has lead to the current direction of just modelling at as an (output only) GPIO: 1. The LED class allows userspace control of the LED which is bad from a privacy pov. This will make it easy, too easy (was the conclusion) for spy-ware to turn on the camera without the LED turning on. Later in the thread it was pointed out that there is a flag to suspend userspace control, we could use this to permanently disable userspace control which I guess would be some what of a solution, although we would then also need to look into disallow changing triggers, because I think those could still be used as a way around this. 2. GPIO(s) can be tied directly to the device so that on a device with both front and back privacy-LEDs (real world example) doing gpiod_get(dev, "privacy-led") gets us the right privacy-led, where as with LED class devices tying the sensor and LED class device is going to be tricky. > IIUC the leds subsystem has gained support for leds as resources. Interesting that would mitigate problem 2 from above and since people keep circling back to "its a LED please use the LED class", this is definitely worth looking into. Do you have any pointers / examples about led class devices as resources? ### Note though that these indicator LED outputs, both functionality wise as well as at the register level of this PMIC only support turning them on/off. So this maps pretty well to the GPIO subsystem and all the functionality of the LED class subsystem is mostly functionality which we want to avoid since we don't want userspace control, neither directly through sysfs or by attaching triggers. So this does map pretty well to just modelling it as a GPIO, if we model this as a LED then we end up having to workaround a bunch of stuff the LED subsytem does which we do not want in this case. And this may even require patches to the LED subsystem to disallow userspace changing the trigger (I would need to check). So from my pov modelling this as an output-only GPIO pin is actually a more KISS solution then involving the LED subsystem... > I don't mind a LED driver inside of the GPIO driver if that is what > it takes as a compromise, just that it should be handled by the right > subsystem. The PMIC already is a MFD device, so if we go the LED class route we can just add a separate MFD child device for the new LED driver to bind to. > Given that flash leds which are used by cameras are already in > drivers/leds/flash this should be no different and there will be more > cameras with these privacy leds. Actually this patch is for the back camera privacy LED on a Microsoft Surface Go tablet. The front camera privacy LED is directly attached to a GPIO of the main SoC. So for that camera just adding a GPIO lookup table entry to map the ACPI provided GPIO info to a "privacy-led" GPIO on the sensor i2c_client device (which we already do for the "reset" and "powerdown" gpios) also by far is the most KISS approach. Doing things this way in the code translating the ACPI "magic" to standard Linux device-model stuff is literary a single line of code (add an extra case: to an existing list of cases in a switch-case). Where as instantiating a LED class device for this and then somehow tying that to the i2c_client for the sensor will be more code. So again treating these on/off only LEDs, where we want to *disallow* userspace control, as a GPIO is by far the most KISS solution. Regards, Hans
Hi Hans, On Mon, Dec 05, 2022 at 04:01:20PM +0100, Hans de Goede wrote: > On 12/3/22 13:28, Hans de Goede wrote: > > On 12/3/22 10:32, Linus Walleij wrote: > >> On Mon, Nov 28, 2022 at 10:44 PM Hans de Goede wrote: > >> > >>> Patches 1-2: 2 small bugfixes to the gpio-tps68470 code > >> > >> Good, please merge this. > >> > >>> Patch3: Add support for the indicator LED outputs on the tps68470 as GPIOs > >>> Patch4: Add support for a privacy LED to the ov8865 sensor driver > >>> Patch5: Add gpio-lookup table entry for the privacy LED. > >> > >> OK so I have to call out the hippo in the room: > >> > >> these "gpios" are not called "gpios" anywhere else than in this > >> patch. General purpose input/output, remember. These are special > >> purpose LED control registers. > >> > >> So can you provide a good explanation why these registers aren't > >> handled in the drivers/led subsystem instead? > > > > This was discussed in another thread: > > > > https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/ > > > > There were 2 problems identified which has lead to the current > > direction of just modelling at as an (output only) GPIO: > > > > 1. The LED class allows userspace control of the LED which is > > bad from a privacy pov. This will make it easy, too easy > > (was the conclusion) for spy-ware to turn on the camera without > > the LED turning on. > > > > Later in the thread it was pointed out that there is a flag to > > suspend userspace control, we could use this to permanently disable > > userspace control which I guess would be some what of a solution, > > although we would then also need to look into disallow changing > > triggers, because I think those could still be used as a way around > > this. > > I have spend today looking into the feasibility of using the LED > class subsystem instead of modelling these on/off only LEDs as a GPIO. > > Good news, there is a LED_SYSFS_DISABLE flag which also stops > userspace from messing with the trigger of the LED, so this > first issue can easily be fixed. > > > 2. GPIO(s) can be tied directly to the device so that on a device > > with both front and back privacy-LEDs (real world example) doing > > gpiod_get(dev, "privacy-led") gets us the right privacy-led, > > where as with LED class devices tying the sensor and LED class > > device is going to be tricky. > > > >> IIUC the leds subsystem has gained support for leds as resources. > > > > Interesting that would mitigate problem 2 from above and since > > people keep circling back to "its a LED please use the LED class", > > this is definitely worth looking into. > > > > Do you have any pointers / examples about led class devices as > > resources? > > I have been looking into this, but atm the only way to tie a > led-classdev to a device is through a fwnode reference. > > Since this is x86 where there is no DTS file where we can > easily add this, I have been looking into doing this with > swnode-s. > > LED directly attached to main SoC GPIO > ====================================== > > For the simple LED is attached to a GPIO on the main > SoC case, this requires 2 steps: > > 1. Have the INT3472 code register a LED classdev for the > privacy-led instead of a GPIO lookup table entry. This LED > classdev must have a swnode as fwnode, so that we can put > a reference to that swnode in a "leds" reference-array > property on the sensor i2c_client. This is about a 100 > lines of extra code and seems fine / doable. > > 2. Add a "leds" reference-array property on the > i2c_client device by adding a swnode with this property > to the i2c_client device. This sounds straight forward > (once we have the swnode for the LED class-device to point to) > but this is actually not straight forward at all. > > There is a whole bunch of properties which needs to be > added on the sensor to describe the media-graph between > the sensor and the IPU, as well as what VCM (if any) is paired > up with the sensor. These properties are all added through > adding a swnode from the CSI bridge driver: > > drivers/media/pci/intel/ipu3/cio2-bridge.c > > But a device can only have one swnode added. So we cannot > add a swnode to the sensor i2c_client in the INT3472 code. > > Instead the only thing which we could do is give the swnode > for the privacy LED classdev a predictable name, derived > from the sensor's device name and then > have drivers/media/pci/intel/ipu3/cio2-bridge.c > call software_node_find_by_name() to get the swnode and > have it add the "leds" reference-array property on the > i2c_client device for the sensor. > > However the INT3472 code is shared between multiple ISP/IPU > implementations, so then we would need to duplicate this code > for the other IPU versions (currently IPU6 which is out of tree), > further complicating things. > > And this is for the direct usage of a SoC GPIO case. > > > LED attached to TPS68470 PMIC indicator LED pin > =============================================== > > In this case the LED-classdev should be instantiated > by a driver for a new TPS68470 MFD cell. But this > also introduces a bunch of probe ordering systems, > so modelling things as a LED classdev here would involve: > > 1. Making the IN3472 code create + register a swnode for > the LED classdev, this must be done here because of > probe ordering. > > 2. Make the IN3472 code create a new TPS68470 MFD cell > and pass the swnode as fwnode to this cell. > > 3. Write a new driver for the new TPS68470 MFD cell, > which registers a LED classdev using the fwnode from > the MFD cell as fwnode for the LED classdev. > > > And this still does not solve the issue of how to get > the privacy-LED as LED classdev model to work on the IPU6. > > Alternative approach > ==================== > > An alternative approach, would be to add support for LED > lookup tables to the LED class code (like we already have > for GPIOs) and use this to allow tying a LED classdev to > a struct device on non devicetree platforms. > > Given the problems with the swnode approach from above > I believe that this would actually be better then > the swnode approach. > > Lookup tables like this use device-names, so we don't need > to have swnode-s ready for both the provider and the consumer > at the time of adding the lookup table entry. Instead all > that is necessary is to know the device-names of both > the provider and the consumer which are both known in > advance. Thank you for all this research. > Is this really worth all the trouble ? > ====================================== > > So I really have to wonder what is using the LED > classdev / framework actually buying us over using > modelling these on/off only LEDs as a GPIO ? > > I know that some (x86) have a flash-LED for the back > camera and given the experience with trying to tie > a LED class dev to a specific struct device (to the > sensor's i2c_client) I guess we are eventually going to > need some sort of lookup tables for tying LED class > devices to a specific device anyways. Probably, but for those, the effort will pay off better, as we need to control the flash from userspace. For the privacy LED, I doubt we'll ever seen a system where we'll have to control it through more than an enable bit (for instance, an RGB or intensity-controlled privacy LED sounds very unlikely), so a GPIO is fine with me. > That and we want to avoid moving from the current > approach (for some INT3472 using devices) of tying > the privacy LED on/off to the INT3472 registered > clk being enabled/disabled to modelling this as > GPIOs, to then later modelling it as LED class > devices after all. > > To avoid this double conversion issue I'm going to > give the LED class route a second go, replacing > the swnode approach which I tried today with > a lookup-table approach. > > > ### > > > > Note though that these indicator LED outputs, both functionality > > wise as well as at the register level of this PMIC only support > > turning them on/off. So this maps pretty well to the GPIO subsystem > > and all the functionality of the LED class subsystem is mostly > > functionality which we want to avoid since we don't want userspace > > control, neither directly through sysfs or by attaching triggers. > > > > So this does map pretty well to just modelling it as a GPIO, > > if we model this as a LED then we end up having to workaround > > a bunch of stuff the LED subsytem does which we do not want in > > this case. And this may even require patches to the LED subsystem > > to disallow userspace changing the trigger (I would need to check). > > > > So from my pov modelling this as an output-only GPIO pin is > > actually a more KISS solution then involving the LED subsystem... > > > >> I don't mind a LED driver inside of the GPIO driver if that is what > >> it takes as a compromise, just that it should be handled by the right > >> subsystem. > > > > The PMIC already is a MFD device, so if we go the LED class route > > we can just add a separate MFD child device for the new LED driver > > to bind to. > > > >> Given that flash leds which are used by cameras are already in > >> drivers/leds/flash this should be no different and there will be more > >> cameras with these privacy leds. > > > > Actually this patch is for the back camera privacy LED on a > > Microsoft Surface Go tablet. The front camera privacy LED is > > directly attached to a GPIO of the main SoC. So for that camera > > just adding a GPIO lookup table entry to map the ACPI provided > > GPIO info to a "privacy-led" GPIO on the sensor i2c_client device > > (which we already do for the "reset" and "powerdown" gpios) also > > by far is the most KISS approach. > > > > Doing things this way in the code translating the ACPI "magic" > > to standard Linux device-model stuff is literary a single line > > of code (add an extra case: to an existing list of cases in a > > switch-case). Where as instantiating a LED class device for this > > and then somehow tying that to the i2c_client for the sensor will > > be more code. > > > > So again treating these on/off only LEDs, where we want to > > *disallow* userspace control, as a GPIO is by far the most KISS > > solution.
Hi, On 12/5/22 22:26, Laurent Pinchart wrote: > Hi Hans, > > On Mon, Dec 05, 2022 at 04:01:20PM +0100, Hans de Goede wrote: >> On 12/3/22 13:28, Hans de Goede wrote: >>> On 12/3/22 10:32, Linus Walleij wrote: >>>> On Mon, Nov 28, 2022 at 10:44 PM Hans de Goede wrote: >>>> >>>>> Patches 1-2: 2 small bugfixes to the gpio-tps68470 code >>>> >>>> Good, please merge this. >>>> >>>>> Patch3: Add support for the indicator LED outputs on the tps68470 as GPIOs >>>>> Patch4: Add support for a privacy LED to the ov8865 sensor driver >>>>> Patch5: Add gpio-lookup table entry for the privacy LED. >>>> >>>> OK so I have to call out the hippo in the room: >>>> >>>> these "gpios" are not called "gpios" anywhere else than in this >>>> patch. General purpose input/output, remember. These are special >>>> purpose LED control registers. >>>> >>>> So can you provide a good explanation why these registers aren't >>>> handled in the drivers/led subsystem instead? >>> >>> This was discussed in another thread: >>> >>> https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/ >>> >>> There were 2 problems identified which has lead to the current >>> direction of just modelling at as an (output only) GPIO: >>> >>> 1. The LED class allows userspace control of the LED which is >>> bad from a privacy pov. This will make it easy, too easy >>> (was the conclusion) for spy-ware to turn on the camera without >>> the LED turning on. >>> >>> Later in the thread it was pointed out that there is a flag to >>> suspend userspace control, we could use this to permanently disable >>> userspace control which I guess would be some what of a solution, >>> although we would then also need to look into disallow changing >>> triggers, because I think those could still be used as a way around >>> this. >> >> I have spend today looking into the feasibility of using the LED >> class subsystem instead of modelling these on/off only LEDs as a GPIO. >> >> Good news, there is a LED_SYSFS_DISABLE flag which also stops >> userspace from messing with the trigger of the LED, so this >> first issue can easily be fixed. >> >>> 2. GPIO(s) can be tied directly to the device so that on a device >>> with both front and back privacy-LEDs (real world example) doing >>> gpiod_get(dev, "privacy-led") gets us the right privacy-led, >>> where as with LED class devices tying the sensor and LED class >>> device is going to be tricky. >>> >>>> IIUC the leds subsystem has gained support for leds as resources. >>> >>> Interesting that would mitigate problem 2 from above and since >>> people keep circling back to "its a LED please use the LED class", >>> this is definitely worth looking into. >>> >>> Do you have any pointers / examples about led class devices as >>> resources? >> >> I have been looking into this, but atm the only way to tie a >> led-classdev to a device is through a fwnode reference. >> >> Since this is x86 where there is no DTS file where we can >> easily add this, I have been looking into doing this with >> swnode-s. >> >> LED directly attached to main SoC GPIO >> ====================================== >> >> For the simple LED is attached to a GPIO on the main >> SoC case, this requires 2 steps: >> >> 1. Have the INT3472 code register a LED classdev for the >> privacy-led instead of a GPIO lookup table entry. This LED >> classdev must have a swnode as fwnode, so that we can put >> a reference to that swnode in a "leds" reference-array >> property on the sensor i2c_client. This is about a 100 >> lines of extra code and seems fine / doable. >> >> 2. Add a "leds" reference-array property on the >> i2c_client device by adding a swnode with this property >> to the i2c_client device. This sounds straight forward >> (once we have the swnode for the LED class-device to point to) >> but this is actually not straight forward at all. >> >> There is a whole bunch of properties which needs to be >> added on the sensor to describe the media-graph between >> the sensor and the IPU, as well as what VCM (if any) is paired >> up with the sensor. These properties are all added through >> adding a swnode from the CSI bridge driver: >> >> drivers/media/pci/intel/ipu3/cio2-bridge.c >> >> But a device can only have one swnode added. So we cannot >> add a swnode to the sensor i2c_client in the INT3472 code. >> >> Instead the only thing which we could do is give the swnode >> for the privacy LED classdev a predictable name, derived >> from the sensor's device name and then >> have drivers/media/pci/intel/ipu3/cio2-bridge.c >> call software_node_find_by_name() to get the swnode and >> have it add the "leds" reference-array property on the >> i2c_client device for the sensor. >> >> However the INT3472 code is shared between multiple ISP/IPU >> implementations, so then we would need to duplicate this code >> for the other IPU versions (currently IPU6 which is out of tree), >> further complicating things. >> >> And this is for the direct usage of a SoC GPIO case. >> >> >> LED attached to TPS68470 PMIC indicator LED pin >> =============================================== >> >> In this case the LED-classdev should be instantiated >> by a driver for a new TPS68470 MFD cell. But this >> also introduces a bunch of probe ordering systems, >> so modelling things as a LED classdev here would involve: >> >> 1. Making the IN3472 code create + register a swnode for >> the LED classdev, this must be done here because of >> probe ordering. >> >> 2. Make the IN3472 code create a new TPS68470 MFD cell >> and pass the swnode as fwnode to this cell. >> >> 3. Write a new driver for the new TPS68470 MFD cell, >> which registers a LED classdev using the fwnode from >> the MFD cell as fwnode for the LED classdev. >> >> >> And this still does not solve the issue of how to get >> the privacy-LED as LED classdev model to work on the IPU6. >> >> Alternative approach >> ==================== >> >> An alternative approach, would be to add support for LED >> lookup tables to the LED class code (like we already have >> for GPIOs) and use this to allow tying a LED classdev to >> a struct device on non devicetree platforms. >> >> Given the problems with the swnode approach from above >> I believe that this would actually be better then >> the swnode approach. >> >> Lookup tables like this use device-names, so we don't need >> to have swnode-s ready for both the provider and the consumer >> at the time of adding the lookup table entry. Instead all >> that is necessary is to know the device-names of both >> the provider and the consumer which are both known in >> advance. > > Thank you for all this research. > >> Is this really worth all the trouble ? >> ====================================== >> >> So I really have to wonder what is using the LED >> classdev / framework actually buying us over using >> modelling these on/off only LEDs as a GPIO ? >> >> I know that some (x86) have a flash-LED for the back >> camera and given the experience with trying to tie >> a LED class dev to a specific struct device (to the >> sensor's i2c_client) I guess we are eventually going to >> need some sort of lookup tables for tying LED class >> devices to a specific device anyways. > > Probably, but for those, the effort will pay off better, as we need to > control the flash from userspace. For the privacy LED, I doubt we'll > ever seen a system where we'll have to control it through more than an > enable bit (for instance, an RGB or intensity-controlled privacy LED > sounds very unlikely), so a GPIO is fine with me. Ok, that is good to know. Going the GPIO route would be a lot more KISS, so that has my preference too. Linus would modelling these simple on/off LEDs as GPIOs be acceptable to you too ? Regards, Hans > >> That and we want to avoid moving from the current >> approach (for some INT3472 using devices) of tying >> the privacy LED on/off to the INT3472 registered >> clk being enabled/disabled to modelling this as >> GPIOs, to then later modelling it as LED class >> devices after all. >> >> To avoid this double conversion issue I'm going to >> give the LED class route a second go, replacing >> the swnode approach which I tried today with >> a lookup-table approach. >> >>> ### >>> >>> Note though that these indicator LED outputs, both functionality >>> wise as well as at the register level of this PMIC only support >>> turning them on/off. So this maps pretty well to the GPIO subsystem >>> and all the functionality of the LED class subsystem is mostly >>> functionality which we want to avoid since we don't want userspace >>> control, neither directly through sysfs or by attaching triggers. >>> >>> So this does map pretty well to just modelling it as a GPIO, >>> if we model this as a LED then we end up having to workaround >>> a bunch of stuff the LED subsytem does which we do not want in >>> this case. And this may even require patches to the LED subsystem >>> to disallow userspace changing the trigger (I would need to check). >>> >>> So from my pov modelling this as an output-only GPIO pin is >>> actually a more KISS solution then involving the LED subsystem... >>> >>>> I don't mind a LED driver inside of the GPIO driver if that is what >>>> it takes as a compromise, just that it should be handled by the right >>>> subsystem. >>> >>> The PMIC already is a MFD device, so if we go the LED class route >>> we can just add a separate MFD child device for the new LED driver >>> to bind to. >>> >>>> Given that flash leds which are used by cameras are already in >>>> drivers/leds/flash this should be no different and there will be more >>>> cameras with these privacy leds. >>> >>> Actually this patch is for the back camera privacy LED on a >>> Microsoft Surface Go tablet. The front camera privacy LED is >>> directly attached to a GPIO of the main SoC. So for that camera >>> just adding a GPIO lookup table entry to map the ACPI provided >>> GPIO info to a "privacy-led" GPIO on the sensor i2c_client device >>> (which we already do for the "reset" and "powerdown" gpios) also >>> by far is the most KISS approach. >>> >>> Doing things this way in the code translating the ACPI "magic" >>> to standard Linux device-model stuff is literary a single line >>> of code (add an extra case: to an existing list of cases in a >>> switch-case). Where as instantiating a LED class device for this >>> and then somehow tying that to the i2c_client for the sensor will >>> be more code. >>> >>> So again treating these on/off only LEDs, where we want to >>> *disallow* userspace control, as a GPIO is by far the most KISS >>> solution. >
Hi, On 12/7/22 01:25, Linus Walleij wrote: > On Mon, Dec 5, 2022 at 4:01 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> An alternative approach, would be to add support for LED >> lookup tables to the LED class code (like we already have >> for GPIOs) and use this to allow tying a LED classdev to >> a struct device on non devicetree platforms. >> >> Given the problems with the swnode approach from above >> I believe that this would actually be better then >> the swnode approach. >> >> Lookup tables like this use device-names, so we don't need >> to have swnode-s ready for both the provider and the consumer >> at the time of adding the lookup table entry. Instead all >> that is necessary is to know the device-names of both >> the provider and the consumer which are both known in >> advance. > > I think this looks like a good idea. > > We attach other resources such as clocks, regulators, > DMA channels, GPIOs exactly this way. So why not LEDs? > > As GPIO maintainer I every now and then get a suggestion > like this to "just let this pass as a GPIO because it makes > my life so much easier", but it is the same curse as the > input subsystem has: it is versatile and well engineered so > it starts to look like a golden hammer (everything start to > look like nails). > > But we are two GPIO maintainers and right now Bartosz > does the majority of the work, and if he thinks it's the best > idea ever I will certainly reconsider. Ok, I will give the LED lookup table approach a go and post a new series replacing this one when its ready. Feel free to drop this series from your queue. (Note given that that is going to be a whole new approach I plan to start the new series at v1 again). Regards, Hans
Hi All, While working on the earlier discussed changes for dealing with the privacy LED for "discrete" INT3472 ACPI devices I was wondering "how are we dealing with the privacy LED when the INT3472 ACPI device is a tps68470?". Well it turns out we were not dealing with this at all, leading to the privacy LED on the back of the Surface Go series not lighting up when the back camera is on. This series fixes this, it consists of: Patches 1-2: 2 small bugfixes to the gpio-tps68470 code Patch3: Add support for the indicator LED outputs on the tps68470 as GPIOs Patch4: Add support for a privacy LED to the ov8865 sensor driver Patch5: Add gpio-lookup table entry for the privacy LED. There is one small issue here, I believe that patches 1-3 need to land before 4 + 5 do. Once 4 + 5 have landed the ov8865 driver will try to get a GPIO with pin number 10 from the gpio-tps68470 provider and without patch 3, that will fail because only pins 0-9 exist until patch 3 lands. The easiest way to avoid this issue would be for me to merge patches 1-3 + 5 through the pdx86 tree. GPIO subsystem maintainers, may I have your ack for this ? Note patch 4 is not a problem without patch 5, it uses gpiod_get_optional, so as long as there is no lookup entry for a "privacy-led" GPIO it is a no-op. Regards, Hans Hans de Goede (5): gpio: tps68470: Fix tps68470_gpio_get() reading from the wrong register gpio: tps68470: Make tps68470_gpio_output() always set the initial value gpio: tps68470: Add support for the indicator LED outputs media: ov8865: Add support for a privacy-led GPIO platform/x86: int3472: Add support for the back privacy LED on Surface Go models drivers/gpio/gpio-tps68470.c | 52 +++++++++++++------ drivers/media/i2c/ov8865.c | 9 ++++ .../x86/intel/int3472/tps68470_board_data.c | 1 + include/linux/mfd/tps68470.h | 4 ++ 4 files changed, 49 insertions(+), 17 deletions(-)