Re: [PATCH v3 10/10] leds: pca995x: Add support for group brightness control

[Lee Jones <lee@kernel.org>]

--- checkpatch.pl: clean (0 issues) ---

On Tue, 22 Sep 2026, Nora Schiffer wrote:

> When LEDs are set to PWM mode with group control enabled, their
> brightness can control using a global "group PWM" setting, modulating
> the individual LEDs' brightness with a second PWM running at a different
> frequency. This lowers the minimum duty cycle from 1/256 to 1/65536
> (averaged over the modulated signal). Group brightness control is
> particularly useful to adjust for different levels of ambient light.
>
> For simplicity, group PWM mode is always enabled, with the reset default
> of 255 as group brightness. This reduces the effective duty cycle by
> 1/256 at all individual brightness levels (the individual PWM signals
> are modulated with the 255/256 duty cycle group PWM), which should be
> imperceptible.
>
> Signed-off-by: Nora Schiffer <nora.schiffer@ew.tq-group.com>
> ---
>  drivers/leds/leds-pca995x.c | 46 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> index 61d2581b976ce..d6438b576b186 100644
> --- a/drivers/leds/leds-pca995x.c
> +++ b/drivers/leds/leds-pca995x.c
> @@ -35,6 +35,7 @@
>  #define PCA995X_LED_OFF			0x0
>  #define PCA995X_LED_ON			0x1
>  #define PCA995X_LED_PWM_MODE		0x2
> +#define PCA995X_LED_PWM_MODE_GRP	0x3
>  #define PCA995X_LDRX_MASK		0x3
>  #define PCA995X_LDRX_BITS		2
>
> @@ -52,6 +53,7 @@
>  struct pca995x_chipdef {
>  	unsigned int num_leds;
>  	u8 pwm_base;
> +	u8 grppwm;
>  	u8 irefall;
>  	u8 eflag_base;
>  };
> @@ -59,6 +61,7 @@ struct pca995x_chipdef {
>  static const struct pca995x_chipdef pca9952_chipdef = {
>  	.num_leds	= 16,
>  	.pwm_base	= 0x0a,
> +	.grppwm		= 0x08,
>  	.irefall	= 0x43,
>  	.eflag_base	= 0x44,
>  };
> @@ -66,6 +69,7 @@ static const struct pca995x_chipdef pca9952_chipdef = {
>  static const struct pca995x_chipdef pca9955b_chipdef = {
>  	.num_leds	= 16,
>  	.pwm_base	= 0x08,
> +	.grppwm		= 0x06,
>  	.irefall	= 0x45,
>  	.eflag_base	= 0x46,
>  };
> @@ -73,6 +77,7 @@ static const struct pca995x_chipdef pca9955b_chipdef = {
>  static const struct pca995x_chipdef pca9956b_chipdef = {
>  	.num_leds	= 24,
>  	.pwm_base	= 0x0a,
> +	.grppwm		= 0x08,
>  	.irefall	= 0x40,
>  	.eflag_base	= 0x41,
>  };
> @@ -114,11 +119,10 @@ static int pca995x_brightness_set(struct led_classdev *led_cdev,
>
>  	/*
>  	 * Change LDRx configuration to individual brightness via PWM.
> -	 * LED will stop blinking if it's doing so.
>  	 */
>  	return regmap_update_bits(chip->regmap, ledout_addr,
>  				  PCA995X_LDRX_MASK << shift,
> -				  PCA995X_LED_PWM_MODE << shift);
> +				  PCA995X_LED_PWM_MODE_GRP << shift);

If we use group PWM mode to scale brightness, what happens when 'brightness' is
'LED_FULL'?  The 'switch' statement above sets 'PCA995X_LED_ON', which typically
bypasses PWM entirely.  Won't this cause an LED set to 255 to ignore the group
brightness and be fully on, while an LED at 254 is scaled down?

>  }
>
>  static ssize_t status_show(struct device *dev, struct device_attribute *attr, char *buf)
> @@ -190,10 +194,41 @@ static ssize_t has_errors_store(struct device *dev, struct device_attribute *att
>  	return ret ?: count;
>  }
>
> +static ssize_t group_brightness_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct pca995x_chip *chip = i2c_get_clientdata(to_i2c_client(dev));

'dev_get_drvdata(dev)' again?

> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(chip->regmap, chip->chipdef->grppwm, &val);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", val);
> +}
> +
> +static ssize_t group_brightness_store(struct device *dev, struct device_attribute *attr,
> +				       const char *buf, size_t count)
> +{
> +	struct pca995x_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
> +	u8 val;
> +	int ret;
> +
> +	ret = kstrtou8(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(chip->regmap, chip->chipdef->grppwm, val);
> +
> +	return ret ?: count;
> +}
> +
>  static DEVICE_ATTR_RW(has_errors);
> +static DEVICE_ATTR_RW(group_brightness);
>
>  static struct attribute *pca995x_attrs[] = {
>  	&dev_attr_has_errors.attr,
> +	&dev_attr_group_brightness.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(pca995x);
> @@ -270,11 +305,16 @@ static int pca995x_probe(struct i2c_client *client)
>  	if (ret)
>  		goto err_put_nodes;
>
> -	/* Clear errors on probe */
> +	/* Clear errors on probe, use GRPPWM register for group brightness control */
>  	ret = regmap_write(chip->regmap, PCA995X_MODE2, PCA995X_MODE2_CFG);
>  	if (ret)
>  		goto err_put_nodes;
>
> +	/* Full group brightness */
> +	ret = regmap_write(chip->regmap, chipdef->grppwm, U8_MAX);
> +	if (ret)
> +		goto err_put_nodes;
> +
>  	/* IREF Output current value for all LEDn outputs */
>  	ret = regmap_write(chip->regmap, chipdef->irefall, iref);
>  	if (ret)
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
>
>

-- 
Lee Jones