| Message ID | 20231004162317.163488-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | platform/x86: int3472: don't use gpiod_toggle_active_low() | expand |
On Wed, Oct 4, 2023 at 6:23 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Switch to devm_get_gpiod() for discrete GPIOs for clks / regulators / LEDs > and let devm do the cleanup for us. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../x86/intel/int3472/clk_and_regulator.c | 23 ++++--------------- > drivers/platform/x86/intel/int3472/discrete.c | 2 +- > drivers/platform/x86/intel/int3472/led.c | 7 +----- > 3 files changed, 6 insertions(+), 26 deletions(-) > > diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c > index 459f96c04ca1..16e36ac0a7b4 100644 > --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c > +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c > @@ -177,10 +177,8 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, > > init.name = kasprintf(GFP_KERNEL, "%s-clk", > acpi_dev_name(int3472->adev)); > - if (!init.name) { > - ret = -ENOMEM; > - goto out_put_gpio; > - } > + if (!init.name) > + return -ENOMEM; > > int3472->clock.frequency = skl_int3472_get_clk_frequency(int3472); > > @@ -206,8 +204,6 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, > clk_unregister(int3472->clock.clk); > out_free_init_name: > kfree(init.name); > -out_put_gpio: > - gpiod_put(int3472->clock.ena_gpio); > > return ret; > } > @@ -219,7 +215,6 @@ void skl_int3472_unregister_clock(struct int3472_discrete_device *int3472) > > clkdev_drop(int3472->clock.cl); > clk_unregister(int3472->clock.clk); > - gpiod_put(int3472->clock.ena_gpio); > } > > /* > @@ -266,7 +261,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, > struct regulator_config cfg = { }; > const char *second_sensor = NULL; > const struct dmi_system_id *id; > - int i, j, ret; > + int i, j; > > id = dmi_first_match(skl_int3472_regulator_second_sensor); > if (id) > @@ -309,21 +304,11 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, > int3472->regulator.rdev = regulator_register(int3472->dev, > &int3472->regulator.rdesc, > &cfg); > - if (IS_ERR(int3472->regulator.rdev)) { > - ret = PTR_ERR(int3472->regulator.rdev); > - goto err_free_gpio; > - } > > - return 0; > - > -err_free_gpio: > - gpiod_put(int3472->regulator.gpio); > - > - return ret; > + return PTR_ERR_OR_ZERO(int3472->regulator.rdev); > } > > void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472) > { > regulator_unregister(int3472->regulator.rdev); > - gpiod_put(int3472->regulator.gpio); > } > diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c > index 0bc7cbefd9ae..07b302e09340 100644 > --- a/drivers/platform/x86/intel/int3472/discrete.c > +++ b/drivers/platform/x86/intel/int3472/discrete.c > @@ -119,7 +119,7 @@ skl_int3472_gpiod_get_from_temp_lookup(struct int3472_discrete_device *int3472, > return ERR_PTR(ret); > > gpiod_add_lookup_table(lookup); > - desc = gpiod_get(int3472->dev, func, GPIOD_OUT_LOW); > + desc = devm_gpiod_get(int3472->dev, func, GPIOD_OUT_LOW); > gpiod_remove_lookup_table(lookup); > > return desc; > diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c > index 476cd637fc51..9cbed694e2ca 100644 > --- a/drivers/platform/x86/intel/int3472/led.c > +++ b/drivers/platform/x86/intel/int3472/led.c > @@ -39,7 +39,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp > > ret = led_classdev_register(int3472->dev, &int3472->pled.classdev); > if (ret) > - goto err_free_gpio; > + return ret; > > int3472->pled.lookup.provider = int3472->pled.name; > int3472->pled.lookup.dev_id = int3472->sensor_name; > @@ -47,10 +47,6 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp > led_add_lookup(&int3472->pled.lookup); > > return 0; > - > -err_free_gpio: > - gpiod_put(int3472->pled.gpio); > - return ret; > } > > void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472) > @@ -60,5 +56,4 @@ void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472) > > led_remove_lookup(&int3472->pled.lookup); > led_classdev_unregister(&int3472->pled.classdev); > - gpiod_put(int3472->pled.gpio); > } > -- > 2.41.0 > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Hi All, Here is v3 of Bartosz' and my "don't use gpiod_toggle_active_low()" series. New in v3: - Correctly preserve Bartosz as author for 2 of the patches - Add Reviewed-by tags from Mika and Andy - Fix wrong size of gpio lookup table allocation in new skl_int3472_gpiod_get_from_temp_lookup() helper. There must also be an empty terminating lookup, so 2 entries must be allocated. New in v2: - Rework to deal with ACPI path vs gpiod_lookup.key differences: acpi_get_handle(path) -> acpi_fetch_acpi_dev(handle) -> acpi_dev_name(adev) Regards, Hans Bartosz Golaszewski (2): platform/x86: int3472: Add new skl_int3472_gpiod_get_from_temp_lookup() helper gpio: acpi: remove acpi_get_and_request_gpiod() Hans de Goede (3): platform/x86: int3472: Add new skl_int3472_fill_gpiod_lookup() helper platform/x86: int3472: Stop using gpiod_toggle_active_low() platform/x86: int3472: Switch to devm_get_gpiod() drivers/gpio/gpiolib-acpi.c | 28 ----- .../x86/intel/int3472/clk_and_regulator.c | 54 ++-------- drivers/platform/x86/intel/int3472/common.h | 7 +- drivers/platform/x86/intel/int3472/discrete.c | 101 ++++++++++++++---- drivers/platform/x86/intel/int3472/led.c | 24 +---- include/linux/gpio/consumer.h | 8 -- 6 files changed, 93 insertions(+), 129 deletions(-)