Re: [RFC v3 3/3] phy,leds: add support for led triggers on phy link state change
On Mon, Oct 10, 2016 at 02:03:32AM -0700, Florian Fainelli wrote: > > + > > +#ifdef CONFIG_LED_TRIGGER_PHY > > + > > +#include > > +#include > > + > > +#define PHY_LINK_LED_MAX_TRIGGERS 5 > > +#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 7 > > +#define PHY_MII_BUS_ID_SIZE(20 - 3) > > This particular constant may be something worth moving to > include/linux/phy.h eventually. > -- > Florian MII_BUS_ID_SIZE is defined in include/linux/phy.h but it's defined after phy_led_triggers.h is included so phy_led_triggers.h doesn't have access. I could move the definition of MII_BUS_ID_SIZE above the include, but that seemed ugly. Do you have any suggestions? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v3 3/3] phy,leds: add support for led triggers on phy link state change
> Andrew, are you happy with this implementation? Sorry, been to busy with other things to follow the discussion. What would be nice to see is a comment about how the link to LEDs in the PHYs is made. Often there is a couple of LEDs in the RJ45 socket driven by the PHY. They can show link, speed, packet Rx/Tx, etc. How are these triggers related to these LEDs? Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v3 3/3] phy,leds: add support for led triggers on phy link state change
On 10/07/2016 02:14 PM, Zach Brown wrote: > From: Josh Cartwright > > Create an option CONFIG_LED_TRIGGER_PHY (default n), which will > create a set of led triggers for each instantiated PHY device. There is > one LED trigger per link-speed, per-phy. > > This allows for a user to configure their system to allow a set of LEDs > to represent link state changes on the phy. Seems like we are past the RFC state now, so you might want to prefix your patches with [PATCH ..] now. Also, the typical prefix used for PHYLIB changes is net: phy: foo Other than that: Reviewed-by: Florian Fainelli Andrew, are you happy with this implementation? [snip] > + > +#ifdef CONFIG_LED_TRIGGER_PHY > + > +#include > +#include > + > +#define PHY_LINK_LED_MAX_TRIGGERS5 > +#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE7 > +#define PHY_MII_BUS_ID_SIZE (20 - 3) This particular constant may be something worth moving to include/linux/phy.h eventually. -- Florian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel