| Message ID | 20231004162317.163488-4-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | 53c5f7f6e7930ff057cefe4960f5bbf29023e5a9 |
| 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: > > Use the new skl_int3472_gpiod_get_from_temp_lookup() helper to get > a gpio to pass to register_gpio_clock(), skl_int3472_register_regulator() > and skl_int3472_register_pled(). > > This removes all use of the deprecated gpiod_toggle_active_low() and > acpi_get_and_request_gpiod() functions. > > Suggested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > 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 | 31 ++----------- > drivers/platform/x86/intel/int3472/common.h | 7 ++- > drivers/platform/x86/intel/int3472/discrete.c | 43 +++++++++++++------ > drivers/platform/x86/intel/int3472/led.c | 17 ++------ > 4 files changed, 40 insertions(+), 58 deletions(-) > > diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c > index ef4b3141efcd..459f96c04ca1 100644 > --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c > +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c > @@ -162,9 +162,8 @@ int skl_int3472_register_dsm_clock(struct int3472_discrete_device *int3472) > } > > int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio, u32 polarity) > + struct gpio_desc *gpio) > { > - char *path = agpio->resource_source.string_ptr; > struct clk_init_data init = { > .ops = &skl_int3472_clock_ops, > .flags = CLK_GET_RATE_NOCACHE, > @@ -174,19 +173,7 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, > if (int3472->clock.cl) > return -EBUSY; > > - int3472->clock.ena_gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], > - "int3472,clk-enable"); > - if (IS_ERR(int3472->clock.ena_gpio)) { > - ret = PTR_ERR(int3472->clock.ena_gpio); > - int3472->clock.ena_gpio = NULL; > - return dev_err_probe(int3472->dev, ret, "getting clk-enable GPIO\n"); > - } > - > - if (polarity == GPIO_ACTIVE_LOW) > - gpiod_toggle_active_low(int3472->clock.ena_gpio); > - > - /* Ensure the pin is in output mode and non-active state */ > - gpiod_direction_output(int3472->clock.ena_gpio, 0); > + int3472->clock.ena_gpio = gpio; > > init.name = kasprintf(GFP_KERNEL, "%s-clk", > acpi_dev_name(int3472->adev)); > @@ -273,9 +260,8 @@ static const struct dmi_system_id skl_int3472_regulator_second_sensor[] = { > }; > > int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio) > + struct gpio_desc *gpio) > { > - char *path = agpio->resource_source.string_ptr; > struct regulator_init_data init_data = { }; > struct regulator_config cfg = { }; > const char *second_sensor = NULL; > @@ -314,16 +300,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, > int3472->regulator.supply_name, > &int3472_gpio_regulator_ops); > > - int3472->regulator.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], > - "int3472,regulator"); > - if (IS_ERR(int3472->regulator.gpio)) { > - ret = PTR_ERR(int3472->regulator.gpio); > - int3472->regulator.gpio = NULL; > - return dev_err_probe(int3472->dev, ret, "getting regulator GPIO\n"); > - } > - > - /* Ensure the pin is in output mode and non-active state */ > - gpiod_direction_output(int3472->regulator.gpio, 0); > + int3472->regulator.gpio = gpio; > > cfg.dev = &int3472->adev->dev; > cfg.init_data = &init_data; > diff --git a/drivers/platform/x86/intel/int3472/common.h b/drivers/platform/x86/intel/int3472/common.h > index 9f29baa13860..145dec66df64 100644 > --- a/drivers/platform/x86/intel/int3472/common.h > +++ b/drivers/platform/x86/intel/int3472/common.h > @@ -117,16 +117,15 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev, > const char **name_ret); > > int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio, u32 polarity); > + struct gpio_desc *gpio); > int skl_int3472_register_dsm_clock(struct int3472_discrete_device *int3472); > void skl_int3472_unregister_clock(struct int3472_discrete_device *int3472); > > int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio); > + struct gpio_desc *gpio); > void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472); > > -int skl_int3472_register_pled(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio, u32 polarity); > +int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio); > void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472); > > #endif > diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c > index b69ef63f75ab..0bc7cbefd9ae 100644 > --- a/drivers/platform/x86/intel/int3472/discrete.c > +++ b/drivers/platform/x86/intel/int3472/discrete.c > @@ -194,6 +194,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares, > struct acpi_resource_gpio *agpio; > u8 active_value, pin, type; > union acpi_object *obj; > + struct gpio_desc *gpio; > const char *err_msg; > const char *func; > u32 polarity; > @@ -244,22 +245,38 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares, > > break; > case INT3472_GPIO_TYPE_CLK_ENABLE: > - ret = skl_int3472_register_gpio_clock(int3472, agpio, polarity); > - if (ret) > - err_msg = "Failed to register clock\n"; > - > - break; > case INT3472_GPIO_TYPE_PRIVACY_LED: > - ret = skl_int3472_register_pled(int3472, agpio, polarity); > - if (ret) > - err_msg = "Failed to register LED\n"; > - > - break; > case INT3472_GPIO_TYPE_POWER_ENABLE: > - ret = skl_int3472_register_regulator(int3472, agpio); > - if (ret) > - err_msg = "Failed to map regulator to sensor\n"; > + gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, func, polarity); > + if (IS_ERR(gpio)) { > + ret = PTR_ERR(gpio); > + err_msg = "Failed to get GPIO\n"; > + break; > + } > > + switch (type) { My brain refused to parse the diff, I needed to apply the patch to understand what it's doing. I can't say I'm a fan of this double switch construct but I don't have a better idea either, so... Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > + case INT3472_GPIO_TYPE_CLK_ENABLE: > + ret = skl_int3472_register_gpio_clock(int3472, gpio); > + if (ret) > + err_msg = "Failed to register clock\n"; > + > + break; > + case INT3472_GPIO_TYPE_PRIVACY_LED: > + ret = skl_int3472_register_pled(int3472, gpio); > + if (ret) > + err_msg = "Failed to register LED\n"; > + > + break; > + case INT3472_GPIO_TYPE_POWER_ENABLE: > + ret = skl_int3472_register_regulator(int3472, gpio); > + if (ret) > + err_msg = "Failed to map regulator to sensor\n"; > + > + break; > + default: /* Never reached */ > + ret = -EINVAL; > + break; > + } > break; > default: > dev_warn(int3472->dev, > diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c > index bca1ce7d0d0c..476cd637fc51 100644 > --- a/drivers/platform/x86/intel/int3472/led.c > +++ b/drivers/platform/x86/intel/int3472/led.c > @@ -16,26 +16,15 @@ static int int3472_pled_set(struct led_classdev *led_cdev, > return 0; > } > > -int skl_int3472_register_pled(struct int3472_discrete_device *int3472, > - struct acpi_resource_gpio *agpio, u32 polarity) > +int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio) > { > - char *p, *path = agpio->resource_source.string_ptr; > + char *p; > int ret; > > if (int3472->pled.classdev.dev) > return -EBUSY; > > - int3472->pled.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], > - "int3472,privacy-led"); > - if (IS_ERR(int3472->pled.gpio)) > - return dev_err_probe(int3472->dev, PTR_ERR(int3472->pled.gpio), > - "getting privacy LED GPIO\n"); > - > - if (polarity == GPIO_ACTIVE_LOW) > - gpiod_toggle_active_low(int3472->pled.gpio); > - > - /* Ensure the pin is in output mode and non-active state */ > - gpiod_direction_output(int3472->pled.gpio, 0); > + int3472->pled.gpio = gpio; > > /* Generate the name, replacing the ':' in the ACPI devname with '_' */ > snprintf(int3472->pled.name, sizeof(int3472->pled.name), > -- > 2.41.0 >
diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c index ef4b3141efcd..459f96c04ca1 100644 --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c @@ -162,9 +162,8 @@ int skl_int3472_register_dsm_clock(struct int3472_discrete_device *int3472) } int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio, u32 polarity) + struct gpio_desc *gpio) { - char *path = agpio->resource_source.string_ptr; struct clk_init_data init = { .ops = &skl_int3472_clock_ops, .flags = CLK_GET_RATE_NOCACHE, @@ -174,19 +173,7 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, if (int3472->clock.cl) return -EBUSY; - int3472->clock.ena_gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], - "int3472,clk-enable"); - if (IS_ERR(int3472->clock.ena_gpio)) { - ret = PTR_ERR(int3472->clock.ena_gpio); - int3472->clock.ena_gpio = NULL; - return dev_err_probe(int3472->dev, ret, "getting clk-enable GPIO\n"); - } - - if (polarity == GPIO_ACTIVE_LOW) - gpiod_toggle_active_low(int3472->clock.ena_gpio); - - /* Ensure the pin is in output mode and non-active state */ - gpiod_direction_output(int3472->clock.ena_gpio, 0); + int3472->clock.ena_gpio = gpio; init.name = kasprintf(GFP_KERNEL, "%s-clk", acpi_dev_name(int3472->adev)); @@ -273,9 +260,8 @@ static const struct dmi_system_id skl_int3472_regulator_second_sensor[] = { }; int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio) + struct gpio_desc *gpio) { - char *path = agpio->resource_source.string_ptr; struct regulator_init_data init_data = { }; struct regulator_config cfg = { }; const char *second_sensor = NULL; @@ -314,16 +300,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, int3472->regulator.supply_name, &int3472_gpio_regulator_ops); - int3472->regulator.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], - "int3472,regulator"); - if (IS_ERR(int3472->regulator.gpio)) { - ret = PTR_ERR(int3472->regulator.gpio); - int3472->regulator.gpio = NULL; - return dev_err_probe(int3472->dev, ret, "getting regulator GPIO\n"); - } - - /* Ensure the pin is in output mode and non-active state */ - gpiod_direction_output(int3472->regulator.gpio, 0); + int3472->regulator.gpio = gpio; cfg.dev = &int3472->adev->dev; cfg.init_data = &init_data; diff --git a/drivers/platform/x86/intel/int3472/common.h b/drivers/platform/x86/intel/int3472/common.h index 9f29baa13860..145dec66df64 100644 --- a/drivers/platform/x86/intel/int3472/common.h +++ b/drivers/platform/x86/intel/int3472/common.h @@ -117,16 +117,15 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev, const char **name_ret); int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio, u32 polarity); + struct gpio_desc *gpio); int skl_int3472_register_dsm_clock(struct int3472_discrete_device *int3472); void skl_int3472_unregister_clock(struct int3472_discrete_device *int3472); int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio); + struct gpio_desc *gpio); void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472); -int skl_int3472_register_pled(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio, u32 polarity); +int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio); void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472); #endif diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c index b69ef63f75ab..0bc7cbefd9ae 100644 --- a/drivers/platform/x86/intel/int3472/discrete.c +++ b/drivers/platform/x86/intel/int3472/discrete.c @@ -194,6 +194,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares, struct acpi_resource_gpio *agpio; u8 active_value, pin, type; union acpi_object *obj; + struct gpio_desc *gpio; const char *err_msg; const char *func; u32 polarity; @@ -244,22 +245,38 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares, break; case INT3472_GPIO_TYPE_CLK_ENABLE: - ret = skl_int3472_register_gpio_clock(int3472, agpio, polarity); - if (ret) - err_msg = "Failed to register clock\n"; - - break; case INT3472_GPIO_TYPE_PRIVACY_LED: - ret = skl_int3472_register_pled(int3472, agpio, polarity); - if (ret) - err_msg = "Failed to register LED\n"; - - break; case INT3472_GPIO_TYPE_POWER_ENABLE: - ret = skl_int3472_register_regulator(int3472, agpio); - if (ret) - err_msg = "Failed to map regulator to sensor\n"; + gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, func, polarity); + if (IS_ERR(gpio)) { + ret = PTR_ERR(gpio); + err_msg = "Failed to get GPIO\n"; + break; + } + switch (type) { + case INT3472_GPIO_TYPE_CLK_ENABLE: + ret = skl_int3472_register_gpio_clock(int3472, gpio); + if (ret) + err_msg = "Failed to register clock\n"; + + break; + case INT3472_GPIO_TYPE_PRIVACY_LED: + ret = skl_int3472_register_pled(int3472, gpio); + if (ret) + err_msg = "Failed to register LED\n"; + + break; + case INT3472_GPIO_TYPE_POWER_ENABLE: + ret = skl_int3472_register_regulator(int3472, gpio); + if (ret) + err_msg = "Failed to map regulator to sensor\n"; + + break; + default: /* Never reached */ + ret = -EINVAL; + break; + } break; default: dev_warn(int3472->dev, diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c index bca1ce7d0d0c..476cd637fc51 100644 --- a/drivers/platform/x86/intel/int3472/led.c +++ b/drivers/platform/x86/intel/int3472/led.c @@ -16,26 +16,15 @@ static int int3472_pled_set(struct led_classdev *led_cdev, return 0; } -int skl_int3472_register_pled(struct int3472_discrete_device *int3472, - struct acpi_resource_gpio *agpio, u32 polarity) +int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio) { - char *p, *path = agpio->resource_source.string_ptr; + char *p; int ret; if (int3472->pled.classdev.dev) return -EBUSY; - int3472->pled.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0], - "int3472,privacy-led"); - if (IS_ERR(int3472->pled.gpio)) - return dev_err_probe(int3472->dev, PTR_ERR(int3472->pled.gpio), - "getting privacy LED GPIO\n"); - - if (polarity == GPIO_ACTIVE_LOW) - gpiod_toggle_active_low(int3472->pled.gpio); - - /* Ensure the pin is in output mode and non-active state */ - gpiod_direction_output(int3472->pled.gpio, 0); + int3472->pled.gpio = gpio; /* Generate the name, replacing the ':' in the ACPI devname with '_' */ snprintf(int3472->pled.name, sizeof(int3472->pled.name),