Re: [PATCH 15/18] ledtrig-blkdev: Add sysfs attributes to [un]link LEDs & devices
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-09-04 05:59:54
Also in:
linux-leds
On Fri, Sep 03, 2021 at 03:45:45PM -0500, Ian Pilcher wrote:
quoted hunk ↗ jump to hunk
/sys/class/leds/<led>/add_blkdev - to create device/LED associations /sys/class/leds/<led>/delete_blkdev to remove device/LED associations For both attributes, accept multiple device names separated by whitespace Signed-off-by: Ian Pilcher <redacted> --- drivers/leds/trigger/ledtrig-blkdev.c | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)diff --git a/drivers/leds/trigger/ledtrig-blkdev.c b/drivers/leds/trigger/ledtrig-blkdev.c index b2ec85b805d0..db82d37fc721 100644 --- a/drivers/leds/trigger/ledtrig-blkdev.c +++ b/drivers/leds/trigger/ledtrig-blkdev.c@@ -509,3 +509,51 @@ static void blkdev_deactivate(struct led_classdev *const led_dev) module_put(THIS_MODULE); } + + +/* + * + * sysfs attributes to add & delete devices from LEDs + * + */ + +static ssize_t blkdev_add_or_del(struct device *const dev, + struct device_attribute *const attr, + const char *const buf, const size_t count); + +static struct device_attribute ledtrig_blkdev_attr_add = + __ATTR(add_blkdev, 0200, NULL, blkdev_add_or_del); + +static struct device_attribute ledtrig_blkdev_attr_del = + __ATTR(delete_blkdev, 0200, NULL, blkdev_add_or_del);
DEVICE_ATTR_RO()? Or something like that? Do not use __ATTR() for device attributes if at all possible, worst case, use DEVICE_ATTR() here. And the mode settings are odd, are you sure you want that?
+static ssize_t blkdev_add_or_del(struct device *const dev,
+ struct device_attribute *const attr,
+ const char *const buf, const size_t count)
+{
+ struct ledtrig_blkdev_led *const led = led_trigger_get_drvdata(dev);
+ const char *const disk_name = blkdev_skip_space(buf);
+ const char *const endp = blkdev_find_space(disk_name);
+ const ptrdiff_t name_len = endp - disk_name; /* always >= 0 */
+ int ret;
+
+ if (name_len == 0) {
+ pr_info("blkdev LED: empty block device name\n");Looks like debugging code, please remove. And how can this ever happen?
+ return -EINVAL;
+ }
+
+ if (attr == &ledtrig_blkdev_attr_del) {
+ blkdev_disk_delete(led, disk_name, name_len);
+ } else { /* attr == &ledtrig_blkdev_attr_add */
+ ret = blkdev_disk_add(led, disk_name, name_len);Why do you have a single attribute callback that does two totally different things? Just have 2 different callback functions please, it makes things much easier to review and maintain over time. thanks, greg k-h