lkml.org 
[lkml]   [2021]   [Oct]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] net: phy: mscc: Add possibilty to disable combined LED mode
From
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

\
 
 \ /
  Last update: 2021-10-01 12:54    [W:0.036 / U:1.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site