Messages in this thread | | | Date | Fri, 1 Oct 2021 12:52:45 +0200 | Subject | Re: [PATCH 1/3] net: phy: mscc: Add possibilty to disable combined LED mode | From | Frieder Schrempf <> |
| |
Hi Marek,
On 01.10.21 12:09, Marek Behún wrote: > On Fri, 1 Oct 2021 11:20:36 +0200 > Frieder Schrempf <frieder.schrempf@kontron.de> wrote: > >> On 01.10.21 02:05, Andrew Lunn wrote: >>> On Thu, Sep 30, 2021 at 02:57:43PM +0200, Frieder Schrempf wrote: >>>> From: Frieder Schrempf <frieder.schrempf@kontron.de> >>>> >>>> By default the LED modes offer to combine two indicators like speed/link >>>> and activity in one LED. In order to use a LED only for the first of the >>>> two modes, the combined feature needs to be disabled. >>>> >>>> In order to do this we introduce a boolean devicetree property >>>> 'vsc8531,led-[N]-combine-disable' and wire it up to the matching >>>> bits in the LED behavior register. >>> >>> Sorry, but no DT property. Each PHY has its own magic combination of >>> DT properties, nothing shared, nothing common. This does not scale. >>> >>> Please look at the work being done to control PHY LEDs using the Linux >>> LED infrastructure. That should give us one uniform interface for all >>> PHY LEDs. >> >> +Cc: Marek >> >> I guess you are referring to this: [1]? >> >> If so, the last version I could find is a year old now. Is anyone still >> working on this? > > Yes, I am still working on this. > > Anyway the last version is not one year old, the last version to add > this support is 4 months old: > https://lore.kernel.org/netdev/20210602144439.4d20b295@dellmb/T/
Thanks for pointing out the latest patches. Good to know that you are still working on this.
> > This version does not add the code for ethernet PHYs, instead it just > tries to touch only the LED subsystem by adding the API for offloading > LED triggers and an example implementation for Turris Omnia LED > controller. > > I will probably send another version this weekend. Sorry this takes > this long.
No worries, and thanks for the work!
> > >> I understand, that the generic approach is the one we want to have, but >> does this really mean adding PHY led configuration via DT to existing >> drivers (that already use DT properties for LED modes) is not accepted >> anymore, even if the new API is not yet in place? > > I don't know about Rob, but I would be against it. > > But if you need to have your PHY LED configured with via devicetree > ASAP, instead of proposing the vendor specific property, you can > propose LED subnodes and properties that will be generic and compatible > with the LED subsystem API, i.e. something like: > > ethernet-phy@1 { > .... eth phy properties; > > leds { > led@0 { > reg = <0>; > color = <LED_COLOR_ID_GREEN>; > /* this LED should indicate link/speed */ > function = LED_FUNCTION_LINK; > }; > }; > } > > Then make your PHY driver parse this, and make it so that if > function is LED_FUNCTION_LINK or LED_FUNCTION_ACTIVITY, the driver will > disable combined mode. > > Afterwards, when LED subsystem has support for trigger offloading, you > can update mscc driver so that instead of just disabling combined mode, > it will register the LEDs via LED subsystem...
Good idea, but I'm not really in a hurry. Now knowing that work on the trigger offloading is still active, I guess I will just wait a bit until the dust has settled and maybe the bindings have been defined. Then I can try to implement this in the PHY driver.
Thanks Frieder
| |