[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250731104623.GD1049189@google.com>
Date: Thu, 31 Jul 2025 11:46:23 +0100
From: Lee Jones <lee@...nel.org>
To: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
Cc: pavel@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, linux-leds@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V8 2/2] leds: pwm: Add optional GPIO enable pin support
On Thu, 24 Jul 2025, LI Qingwu wrote:
> Add support for optional GPIO-based enable pin control to PWM LED driver.
> Some PWM LED driver chips like TPS92380 and LT3743 require a separate
> enable signal in addition to PWM control. Implement support for such
> GPIO control through the "enable-gpios" device tree property, activating
> the pin when LED brightness is non-zero and deactivating it when off.
>
> Tested on i.MX8MP EVK with TPS92380 LED driver chip
>
> Signed-off-by: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
> ---
> drivers/leds/leds-pwm.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index c73134e7b9514..08a1f735166ad 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -17,6 +17,7 @@
> #include <linux/err.h>
> #include <linux/pwm.h>
> #include <linux/slab.h>
> +#include <linux/gpio/consumer.h>
This list is alphabetical.
> struct led_pwm {
> const char *name;
> @@ -29,6 +30,7 @@ struct led_pwm_data {
> struct led_classdev cdev;
> struct pwm_device *pwm;
> struct pwm_state pwmstate;
> + struct gpio_desc *enable_gpio;
> unsigned int active_low;
> };
>
> @@ -51,6 +53,8 @@ static int led_pwm_set(struct led_classdev *led_cdev,
> if (led_dat->active_low)
> duty = led_dat->pwmstate.period - duty;
>
> + gpiod_set_value_cansleep(led_dat->enable_gpio, brightness == LED_OFF ? 0 : 1);
How about the more succinct:
brightness ? 1 : 0
Or:
!!brightness
> +
> led_dat->pwmstate.duty_cycle = duty;
> /*
> * Disabling a PWM doesn't guarantee that it emits the inactive level.
> @@ -132,6 +136,22 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> break;
> }
>
> + /*
> + * Claim the GPIO as GPIOD_ASIS and set the value
> + * later on to honor the different default states
> + */
> + led_data->enable_gpio = devm_fwnode_gpiod_get(dev, fwnode, "enable", GPIOD_ASIS, NULL);
> +
Remove this line.
> + if (IS_ERR(led_data->enable_gpio)) {
> + if (PTR_ERR(led_data->enable_gpio) == -ENOENT)
> + /* Enable GPIO is optional */
> + led_data->enable_gpio = NULL;
> + else
> + return PTR_ERR(led_data->enable_gpio);
> + }
> +
> + gpiod_direction_output(led_data->enable_gpio, !!led_data->cdev.brightness);
> +
> ret = devm_led_classdev_register_ext(dev, &led_data->cdev, &init_data);
> if (ret) {
> dev_err(dev, "failed to register PWM led for %s: %d\n",
> --
> 2.43.0
>
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists