diff mbox series

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

Message ID 20220427184513.21192-1-eajames@linux.ibm.com
State New
Headers show
Series [linux,dev-5.15] leds: pca955x: Set blink duty cycle to fifty percent | expand

Commit Message

Eddie James April 27, 2022, 6:45 p.m. UTC
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.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/leds/leds-pca955x.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Paul Menzel April 28, 2022, 3:12 p.m. UTC | #1
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) {
Eddie James May 2, 2022, 1:35 p.m. UTC | #2
On 4/28/22 10:12, Paul Menzel wrote:
> 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?


It is in the PCA955X datasheets 
(https://www.nxp.com/docs/en/data-sheet/PCA9552.pdf) but it's not 
exactly clear. The relationship between the PWM rate and the blink rate 
is not specified, but for example, setting maximum duty cycle and 1 
second blink period results in the LED not blinking. Setting 50% duty 
cycle blinks the LED at 1 second intervals, as expected.


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


Ah ok. Thanks!

Eddie


>
>
> 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) {
diff mbox series

Patch

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) {