Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hi Fabio! > On Mon, Dec 17, 2012 at 09:20:57PM +0100, "Bernd Krumböck" wrote: >> > If you think it's useful for USB controller, just tell me or modify >> the >> > driver by yourself! As you see the patch is really easy. >> >> At least it is useful for the usb_8dev driver. I'll write a patch. >> >> Photo of the device: >> http://www.8devices.com/product/2/usb2can > > Sure that it's needed? That status LED is probably controlled directly > by the firmware itself. Anyway I think it makes sense to support all > the mainline drivers and I'm really happy if you test the patch on > your hardware! Yes, it is controlled by the firmware, but it does not show rx/tx. Whereas my OpenWRT hardware has enough leds. ;-) regards, Bernd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
On Mon, Dec 17, 2012 at 09:20:57PM +0100, "Bernd Krumböck" wrote: > > If you think it's useful for USB controller, just tell me or modify the > > driver by yourself! As you see the patch is really easy. > > At least it is useful for the usb_8dev driver. I'll write a patch. > > Photo of the device: > http://www.8devices.com/product/2/usb2can Sure that it's needed? That status LED is probably controlled directly by the firmware itself. Anyway I think it makes sense to support all the mainline drivers and I'm really happy if you test the patch on your hardware! My only suggestion is to keep the LED support on a separate patch from the "add driver" one. Cheers, Fabio -- Fabio Baltieri -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
> Hi Bernd, > > On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: >> Why there is no patch for any usb can device? > > Because USB canbus interfaces usually already has some dedicated > activity LED on the device itself, while this patch is meant to give an > equivalent functionality for Embedded SoC with GPIO based LEDs, so I > just started by modifying some Embedded CAN drivers. > > If you think it's useful for USB controller, just tell me or modify the > driver by yourself! As you see the patch is really easy. At least it is useful for the usb_8dev driver. I'll write a patch. Photo of the device: http://www.8devices.com/product/2/usb2can >> Can this be done in a more general way, except patching every driver? > > Actually this started as a generic patch bolted to the CAN stack, but > the implementation was too invasive and a bit too hacky. A generic > implementation at can-dev level is hard to obtain because the can-dev > layer is not used in all the device operation, as the most of the driver > calls generick network API directly. > Thanks! regards, Bernd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hi Bernd, On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: > Why there is no patch for any usb can device? Because USB canbus interfaces usually already has some dedicated activity LED on the device itself, while this patch is meant to give an equivalent functionality for Embedded SoC with GPIO based LEDs, so I just started by modifying some Embedded CAN drivers. If you think it's useful for USB controller, just tell me or modify the driver by yourself! As you see the patch is really easy. > Can this be done in a more general way, except patching every driver? Actually this started as a generic patch bolted to the CAN stack, but the implementation was too invasive and a bit too hacky. A generic implementation at can-dev level is hard to obtain because the can-dev layer is not used in all the device operation, as the most of the driver calls generick network API directly. Fabio -- Fabio Baltieri -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: > Hello Fabio! > > Am 2012-12-16 12:08, schrieb Fabio Baltieri: > >This patch implements the functions to add two LED triggers, named > >-tx and -rx, to a canbus device driver. > > > >Triggers are called from specific handlers by each CAN device driver and > >can be disabled altogether with a Kconfig option. > > > >The implementation keeps the LED on when the interface is UP and blinks > >the LED on network activity at a configurable rate. > > > >This only supports can-dev based drivers, as it uses some support field > >in the can_priv structure. > > > >Supported drivers should call devm_can_led_init() and can_led_event() as > >needed. > > > >Cleanup is handled automatically by devres, so no *_exit function is > >needed. > > > >Supported events are: > >- CAN_LED_EVENT_OPEN: turn on tx/rx LEDs > >- CAN_LED_EVENT_STOP: turn off tx/rx LEDs > >- CAN_LED_EVENT_TX: trigger tx LED blink > >- CAN_LED_EVENT_RX: trigger tx LED blink > > Why there is no patch for any usb can device? > > Can this be done in a more general way, except patching every driver? There must be a way. Up to this series, there were a few minor problems: * determine if a CAN device is can_dev based * led trigger calls. After this series, I think it's possible to make this functionality generic. But up to then, this per-driver code is a working model. Kind regards, Kurt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hi Bernd, On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: Why there is no patch for any usb can device? Because USB canbus interfaces usually already has some dedicated activity LED on the device itself, while this patch is meant to give an equivalent functionality for Embedded SoC with GPIO based LEDs, so I just started by modifying some Embedded CAN drivers. If you think it's useful for USB controller, just tell me or modify the driver by yourself! As you see the patch is really easy. Can this be done in a more general way, except patching every driver? Actually this started as a generic patch bolted to the CAN stack, but the implementation was too invasive and a bit too hacky. A generic implementation at can-dev level is hard to obtain because the can-dev layer is not used in all the device operation, as the most of the driver calls generick network API directly. Fabio -- Fabio Baltieri -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hi Bernd, On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: Why there is no patch for any usb can device? Because USB canbus interfaces usually already has some dedicated activity LED on the device itself, while this patch is meant to give an equivalent functionality for Embedded SoC with GPIO based LEDs, so I just started by modifying some Embedded CAN drivers. If you think it's useful for USB controller, just tell me or modify the driver by yourself! As you see the patch is really easy. At least it is useful for the usb_8dev driver. I'll write a patch. Photo of the device: http://www.8devices.com/product/2/usb2can Can this be done in a more general way, except patching every driver? Actually this started as a generic patch bolted to the CAN stack, but the implementation was too invasive and a bit too hacky. A generic implementation at can-dev level is hard to obtain because the can-dev layer is not used in all the device operation, as the most of the driver calls generick network API directly. Thanks! regards, Bernd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
On Mon, Dec 17, 2012 at 09:20:57PM +0100, Bernd Krumböck wrote: If you think it's useful for USB controller, just tell me or modify the driver by yourself! As you see the patch is really easy. At least it is useful for the usb_8dev driver. I'll write a patch. Photo of the device: http://www.8devices.com/product/2/usb2can Sure that it's needed? That status LED is probably controlled directly by the firmware itself. Anyway I think it makes sense to support all the mainline drivers and I'm really happy if you test the patch on your hardware! My only suggestion is to keep the LED support on a separate patch from the add driver one. Cheers, Fabio -- Fabio Baltieri -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hi Fabio! On Mon, Dec 17, 2012 at 09:20:57PM +0100, Bernd Krumböck wrote: If you think it's useful for USB controller, just tell me or modify the driver by yourself! As you see the patch is really easy. At least it is useful for the usb_8dev driver. I'll write a patch. Photo of the device: http://www.8devices.com/product/2/usb2can Sure that it's needed? That status LED is probably controlled directly by the firmware itself. Anyway I think it makes sense to support all the mainline drivers and I'm really happy if you test the patch on your hardware! Yes, it is controlled by the firmware, but it does not show rx/tx. Whereas my OpenWRT hardware has enough leds. ;-) regards, Bernd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
On Mon, Dec 17, 2012 at 08:28:48AM +0100, Bernd Krumboeck wrote: Hello Fabio! Am 2012-12-16 12:08, schrieb Fabio Baltieri: This patch implements the functions to add two LED triggers, named ifname-tx and ifname-rx, to a canbus device driver. Triggers are called from specific handlers by each CAN device driver and can be disabled altogether with a Kconfig option. The implementation keeps the LED on when the interface is UP and blinks the LED on network activity at a configurable rate. This only supports can-dev based drivers, as it uses some support field in the can_priv structure. Supported drivers should call devm_can_led_init() and can_led_event() as needed. Cleanup is handled automatically by devres, so no *_exit function is needed. Supported events are: - CAN_LED_EVENT_OPEN: turn on tx/rx LEDs - CAN_LED_EVENT_STOP: turn off tx/rx LEDs - CAN_LED_EVENT_TX: trigger tx LED blink - CAN_LED_EVENT_RX: trigger tx LED blink Why there is no patch for any usb can device? Can this be done in a more general way, except patching every driver? There must be a way. Up to this series, there were a few minor problems: * determine if a CAN device is can_dev based * led trigger calls. After this series, I think it's possible to make this functionality generic. But up to then, this per-driver code is a working model. Kind regards, Kurt -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hello Fabio! Am 2012-12-16 12:08, schrieb Fabio Baltieri: This patch implements the functions to add two LED triggers, named -tx and -rx, to a canbus device driver. Triggers are called from specific handlers by each CAN device driver and can be disabled altogether with a Kconfig option. The implementation keeps the LED on when the interface is UP and blinks the LED on network activity at a configurable rate. This only supports can-dev based drivers, as it uses some support field in the can_priv structure. Supported drivers should call devm_can_led_init() and can_led_event() as needed. Cleanup is handled automatically by devres, so no *_exit function is needed. Supported events are: - CAN_LED_EVENT_OPEN: turn on tx/rx LEDs - CAN_LED_EVENT_STOP: turn off tx/rx LEDs - CAN_LED_EVENT_TX: trigger tx LED blink - CAN_LED_EVENT_RX: trigger tx LED blink Why there is no patch for any usb can device? Can this be done in a more general way, except patching every driver? regards, Bernd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] can: add tx/rx LED trigger support
This patch implements the functions to add two LED triggers, named -tx and -rx, to a canbus device driver. Triggers are called from specific handlers by each CAN device driver and can be disabled altogether with a Kconfig option. The implementation keeps the LED on when the interface is UP and blinks the LED on network activity at a configurable rate. This only supports can-dev based drivers, as it uses some support field in the can_priv structure. Supported drivers should call devm_can_led_init() and can_led_event() as needed. Cleanup is handled automatically by devres, so no *_exit function is needed. Supported events are: - CAN_LED_EVENT_OPEN: turn on tx/rx LEDs - CAN_LED_EVENT_STOP: turn off tx/rx LEDs - CAN_LED_EVENT_TX: trigger tx LED blink - CAN_LED_EVENT_RX: trigger tx LED blink Cc: Wolfgang Grandegger Cc: Marc Kleine-Budde Signed-off-by: Fabio Baltieri Acked-by: Oliver Hartkopp --- drivers/net/can/Kconfig | 12 +++ drivers/net/can/Makefile | 2 ++ drivers/net/can/led.c| 89 include/linux/can/dev.h | 8 + include/linux/can/led.h | 42 +++ 5 files changed, 153 insertions(+) create mode 100644 drivers/net/can/led.c create mode 100644 include/linux/can/led.h diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index b56bd9e..8e59120 100644 --- a/drivers/net/can/Kconfig +++ b/drivers/net/can/Kconfig @@ -54,6 +54,18 @@ config CAN_CALC_BITTIMING arguments "tq", "prop_seg", "phase_seg1", "phase_seg2" and "sjw". If unsure, say Y. +config CAN_LEDS + bool "Enable LED triggers for Netlink based drivers" + depends on CAN_DEV + depends on LEDS_CLASS + select LEDS_TRIGGERS + ---help--- + This option adds two LED triggers for packet receive and transmit + events on each supported CAN device. + + Say Y here if you are working on a system with led-class supported + LEDs and you want to use them as canbus activity indicators. + config CAN_AT91 tristate "Atmel AT91 onchip CAN controller" depends on CAN_DEV && (ARCH_AT91SAM9263 || ARCH_AT91SAM9X5) diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index 7de5986..c744039 100644 --- a/drivers/net/can/Makefile +++ b/drivers/net/can/Makefile @@ -8,6 +8,8 @@ obj-$(CONFIG_CAN_SLCAN) += slcan.o obj-$(CONFIG_CAN_DEV) += can-dev.o can-dev-y := dev.o +can-dev-$(CONFIG_CAN_LEDS) += led.o + obj-y += usb/ obj-y += softing/ diff --git a/drivers/net/can/led.c b/drivers/net/can/led.c new file mode 100644 index 000..eaa14ac --- /dev/null +++ b/drivers/net/can/led.c @@ -0,0 +1,89 @@ +/* + * Copyright 2012, Fabio Baltieri + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +#include + +static unsigned long led_delay = 50; +module_param(led_delay, ulong, 0644); +MODULE_PARM_DESC(led_delay, + "blink delay time for activity leds (msecs, default: 50)."); + +/* + * Trigger a LED event in response to a CAN device event + */ +void can_led_event(struct net_device *netdev, enum can_led_event event) +{ + struct can_priv *priv = netdev_priv(netdev); + + switch (event) { + case CAN_LED_EVENT_OPEN: + led_trigger_event(priv->tx_led_trig, LED_FULL); + led_trigger_event(priv->rx_led_trig, LED_FULL); + break; + case CAN_LED_EVENT_STOP: + led_trigger_event(priv->tx_led_trig, LED_OFF); + led_trigger_event(priv->rx_led_trig, LED_OFF); + break; + case CAN_LED_EVENT_TX: + if (led_delay) + led_trigger_blink_oneshot(priv->tx_led_trig, + _delay, _delay, 1); + break; + case CAN_LED_EVENT_RX: + if (led_delay) + led_trigger_blink_oneshot(priv->rx_led_trig, + _delay, _delay, 1); + break; + } +} +EXPORT_SYMBOL_GPL(can_led_event); + +static void can_led_release(struct device *gendev, void *res) +{ + struct can_priv *priv = netdev_priv(to_net_dev(gendev)); + + led_trigger_unregister_simple(priv->tx_led_trig); + led_trigger_unregister_simple(priv->rx_led_trig); +} + +/* + * Register CAN LED triggers for a CAN device + * + * This is normally called from a driver's probe function + */ +void devm_can_led_init(struct net_device *netdev) +{ + struct can_priv *priv = netdev_priv(netdev); + void *res; + + res = devres_alloc(can_led_release, 0, GFP_KERNEL); + if (!res) { + netdev_err(netdev, "cannot
[PATCH 1/9] can: add tx/rx LED trigger support
This patch implements the functions to add two LED triggers, named ifname-tx and ifname-rx, to a canbus device driver. Triggers are called from specific handlers by each CAN device driver and can be disabled altogether with a Kconfig option. The implementation keeps the LED on when the interface is UP and blinks the LED on network activity at a configurable rate. This only supports can-dev based drivers, as it uses some support field in the can_priv structure. Supported drivers should call devm_can_led_init() and can_led_event() as needed. Cleanup is handled automatically by devres, so no *_exit function is needed. Supported events are: - CAN_LED_EVENT_OPEN: turn on tx/rx LEDs - CAN_LED_EVENT_STOP: turn off tx/rx LEDs - CAN_LED_EVENT_TX: trigger tx LED blink - CAN_LED_EVENT_RX: trigger tx LED blink Cc: Wolfgang Grandegger w...@grandegger.com Cc: Marc Kleine-Budde m...@pengutronix.de Signed-off-by: Fabio Baltieri fabio.balti...@gmail.com Acked-by: Oliver Hartkopp socket...@hartkopp.net --- drivers/net/can/Kconfig | 12 +++ drivers/net/can/Makefile | 2 ++ drivers/net/can/led.c| 89 include/linux/can/dev.h | 8 + include/linux/can/led.h | 42 +++ 5 files changed, 153 insertions(+) create mode 100644 drivers/net/can/led.c create mode 100644 include/linux/can/led.h diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index b56bd9e..8e59120 100644 --- a/drivers/net/can/Kconfig +++ b/drivers/net/can/Kconfig @@ -54,6 +54,18 @@ config CAN_CALC_BITTIMING arguments tq, prop_seg, phase_seg1, phase_seg2 and sjw. If unsure, say Y. +config CAN_LEDS + bool Enable LED triggers for Netlink based drivers + depends on CAN_DEV + depends on LEDS_CLASS + select LEDS_TRIGGERS + ---help--- + This option adds two LED triggers for packet receive and transmit + events on each supported CAN device. + + Say Y here if you are working on a system with led-class supported + LEDs and you want to use them as canbus activity indicators. + config CAN_AT91 tristate Atmel AT91 onchip CAN controller depends on CAN_DEV (ARCH_AT91SAM9263 || ARCH_AT91SAM9X5) diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index 7de5986..c744039 100644 --- a/drivers/net/can/Makefile +++ b/drivers/net/can/Makefile @@ -8,6 +8,8 @@ obj-$(CONFIG_CAN_SLCAN) += slcan.o obj-$(CONFIG_CAN_DEV) += can-dev.o can-dev-y := dev.o +can-dev-$(CONFIG_CAN_LEDS) += led.o + obj-y += usb/ obj-y += softing/ diff --git a/drivers/net/can/led.c b/drivers/net/can/led.c new file mode 100644 index 000..eaa14ac --- /dev/null +++ b/drivers/net/can/led.c @@ -0,0 +1,89 @@ +/* + * Copyright 2012, Fabio Baltieri fabio.balti...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/device.h +#include linux/kernel.h +#include linux/slab.h +#include linux/netdevice.h +#include linux/can/dev.h + +#include linux/can/led.h + +static unsigned long led_delay = 50; +module_param(led_delay, ulong, 0644); +MODULE_PARM_DESC(led_delay, + blink delay time for activity leds (msecs, default: 50).); + +/* + * Trigger a LED event in response to a CAN device event + */ +void can_led_event(struct net_device *netdev, enum can_led_event event) +{ + struct can_priv *priv = netdev_priv(netdev); + + switch (event) { + case CAN_LED_EVENT_OPEN: + led_trigger_event(priv-tx_led_trig, LED_FULL); + led_trigger_event(priv-rx_led_trig, LED_FULL); + break; + case CAN_LED_EVENT_STOP: + led_trigger_event(priv-tx_led_trig, LED_OFF); + led_trigger_event(priv-rx_led_trig, LED_OFF); + break; + case CAN_LED_EVENT_TX: + if (led_delay) + led_trigger_blink_oneshot(priv-tx_led_trig, + led_delay, led_delay, 1); + break; + case CAN_LED_EVENT_RX: + if (led_delay) + led_trigger_blink_oneshot(priv-rx_led_trig, + led_delay, led_delay, 1); + break; + } +} +EXPORT_SYMBOL_GPL(can_led_event); + +static void can_led_release(struct device *gendev, void *res) +{ + struct can_priv *priv = netdev_priv(to_net_dev(gendev)); + + led_trigger_unregister_simple(priv-tx_led_trig); + led_trigger_unregister_simple(priv-rx_led_trig); +} + +/* + * Register CAN LED triggers for a CAN device + * + * This is normally called from a driver's probe function + */ +void devm_can_led_init(struct net_device
Re: [PATCH 1/9] can: add tx/rx LED trigger support
Hello Fabio! Am 2012-12-16 12:08, schrieb Fabio Baltieri: This patch implements the functions to add two LED triggers, named ifname-tx and ifname-rx, to a canbus device driver. Triggers are called from specific handlers by each CAN device driver and can be disabled altogether with a Kconfig option. The implementation keeps the LED on when the interface is UP and blinks the LED on network activity at a configurable rate. This only supports can-dev based drivers, as it uses some support field in the can_priv structure. Supported drivers should call devm_can_led_init() and can_led_event() as needed. Cleanup is handled automatically by devres, so no *_exit function is needed. Supported events are: - CAN_LED_EVENT_OPEN: turn on tx/rx LEDs - CAN_LED_EVENT_STOP: turn off tx/rx LEDs - CAN_LED_EVENT_TX: trigger tx LED blink - CAN_LED_EVENT_RX: trigger tx LED blink Why there is no patch for any usb can device? Can this be done in a more general way, except patching every driver? regards, Bernd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/