diff mbox series

[08/21] gpio: acpi: provide acpi_gpio_device_free_interrupts()

Message ID 20230905185309.131295-9-brgl@bgdev.pl (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series gpio: convert users to gpio_device_find() and remove gpiochip_find() | expand

Commit Message

Bartosz Golaszewski Sept. 5, 2023, 6:52 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We're moving away from public functions that take struct gpio_chip as
argument as the chip - unlike struct gpio_device - is owned by its
provider and tied to its lifetime.

Provide an alternative to acpi_gpiochip_free_interrupts().

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-acpi.c | 29 +++++++++++++++++++++++------
 include/linux/gpio/driver.h | 12 ++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)

Comments

Mika Westerberg Sept. 6, 2023, 7:10 a.m. UTC | #1
Hi,

On Tue, Sep 05, 2023 at 08:52:56PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> We're moving away from public functions that take struct gpio_chip as
> argument as the chip - unlike struct gpio_device - is owned by its
> provider and tied to its lifetime.
> 
> Provide an alternative to acpi_gpiochip_free_interrupts().

Looks good to me, few minor comments below.

> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

> ---
>  drivers/gpio/gpiolib-acpi.c | 29 +++++++++++++++++++++++------
>  include/linux/gpio/driver.h | 12 ++++++++++++
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index fbda452fb4d6..5633e39396bc 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -558,12 +558,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(acpi_gpiochip_request_interrupts);
>  
> -/**
> - * acpi_gpiochip_free_interrupts() - Free GPIO ACPI event interrupts.
> - * @chip:      GPIO chip
> - *
> - * Free interrupts associated with GPIO ACPI event method for the given
> - * GPIO chip.
> +/*
> + * This function is deprecated, use acpi_gpio_device_free_interrupts()
> + * instead.
>   */
>  void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
>  {
> @@ -604,6 +601,26 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(acpi_gpiochip_free_interrupts);
>  
> +/**
> + * acpi_gpio_device_free_interrupts() - Free GPIO ACPI event interrupts.
> + * @chip	GPIO device

Should be:

@chip: GPIO device

> + *
> + * Free interrupts associated with GPIO ACPI event method for the given
> + * GPIO device.
> + */
> +void acpi_gpio_device_free_interrupts(struct gpio_device *gdev)
> +{
> +	struct gpio_chip *gc;
> +
> +	/* TODO: protect gdev->chip once SRCU is in place in GPIOLIB. */
> +	gc = gdev->chip;
> +	if (!gc)
> +		return;
> +
> +	acpi_gpiochip_free_interrupts(gc);
> +}
> +EXPORT_SYMBOL_GPL(acpi_gpio_device_free_interrupts);
> +
>  int acpi_dev_add_driver_gpios(struct acpi_device *adev,
>  			      const struct acpi_gpio_mapping *gpios)
>  {
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index b68b3493b29d..47906bc56b3d 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -835,4 +835,16 @@ static inline struct fwnode_handle *gpiochip_node_get_first(struct device *dev)
>  	return NULL;
>  }
>  
> +/*
> + * FIXME: Remove this once the only driver that uses it - android tablets -
> + * becomes a good citizen and stops abusing GPIOLIB.

There are a acouple of more when grepping for acpi_gpiochip_free_interrupts().

I'm not entirely sure why these functions are exposed to the drivers in
the first place. IMHO GPIOLIB should deal with these but perhaps there
is some good reason these drivers do it...

> + */
> +#ifdef CONFIG_ACPI
> +void acpi_gpio_device_free_interrupts(struct gpio_device *gdev);
> +#else
> +static inline void acpi_gpio_device_free_interrupts(struct gpio_device *gdev)
> +{
> +}

I would put these {} to the same line:

static inline void acpi_gpio_device_free_interrupts(struct gpio_device *gdev) { }

> +#endif
> +
>  #endif /* __LINUX_GPIO_DRIVER_H */
> -- 
> 2.39.2
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index fbda452fb4d6..5633e39396bc 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -558,12 +558,9 @@  void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 }
 EXPORT_SYMBOL_GPL(acpi_gpiochip_request_interrupts);
 
-/**
- * acpi_gpiochip_free_interrupts() - Free GPIO ACPI event interrupts.
- * @chip:      GPIO chip
- *
- * Free interrupts associated with GPIO ACPI event method for the given
- * GPIO chip.
+/*
+ * This function is deprecated, use acpi_gpio_device_free_interrupts()
+ * instead.
  */
 void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 {
@@ -604,6 +601,26 @@  void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 }
 EXPORT_SYMBOL_GPL(acpi_gpiochip_free_interrupts);
 
+/**
+ * acpi_gpio_device_free_interrupts() - Free GPIO ACPI event interrupts.
+ * @chip	GPIO device
+ *
+ * Free interrupts associated with GPIO ACPI event method for the given
+ * GPIO device.
+ */
+void acpi_gpio_device_free_interrupts(struct gpio_device *gdev)
+{
+	struct gpio_chip *gc;
+
+	/* TODO: protect gdev->chip once SRCU is in place in GPIOLIB. */
+	gc = gdev->chip;
+	if (!gc)
+		return;
+
+	acpi_gpiochip_free_interrupts(gc);
+}
+EXPORT_SYMBOL_GPL(acpi_gpio_device_free_interrupts);
+
 int acpi_dev_add_driver_gpios(struct acpi_device *adev,
 			      const struct acpi_gpio_mapping *gpios)
 {
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index b68b3493b29d..47906bc56b3d 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -835,4 +835,16 @@  static inline struct fwnode_handle *gpiochip_node_get_first(struct device *dev)
 	return NULL;
 }
 
+/*
+ * FIXME: Remove this once the only driver that uses it - android tablets -
+ * becomes a good citizen and stops abusing GPIOLIB.
+ */
+#ifdef CONFIG_ACPI
+void acpi_gpio_device_free_interrupts(struct gpio_device *gdev);
+#else
+static inline void acpi_gpio_device_free_interrupts(struct gpio_device *gdev)
+{
+}
+#endif
+
 #endif /* __LINUX_GPIO_DRIVER_H */