Re: [PATCH 1/3] net: phy: mscc: Add possibilty to disable combined LED mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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@xxxxxxxxxx>

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux