merez@codeaurora.org writes:

On 2016-04-25 21:56, Kalle Valo wrote:

quoted

Maya Erez [off-list ref] writes:

quoted

Add the ability to configure the device led to be used for notifying
the AP activity (60G device supports leds 0-2).
The host can also configure the blinking frequency of the led in
three states.

[...]

quoted

+/* led_blink_time, write:
+ * "<blink_on_slow> <blink_off_slow> <blink_on_med> <blink_off_med>

<blink_on_fast> <blink_off_fast>

quoted

+ */
+static ssize_t wil_write_led_blink_time(struct file *file,
+					const char __user *buf,
+					size_t len, loff_t *ppos)
+{
+	int rc;
+	char *kbuf = kmalloc(len + 1, GFP_KERNEL);
+
+	if (!kbuf)
+		return -ENOMEM;
+
+	rc = simple_write_to_buffer(kbuf, len, ppos, buf, len);
+	if (rc != len) {
+		kfree(kbuf);
+		return rc >= 0 ? -EIO : rc;
+	}
+
+	kbuf[len] = '\0';
+	rc = sscanf(kbuf, "%d %d %d %d %d %d",
+		    &led_blink_time[WIL_LED_TIME_SLOW].on_ms,
+		    &led_blink_time[WIL_LED_TIME_SLOW].off_ms,
+		    &led_blink_time[WIL_LED_TIME_MED].on_ms,
+		    &led_blink_time[WIL_LED_TIME_MED].off_ms,
+		    &led_blink_time[WIL_LED_TIME_FAST].on_ms,
+		    &led_blink_time[WIL_LED_TIME_FAST].off_ms);
+	kfree(kbuf);
+
+	if (rc < 0)
+		return rc;
+	if (rc < 6)
+		return -EINVAL;
+
+	return len;
+}

Don't we already have a proper framework for leds? At least
include/linux/led.h and drivers/led/ makes me suspect that. I'm not
really fond of the idea reinventing the wheel, unless there's a really
good reason.

The kernel led framework provides the host the ability to toggle a
defined led, but in our case the led toggling is fully offloaded to
the device. The host only configures the device with the led to
activate and its parameters. I agree that reusing an existing
framework should be enforced when possible, but as the WIL6210 host
needs to align to the device led configuration we cannot reuse the led
kernel framework.

Ok, thanks for checking. I'll apply the patch now.

-- 
Kalle Valo