linux-kernel - Re: [PATCH V9 2/3] leds: pwm: Add optional GPIO enable pin support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251113155343.GM1949330@google.com>
Date: Thu, 13 Nov 2025 15:53:43 +0000
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 V9 2/3] leds: pwm: Add optional GPIO enable pin support

On Wed, 05 Nov 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 | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index c73134e7b951..aa9e14d912bf 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -9,6 +9,7 @@
>   * based on leds-gpio.c by Raphael Assenat <raph@...com>
>   */
>  
> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
> @@ -26,6 +27,7 @@ struct led_pwm {
>  };
>  
>  struct led_pwm_data {
> +	struct gpio_desc	*enable_gpio;
>  	struct led_classdev	cdev;
>  	struct pwm_device	*pwm;
>  	struct pwm_state	pwmstate;
> @@ -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);

Nit: Too many spaces.

Didn't checkpatch.pl pick this up?

The rest of the set looks okay.

> +
>  	led_dat->pwmstate.duty_cycle = duty;
>  	/*
>  	 * Disabling a PWM doesn't guarantee that it emits the inactive level.
> @@ -132,6 +136,21 @@ 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);
> +	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 [李琼斯]