From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932649AbcILITY (ORCPT ); Mon, 12 Sep 2016 04:19:24 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:50757 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932404AbcILITU (ORCPT ); Mon, 12 Sep 2016 04:19:20 -0400 X-AuditID: cbfec7f2-f79556d000002c42-e0-57d664e6c83f Subject: Re: [PATCH v3] leds: Introduce userspace leds driver To: David Lechner , Richard Purdie Cc: linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, Marcel Holtmann From: Jacek Anaszewski Message-id: <80597ded-f4b4-2990-3eae-e72276296d1a@samsung.com> Date: Mon, 12 Sep 2016 10:18:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <1473439776-15655-1-git-send-email-david@lechnology.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpgleLIzCtJLcpLzFFi42LZduznOd1nKdfCDa4utLFY1CBmcXnXHDaL rW/WMVp8+/SL0WL3rqesDqwe63cvZ/f41H+S1WPP/B+sHp83yQWwRHHZpKTmZJalFunbJXBl 3JtiU3Ays+LtgofsDYwXA7sYOTgkBEwkmleKdDFyApliEhfurWfrYuTiEBJYyiixduo3KOcz o8Tu9utsEFUmEn173zBCJJYxSpz7tZodwnnGKNFy+Ro7SJWwgK3Euo3XmEBsEQEfiYVnF4DF mQVSJa59P8UMYrMJGEr8fPEarIZXwE7iwKpGsA0sAqoSyx9MZwM5T1QgQmL33VSIEkGJH5Pv sYDYnAKuEnfPdDFBjHSUeLBoJyuELS+xec1bZpB7JAS62SWudveyQLwpK7HpADPEAy4SS148 Y4GwhSVeHd/CDmHLSFye3M0C0TuZUeLisZusEM5qRomNnZ1QHdYSDf9/sUBs45OYtG06M8QC XomONiGIEg+JlhX/oJY5SnxonQgNoBmMEi/fHWWbwCg/C8lDs5A8MQvJEwsYmVcxiqSWFuem pxYb6xUn5haX5qXrJefnbmIEpovT/45/2sH49YTVIUYBDkYlHt6A0KvhQqyJZcWVuYcYJTiY lUR4v0dfCxfiTUmsrEotyo8vKs1JLT7EKM3BoiTOu2fBlXAhgfTEktTs1NSC1CKYLBMHp1QD o9vZPQtvKW03enT7vt1rWUapKxLba7cFapxoWtpuryQ4k3tHVenh+Uo8Oys56h+dumGpLpq3 pPeQjTBDOtP6zQFKPFNVen8de/Nla8Ta66HqRXq/lsxxPNTBOM+lU2SVulBAity6A0bdR4+1 66QvNKgTYPnvWV3wXulL5MTZYXxlRjFCCw2OKrEUZyQaajEXFScCADSKj+0TAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHIsWRmVeSWpSXmKPExsVy+t/xy7pbU66FG0x8o2WxqEHM4vKuOWwW W9+sY7T49ukXo8XuXU9ZHVg91u9ezu7xqf8kq8ee+T9YPT5vkgtgiXKzyUhNTEktUkjNS85P ycxLt1UKDXHTtVBSyEvMTbVVitD1DQlSUihLzCkF8owM0ICDc4B7sJK+XYJbxr0pNgUnMyve LnjI3sB4MbCLkZNDQsBEom/vG0YIW0ziwr31bF2MXBxCAksYJeY/W8sOkhASeMYo0fRWHMQW FrCVWLfxGhOILSLgI7Hw7AKoGheJG3euAQ3i4GAWSJVYuckFJMwmYCjx88VrsHJeATuJA6sa 2UBsFgFVieUPpoPZogIRErdWfWSEqBGU+DH5HguIzSngKnH3TBdYLzPQ2gXv17FA2PISm9e8 ZZ7AKDALScssJGWzkJQtYGRexSiSWlqcm55bbKRXnJhbXJqXrpecn7uJERg324793LKDsetd 8CFGAQ5GJR7egNCr4UKsiWXFlbmHGCU4mJVEeL9HXwsX4k1JrKxKLcqPLyrNSS0+xGgK9MRE ZinR5HxgTOeVxBuaGJpbGhoZW1iYGxkpifNO/XAlXEggPbEkNTs1tSC1CKaPiYNTqoGRR++v XJHM5g+eNdsEZjvEiiV7OSRdnn97ffLUs9pyEgJnIoL/Zn202PRz74pFz37osT7Y46N04O6l OaxBBrv+zE6VFLx5N2eW4a/lj3eJPz7uVsMbeYW305z1QfiJitoSjVeLHknzXIwqqlOLZ5zv Zc7IyPXMwndF+wVm9Rtst+66xhrd2sipxFKckWioxVxUnAgAZsZOXrECAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160912081846eucas1p255044e49034685ad44400d6830ef0b95 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?Qikb7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?QikbU2Ftc3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgU29mdHdhcmUgRW5naW5l?= =?UTF-8?B?ZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjc1MjY=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160912081846eucas1p255044e49034685ad44400d6830ef0b95 X-RootMTR: 20160912081846eucas1p255044e49034685ad44400d6830ef0b95 References: <1473439776-15655-1-git-send-email-david@lechnology.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi David, Thanks for addressing the issues. I've applied the patch to the for-next branch of linux-leds.git, after making few more modifications indicated below. Thanks, Jacek Anaszewski On 09/09/2016 06:49 PM, David Lechner wrote: > This driver creates a userspace leds driver similar to uinput. s/leds/LED class/ Also in the commit title: s/leds/LED class/ > > New leds are created by opening /dev/uleds and writing a uleds_user_dev s/leds/LED class devices/ > struct. A new leds class device is registered with the name given in the s/leds/LED/ > struct. Reading will return a single byte that is the current brightness. > The poll() syscall is also supported. It will be triggered whenever the > brightness changes. Closing the file handle to /dev/uleds will remove > the leds class device. s/leds/LED/ > > Signed-off-by: David Lechner > --- > > v2 changes: > > * sort #includes > * fix typo in Kconfig > * Add Documentation text file > > v3 changes: > > * fix typos in docs > * rename "leds class" to "LED class" in docs > * rebase on linux-leds/for-next > > > Documentation/leds/uleds.txt | 105 ++++++++++++++++++++ > drivers/leds/Kconfig | 8 ++ > drivers/leds/Makefile | 3 + > drivers/leds/uleds.c | 224 +++++++++++++++++++++++++++++++++++++++++++ > include/uapi/linux/Kbuild | 1 + > include/uapi/linux/uleds.h | 23 +++++ > 6 files changed, 364 insertions(+) > create mode 100644 Documentation/leds/uleds.txt > create mode 100644 drivers/leds/uleds.c > create mode 100644 include/uapi/linux/uleds.h > > diff --git a/Documentation/leds/uleds.txt b/Documentation/leds/uleds.txt > new file mode 100644 > index 0000000..de11dbb > --- /dev/null > +++ b/Documentation/leds/uleds.txt > @@ -0,0 +1,105 @@ > +Userspace LEDs > +============== > + > +The uleds driver supports userspace LEDs. This can be useful for testing > +triggers and can also be used to implement virtual LEDs. > + > + > +Usage > +===== > + > +When the driver is loaded, a character device is created at /dev/uleds. To > +create a new LED class device, open /dev/uleds and write a uleds_user_dev > +structure to it (found in kernel public header file linux/uleds.h). > + > + #define ULEDS_MAX_NAME_SIZE 80 > + > + struct uleds_user_dev { > + char name[ULEDS_MAX_NAME_SIZE]; > + }; > + > +A new LED class device will be created with the name given. The name can be > +any valid file name, but consider using the LED class naming convention of > +"devicename:color:function". > + > +The current brightness is found by reading a single byte from the character > +device. Values are unsigned: 0 to 255. Reading does not block and always returns > +the most recent brightness value. The device node can also be polled to notify > +when the brightness value changes. > + > +The LED class device will be removed when the open file handle to /dev/uleds > +is closed. > + > + > +Example > +======= > + > +/* > + * uledmon.c > + * > + * This program creates a new userspace LED class device and monitors it. A > + * timestamp and brightness value is printed each time the brightness changes. > + * > + * Usage: uledmon > + * > + * is the name of the LED class device to be created. Pressing > + * CTRL+C will exit. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +int main(int argc, char const *argv[]) > +{ > + struct uleds_user_dev uleds_dev; > + int fd, ret; > + struct pollfd pfd; > + unsigned char brightness; > + struct timespec ts; > + > + if (argc != 2) { > + fprintf(stderr, "Requires argument\n"); > + return 1; > + } > + > + strncpy(uleds_dev.name, argv[1], ULEDS_MAX_NAME_SIZE); > + > + fd = open("/dev/uleds", O_RDWR); > + if (fd == -1) { > + perror("Failed to open /dev/uleds"); > + return 1; > + } > + > + ret = write(fd, &uleds_dev, sizeof(uleds_dev)); > + if (ret == -1) { > + perror("Failed to write to /dev/uleds"); > + close(fd); > + return 1; > + } > + > + pfd.fd = fd; > + pfd.events = POLLIN; > + pfd.revents = 0; > + > + while (!(pfd.revents & (POLLERR | POLLHUP | POLLNVAL))) { > + ret = read(fd, &brightness, 1); > + if (ret == -1) { > + perror("Failed to read from /dev/uleds"); > + close(fd); > + return 1; > + } > + clock_gettime(CLOCK_MONOTONIC, &ts); > + printf("[%ld.%09ld] %u\n", ts.tv_sec, ts.tv_nsec, brightness); > + poll(&pfd, 1, -1); > + } > + > + close(fd); > + > + return 0; > +} > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 7a628c6..5fd3f4c 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -659,6 +659,14 @@ config LEDS_MLXCPLD > This option enabled support for the LEDs on the Mellanox > boards. Say Y to enabled these. > > +config LEDS_USER > + tristate "Userspace LED support" > + depends on LEDS_CLASS > + help > + This option enables support for userspace LEDs. Say 'y' to enable this > + support in kernel. To compile this driver as a module, choose 'm' here: > + the module will be called uleds. > + > comment "LED Triggers" > source "drivers/leds/trigger/Kconfig" > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 3965070..d5331ff 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -75,5 +75,8 @@ obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o > # LED SPI Drivers > obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o > > +# LED Userspace Drivers > +obj-$(CONFIG_LEDS_USER) += uleds.o > + > # LED Triggers > obj-$(CONFIG_LEDS_TRIGGERS) += trigger/ > diff --git a/drivers/leds/uleds.c b/drivers/leds/uleds.c > new file mode 100644 > index 0000000..77a0bee > --- /dev/null > +++ b/drivers/leds/uleds.c > @@ -0,0 +1,224 @@ > +/* > + * Userspace driver for leds subsystem s/leds subsystem/the LED subsystem/ > + * > + * Copyright (C) 2016 David Lechner > + * > + * Based on uinput.c: Aristeu Sergio Rozanski Filho > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define ULEDS_NAME "uleds" > + > +enum uleds_state { > + ULEDS_STATE_UNKNOWN, > + ULEDS_STATE_REGISTERED, > +}; > + > +struct uleds_device { > + struct uleds_user_dev user_dev; > + struct led_classdev led_cdev; > + struct mutex mutex; > + enum uleds_state state; > + wait_queue_head_t waitq; > + unsigned char brightness; > + unsigned char new_data; > +}; > + > +static void uleds_brightness_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + struct uleds_device *udev = container_of(led_cdev, struct uleds_device, > + led_cdev); > + > + if (udev->brightness != brightness) { > + udev->brightness = brightness; > + udev->new_data = 1; > + wake_up_interruptible(&udev->waitq); > + } > +} > + > +static int uleds_open(struct inode *inode, struct file *file) > +{ > + struct uleds_device *udev; > + > + udev = kzalloc(sizeof(*udev), GFP_KERNEL); > + if (!udev) > + return -ENOMEM; > + > + udev->led_cdev.name = udev->user_dev.name; > + udev->led_cdev.max_brightness = LED_FULL; > + udev->led_cdev.brightness_set = uleds_brightness_set; > + > + mutex_init(&udev->mutex); > + init_waitqueue_head(&udev->waitq); > + udev->state = ULEDS_STATE_UNKNOWN; > + > + file->private_data = udev; > + nonseekable_open(inode, file); > + > + return 0; > +} > + > +static ssize_t uleds_write(struct file *file, const char __user *buffer, > + size_t count, loff_t *ppos) > +{ > + struct uleds_device *udev = file->private_data; > + int ret; > + > + if (count == 0) > + return 0; > + > + ret = mutex_lock_interruptible(&udev->mutex); > + if (ret) > + return ret; > + > + if (udev->state == ULEDS_STATE_REGISTERED) { > + ret = -EBUSY; > + goto out; > + } > + > + if (count != sizeof(struct uleds_user_dev)) { > + ret = -EINVAL; > + goto out; > + } > + > + if (copy_from_user(&udev->user_dev, buffer, > + sizeof(struct uleds_user_dev))) { > + ret = -EFAULT; > + goto out; > + } > + > + if (!udev->user_dev.name[0]) { > + ret = -EINVAL; > + goto out; > + } > + > + ret = led_classdev_register(NULL, &udev->led_cdev); > + if (ret < 0) > + goto out; > + > + udev->state = ULEDS_STATE_REGISTERED; > + ret = count; > + > +out: > + mutex_unlock(&udev->mutex); > + > + return ret; > +} > + > +static ssize_t uleds_read(struct file *file, char __user *buffer, size_t count, > + loff_t *ppos) > +{ > + struct uleds_device *udev = file->private_data; > + ssize_t retval; > + > + if (count == 0) > + return 0; > + > + if (count != 1) > + return -EINVAL; > + > + do { > + retval = mutex_lock_interruptible(&udev->mutex); > + if (retval) > + return retval; > + > + if (udev->state != ULEDS_STATE_REGISTERED) { > + retval = -ENODEV; > + } else if (!udev->new_data && (file->f_flags & O_NONBLOCK)) { > + retval = -EAGAIN; > + } else { > + retval = copy_to_user(buffer, &udev->brightness, 1); > + udev->new_data = 0; > + retval = 1; > + } > + > + mutex_unlock(&udev->mutex); > + > + if (retval || count == 0) > + break; > + > + if (!(file->f_flags & O_NONBLOCK)) > + retval = wait_event_interruptible(udev->waitq, > + udev->new_data || > + udev->state != ULEDS_STATE_REGISTERED); > + } while (retval == 0); > + > + return retval; > +} > + > +static unsigned int uleds_poll(struct file *file, poll_table *wait) > +{ > + struct uleds_device *udev = file->private_data; > + > + poll_wait(file, &udev->waitq, wait); > + > + if (udev->new_data) > + return POLLIN | POLLRDNORM; > + > + return 0; > +} > + > +static int uleds_release(struct inode *inode, struct file *file) > +{ > + struct uleds_device *udev = file->private_data; > + > + if (udev->state == ULEDS_STATE_REGISTERED) { > + udev->state = ULEDS_STATE_UNKNOWN; > + led_classdev_unregister(&udev->led_cdev); > + } > + kfree(udev); > + > + return 0; > +} > + > +static const struct file_operations uleds_fops = { > + .owner = THIS_MODULE, > + .open = uleds_open, > + .release = uleds_release, > + .read = uleds_read, > + .write = uleds_write, > + .poll = uleds_poll, > + .llseek = no_llseek, > +}; > + > +static struct miscdevice uleds_misc = { > + .fops = &uleds_fops, > + .minor = MISC_DYNAMIC_MINOR, > + .name = ULEDS_NAME, > +}; > + > +static int __init uleds_init(void) > +{ > + return misc_register(&uleds_misc); > +} > +module_init(uleds_init); > + > +static void __exit uleds_exit(void) > +{ > + misc_deregister(&uleds_misc); > +} > +module_exit(uleds_exit); > + > +MODULE_AUTHOR("David Lechner "); > +MODULE_DESCRIPTION("Userspace driver for leds subsystem"); s/leds subsystem/the LED subsystem/ > +MODULE_LICENSE("GPL"); > diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild > index 185f8ea..416f5e6 100644 > --- a/include/uapi/linux/Kbuild > +++ b/include/uapi/linux/Kbuild > @@ -421,6 +421,7 @@ header-y += udp.h > header-y += uhid.h > header-y += uinput.h > header-y += uio.h > +header-y += uleds.h > header-y += ultrasound.h > header-y += un.h > header-y += unistd.h > diff --git a/include/uapi/linux/uleds.h b/include/uapi/linux/uleds.h > new file mode 100644 > index 0000000..e78ed46 > --- /dev/null > +++ b/include/uapi/linux/uleds.h > @@ -0,0 +1,23 @@ > +/* > + * Userspace driver support for leds subsystem s/leds subsystem/the LED subsystem/ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#ifndef _UAPI__ULEDS_H_ > +#define _UAPI__ULEDS_H_ > + > +#define ULEDS_MAX_NAME_SIZE 80 > + > +struct uleds_user_dev { > + char name[ULEDS_MAX_NAME_SIZE]; > +}; > + > +#endif /* _UAPI__ULEDS_H_ */ >