Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem

[Andrew Lunn <andrew@lunn.ch>]

On Wed, Mar 23, 2016 at 10:24:45PM +0100, Vishal Thanki wrote:
> On Wed, Mar 23, 2016 at 10:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
> >> Hi,
> >>
> >> > My suggestion was that the hardware needs to control the LEDs. You
> >> > have software doing it. You might be able to do this with the PHY
> >> > state machine for link. But activity is never going to be accepted if
> >> > software control.
> >> >
> >> > The LED trigger attached to an LED should be used to configure the
> >> > hardware to drive the LED as wanted.
> >> >
> >>
> >> The eth-phy-activity trigger uses the blink_set which I think uses the
> >> hardware acceleration if available. I am not sure how to handles LEDs
> >> which does not have hardware acceleration for this (eth-phy-activity)
> >> trigger.
> >
> > We want the LED to blink on activity, real packets coming in and
> > out. The PHY can do this, so let the PHY control the LED. In this
> > case, the trigger is just mechanism for the user to say what the LED
> > should be used for. The trigger is not itself controlling the LED, it
> > has no idea about packets coming and going.
> >
> 
> Yes, I understand that. But PHY can only control the LEDs attached to
> it directly. The at803x led driver configures the PHY to blink the
> activity LED based on traffic but I think it is not possible for PHY
> to control other LEDs in system, for example some other LEDs in system
> controlled only via GPIO. In such cases, putting PHY activity trigger
> on the GPIO LEDs would not make sense. Correct me if I am wrong.

Hi Vishal

All correct. Which is why i said in my original email, you need to
extend the LED core to associate triggers to LEDs. You can then
associate the eth-phy-activity trigger to only PHY leds which can
implement that functionality.

drivers/leds/led-triggers.c contains a global list
LIST_HEAD(trigger_list) which triggers get added to using
led_trigger_register(). You could add a second list to the
led_classdev structure, and add an led_trigger_register_to_led()
function which registers a trigger to a specific LED, on its own
trigger list. led_trigger_store() and led_trigger_show() would use
both lists.

     Andrew