{"id":1839963,"url":"http://patchwork.ozlabs.org/api/patches/1839963/?format=json","web_url":"http://patchwork.ozlabs.org/project/linux-gpio/patch/20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org/","project":{"id":42,"url":"http://patchwork.ozlabs.org/api/projects/42/?format=json","name":"Linux GPIO development","link_name":"linux-gpio","list_id":"linux-gpio.vger.kernel.org","list_email":"linux-gpio@vger.kernel.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org>","list_archive_url":null,"date":"2023-09-26T21:48:13","name":"[v2,3/3] leds: triggers: gpio: Rewrite to use trigger-sources","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"574c21b2770465be3ae399881e48dd41c4690900","submitter":{"id":7055,"url":"http://patchwork.ozlabs.org/api/people/7055/?format=json","name":"Linus Walleij","email":"linus.walleij@linaro.org"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/linux-gpio/patch/20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org/mbox/","series":[{"id":375009,"url":"http://patchwork.ozlabs.org/api/series/375009/?format=json","web_url":"http://patchwork.ozlabs.org/project/linux-gpio/list/?series=375009","date":"2023-09-26T21:48:11","name":"Rewrite GPIO LED trigger to use trigger-sources","version":2,"mbox":"http://patchwork.ozlabs.org/series/375009/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/1839963/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/1839963/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256\n header.s=google header.b=EwC4MXuF;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-gpio-owner@vger.kernel.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby legolas.ozlabs.org (Postfix) with ESMTP id 4RwFFy1KqZz1ypJ\n\tfor ; Wed, 27 Sep 2023 08:45:58 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S230197AbjIZWpz (ORCPT );\n Tue, 26 Sep 2023 18:45:55 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:40992 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S230525AbjIZWny (ORCPT\n ); Tue, 26 Sep 2023 18:43:54 -0400","from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com\n [IPv6:2a00:1450:4864:20::22d])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8180218E83\n for ;\n Tue, 26 Sep 2023 14:48:19 -0700 (PDT)","by mail-lj1-x22d.google.com with SMTP id\n 38308e7fff4ca-2c12ae20a5cso159623261fa.2\n for ;\n Tue, 26 Sep 2023 14:48:19 -0700 (PDT)","from [127.0.1.1] ([85.235.12.238])\n by smtp.gmail.com with ESMTPSA id\n f10-20020a19ae0a000000b0050334e5f5a8sm2299982lfc.271.2023.09.26.14.48.16\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Tue, 26 Sep 2023 14:48:17 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=linaro.org; s=google; t=1695764897; x=1696369697;\n darn=vger.kernel.org;\n h=cc:to:in-reply-to:references:message-id:content-transfer-encoding\n :mime-version:subject:date:from:from:to:cc:subject:date:message-id\n :reply-to;\n bh=xCuTZhEJPz8ZqCyb/xY7ocE2o3PkqDSRE/J2TdY26/o=;\n b=EwC4MXuFj2KrB2CN2IakXRhv5gAh+jzTrViJkl4faVqa7/m+CPj2MsgMeoYlfFg3Qi\n Z/2oI2zSMem1yHL/Jl9EyPTjQ7Od0aGyjNcFNIP85BuO9CqUDh54hBLaJKg6H5cp9nY/\n PUdSuVqYxewnn9KlRtU/M1niG5mLWe+CI2hVdqK5NtEu1znrl12OMzk7FZTo0L1esxRq\n JRVCnXFkJTBprlSdbbOypOSCmPkMlEuqrhGwnqr3XhOrIuYSb0A1Vx7hE+DSYhLYBBjV\n wwN4ucX+T8qngUMQt87HlTOwkp4BVoPJ9pCHCg4O/k2KZGrShpjOHaXzlh9XQugO59Rq\n G8Mg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20230601; t=1695764897; x=1696369697;\n h=cc:to:in-reply-to:references:message-id:content-transfer-encoding\n :mime-version:subject:date:from:x-gm-message-state:from:to:cc\n :subject:date:message-id:reply-to;\n bh=xCuTZhEJPz8ZqCyb/xY7ocE2o3PkqDSRE/J2TdY26/o=;\n b=TkUV2lcM7c528Sr8hJBUFegu1CX738pZW48nkEWNOUci3ocNwOklCAr2s/lpGUAlcu\n 7HT8Jt+PxXPFO5uH83TAo9a7HpopbNES195LcPIZrA1l3Bjis/K8MbYQFSjGGM21JB8w\n qXS3didgPHxBDBjo//3xW9IV2Ea8MHOMdcNb7ILGS0MNRNkJoNJ2Aiv/ceU3m9PatIpp\n dkGzV2B/QmNcgp9e7PMzG8gY79knIfbF8o1768xrmzcwnWyjaCFQPI3g0egI6GwzDJMX\n WhJPc3XMv3LwszXuvApTQxcR0NGm99znHk0uTpnOHG4bs3KFcaLMF6xlmEiOzqneQvF7\n htHw==","X-Gm-Message-State":"AOJu0YyYK4UeoDYMRjeYKZNJxgbdzj64/4aPRLxvyg7WaG/Ceb2uKo+a\n tZXpDXqcYpJhECxt0kgh2AfGMg==","X-Google-Smtp-Source":"\n AGHT+IGBrC0bECecJjFp+EqYLonAlaFQLJT/bnX6STpQt3TRBoCa+su9DN31qD3xXpVwLjbs+7PzsA==","X-Received":"by 2002:ac2:58ef:0:b0:503:9c2:e44e with SMTP id\n v15-20020ac258ef000000b0050309c2e44emr16458lfo.55.1695764897557;\n Tue, 26 Sep 2023 14:48:17 -0700 (PDT)","From":"Linus Walleij ","Date":"Tue, 26 Sep 2023 23:48:13 +0200","Subject":"[PATCH v2 3/3] leds: triggers: gpio: Rewrite to use\n trigger-sources","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"7bit","Message-Id":"<20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org>","References":"<20230926-gpio-led-trigger-dt-v2-0-e06e458b788e@linaro.org>","In-Reply-To":"<20230926-gpio-led-trigger-dt-v2-0-e06e458b788e@linaro.org>","To":"=?utf-8?q?Jan_Kundr=C3=A1t?= ,\n Pavel Machek , Lee Jones ,\n Rob Herring ,\n Krzysztof Kozlowski ,\n Conor Dooley ,\n Jacek Anaszewski ","Cc":"linux-leds@vger.kernel.org, linux-gpio@vger.kernel.org,\n devicetree@vger.kernel.org,\n Linus Walleij ","X-Mailer":"b4 0.12.3","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,\n SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no\n version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"},"content":"By providing a GPIO line as \"trigger-sources\" in the FWNODE\n(such as from the device tree) and combining with the\nGPIO trigger, we can support a GPIO LED trigger in a natural\nway from the hardware description instead of using the\ncustom sysfs and deprecated global GPIO numberspace.\n\nExample:\n\ngpio: gpio@0 {\n compatible \"my-gpio\";\n gpio-controller;\n #gpio-cells = <2>;\n interrupt-controller;\n #interrupt-cells = <2>;\n #trigger-source-cells = <2>;\n};\n\nleds {\n compatible = \"gpio-leds\";\n led-my-gpio {\n label = \"device:blue:myled\";\n gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;\n default-state = \"off\";\n linux,default-trigger = \"gpio\";\n trigger-sources = <&gpio 1 GPIO_ACTIVE_HIGH>;\n };\n};\n\nMake this the norm, unmark the driver as broken.\n\nDelete the sysfs handling of GPIOs.\n\nSince GPIO descriptors inherently can describe inversion,\nthe inversion handling can just be deleted.\n\nSigned-off-by: Linus Walleij \n---\nChangeLog v1->v2:\n- Fix a use-after-free bug found by Dan Carpenter\n---\n drivers/leds/trigger/Kconfig | 5 +-\n drivers/leds/trigger/ledtrig-gpio.c | 137 +++++++++++-------------------------\n 2 files changed, 41 insertions(+), 101 deletions(-)","diff":"diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig\nindex 2a57328eca20..d11d80176fc0 100644\n--- a/drivers/leds/trigger/Kconfig\n+++ b/drivers/leds/trigger/Kconfig\n@@ -83,13 +83,10 @@ config LEDS_TRIGGER_ACTIVITY\n config LEDS_TRIGGER_GPIO\n \ttristate \"LED GPIO Trigger\"\n \tdepends on GPIOLIB || COMPILE_TEST\n-\tdepends on BROKEN\n \thelp\n \t This allows LEDs to be controlled by gpio events. It's good\n \t when using gpios as switches and triggering the needed LEDs\n-\t from there. One use case is n810's keypad LEDs that could\n-\t be triggered by this trigger when user slides up to show\n-\t keypad.\n+\t from there. Triggers are defined as device properties.\n \n \t If unsure, say N.\n \ndiff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c\nindex 0120faa3dafa..9b7fe5dd5208 100644\n--- a/drivers/leds/trigger/ledtrig-gpio.c\n+++ b/drivers/leds/trigger/ledtrig-gpio.c\n@@ -3,12 +3,13 @@\n * ledtrig-gio.c - LED Trigger Based on GPIO events\n *\n * Copyright 2009 Felipe Balbi \n+ * Copyright 2023 Linus Walleij \n */\n \n #include \n #include \n #include \n-#include \n+#include \n #include \n #include \n #include \n@@ -16,10 +17,8 @@\n \n struct gpio_trig_data {\n \tstruct led_classdev *led;\n-\n \tunsigned desired_brightness;\t/* desired brightness when led is on */\n-\tunsigned inverted;\t\t/* true when gpio is inverted */\n-\tunsigned gpio;\t\t\t/* gpio that triggers the leds */\n+\tstruct gpio_desc *gpiod;\t/* gpio that triggers the led */\n };\n \n static irqreturn_t gpio_trig_irq(int irq, void *_led)\n@@ -28,10 +27,7 @@ static irqreturn_t gpio_trig_irq(int irq, void *_led)\n \tstruct gpio_trig_data *gpio_data = led_get_trigger_data(led);\n \tint tmp;\n \n-\ttmp = gpio_get_value_cansleep(gpio_data->gpio);\n-\tif (gpio_data->inverted)\n-\t\ttmp = !tmp;\n-\n+\ttmp = gpiod_get_value_cansleep(gpio_data->gpiod);\n \tif (tmp) {\n \t\tif (gpio_data->desired_brightness)\n \t\t\tled_set_brightness_nosleep(gpio_data->led,\n@@ -73,93 +69,8 @@ static ssize_t gpio_trig_brightness_store(struct device *dev,\n static DEVICE_ATTR(desired_brightness, 0644, gpio_trig_brightness_show,\n \t\tgpio_trig_brightness_store);\n \n-static ssize_t gpio_trig_inverted_show(struct device *dev,\n-\t\tstruct device_attribute *attr, char *buf)\n-{\n-\tstruct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);\n-\n-\treturn sprintf(buf, \"%u\\n\", gpio_data->inverted);\n-}\n-\n-static ssize_t gpio_trig_inverted_store(struct device *dev,\n-\t\tstruct device_attribute *attr, const char *buf, size_t n)\n-{\n-\tstruct led_classdev *led = led_trigger_get_led(dev);\n-\tstruct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);\n-\tunsigned long inverted;\n-\tint ret;\n-\n-\tret = kstrtoul(buf, 10, &inverted);\n-\tif (ret < 0)\n-\t\treturn ret;\n-\n-\tif (inverted > 1)\n-\t\treturn -EINVAL;\n-\n-\tgpio_data->inverted = inverted;\n-\n-\t/* After inverting, we need to update the LED. */\n-\tif (gpio_is_valid(gpio_data->gpio))\n-\t\tgpio_trig_irq(0, led);\n-\n-\treturn n;\n-}\n-static DEVICE_ATTR(inverted, 0644, gpio_trig_inverted_show,\n-\t\tgpio_trig_inverted_store);\n-\n-static ssize_t gpio_trig_gpio_show(struct device *dev,\n-\t\tstruct device_attribute *attr, char *buf)\n-{\n-\tstruct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);\n-\n-\treturn sprintf(buf, \"%u\\n\", gpio_data->gpio);\n-}\n-\n-static ssize_t gpio_trig_gpio_store(struct device *dev,\n-\t\tstruct device_attribute *attr, const char *buf, size_t n)\n-{\n-\tstruct led_classdev *led = led_trigger_get_led(dev);\n-\tstruct gpio_trig_data *gpio_data = led_trigger_get_drvdata(dev);\n-\tunsigned gpio;\n-\tint ret;\n-\n-\tret = sscanf(buf, \"%u\", &gpio);\n-\tif (ret < 1) {\n-\t\tdev_err(dev, \"couldn't read gpio number\\n\");\n-\t\treturn -EINVAL;\n-\t}\n-\n-\tif (gpio_data->gpio == gpio)\n-\t\treturn n;\n-\n-\tif (!gpio_is_valid(gpio)) {\n-\t\tif (gpio_is_valid(gpio_data->gpio))\n-\t\t\tfree_irq(gpio_to_irq(gpio_data->gpio), led);\n-\t\tgpio_data->gpio = gpio;\n-\t\treturn n;\n-\t}\n-\n-\tret = request_threaded_irq(gpio_to_irq(gpio), NULL, gpio_trig_irq,\n-\t\t\tIRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_RISING\n-\t\t\t| IRQF_TRIGGER_FALLING, \"ledtrig-gpio\", led);\n-\tif (ret) {\n-\t\tdev_err(dev, \"request_irq failed with error %d\\n\", ret);\n-\t} else {\n-\t\tif (gpio_is_valid(gpio_data->gpio))\n-\t\t\tfree_irq(gpio_to_irq(gpio_data->gpio), led);\n-\t\tgpio_data->gpio = gpio;\n-\t\t/* After changing the GPIO, we need to update the LED. */\n-\t\tgpio_trig_irq(0, led);\n-\t}\n-\n-\treturn ret ? ret : n;\n-}\n-static DEVICE_ATTR(gpio, 0644, gpio_trig_gpio_show, gpio_trig_gpio_store);\n-\n static struct attribute *gpio_trig_attrs[] = {\n \t&dev_attr_desired_brightness.attr,\n-\t&dev_attr_inverted.attr,\n-\t&dev_attr_gpio.attr,\n \tNULL\n };\n ATTRIBUTE_GROUPS(gpio_trig);\n@@ -167,16 +78,48 @@ ATTRIBUTE_GROUPS(gpio_trig);\n static int gpio_trig_activate(struct led_classdev *led)\n {\n \tstruct gpio_trig_data *gpio_data;\n+\tstruct device *dev = led->dev;\n+\tint ret;\n \n \tgpio_data = kzalloc(sizeof(*gpio_data), GFP_KERNEL);\n \tif (!gpio_data)\n \t\treturn -ENOMEM;\n \n-\tgpio_data->led = led;\n-\tgpio_data->gpio = -ENOENT;\n+\t/*\n+\t * The generic property \"trigger-sources\" is followed,\n+\t * and we hope that this is a GPIO.\n+\t */\n+\tgpio_data->gpiod = fwnode_gpiod_get_index(dev->fwnode,\n+\t\t\t\t\t\t \"trigger-sources\",\n+\t\t\t\t\t\t 0, GPIOD_IN,\n+\t\t\t\t\t\t \"led-trigger\");\n+\tif (IS_ERR(gpio_data->gpiod)) {\n+\t\tret = PTR_ERR(gpio_data->gpiod);\n+\t\tkfree(gpio_data);\n+\t\treturn ret;\n+\t}\n+\tif (!gpio_data->gpiod) {\n+\t\tdev_err(dev, \"no valid GPIO for the trigger\\n\");\n+\t\tkfree(gpio_data);\n+\t\treturn -EINVAL;\n+\t}\n \n+\tgpio_data->led = led;\n \tled_set_trigger_data(led, gpio_data);\n \n+\tret = request_threaded_irq(gpiod_to_irq(gpio_data->gpiod), NULL, gpio_trig_irq,\n+\t\t\tIRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_RISING\n+\t\t\t| IRQF_TRIGGER_FALLING, \"ledtrig-gpio\", led);\n+\tif (ret) {\n+\t\tdev_err(dev, \"request_irq failed with error %d\\n\", ret);\n+\t\tgpiod_put(gpio_data->gpiod);\n+\t\tkfree(gpio_data);\n+\t\treturn ret;\n+\t}\n+\n+\t/* Finally update the LED to initial status */\n+\tgpio_trig_irq(0, led);\n+\n \treturn 0;\n }\n \n@@ -184,8 +127,8 @@ static void gpio_trig_deactivate(struct led_classdev *led)\n {\n \tstruct gpio_trig_data *gpio_data = led_get_trigger_data(led);\n \n-\tif (gpio_is_valid(gpio_data->gpio))\n-\t\tfree_irq(gpio_to_irq(gpio_data->gpio), led);\n+\tfree_irq(gpiod_to_irq(gpio_data->gpiod), led);\n+\tgpiod_put(gpio_data->gpiod);\n \tkfree(gpio_data);\n }\n \n","prefixes":["v2","3/3"]}