| Message ID | 20221129231149.697154-6-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ov5693/int3472: Privacy LED handling changes + IPU6 compatibility | expand |
On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@redhat.com> wrote: > > acpi_get_and_request_gpiod() does not take a gpio_lookup_flags argument > specifying that the pins direction should be initialized to a specific > value. > > This means that in some cases the pins might be left in input mode, causing > the gpiod_set() calls made to enable the clk / regulator to not work. > > One example of this problem is the clk-enable GPIO for the ov01a1s sensor > on a Dell Latitude 9420 being left in input mode causing the clk to > never get enabled. > > Explicitly set the direction of the pins to output to fix this. ... > + /* Ensure the pin is in output mode */ ...in output mode and non-active state */ > + gpiod_direction_output(int3472->clock.ena_gpio, 0); ... > + /* Ensure the pin is in output mode */ > + gpiod_direction_output(int3472->regulator.gpio, 0); So, previously it was AS IS and now it's non-active state. I believe this makes no regressions for the other laptops / platforms.
Hi, On 11/30/22 10:59, Andy Shevchenko wrote: > On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> acpi_get_and_request_gpiod() does not take a gpio_lookup_flags argument >> specifying that the pins direction should be initialized to a specific >> value. >> >> This means that in some cases the pins might be left in input mode, causing >> the gpiod_set() calls made to enable the clk / regulator to not work. >> >> One example of this problem is the clk-enable GPIO for the ov01a1s sensor >> on a Dell Latitude 9420 being left in input mode causing the clk to >> never get enabled. >> >> Explicitly set the direction of the pins to output to fix this. > > ... > >> + /* Ensure the pin is in output mode */ > > ...in output mode and non-active state */ Ack. >> + gpiod_direction_output(int3472->clock.ena_gpio, 0); > > ... > >> + /* Ensure the pin is in output mode */ >> + gpiod_direction_output(int3472->regulator.gpio, 0); > > So, previously it was AS IS and now it's non-active state. I believe > this makes no regressions for the other laptops / platforms. Correct, and no regression intended indeed. I'll add the improved comment when merging this (unless there are other / bigger reasons for a v2). Regards, Hans
diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c index 89894ec873f2..dc4ab7821b56 100644 --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c @@ -103,6 +103,9 @@ int skl_int3472_register_clock(struct int3472_discrete_device *int3472, return dev_err_probe(int3472->dev, ret, "getting regulator GPIO\n"); } + /* Ensure the pin is in output mode */ + gpiod_direction_output(int3472->clock.ena_gpio, 0); + init.name = kasprintf(GFP_KERNEL, "%s-clk", acpi_dev_name(int3472->adev)); if (!init.name) { @@ -195,6 +198,9 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, return PTR_ERR(int3472->regulator.gpio); } + /* Ensure the pin is in output mode */ + gpiod_direction_output(int3472->regulator.gpio, 0); + cfg.dev = &int3472->adev->dev; cfg.init_data = &init_data; cfg.ena_gpiod = int3472->regulator.gpio;
acpi_get_and_request_gpiod() does not take a gpio_lookup_flags argument specifying that the pins direction should be initialized to a specific value. This means that in some cases the pins might be left in input mode, causing the gpiod_set() calls made to enable the clk / regulator to not work. One example of this problem is the clk-enable GPIO for the ov01a1s sensor on a Dell Latitude 9420 being left in input mode causing the clk to never get enabled. Explicitly set the direction of the pins to output to fix this. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/intel/int3472/clk_and_regulator.c | 6 ++++++ 1 file changed, 6 insertions(+)