| Message ID | 1512923050-7297-1-git-send-email-ben.whitten@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v4] leds: trigger: Introduce a NETDEV trigger | expand |
Hi Ben, Thanks for the update. I have one doubt about comment style at the top of the file. Please refer below. On 12/10/2017 05:24 PM, Ben Whitten wrote: > This commit introduces a NETDEV trigger for named device > activity. Available triggers are link, rx, and tx. > > Signed-off-by: Ben Whitten <ben.whitten@gmail.com> > > --- > Changes in v4: > Adopt SPDX licence header > Changes in v3: > Cancel the software blink prior to a oneshot re-queue > Changes in v2: > Sort includes and redate documentation > Correct licence > Remove macro and replace with generic function using enums > Convert blink logic in stats work to use led_blink_oneshot > Uses configured brightness instead of FULL > --- > .../ABI/testing/sysfs-class-led-trigger-netdev | 45 ++ > drivers/leds/trigger/Kconfig | 7 + > drivers/leds/trigger/Makefile | 1 + > drivers/leds/trigger/ledtrig-netdev.c | 498 +++++++++++++++++++++ > 4 files changed, 551 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev > create mode 100644 drivers/leds/trigger/ledtrig-netdev.c > > diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev > new file mode 100644 > index 0000000..451af6d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev > @@ -0,0 +1,45 @@ > +What: /sys/class/leds/<led>/device_name > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Specifies the network device name to monitor. > + > +What: /sys/class/leds/<led>/interval > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Specifies the duration of the LED blink in milliseconds. > + Defaults to 50 ms. > + > +What: /sys/class/leds/<led>/link > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal the link state of the named network device. > + If set to 0 (default), the LED's normal state is off. > + If set to 1, the LED's normal state reflects the link state > + of the named network device. > + Setting this value also immediately changes the LED state. > + > +What: /sys/class/leds/<led>/tx > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal transmission of data on the named network device. > + If set to 0 (default), the LED will not blink on transmission. > + If set to 1, the LED will blink for the milliseconds specified > + in interval to signal transmission. > + > +What: /sys/class/leds/<led>/rx > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal reception of data on the named network device. > + If set to 0 (default), the LED will not blink on reception. > + If set to 1, the LED will blink for the milliseconds specified > + in interval to signal reception. > diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig > index 3f9ddb9..4ec1853 100644 > --- a/drivers/leds/trigger/Kconfig > +++ b/drivers/leds/trigger/Kconfig > @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC > a different trigger. > If unsure, say Y. > > +config LEDS_TRIGGER_NETDEV > + tristate "LED Netdev Trigger" > + depends on NET && LEDS_TRIGGERS > + help > + This allows LEDs to be controlled by network device activity. > + If unsure, say Y. > + > endif # LEDS_TRIGGERS > diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile > index 9f2e868..59e163d 100644 > --- a/drivers/leds/trigger/Makefile > +++ b/drivers/leds/trigger/Makefile > @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o > obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o > obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o > obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o > +obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o > diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c > new file mode 100644 > index 0000000..3d24573 > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-netdev.c > @@ -0,0 +1,498 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com> > +// Copyright 2007 Oliver Jowett <oliver@opencloud.com> > +/* > + * LED Kernel Netdev Trigger > + * > + * Toggles the LED to reflect the link and traffic state of a named net device > + * > + * Derived from ledtrig-timer.c which is: > + * Copyright 2005-2006 Openedhand Ltd. > + * Author: Richard Purdie <rpurdie@openedhand.com> > + * > + */ This mixed comment style looks odd to me. I'd go for // style on the whole span of this block of commented text. Especially taking into account following Linus' statement from [0]: "And yes, feel free to replace block comments with // while at it." Otherwise the driver looks good to me. [0] https://lkml.org/lkml/2017/11/2/715
On Sun, Dec 10, 2017 at 5:24 PM, Ben Whitten <ben.whitten@gmail.com> wrote: > This commit introduces a NETDEV trigger for named device > activity. Available triggers are link, rx, and tx. > > Signed-off-by: Ben Whitten <ben.whitten@gmail.com> > > --- > Changes in v4: > Adopt SPDX licence header Thanks you! Acked-by: Philippe Ombredanne <pombredanne@nexb.com> > Changes in v3: > Cancel the software blink prior to a oneshot re-queue > Changes in v2: > Sort includes and redate documentation > Correct licence > Remove macro and replace with generic function using enums > Convert blink logic in stats work to use led_blink_oneshot > Uses configured brightness instead of FULL > --- > .../ABI/testing/sysfs-class-led-trigger-netdev | 45 ++ > drivers/leds/trigger/Kconfig | 7 + > drivers/leds/trigger/Makefile | 1 + > drivers/leds/trigger/ledtrig-netdev.c | 498 +++++++++++++++++++++ > 4 files changed, 551 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev > create mode 100644 drivers/leds/trigger/ledtrig-netdev.c > > diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev > new file mode 100644 > index 0000000..451af6d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev > @@ -0,0 +1,45 @@ > +What: /sys/class/leds/<led>/device_name > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Specifies the network device name to monitor. > + > +What: /sys/class/leds/<led>/interval > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Specifies the duration of the LED blink in milliseconds. > + Defaults to 50 ms. > + > +What: /sys/class/leds/<led>/link > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal the link state of the named network device. > + If set to 0 (default), the LED's normal state is off. > + If set to 1, the LED's normal state reflects the link state > + of the named network device. > + Setting this value also immediately changes the LED state. > + > +What: /sys/class/leds/<led>/tx > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal transmission of data on the named network device. > + If set to 0 (default), the LED will not blink on transmission. > + If set to 1, the LED will blink for the milliseconds specified > + in interval to signal transmission. > + > +What: /sys/class/leds/<led>/rx > +Date: Dec 2017 > +KernelVersion: 4.16 > +Contact: linux-leds@vger.kernel.org > +Description: > + Signal reception of data on the named network device. > + If set to 0 (default), the LED will not blink on reception. > + If set to 1, the LED will blink for the milliseconds specified > + in interval to signal reception. > diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig > index 3f9ddb9..4ec1853 100644 > --- a/drivers/leds/trigger/Kconfig > +++ b/drivers/leds/trigger/Kconfig > @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC > a different trigger. > If unsure, say Y. > > +config LEDS_TRIGGER_NETDEV > + tristate "LED Netdev Trigger" > + depends on NET && LEDS_TRIGGERS > + help > + This allows LEDs to be controlled by network device activity. > + If unsure, say Y. > + > endif # LEDS_TRIGGERS > diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile > index 9f2e868..59e163d 100644 > --- a/drivers/leds/trigger/Makefile > +++ b/drivers/leds/trigger/Makefile > @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o > obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o > obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o > obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o > +obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o > diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c > new file mode 100644 > index 0000000..3d24573 > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-netdev.c > @@ -0,0 +1,498 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com> > +// Copyright 2007 Oliver Jowett <oliver@opencloud.com> > +/* > + * LED Kernel Netdev Trigger > + * > + * Toggles the LED to reflect the link and traffic state of a named net device > + * > + * Derived from ledtrig-timer.c which is: > + * Copyright 2005-2006 Openedhand Ltd. > + * Author: Richard Purdie <rpurdie@openedhand.com> > + * > + */ > + > +#include <linux/atomic.h> > +#include <linux/ctype.h> > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/jiffies.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/list.h> > +#include <linux/module.h> > +#include <linux/netdevice.h> > +#include <linux/spinlock.h> > +#include <linux/timer.h> > +#include "../leds.h" > + > +/* > + * Configurable sysfs attributes: > + * > + * device_name - network device name to monitor > + * interval - duration of LED blink, in milliseconds > + * link - LED's normal state reflects whether the link is up > + * (has carrier) or not > + * tx - LED blinks on transmitted data > + * rx - LED blinks on receive data > + * > + */ > + > +struct led_netdev_data { > + spinlock_t lock; > + > + struct delayed_work work; > + struct notifier_block notifier; > + > + struct led_classdev *led_cdev; > + struct net_device *net_dev; > + > + char device_name[IFNAMSIZ]; > + atomic_t interval; > + unsigned int last_activity; > + > + unsigned long mode; > +#define NETDEV_LED_LINK 0 > +#define NETDEV_LED_TX 1 > +#define NETDEV_LED_RX 2 > +#define NETDEV_LED_MODE_LINKUP 3 > +}; > + > +enum netdev_led_attr { > + NETDEV_ATTR_LINK, > + NETDEV_ATTR_TX, > + NETDEV_ATTR_RX > +}; > + > +static void set_baseline_state(struct led_netdev_data *trigger_data) > +{ > + int current_brightness; > + struct led_classdev *led_cdev = trigger_data->led_cdev; > + > + current_brightness = led_cdev->brightness; > + if (current_brightness) > + led_cdev->blink_brightness = current_brightness; > + if (!led_cdev->blink_brightness) > + led_cdev->blink_brightness = led_cdev->max_brightness; > + > + if (!test_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode)) > + led_set_brightness(led_cdev, LED_OFF); > + else { > + if (test_bit(NETDEV_LED_LINK, &trigger_data->mode)) > + led_set_brightness(led_cdev, > + led_cdev->blink_brightness); > + else > + led_set_brightness(led_cdev, LED_OFF); > + > + /* If we are looking for RX/TX start periodically > + * checking stats > + */ > + if (test_bit(NETDEV_LED_TX, &trigger_data->mode) || > + test_bit(NETDEV_LED_RX, &trigger_data->mode)) > + schedule_delayed_work(&trigger_data->work, 0); > + } > +} > + > +static ssize_t device_name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + ssize_t len; > + > + spin_lock_bh(&trigger_data->lock); > + len = sprintf(buf, "%s\n", trigger_data->device_name); > + spin_unlock_bh(&trigger_data->lock); > + > + return len; > +} > + > +static ssize_t device_name_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + > + if (size >= IFNAMSIZ) > + return -EINVAL; > + > + cancel_delayed_work_sync(&trigger_data->work); > + > + spin_lock_bh(&trigger_data->lock); > + > + if (trigger_data->net_dev) { > + dev_put(trigger_data->net_dev); > + trigger_data->net_dev = NULL; > + } > + > + strncpy(trigger_data->device_name, buf, size); > + if (size > 0 && trigger_data->device_name[size - 1] == '\n') > + trigger_data->device_name[size - 1] = 0; > + > + if (trigger_data->device_name[0] != 0) > + trigger_data->net_dev = > + dev_get_by_name(&init_net, trigger_data->device_name); > + > + clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); > + if (trigger_data->net_dev != NULL) > + if (netif_carrier_ok(trigger_data->net_dev)) > + set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); > + > + trigger_data->last_activity = 0; > + > + set_baseline_state(trigger_data); > + spin_unlock_bh(&trigger_data->lock); > + > + return size; > +} > + > +static DEVICE_ATTR_RW(device_name); > + > +static ssize_t netdev_led_attr_show(struct device *dev, char *buf, > + enum netdev_led_attr attr) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + int bit; > + > + switch (attr) { > + case NETDEV_ATTR_LINK: > + bit = NETDEV_LED_LINK; > + break; > + case NETDEV_ATTR_TX: > + bit = NETDEV_LED_TX; > + break; > + case NETDEV_ATTR_RX: > + bit = NETDEV_LED_RX; > + break; > + default: > + return -EINVAL; > + } > + > + return sprintf(buf, "%u\n", test_bit(bit, &trigger_data->mode)); > +} > + > +static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, > + size_t size, enum netdev_led_attr attr) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + unsigned long state; > + int ret; > + int bit; > + > + ret = kstrtoul(buf, 0, &state); > + if (ret) > + return ret; > + > + switch (attr) { > + case NETDEV_ATTR_LINK: > + bit = NETDEV_LED_LINK; > + break; > + case NETDEV_ATTR_TX: > + bit = NETDEV_LED_TX; > + break; > + case NETDEV_ATTR_RX: > + bit = NETDEV_LED_RX; > + break; > + default: > + return -EINVAL; > + } > + > + cancel_delayed_work_sync(&trigger_data->work); > + > + if (state) > + set_bit(bit, &trigger_data->mode); > + else > + clear_bit(bit, &trigger_data->mode); > + > + set_baseline_state(trigger_data); > + > + return size; > +} > + > +static ssize_t link_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_LINK); > +} > + > +static ssize_t link_store(struct device *dev, > + struct device_attribute *attr, const char *buf, size_t size) > +{ > + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_LINK); > +} > + > +static DEVICE_ATTR_RW(link); > + > +static ssize_t tx_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_TX); > +} > + > +static ssize_t tx_store(struct device *dev, > + struct device_attribute *attr, const char *buf, size_t size) > +{ > + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_TX); > +} > + > +static DEVICE_ATTR_RW(tx); > + > +static ssize_t rx_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_RX); > +} > + > +static ssize_t rx_store(struct device *dev, > + struct device_attribute *attr, const char *buf, size_t size) > +{ > + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_RX); > +} > + > +static DEVICE_ATTR_RW(rx); > + > +static ssize_t interval_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + > + return sprintf(buf, "%u\n", > + jiffies_to_msecs(atomic_read(&trigger_data->interval))); > +} > + > +static ssize_t interval_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + unsigned long value; > + int ret; > + > + ret = kstrtoul(buf, 0, &value); > + if (ret) > + return ret; > + > + /* impose some basic bounds on the timer interval */ > + if (value >= 5 && value <= 10000) { > + cancel_delayed_work_sync(&trigger_data->work); > + > + atomic_set(&trigger_data->interval, msecs_to_jiffies(value)); > + set_baseline_state(trigger_data); /* resets timer */ > + } > + > + return size; > +} > + > +static DEVICE_ATTR_RW(interval); > + > +static int netdev_trig_notify(struct notifier_block *nb, > + unsigned long evt, void *dv) > +{ > + struct net_device *dev = > + netdev_notifier_info_to_dev((struct netdev_notifier_info *)dv); > + struct led_netdev_data *trigger_data = container_of(nb, > + struct > + led_netdev_data, > + notifier); > + > + if (evt != NETDEV_UP && evt != NETDEV_DOWN && evt != NETDEV_CHANGE > + && evt != NETDEV_REGISTER && evt != NETDEV_UNREGISTER > + && evt != NETDEV_CHANGENAME) > + return NOTIFY_DONE; > + > + if (strcmp(dev->name, trigger_data->device_name)) > + return NOTIFY_DONE; > + > + cancel_delayed_work_sync(&trigger_data->work); > + > + spin_lock_bh(&trigger_data->lock); > + > + clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); > + switch (evt) { > + case NETDEV_REGISTER: > + if (trigger_data->net_dev) > + dev_put(trigger_data->net_dev); > + dev_hold(dev); > + trigger_data->net_dev = dev; > + break; > + case NETDEV_CHANGENAME: > + case NETDEV_UNREGISTER: > + if (trigger_data->net_dev) { > + dev_put(trigger_data->net_dev); > + trigger_data->net_dev = NULL; > + } > + break; > + case NETDEV_UP: > + case NETDEV_CHANGE: > + if (netif_carrier_ok(dev)) > + set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); > + break; > + } > + > + set_baseline_state(trigger_data); > + > + spin_unlock_bh(&trigger_data->lock); > + > + return NOTIFY_DONE; > +} > + > +/* here's the real work! */ > +static void netdev_trig_work(struct work_struct *work) > +{ > + struct led_netdev_data *trigger_data = container_of(work, > + struct > + led_netdev_data, > + work.work); > + struct rtnl_link_stats64 *dev_stats; > + unsigned int new_activity; > + struct rtnl_link_stats64 temp; > + unsigned long interval; > + int invert; > + > + /* If we dont have a device, insure we are off */ > + if (!trigger_data->net_dev) { > + led_set_brightness(trigger_data->led_cdev, LED_OFF); > + return; > + } > + > + /* If we are not looking for RX/TX then return */ > + if (!test_bit(NETDEV_LED_TX, &trigger_data->mode) && > + !test_bit(NETDEV_LED_RX, &trigger_data->mode)) > + return; > + > + dev_stats = dev_get_stats(trigger_data->net_dev, &temp); > + new_activity = > + (test_bit(NETDEV_LED_TX, &trigger_data->mode) ? > + dev_stats->tx_packets : 0) + > + (test_bit(NETDEV_LED_RX, &trigger_data->mode) ? > + dev_stats->rx_packets : 0); > + > + if (trigger_data->last_activity != new_activity) { > + led_stop_software_blink(trigger_data->led_cdev); > + > + invert = test_bit(NETDEV_LED_LINK, &trigger_data->mode); > + interval = jiffies_to_msecs( > + atomic_read(&trigger_data->interval)); > + /* base state is ON (link present) */ > + led_blink_set_oneshot(trigger_data->led_cdev, > + &interval, > + &interval, > + invert); > + trigger_data->last_activity = new_activity; > + } > + > + schedule_delayed_work(&trigger_data->work, > + (atomic_read(&trigger_data->interval)*2)); > +} > + > +static void netdev_trig_activate(struct led_classdev *led_cdev) > +{ > + struct led_netdev_data *trigger_data; > + int rc; > + > + trigger_data = kzalloc(sizeof(struct led_netdev_data), GFP_KERNEL); > + if (!trigger_data) > + return; > + > + spin_lock_init(&trigger_data->lock); > + > + trigger_data->notifier.notifier_call = netdev_trig_notify; > + trigger_data->notifier.priority = 10; > + > + INIT_DELAYED_WORK(&trigger_data->work, netdev_trig_work); > + > + trigger_data->led_cdev = led_cdev; > + trigger_data->net_dev = NULL; > + trigger_data->device_name[0] = 0; > + > + trigger_data->mode = 0; > + atomic_set(&trigger_data->interval, msecs_to_jiffies(50)); > + trigger_data->last_activity = 0; > + > + led_cdev->trigger_data = trigger_data; > + > + rc = device_create_file(led_cdev->dev, &dev_attr_device_name); > + if (rc) > + goto err_out; > + rc = device_create_file(led_cdev->dev, &dev_attr_link); > + if (rc) > + goto err_out_device_name; > + rc = device_create_file(led_cdev->dev, &dev_attr_rx); > + if (rc) > + goto err_out_link; > + rc = device_create_file(led_cdev->dev, &dev_attr_tx); > + if (rc) > + goto err_out_rx; > + rc = device_create_file(led_cdev->dev, &dev_attr_interval); > + if (rc) > + goto err_out_tx; > + rc = register_netdevice_notifier(&trigger_data->notifier); > + if (rc) > + goto err_out_interval; > + return; > + > +err_out_interval: > + device_remove_file(led_cdev->dev, &dev_attr_interval); > +err_out_tx: > + device_remove_file(led_cdev->dev, &dev_attr_tx); > +err_out_rx: > + device_remove_file(led_cdev->dev, &dev_attr_rx); > +err_out_link: > + device_remove_file(led_cdev->dev, &dev_attr_link); > +err_out_device_name: > + device_remove_file(led_cdev->dev, &dev_attr_device_name); > +err_out: > + led_cdev->trigger_data = NULL; > + kfree(trigger_data); > +} > + > +static void netdev_trig_deactivate(struct led_classdev *led_cdev) > +{ > + struct led_netdev_data *trigger_data = led_cdev->trigger_data; > + > + if (trigger_data) { > + unregister_netdevice_notifier(&trigger_data->notifier); > + > + device_remove_file(led_cdev->dev, &dev_attr_device_name); > + device_remove_file(led_cdev->dev, &dev_attr_link); > + device_remove_file(led_cdev->dev, &dev_attr_rx); > + device_remove_file(led_cdev->dev, &dev_attr_tx); > + device_remove_file(led_cdev->dev, &dev_attr_interval); > + > + cancel_delayed_work_sync(&trigger_data->work); > + > + if (trigger_data->net_dev) > + dev_put(trigger_data->net_dev); > + > + kfree(trigger_data); > + } > +} > + > +static struct led_trigger netdev_led_trigger = { > + .name = "netdev", > + .activate = netdev_trig_activate, > + .deactivate = netdev_trig_deactivate, > +}; > + > +static int __init netdev_trig_init(void) > +{ > + return led_trigger_register(&netdev_led_trigger); > +} > + > +static void __exit netdev_trig_exit(void) > +{ > + led_trigger_unregister(&netdev_led_trigger); > +} > + > +module_init(netdev_trig_init); > +module_exit(netdev_trig_exit); > + > +MODULE_AUTHOR("Ben Whitten <ben.whitten@gmail.com>"); > +MODULE_AUTHOR("Oliver Jowett <oliver@opencloud.com>"); > +MODULE_DESCRIPTION("Netdev LED trigger"); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 >
Hi Jacek, On 10 December 2017 at 18:31, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Hi Ben, > > Thanks for the update. I have one doubt about comment style > at the top of the file. Please refer below. > > On 12/10/2017 05:24 PM, Ben Whitten wrote: >> This commit introduces a NETDEV trigger for named device >> activity. Available triggers are link, rx, and tx. >> >> Signed-off-by: Ben Whitten <ben.whitten@gmail.com> >> >> --- >> Changes in v4: >> Adopt SPDX licence header >> Changes in v3: >> Cancel the software blink prior to a oneshot re-queue >> Changes in v2: >> Sort includes and redate documentation >> Correct licence >> Remove macro and replace with generic function using enums >> Convert blink logic in stats work to use led_blink_oneshot >> Uses configured brightness instead of FULL >> --- >> .../ABI/testing/sysfs-class-led-trigger-netdev | 45 ++ >> drivers/leds/trigger/Kconfig | 7 + >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-netdev.c | 498 +++++++++++++++++++++ >> 4 files changed, 551 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev >> create mode 100644 drivers/leds/trigger/ledtrig-netdev.c >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev >> new file mode 100644 >> index 0000000..451af6d >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev >> @@ -0,0 +1,45 @@ >> +What: /sys/class/leds/<led>/device_name >> +Date: Dec 2017 >> +KernelVersion: 4.16 >> +Contact: linux-leds@vger.kernel.org >> +Description: >> + Specifies the network device name to monitor. >> + >> +What: /sys/class/leds/<led>/interval >> +Date: Dec 2017 >> +KernelVersion: 4.16 >> +Contact: linux-leds@vger.kernel.org >> +Description: >> + Specifies the duration of the LED blink in milliseconds. >> + Defaults to 50 ms. >> + >> +What: /sys/class/leds/<led>/link >> +Date: Dec 2017 >> +KernelVersion: 4.16 >> +Contact: linux-leds@vger.kernel.org >> +Description: >> + Signal the link state of the named network device. >> + If set to 0 (default), the LED's normal state is off. >> + If set to 1, the LED's normal state reflects the link state >> + of the named network device. >> + Setting this value also immediately changes the LED state. >> + >> +What: /sys/class/leds/<led>/tx >> +Date: Dec 2017 >> +KernelVersion: 4.16 >> +Contact: linux-leds@vger.kernel.org >> +Description: >> + Signal transmission of data on the named network device. >> + If set to 0 (default), the LED will not blink on transmission. >> + If set to 1, the LED will blink for the milliseconds specified >> + in interval to signal transmission. >> + >> +What: /sys/class/leds/<led>/rx >> +Date: Dec 2017 >> +KernelVersion: 4.16 >> +Contact: linux-leds@vger.kernel.org >> +Description: >> + Signal reception of data on the named network device. >> + If set to 0 (default), the LED will not blink on reception. >> + If set to 1, the LED will blink for the milliseconds specified >> + in interval to signal reception. >> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig >> index 3f9ddb9..4ec1853 100644 >> --- a/drivers/leds/trigger/Kconfig >> +++ b/drivers/leds/trigger/Kconfig >> @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC >> a different trigger. >> If unsure, say Y. >> >> +config LEDS_TRIGGER_NETDEV >> + tristate "LED Netdev Trigger" >> + depends on NET && LEDS_TRIGGERS >> + help >> + This allows LEDs to be controlled by network device activity. >> + If unsure, say Y. >> + >> endif # LEDS_TRIGGERS >> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile >> index 9f2e868..59e163d 100644 >> --- a/drivers/leds/trigger/Makefile >> +++ b/drivers/leds/trigger/Makefile >> @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o >> obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o >> obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o >> obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o >> +obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o >> diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c >> new file mode 100644 >> index 0000000..3d24573 >> --- /dev/null >> +++ b/drivers/leds/trigger/ledtrig-netdev.c >> @@ -0,0 +1,498 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com> >> +// Copyright 2007 Oliver Jowett <oliver@opencloud.com> >> +/* >> + * LED Kernel Netdev Trigger >> + * >> + * Toggles the LED to reflect the link and traffic state of a named net device >> + * >> + * Derived from ledtrig-timer.c which is: >> + * Copyright 2005-2006 Openedhand Ltd. >> + * Author: Richard Purdie <rpurdie@openedhand.com> >> + * >> + */ > > This mixed comment style looks odd to me. I'd go for // style > on the whole span of this block of commented text. > Especially taking into account following Linus' statement > from [0]: > > "And yes, feel free to replace block comments with // while at it." > Sure thing, should I apply the same to other block comments and single line /* */'s whilst at it, to keep the overall style? Or just this header. > Otherwise the driver looks good to me. > > [0] https://lkml.org/lkml/2017/11/2/715 > > -- > Best regards, > Jacek Anaszewski
On 12/10/2017 08:12 PM, Ben Whitten wrote: > Hi Jacek, > > On 10 December 2017 at 18:31, Jacek Anaszewski > <jacek.anaszewski@gmail.com> wrote: >> Hi Ben, >> >> Thanks for the update. I have one doubt about comment style >> at the top of the file. Please refer below. >> >> On 12/10/2017 05:24 PM, Ben Whitten wrote: >>> This commit introduces a NETDEV trigger for named device >>> activity. Available triggers are link, rx, and tx. >>> >>> Signed-off-by: Ben Whitten <ben.whitten@gmail.com> >>> >>> --- >>> Changes in v4: >>> Adopt SPDX licence header >>> Changes in v3: >>> Cancel the software blink prior to a oneshot re-queue >>> Changes in v2: >>> Sort includes and redate documentation >>> Correct licence >>> Remove macro and replace with generic function using enums >>> Convert blink logic in stats work to use led_blink_oneshot >>> Uses configured brightness instead of FULL >>> --- >>> .../ABI/testing/sysfs-class-led-trigger-netdev | 45 ++ >>> drivers/leds/trigger/Kconfig | 7 + >>> drivers/leds/trigger/Makefile | 1 + >>> drivers/leds/trigger/ledtrig-netdev.c | 498 +++++++++++++++++++++ >>> 4 files changed, 551 insertions(+) >>> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev >>> create mode 100644 drivers/leds/trigger/ledtrig-netdev.c >>> >>> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev >>> new file mode 100644 >>> index 0000000..451af6d >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev >>> @@ -0,0 +1,45 @@ >>> +What: /sys/class/leds/<led>/device_name >>> +Date: Dec 2017 >>> +KernelVersion: 4.16 >>> +Contact: linux-leds@vger.kernel.org >>> +Description: >>> + Specifies the network device name to monitor. >>> + >>> +What: /sys/class/leds/<led>/interval >>> +Date: Dec 2017 >>> +KernelVersion: 4.16 >>> +Contact: linux-leds@vger.kernel.org >>> +Description: >>> + Specifies the duration of the LED blink in milliseconds. >>> + Defaults to 50 ms. >>> + >>> +What: /sys/class/leds/<led>/link >>> +Date: Dec 2017 >>> +KernelVersion: 4.16 >>> +Contact: linux-leds@vger.kernel.org >>> +Description: >>> + Signal the link state of the named network device. >>> + If set to 0 (default), the LED's normal state is off. >>> + If set to 1, the LED's normal state reflects the link state >>> + of the named network device. >>> + Setting this value also immediately changes the LED state. >>> + >>> +What: /sys/class/leds/<led>/tx >>> +Date: Dec 2017 >>> +KernelVersion: 4.16 >>> +Contact: linux-leds@vger.kernel.org >>> +Description: >>> + Signal transmission of data on the named network device. >>> + If set to 0 (default), the LED will not blink on transmission. >>> + If set to 1, the LED will blink for the milliseconds specified >>> + in interval to signal transmission. >>> + >>> +What: /sys/class/leds/<led>/rx >>> +Date: Dec 2017 >>> +KernelVersion: 4.16 >>> +Contact: linux-leds@vger.kernel.org >>> +Description: >>> + Signal reception of data on the named network device. >>> + If set to 0 (default), the LED will not blink on reception. >>> + If set to 1, the LED will blink for the milliseconds specified >>> + in interval to signal reception. >>> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig >>> index 3f9ddb9..4ec1853 100644 >>> --- a/drivers/leds/trigger/Kconfig >>> +++ b/drivers/leds/trigger/Kconfig >>> @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC >>> a different trigger. >>> If unsure, say Y. >>> >>> +config LEDS_TRIGGER_NETDEV >>> + tristate "LED Netdev Trigger" >>> + depends on NET && LEDS_TRIGGERS >>> + help >>> + This allows LEDs to be controlled by network device activity. >>> + If unsure, say Y. >>> + >>> endif # LEDS_TRIGGERS >>> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile >>> index 9f2e868..59e163d 100644 >>> --- a/drivers/leds/trigger/Makefile >>> +++ b/drivers/leds/trigger/Makefile >>> @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o >>> obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o >>> obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o >>> obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o >>> +obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o >>> diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c >>> new file mode 100644 >>> index 0000000..3d24573 >>> --- /dev/null >>> +++ b/drivers/leds/trigger/ledtrig-netdev.c >>> @@ -0,0 +1,498 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com> >>> +// Copyright 2007 Oliver Jowett <oliver@opencloud.com> >>> +/* >>> + * LED Kernel Netdev Trigger >>> + * >>> + * Toggles the LED to reflect the link and traffic state of a named net device >>> + * >>> + * Derived from ledtrig-timer.c which is: >>> + * Copyright 2005-2006 Openedhand Ltd. >>> + * Author: Richard Purdie <rpurdie@openedhand.com> >>> + * >>> + */ >> >> This mixed comment style looks odd to me. I'd go for // style >> on the whole span of this block of commented text. >> Especially taking into account following Linus' statement >> from [0]: >> >> "And yes, feel free to replace block comments with // while at it." >> > > Sure thing, should I apply the same to other block comments and single line > /* */'s whilst at it, to keep the overall style? Or just this header. Since it is not clear if this should apply also to the struct documentation, let's confine ourselves to the header for now.
diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev new file mode 100644 index 0000000..451af6d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev @@ -0,0 +1,45 @@ +What: /sys/class/leds/<led>/device_name +Date: Dec 2017 +KernelVersion: 4.16 +Contact: linux-leds@vger.kernel.org +Description: + Specifies the network device name to monitor. + +What: /sys/class/leds/<led>/interval +Date: Dec 2017 +KernelVersion: 4.16 +Contact: linux-leds@vger.kernel.org +Description: + Specifies the duration of the LED blink in milliseconds. + Defaults to 50 ms. + +What: /sys/class/leds/<led>/link +Date: Dec 2017 +KernelVersion: 4.16 +Contact: linux-leds@vger.kernel.org +Description: + Signal the link state of the named network device. + If set to 0 (default), the LED's normal state is off. + If set to 1, the LED's normal state reflects the link state + of the named network device. + Setting this value also immediately changes the LED state. + +What: /sys/class/leds/<led>/tx +Date: Dec 2017 +KernelVersion: 4.16 +Contact: linux-leds@vger.kernel.org +Description: + Signal transmission of data on the named network device. + If set to 0 (default), the LED will not blink on transmission. + If set to 1, the LED will blink for the milliseconds specified + in interval to signal transmission. + +What: /sys/class/leds/<led>/rx +Date: Dec 2017 +KernelVersion: 4.16 +Contact: linux-leds@vger.kernel.org +Description: + Signal reception of data on the named network device. + If set to 0 (default), the LED will not blink on reception. + If set to 1, the LED will blink for the milliseconds specified + in interval to signal reception. diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig index 3f9ddb9..4ec1853 100644 --- a/drivers/leds/trigger/Kconfig +++ b/drivers/leds/trigger/Kconfig @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC a different trigger. If unsure, say Y. +config LEDS_TRIGGER_NETDEV + tristate "LED Netdev Trigger" + depends on NET && LEDS_TRIGGERS + help + This allows LEDs to be controlled by network device activity. + If unsure, say Y. + endif # LEDS_TRIGGERS diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile index 9f2e868..59e163d 100644 --- a/drivers/leds/trigger/Makefile +++ b/drivers/leds/trigger/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o +obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c new file mode 100644 index 0000000..3d24573 --- /dev/null +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -0,0 +1,498 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com> +// Copyright 2007 Oliver Jowett <oliver@opencloud.com> +/* + * LED Kernel Netdev Trigger + * + * Toggles the LED to reflect the link and traffic state of a named net device + * + * Derived from ledtrig-timer.c which is: + * Copyright 2005-2006 Openedhand Ltd. + * Author: Richard Purdie <rpurdie@openedhand.com> + * + */ + +#include <linux/atomic.h> +#include <linux/ctype.h> +#include <linux/device.h> +#include <linux/init.h> +#include <linux/jiffies.h> +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/list.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/spinlock.h> +#include <linux/timer.h> +#include "../leds.h" + +/* + * Configurable sysfs attributes: + * + * device_name - network device name to monitor + * interval - duration of LED blink, in milliseconds + * link - LED's normal state reflects whether the link is up + * (has carrier) or not + * tx - LED blinks on transmitted data + * rx - LED blinks on receive data + * + */ + +struct led_netdev_data { + spinlock_t lock; + + struct delayed_work work; + struct notifier_block notifier; + + struct led_classdev *led_cdev; + struct net_device *net_dev; + + char device_name[IFNAMSIZ]; + atomic_t interval; + unsigned int last_activity; + + unsigned long mode; +#define NETDEV_LED_LINK 0 +#define NETDEV_LED_TX 1 +#define NETDEV_LED_RX 2 +#define NETDEV_LED_MODE_LINKUP 3 +}; + +enum netdev_led_attr { + NETDEV_ATTR_LINK, + NETDEV_ATTR_TX, + NETDEV_ATTR_RX +}; + +static void set_baseline_state(struct led_netdev_data *trigger_data) +{ + int current_brightness; + struct led_classdev *led_cdev = trigger_data->led_cdev; + + current_brightness = led_cdev->brightness; + if (current_brightness) + led_cdev->blink_brightness = current_brightness; + if (!led_cdev->blink_brightness) + led_cdev->blink_brightness = led_cdev->max_brightness; + + if (!test_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode)) + led_set_brightness(led_cdev, LED_OFF); + else { + if (test_bit(NETDEV_LED_LINK, &trigger_data->mode)) + led_set_brightness(led_cdev, + led_cdev->blink_brightness); + else + led_set_brightness(led_cdev, LED_OFF); + + /* If we are looking for RX/TX start periodically + * checking stats + */ + if (test_bit(NETDEV_LED_TX, &trigger_data->mode) || + test_bit(NETDEV_LED_RX, &trigger_data->mode)) + schedule_delayed_work(&trigger_data->work, 0); + } +} + +static ssize_t device_name_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + ssize_t len; + + spin_lock_bh(&trigger_data->lock); + len = sprintf(buf, "%s\n", trigger_data->device_name); + spin_unlock_bh(&trigger_data->lock); + + return len; +} + +static ssize_t device_name_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t size) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + + if (size >= IFNAMSIZ) + return -EINVAL; + + cancel_delayed_work_sync(&trigger_data->work); + + spin_lock_bh(&trigger_data->lock); + + if (trigger_data->net_dev) { + dev_put(trigger_data->net_dev); + trigger_data->net_dev = NULL; + } + + strncpy(trigger_data->device_name, buf, size); + if (size > 0 && trigger_data->device_name[size - 1] == '\n') + trigger_data->device_name[size - 1] = 0; + + if (trigger_data->device_name[0] != 0) + trigger_data->net_dev = + dev_get_by_name(&init_net, trigger_data->device_name); + + clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); + if (trigger_data->net_dev != NULL) + if (netif_carrier_ok(trigger_data->net_dev)) + set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); + + trigger_data->last_activity = 0; + + set_baseline_state(trigger_data); + spin_unlock_bh(&trigger_data->lock); + + return size; +} + +static DEVICE_ATTR_RW(device_name); + +static ssize_t netdev_led_attr_show(struct device *dev, char *buf, + enum netdev_led_attr attr) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + int bit; + + switch (attr) { + case NETDEV_ATTR_LINK: + bit = NETDEV_LED_LINK; + break; + case NETDEV_ATTR_TX: + bit = NETDEV_LED_TX; + break; + case NETDEV_ATTR_RX: + bit = NETDEV_LED_RX; + break; + default: + return -EINVAL; + } + + return sprintf(buf, "%u\n", test_bit(bit, &trigger_data->mode)); +} + +static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, + size_t size, enum netdev_led_attr attr) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + unsigned long state; + int ret; + int bit; + + ret = kstrtoul(buf, 0, &state); + if (ret) + return ret; + + switch (attr) { + case NETDEV_ATTR_LINK: + bit = NETDEV_LED_LINK; + break; + case NETDEV_ATTR_TX: + bit = NETDEV_LED_TX; + break; + case NETDEV_ATTR_RX: + bit = NETDEV_LED_RX; + break; + default: + return -EINVAL; + } + + cancel_delayed_work_sync(&trigger_data->work); + + if (state) + set_bit(bit, &trigger_data->mode); + else + clear_bit(bit, &trigger_data->mode); + + set_baseline_state(trigger_data); + + return size; +} + +static ssize_t link_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_LINK); +} + +static ssize_t link_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t size) +{ + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_LINK); +} + +static DEVICE_ATTR_RW(link); + +static ssize_t tx_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_TX); +} + +static ssize_t tx_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t size) +{ + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_TX); +} + +static DEVICE_ATTR_RW(tx); + +static ssize_t rx_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return netdev_led_attr_show(dev, buf, NETDEV_ATTR_RX); +} + +static ssize_t rx_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t size) +{ + return netdev_led_attr_store(dev, buf, size, NETDEV_ATTR_RX); +} + +static DEVICE_ATTR_RW(rx); + +static ssize_t interval_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + + return sprintf(buf, "%u\n", + jiffies_to_msecs(atomic_read(&trigger_data->interval))); +} + +static ssize_t interval_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t size) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + unsigned long value; + int ret; + + ret = kstrtoul(buf, 0, &value); + if (ret) + return ret; + + /* impose some basic bounds on the timer interval */ + if (value >= 5 && value <= 10000) { + cancel_delayed_work_sync(&trigger_data->work); + + atomic_set(&trigger_data->interval, msecs_to_jiffies(value)); + set_baseline_state(trigger_data); /* resets timer */ + } + + return size; +} + +static DEVICE_ATTR_RW(interval); + +static int netdev_trig_notify(struct notifier_block *nb, + unsigned long evt, void *dv) +{ + struct net_device *dev = + netdev_notifier_info_to_dev((struct netdev_notifier_info *)dv); + struct led_netdev_data *trigger_data = container_of(nb, + struct + led_netdev_data, + notifier); + + if (evt != NETDEV_UP && evt != NETDEV_DOWN && evt != NETDEV_CHANGE + && evt != NETDEV_REGISTER && evt != NETDEV_UNREGISTER + && evt != NETDEV_CHANGENAME) + return NOTIFY_DONE; + + if (strcmp(dev->name, trigger_data->device_name)) + return NOTIFY_DONE; + + cancel_delayed_work_sync(&trigger_data->work); + + spin_lock_bh(&trigger_data->lock); + + clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); + switch (evt) { + case NETDEV_REGISTER: + if (trigger_data->net_dev) + dev_put(trigger_data->net_dev); + dev_hold(dev); + trigger_data->net_dev = dev; + break; + case NETDEV_CHANGENAME: + case NETDEV_UNREGISTER: + if (trigger_data->net_dev) { + dev_put(trigger_data->net_dev); + trigger_data->net_dev = NULL; + } + break; + case NETDEV_UP: + case NETDEV_CHANGE: + if (netif_carrier_ok(dev)) + set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode); + break; + } + + set_baseline_state(trigger_data); + + spin_unlock_bh(&trigger_data->lock); + + return NOTIFY_DONE; +} + +/* here's the real work! */ +static void netdev_trig_work(struct work_struct *work) +{ + struct led_netdev_data *trigger_data = container_of(work, + struct + led_netdev_data, + work.work); + struct rtnl_link_stats64 *dev_stats; + unsigned int new_activity; + struct rtnl_link_stats64 temp; + unsigned long interval; + int invert; + + /* If we dont have a device, insure we are off */ + if (!trigger_data->net_dev) { + led_set_brightness(trigger_data->led_cdev, LED_OFF); + return; + } + + /* If we are not looking for RX/TX then return */ + if (!test_bit(NETDEV_LED_TX, &trigger_data->mode) && + !test_bit(NETDEV_LED_RX, &trigger_data->mode)) + return; + + dev_stats = dev_get_stats(trigger_data->net_dev, &temp); + new_activity = + (test_bit(NETDEV_LED_TX, &trigger_data->mode) ? + dev_stats->tx_packets : 0) + + (test_bit(NETDEV_LED_RX, &trigger_data->mode) ? + dev_stats->rx_packets : 0); + + if (trigger_data->last_activity != new_activity) { + led_stop_software_blink(trigger_data->led_cdev); + + invert = test_bit(NETDEV_LED_LINK, &trigger_data->mode); + interval = jiffies_to_msecs( + atomic_read(&trigger_data->interval)); + /* base state is ON (link present) */ + led_blink_set_oneshot(trigger_data->led_cdev, + &interval, + &interval, + invert); + trigger_data->last_activity = new_activity; + } + + schedule_delayed_work(&trigger_data->work, + (atomic_read(&trigger_data->interval)*2)); +} + +static void netdev_trig_activate(struct led_classdev *led_cdev) +{ + struct led_netdev_data *trigger_data; + int rc; + + trigger_data = kzalloc(sizeof(struct led_netdev_data), GFP_KERNEL); + if (!trigger_data) + return; + + spin_lock_init(&trigger_data->lock); + + trigger_data->notifier.notifier_call = netdev_trig_notify; + trigger_data->notifier.priority = 10; + + INIT_DELAYED_WORK(&trigger_data->work, netdev_trig_work); + + trigger_data->led_cdev = led_cdev; + trigger_data->net_dev = NULL; + trigger_data->device_name[0] = 0; + + trigger_data->mode = 0; + atomic_set(&trigger_data->interval, msecs_to_jiffies(50)); + trigger_data->last_activity = 0; + + led_cdev->trigger_data = trigger_data; + + rc = device_create_file(led_cdev->dev, &dev_attr_device_name); + if (rc) + goto err_out; + rc = device_create_file(led_cdev->dev, &dev_attr_link); + if (rc) + goto err_out_device_name; + rc = device_create_file(led_cdev->dev, &dev_attr_rx); + if (rc) + goto err_out_link; + rc = device_create_file(led_cdev->dev, &dev_attr_tx); + if (rc) + goto err_out_rx; + rc = device_create_file(led_cdev->dev, &dev_attr_interval); + if (rc) + goto err_out_tx; + rc = register_netdevice_notifier(&trigger_data->notifier); + if (rc) + goto err_out_interval; + return; + +err_out_interval: + device_remove_file(led_cdev->dev, &dev_attr_interval); +err_out_tx: + device_remove_file(led_cdev->dev, &dev_attr_tx); +err_out_rx: + device_remove_file(led_cdev->dev, &dev_attr_rx); +err_out_link: + device_remove_file(led_cdev->dev, &dev_attr_link); +err_out_device_name: + device_remove_file(led_cdev->dev, &dev_attr_device_name); +err_out: + led_cdev->trigger_data = NULL; + kfree(trigger_data); +} + +static void netdev_trig_deactivate(struct led_classdev *led_cdev) +{ + struct led_netdev_data *trigger_data = led_cdev->trigger_data; + + if (trigger_data) { + unregister_netdevice_notifier(&trigger_data->notifier); + + device_remove_file(led_cdev->dev, &dev_attr_device_name); + device_remove_file(led_cdev->dev, &dev_attr_link); + device_remove_file(led_cdev->dev, &dev_attr_rx); + device_remove_file(led_cdev->dev, &dev_attr_tx); + device_remove_file(led_cdev->dev, &dev_attr_interval); + + cancel_delayed_work_sync(&trigger_data->work); + + if (trigger_data->net_dev) + dev_put(trigger_data->net_dev); + + kfree(trigger_data); + } +} + +static struct led_trigger netdev_led_trigger = { + .name = "netdev", + .activate = netdev_trig_activate, + .deactivate = netdev_trig_deactivate, +}; + +static int __init netdev_trig_init(void) +{ + return led_trigger_register(&netdev_led_trigger); +} + +static void __exit netdev_trig_exit(void) +{ + led_trigger_unregister(&netdev_led_trigger); +} + +module_init(netdev_trig_init); +module_exit(netdev_trig_exit); + +MODULE_AUTHOR("Ben Whitten <ben.whitten@gmail.com>"); +MODULE_AUTHOR("Oliver Jowett <oliver@opencloud.com>"); +MODULE_DESCRIPTION("Netdev LED trigger"); +MODULE_LICENSE("GPL v2");
This commit introduces a NETDEV trigger for named device activity. Available triggers are link, rx, and tx. Signed-off-by: Ben Whitten <ben.whitten@gmail.com> --- Changes in v4: Adopt SPDX licence header Changes in v3: Cancel the software blink prior to a oneshot re-queue Changes in v2: Sort includes and redate documentation Correct licence Remove macro and replace with generic function using enums Convert blink logic in stats work to use led_blink_oneshot Uses configured brightness instead of FULL --- .../ABI/testing/sysfs-class-led-trigger-netdev | 45 ++ drivers/leds/trigger/Kconfig | 7 + drivers/leds/trigger/Makefile | 1 + drivers/leds/trigger/ledtrig-netdev.c | 498 +++++++++++++++++++++ 4 files changed, 551 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev create mode 100644 drivers/leds/trigger/ledtrig-netdev.c