Re: [PATCH linux dev-5.15] leds: pca955x: Set blink duty cycle to fifty percent

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Eddie,


Am 27.04.22 um 20:45 schrieb Eddie James:
> In order to blink at the specified rate, the blinking LEDs
> need to maintain a 50% duty cycle. Therefore, don't use PWM0
> for any attempted brightness change, and set PWM0 when
> enabling blinking.

Is that specified in some datasheet?

Nit: I’d write 50 % in the commit message summary.


Kind regards,

Paul


> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>   drivers/leds/leds-pca955x.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index 2570f92b6754..da93f04e4d10 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -289,17 +289,12 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev)
>   
>   	switch (pca955x_ledstate(ls, pca955x_led->led_num % 4)) {
>   	case PCA955X_LS_LED_ON:
> +	case PCA955X_LS_BLINK0:
>   		ret = LED_FULL;
>   		break;
>   	case PCA955X_LS_LED_OFF:
>   		ret = LED_OFF;
>   		break;
> -	case PCA955X_LS_BLINK0:
> -		ret = pca955x_read_pwm(pca955x, 0, &pwm);
> -		if (ret)
> -			return ret;
> -		ret = 256 - pwm;
> -		break;
>   	case PCA955X_LS_BLINK1:
>   		ret = pca955x_read_pwm(pca955x, 1, &pwm);
>   		if (ret)
> @@ -332,7 +327,7 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
>   			clear_bit(pca955x_led->led_num, &pca955x->active_blink);
>   			ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_OFF);
>   		} else {
> -			ret = pca955x_write_pwm(pca955x, 0, 256 - value);
> +			/* No variable brightness for blinking LEDs */
>   			goto out;
>   		}
>   	} else {
> @@ -432,6 +427,14 @@ static int pca955x_led_blink(struct led_classdev *led_cdev,
>   			ret = pca955x_write_ls(pca955x, reg, ls);
>   			if (ret)
>   				goto out;
> +
> +			/*
> +			 * Force 50% duty cycle to maintain the specified
> +			 * blink rate.
> +			 */
> +			ret = pca955x_write_pwm(pca955x, 0, 128);
> +			if (ret)
> +				goto out;
>   		}
>   
>   		if (pca955x->blink_period != p) {