Hi Pavel,
> El 4 jun 2020, a las 15:40, Pavel Machek <pavel@xxxxxx> escribió:
>
> 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
Yes, the controller supports two different blinking delays (intervals), and LEDs can either be on, off or assigned to one of the two blinking delays.
However, the current upstream controller only supports one blinking delay. This patch just adds support for both blinking delays instead of just one.
>
>>> El 4 jun 2020, a las 15:24, Pavel Machek <pavel@xxxxxx> 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 <noltari@xxxxxxxxx>
>>>> ---
>>>> 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
