Re: [RFC] led-class: always implement blinking

[Andrew Morton]

On Fri, 20 Aug 2010 11:21:42 +0200
Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:

> +static int led_blink_set(struct led_classdev *led_cdev,
> +			 unsigned long *delay_on, unsigned long *delay_off)
> +{
> +	int current_brightness;
> +
> +	current_brightness = led_get_brightness(led_cdev);
> +	if (current_brightness)
> +		led_cdev->blink_brightness = current_brightness;
> +	if (!led_cdev->blink_brightness)
> +		led_cdev->blink_brightness = led_cdev->max_brightness;
> +
> +	if (*delay_on == led_cdev->blink_delay_on &&
> +	    *delay_off == led_cdev->blink_delay_off)
> +		return 0;
> +
> +	/* deactivate previous settings */
> +	del_timer_sync(&led_cdev->blink_timer);
> +
> +	led_cdev->blink_delay_on = *delay_on;
> +	led_cdev->blink_delay_off = *delay_off;
> +
> +	/* never on - don't blink */
> +	if (!*delay_on)
> +		return 0;
> +
> +	/* never off - just set to brightness */
> +	if (!*delay_off) {
> +		led_set_brightness(led_cdev, led_cdev->blink_brightness);
> +		return 0;
> +	}
> +
> +	mod_timer(&led_cdev->blink_timer, jiffies + 1);
> +
> +	return 0;
> +}

delay_on and delay_off could have been pass-by-value rather than
pass-by-reference?  That would clean up some gunk in callers, too.

If there was some reason for doing it with pass-by-reference then that
reason should have been documented!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/