On Wed, Nov 30, 2022 at 11:34:57AM +0100, Hans de Goede wrote: > On 11/30/22 10:54, Andy Shevchenko wrote: > > On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >> On some systems, e.g. the Lenovo ThinkPad X1 Yoga gen 7 and the ThinkPad > >> X1 Nano gen 2 there is no clock-enable pin, triggering the: > >> "No clk GPIO. The privacy LED won't work" warning and causing the privacy > >> LED to not work. > >> > >> Fix this by treating the privacy LED as a regular GPIO rather then > >> integrating it with the registered clock. > >> > >> Note this relies on the ov5693 driver change to support an (optional) > >> privacy-led GPIO to avoid the front cam privacy LED regressing on some > >> models. > > > > ... > > > >> - case INT3472_GPIO_TYPE_PRIVACY_LED: > >> - gpio = acpi_get_and_request_gpiod(path, pin, "int3472,privacy-led"); > >> - if (IS_ERR(gpio)) > >> - return (PTR_ERR(gpio)); > >> > >> - int3472->clock.led_gpio = gpio; > >> - break; > > > > I'm not sure how the previous patch makes this one work without > > regressions. We have a "privacy-led" GPIO name there and here it used > > to be with a prefix. Maybe I'm missing something... > > The GPIO used to be controlled as part of the clk-provider, > and the "int3472,privacy-led" name was the name of the consumer > of the GPIO shown in /sys/kernel/debug/gpio. The "int3472,privacy-led" > name has no lookup meaning since the pin is directly looked up by > GPIO chip ACPI path + pin offset here. > > Since not all devices with a privacy LED also have a clk-enable GPIO > and thus a clk provider this did not work anywhere. > > So this patch removes the code which controls the privacy LED > through the clk-provider (which used the "int3472,privacy-led" > and instead now adds an entry to the GPIO lookup table attached > to the sensor. That new GPIO lookup table entry uses the name > "privacy-led" since the LED no now longer is controlled by > the INT3472 code (*). The matching sensor driver patch > (patch 1/6) to make the sensor driver directly control the > privacy-led also uses "privacy-led" when calling gpiod_get() > for it. > > I hope this helps explain. Definitely, thanks! > *) all the INT3472 code now does is add the lookup table entry > gpio lookup table -- With Best Regards, Andy Shevchenko