Re: [PATCH v2] leds-bcm6328: support second hw blinking interval

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 4627 bytes --]

Hi!

> 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@gmail.com>
> ---
>  v2: remove LED from the other interval
> 
>  drivers/leds/leds-bcm6328.c | 56 ++++++++++++++++++++++++++-----------
>  1 file changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> index 42e1b7598c3a..a5a57a8d2a1c 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,7 +131,8 @@ 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);
> +	led->blink_leds[0] &= ~BIT(led->pin);
> +	led->blink_leds[1] &= ~BIT(led->pin);
>  	if ((led->active_low && value == LED_OFF) ||
>  	    (!led->active_low && value != LED_OFF))
>  		bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
> @@ -176,20 +181,37 @@ 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) {
> +	if (led->blink_leds[0] == 0 ||
> +	    led->blink_leds[0] == BIT(led->pin) ||
> +	    led->blink_delay[0] == delay) {
>  		unsigned long val;
>  
> -		*(led->blink_leds) |= BIT(led->pin);
> -		*(led->blink_delay) = delay;
> +		led->blink_leds[0] |= BIT(led->pin);
> +		led->blink_leds[1] &= ~BIT(led->pin);
> +		led->blink_delay[0] = delay;
>  
>  		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_INTV1_MASK;
> +		val |= (delay << BCM6328_LED_INTV1_SHIFT);
>  		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
>  
> -		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
> +		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[0] &= ~BIT(led->pin);
> +		led->blink_leds[1] |= BIT(led->pin);
> +		led->blink_delay[1] = delay;
> +
> +		val = bcm6328_led_read(led->mem + BCM6328_REG_INIT);
> +		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_INTV2);
>  		rc = 0;
>  	} else {
>  		dev_dbg(led_cdev->dev,
> @@ -358,11 +380,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;
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]