On Thu 2020-06-04 15:35:06, Álvaro Fernández Rojas wrote: > HI Pavel, > > Right now, only one hw blinking interval is supported, but up to two hw blinking intervals can be configured in the controller. > This patch just adds support for the second hw blinking interval. > > BTW, I will send v4 which should clarify that each LED can be configured in 4 modes: > 0: On > 1: HW Blinking (Interval 1) > 2: HW Blinking (Interval 2) > 3: Off > > Until now we were only using 3 modes: > 0: On > 1: HW Blinking (Interval 1) > 3: Off So the issue is .... there are like 5 LEDs and they can be on or off or blinking with at most 2 different intervals? Pavel > > El 4 jun 2020, a las 15:24, Pavel Machek escribió: > > > > On Tue 2020-05-12 12:01:36, Álvaro Fernández Rojas wrote: > >> Add support for both configurable HW blinking intervals. > >> > > > > The code could use ... better documentation and better changelog. What > > is the current blinking interval and what other interval does this > > add? > > > > Best regards, > > Pavel > > > >> Signed-off-by: Álvaro Fernández Rojas > >> --- > >> v3: add code documentation > >> v2: remove LED from the other interval > >> > >> drivers/leds/leds-bcm6328.c | 83 ++++++++++++++++++++++++++++++------- > >> 1 file changed, 67 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c > >> index 42e1b7598c3a..050a77591f0b 100644 > >> --- a/drivers/leds/leds-bcm6328.c > >> +++ b/drivers/leds/leds-bcm6328.c > >> @@ -24,12 +24,16 @@ > >> > >> #define BCM6328_LED_MAX_COUNT 24 > >> #define BCM6328_LED_DEF_DELAY 500 > >> +#define BCM6328_LED_INTERVAL_NUM 2 > >> #define BCM6328_LED_INTERVAL_MS 20 > >> > >> #define BCM6328_LED_INTV_MASK 0x3f > >> -#define BCM6328_LED_FAST_INTV_SHIFT 6 > >> -#define BCM6328_LED_FAST_INTV_MASK (BCM6328_LED_INTV_MASK << \ > >> - BCM6328_LED_FAST_INTV_SHIFT) > >> +#define BCM6328_LED_INTV1_SHIFT 0 > >> +#define BCM6328_LED_INTV1_MASK (BCM6328_LED_INTV_MASK << \ > >> + BCM6328_LED_INTV1_SHIFT) > >> +#define BCM6328_LED_INTV2_SHIFT 6 > >> +#define BCM6328_LED_INTV2_MASK (BCM6328_LED_INTV_MASK << \ > >> + BCM6328_LED_INTV2_SHIFT) > >> #define BCM6328_SERIAL_LED_EN BIT(12) > >> #define BCM6328_SERIAL_LED_MUX BIT(13) > >> #define BCM6328_SERIAL_LED_CLK_NPOL BIT(14) > >> @@ -45,8 +49,8 @@ > >> > >> #define BCM6328_LED_MODE_MASK 3 > >> #define BCM6328_LED_MODE_ON 0 > >> -#define BCM6328_LED_MODE_FAST 1 > >> -#define BCM6328_LED_MODE_BLINK 2 > >> +#define BCM6328_LED_MODE_INTV1 1 > >> +#define BCM6328_LED_MODE_INTV2 2 > >> #define BCM6328_LED_MODE_OFF 3 > >> #define BCM6328_LED_SHIFT(X) ((X) << 1) > >> > >> @@ -127,12 +131,18 @@ static void bcm6328_led_set(struct led_classdev *led_cdev, > >> unsigned long flags; > >> > >> spin_lock_irqsave(led->lock, flags); > >> - *(led->blink_leds) &= ~BIT(led->pin); > >> + > >> + /* Remove LED from cached HW blinking intervals */ > >> + led->blink_leds[0] &= ~BIT(led->pin); > >> + led->blink_leds[1] &= ~BIT(led->pin); > >> + > >> + /* Set LED on/off */ > >> if ((led->active_low && value == LED_OFF) || > >> (!led->active_low && value != LED_OFF)) > >> bcm6328_led_mode(led, BCM6328_LED_MODE_ON); > >> else > >> bcm6328_led_mode(led, BCM6328_LED_MODE_OFF); > >> + > >> spin_unlock_irqrestore(led->lock, flags); > >> } > >> > >> @@ -176,20 +186,59 @@ static int bcm6328_blink_set(struct led_classdev *led_cdev, > >> } > >> > >> spin_lock_irqsave(led->lock, flags); > >> - if (*(led->blink_leds) == 0 || > >> - *(led->blink_leds) == BIT(led->pin) || > >> - *(led->blink_delay) == delay) { > >> + /* > >> + * Check if any of the two configurable HW blinking intervals is > >> + * available: > >> + * 1. No LEDs assigned to the HW blinking interval. > >> + * 2. LEDs with the same delay assigned. > >> + */ > >> + if (led->blink_leds[0] == 0 || > >> + led->blink_leds[0] == BIT(led->pin) || > >> + led->blink_delay[0] == delay) { > >> + unsigned long val; > >> + > >> + /* Add LED to the first HW blinking interval cache */ > >> + led->blink_leds[0] |= BIT(led->pin); > >> + > >> + /* Remove LED from the second HW blinking interval cache */ > >> + led->blink_leds[1] &= ~BIT(led->pin); > >> + > >> + /* Cache the LED in the first HW blinking interval delay */ > >> + led->blink_delay[0] = delay; > >> + > >> + /* Update the delay for the first HW blinking interval */ > >> + val = bcm6328_led_read(led->mem + BCM6328_REG_INIT); > >> + val &= ~BCM6328_LED_INTV1_MASK; > >> + val |= (delay << BCM6328_LED_INTV1_SHIFT); > >> + bcm6328_led_write(led->mem + BCM6328_REG_INIT, val); > >> + > >> + /* Set the LED to first HW blinking interval */ > >> + bcm6328_led_mode(led, BCM6328_LED_MODE_INTV1); > >> + > >> + rc = 0; > >> + } else if (led->blink_leds[1] == 0 || > >> + led->blink_leds[1] == BIT(led->pin) || > >> + led->blink_delay[1] == delay) { > >> unsigned long val; > >> > >> - *(led->blink_leds) |= BIT(led->pin); > >> - *(led->blink_delay) = delay; > >> + /* Remove LED from the first HW blinking interval */ > >> + led->blink_leds[0] &= ~BIT(led->pin); > >> + > >> + /* Add LED to the second HW blinking interval */ > >> + led->blink_leds[1] |= BIT(led->pin); > >> > >> + /* Cache the LED in the first HW blinking interval delay */ > >> + led->blink_delay[1] = delay; > >> + > >> + /* Update the delay for the second HW blinking interval */ > >> val = bcm6328_led_read(led->mem + BCM6328_REG_INIT); > >> - val &= ~BCM6328_LED_FAST_INTV_MASK; > >> - val |= (delay << BCM6328_LED_FAST_INTV_SHIFT); > >> + val &= ~BCM6328_LED_INTV2_MASK; > >> + val |= (delay << BCM6328_LED_INTV2_SHIFT); > >> bcm6328_led_write(led->mem + BCM6328_REG_INIT, val); > >> > >> - bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK); > >> + /* Set the LED to second HW blinking interval */ > >> + bcm6328_led_mode(led, BCM6328_LED_MODE_INTV2); > >> + > >> rc = 0; > >> } else { > >> dev_dbg(led_cdev->dev, > >> @@ -358,11 +407,13 @@ static int bcm6328_leds_probe(struct platform_device *pdev) > >> if (!lock) > >> return -ENOMEM; > >> > >> - blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL); > >> + blink_leds = devm_kcalloc(dev, BCM6328_LED_INTERVAL_NUM, > >> + sizeof(*blink_leds), GFP_KERNEL); > >> if (!blink_leds) > >> return -ENOMEM; > >> > >> - blink_delay = devm_kzalloc(dev, sizeof(*blink_delay), GFP_KERNEL); > >> + blink_delay = devm_kcalloc(dev, BCM6328_LED_INTERVAL_NUM, > >> + sizeof(*blink_delay), GFP_KERNEL); > >> if (!blink_delay) > >> return -ENOMEM; > >> > >> -- > >> 2.26.2 > > > > -- > > (english) http://www.livejournal.com/~pavelmachek > > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html