Re: [PATCH v3 4/5] platform/x86: int3472: Switch to devm_get_gpiod()

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, Oct 4, 2023 at 6:23 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxx>