On Thu, 7 Jan 2010, Rick L. Vinyard, Jr. wrote:
> >> +static ssize_t g13_mled_store(struct device *dev,
> >> + struct device_attribute *attr,
> >> + const char *buf, size_t count)
> >> +{
> >> + struct hid_device *hdev;
> >> + int i;
> >> + unsigned m[4];
> >> + unsigned mled;
> >> + ssize_t set_result;
> >> +
> >> + /* Get the hid associated with the device */
> >> + hdev = container_of(dev, struct hid_device, dev);
> >> +
> >> + /* If we have an invalid pointer we'll return ENODATA */
> >> + if (hdev == NULL || &(hdev->dev) != dev)
> >> + return -ENODATA;
> >> +
> >> + i = sscanf(buf, "%u %u %u %u", m, m+1, m+2, m+3);
> >> + if (!(i == 4 || i == 1)) {
> >> + printk(KERN_ERR "unrecognized input: %s", buf);
> >> + return -1;
> >> + }
> >> +
> >> + if (i == 1)
> >> + mled = m[0];
> >> + else
> >> + mled = (m[0] ? 1 : 0) | (m[1] ? 2 : 0) |
> >> + (m[2] ? 4 : 0) | (m[3] ? 8 : 0);
> >> +
> >> + set_result = g13_set_mled(hdev, mled);
> >> +
> >> + if (set_result < 0)
> >> + return set_result;
> >> +
> >> + return count;
> >> +}
> >> +
> >> +static DEVICE_ATTR(mled, 0666, g13_mled_show, g13_mled_store);
> >
> > Have you considered the use of the LED class driver as an alternative
> > to introducing these sysfs led controls for the device?
> >
>
> I did, but this seemed a simpler approach to let user space (such as a
> daemon) manage the leds. In particular this could be used by a user space
> program to map the keys. The MR led could be used to indicate an active
> record mode, etc.
I finally had some time to go through the driver as well (thanks Dmitry
for beating me with proper review).
Could you be more specific about reasons why you are hesitating to use LED
subsystem instead of the whole mled stuff in the driver?
I don't see anything that couldn't be achieved using LED class.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
