Re: [PATCH 3/6] platform/x86: int3472/discrete: Treat privacy LED as regular GPIO

[Andy Shevchenko <andy.shevchenko@gmail.com>]

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@redhat.com> 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