On 10/31/20 12:45 AM, Marek Behún wrote:
On Fri, 30 Oct 2020 23:37:52 +0100
Jacek Anaszewski wrote:
Hi Marek,
Bitops are guaranteed to be atomic and this was done for a reason.
Hmm okay...
Sooo, netdev_trig_work cannot be executed at the same time as the
link/linkup/rx/tx changing stuff
->blink_brightness)
led_cdev->blink_brightness = led_cdev->max_brightness;
- if (!test_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode))
+ if (!trigger_data->linkup)
led_set_brightness(led_cdev, LED_OFF);
[...]
--
Best regards,
Jacek Anaszewski
void(*deactivate)(struct led_classdev *led_cdev);
+ /* LED-private triggers have this set */
+ struct led_hw_trigger_type *trigger_type;
+
/* LEDs under control by this trigger (for simple triggers) */
rwlock_t leddev_list_lock;
struct list_head led_cdevs;
Looks good to me:
Acked-by: Jacek Anaszewski
--
Best regards,
Jacek Anaszewski
/can/led.h | 54 --
include/linux/leds.h | 18
24 files changed, 1 insertion(+), 395 deletions(-)
delete mode 100644 drivers/net/can/led.c
delete mode 100644 include/linux/can/led.h
--
Best regards,
Jacek Anaszewski
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 ");
> +MODULE_AUTHOR("Oliver Jowett ");
> +MODULE_DESCRIPTION("Netdev LED trigger");
> +MODULE_LICENSE("GPL v2");
>
Applied to the for-next branch of linux-leds.git.
--
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
> wrote:
>> Hi Ben,
>>
>> Thanks for the update. I have one doubt about comment style
>> at the top of the file. Please refer below.
>>
>> On
wett
> +/*
> + * 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
> + *
> + */
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
--
Best regards,
Jacek Anaszewski
+
> drivers/leds/trigger/Makefile | 1 +
> drivers/leds/trigger/ledtrig-netdev.c | 507
> +
> 4 files changed, 560 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev
> create mode 100644 drivers/leds/trigger/ledtrig-netdev.c
>
--
Best regards,
Jacek Anaszewski
le(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 ");
> +MODULE_AUTHOR("Oliver Jowett ");
> +MODULE_DESCRIPTION("Netdev LED trigger");
> +MODULE_LICENSE("GPL");
"GPL v2" according to the licensing information from the top
of the file.
--
Best regards,
Jacek Anaszewski
ses as he finds it
clearer. I think it applies to patches 29-36. I am not sure about patches
26-28,37.
Dropped 30/38 and 31/38 from LED tree then.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a messag
10 matches
Mail list logo
