leds: triggers: gpio: Rewrite to use trigger-sources

By providing a GPIO line as "trigger-sources" in the FWNODE
(such as from the device tree) and combining with the
GPIO trigger, we can support a GPIO LED trigger in a natural
way from the hardware description instead of using the
custom sysfs and deprecated global GPIO numberspace.

Example:

gpio: gpio@0 {
    compatible "my-gpio";
    gpio-controller;
    #gpio-cells = <2>;
    interrupt-controller;
    #interrupt-cells = <2>;
    #trigger-source-cells = <2>;
};

leds {
    compatible = "gpio-leds";
    led-my-gpio {
        label = "device:blue:myled";
        gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
        default-state = "off";
        linux,default-trigger = "gpio";
        trigger-sources = <&gpio 1 GPIO_ACTIVE_HIGH>;
    };
};

Make this the norm, unmark the driver as broken.

Delete the sysfs handling of GPIOs.

Since GPIO descriptors inherently can describe inversion,
the inversion handling can just be deleted.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org
Signed-off-by: Lee Jones <lee@kernel.org>

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 2a57328eca2077180c07517609d775dd0167bfa8..d11d80176fc0fd271a859bc2da333d42e8f7992d 100644 (file)
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -83,13 +83,10 @@ config LEDS_TRIGGER_ACTIVITY
 config LEDS_TRIGGER_GPIO
        tristate "LED GPIO Trigger"
        depends on GPIOLIB || COMPILE_TEST
-       depends on BROKEN
        help
          This allows LEDs to be controlled by gpio events. It's good
          when using gpios as switches and triggering the needed LEDs
-         from there. One use case is n810's keypad LEDs that could
-         be triggered by this trigger when user slides up to show
-         keypad.
+         from there. Triggers are defined as device properties.
 
          If unsure, say N.
 

diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
index 0120faa3dafa644f61d94d544b47400c5f4744c2..9b7fe5dd52088f1456bd3657614f589ab3c4ceb9 100644 (file)
--- a/drivers/leds/trigger/ledtrig-gpio.c
+++ b/drivers/leds/trigger/ledtrig-gpio.c
@@ -3,12 +3,13 @@
  * ledtrig-gio.c - LED Trigger Based on GPIO events
  *
  * Copyright 2009 Felipe Balbi <me@felipebalbi.com>
+ * Copyright 2023 Linus Walleij <linus.walleij@linaro.org>
  */
 
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/leds.h>
 #include <linux/slab.h>
@@ -16,10 +17,8 @@
 
 struct gpio_trig_data {
        struct led_classdev *led;
-
        unsigned desired_brightness;    /* desired brightness when led is on */
-       unsigned inverted;              /* true when gpio is inverted */
-       unsigned gpio;                  /* gpio that triggers the leds */
+       struct gpio_desc *gpiod;        /* gpio that triggers the led */
 };
 
 static irqreturn_t gpio_trig_irq(int irq, void *_led)
@@ -28,10 +27,7 @@ static irqreturn_t gpio_trig_irq(int irq, void *_led)
        struct gpio_trig_data *gpio_data = led_get_trigger_data(led);
        int tmp;
 
-       tmp = gpio_get_value_cansleep(gpio_data->gpio);
-       if (gpio_data->inverted)
-               tmp = !tmp;
-
+       tmp = gpiod_get_value_cansleep(gpio_data->gpiod);
        if (tmp) {
                if (gpio_data->desired_brightness)
                        led_set_brightness_nosleep(gpio_data->led,
@@ -73,93 +69,8 @@ static ssize_t gpio_trig_brightness_store(struct device *dev,
 static DEVICE_ATTR(desired_brightness, 0644, gpio_trig_brightness_show,
                gpio_trig_brightness_store);
 
-static ssize_t gpio_trig_inverted_show(struct device *dev,
-               struct device_attribute *attr, char *buf)
-{
-       struct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);
-
-       return sprintf(buf, "%u\n", gpio_data->inverted);
-}
-
-static ssize_t gpio_trig_inverted_store(struct device *dev,
-               struct device_attribute *attr, const char *buf, size_t n)
-{
-       struct led_classdev *led = led_trigger_get_led(dev);
-       struct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);
-       unsigned long inverted;
-       int ret;
-
-       ret = kstrtoul(buf, 10, &inverted);
-       if (ret < 0)
-               return ret;
-
-       if (inverted > 1)
-               return -EINVAL;
-
-       gpio_data->inverted = inverted;
-
-       /* After inverting, we need to update the LED. */
-       if (gpio_is_valid(gpio_data->gpio))
-               gpio_trig_irq(0, led);
-
-       return n;
-}
-static DEVICE_ATTR(inverted, 0644, gpio_trig_inverted_show,
-               gpio_trig_inverted_store);
-
-static ssize_t gpio_trig_gpio_show(struct device *dev,
-               struct device_attribute *attr, char *buf)
-{
-       struct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);
-
-       return sprintf(buf, "%u\n", gpio_data->gpio);
-}
-
-static ssize_t gpio_trig_gpio_store(struct device *dev,
-               struct device_attribute *attr, const char *buf, size_t n)
-{
-       struct led_classdev *led = led_trigger_get_led(dev);
-       struct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);
-       unsigned gpio;
-       int ret;
-
-       ret = sscanf(buf, "%u", &gpio);
-       if (ret < 1) {
-               dev_err(dev, "couldn't read gpio number\n");
-               return -EINVAL;
-       }
-
-       if (gpio_data->gpio == gpio)
-               return n;
-
-       if (!gpio_is_valid(gpio)) {
-               if (gpio_is_valid(gpio_data->gpio))
-                       free_irq(gpio_to_irq(gpio_data->gpio), led);
-               gpio_data->gpio = gpio;
-               return n;
-       }
-
-       ret = request_threaded_irq(gpio_to_irq(gpio), NULL, gpio_trig_irq,
-                       IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_RISING
-                       | IRQF_TRIGGER_FALLING, "ledtrig-gpio", led);
-       if (ret) {
-               dev_err(dev, "request_irq failed with error %d\n", ret);
-       } else {
-               if (gpio_is_valid(gpio_data->gpio))
-                       free_irq(gpio_to_irq(gpio_data->gpio), led);
-               gpio_data->gpio = gpio;
-               /* After changing the GPIO, we need to update the LED. */
-               gpio_trig_irq(0, led);
-       }
-
-       return ret ? ret : n;
-}
-static DEVICE_ATTR(gpio, 0644, gpio_trig_gpio_show, gpio_trig_gpio_store);
-
 static struct attribute *gpio_trig_attrs[] = {
        &dev_attr_desired_brightness.attr,
-       &dev_attr_inverted.attr,
-       &dev_attr_gpio.attr,
        NULL
 };
 ATTRIBUTE_GROUPS(gpio_trig);
@@ -167,16 +78,48 @@ ATTRIBUTE_GROUPS(gpio_trig);
 static int gpio_trig_activate(struct led_classdev *led)
 {
        struct gpio_trig_data *gpio_data;
+       struct device *dev = led->dev;
+       int ret;
 
        gpio_data = kzalloc(sizeof(*gpio_data), GFP_KERNEL);
        if (!gpio_data)
                return -ENOMEM;
 
-       gpio_data->led = led;
-       gpio_data->gpio = -ENOENT;
+       /*
+        * The generic property "trigger-sources" is followed,
+        * and we hope that this is a GPIO.
+        */
+       gpio_data->gpiod = fwnode_gpiod_get_index(dev->fwnode,
+                                                 "trigger-sources",
+                                                 0, GPIOD_IN,
+                                                 "led-trigger");
+       if (IS_ERR(gpio_data->gpiod)) {
+               ret = PTR_ERR(gpio_data->gpiod);
+               kfree(gpio_data);
+               return ret;
+       }
+       if (!gpio_data->gpiod) {
+               dev_err(dev, "no valid GPIO for the trigger\n");
+               kfree(gpio_data);
+               return -EINVAL;
+       }
 
+       gpio_data->led = led;
        led_set_trigger_data(led, gpio_data);
 
+       ret = request_threaded_irq(gpiod_to_irq(gpio_data->gpiod), NULL, gpio_trig_irq,
+                       IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_RISING
+                       | IRQF_TRIGGER_FALLING, "ledtrig-gpio", led);
+       if (ret) {
+               dev_err(dev, "request_irq failed with error %d\n", ret);
+               gpiod_put(gpio_data->gpiod);
+               kfree(gpio_data);
+               return ret;
+       }
+
+       /* Finally update the LED to initial status */
+       gpio_trig_irq(0, led);
+
        return 0;
 }
 
@@ -184,8 +127,8 @@ static void gpio_trig_deactivate(struct led_classdev *led)
 {
        struct gpio_trig_data *gpio_data = led_get_trigger_data(led);
 
-       if (gpio_is_valid(gpio_data->gpio))
-               free_irq(gpio_to_irq(gpio_data->gpio), led);
+       free_irq(gpiod_to_irq(gpio_data->gpiod), led);
+       gpiod_put(gpio_data->gpiod);
        kfree(gpio_data);
 }