lkml.org 
[lkml]   [2020]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RFC leds + net-next v4 0/2] Add support for LEDs on Marvell PHYs
On Tue, 25 Aug 2020 10:13:59 +0200
Matthias Schiffer <matthias.schiffer@ew.tq-group.com> wrote:

> On Tue, 2020-07-28 at 17:05 +0200, Marek Behún wrote:
> > Hi,
> >
> > this is v4 of my RFC adding support for LEDs connected to Marvell
> > PHYs.
> >
> > Please note that if you want to test this, you still need to first
> > apply
> > the patch adding the LED private triggers support from Pavel's tree.
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/commit/?h=for-next&id=93690cdf3060c61dfce813121d0bfc055e7fa30d
> >
> > What I still don't like about this is that the LEDs created by the
> > code
> > don't properly support device names. LEDs should have name in format
> > "device:color:function", for example "eth0:green:activity".
> >
> > The code currently looks for attached netdev for a given PHY, but
> > at the time this happens there is no netdev attached, so the LEDs
> > gets
> > names without the device part (ie ":green:activity").
> >
> > This can be addressed in next version by renaming the LED when a
> > netdev
> > is attached to the PHY, but first a API for LED device renaming needs
> > to
> > be proposed. I am going to try to do that. This would also solve the
> > same problem when userspace renames an interface.
> >
> > And no, I don't want phydev name there.
>
>
> Hello Marek,
>
> thanks for your patches - Andrew suggested me to have a look at them as
> I'm currently trying to add LED trigger support to the TI DP83867 PHY.
>
> Is there already a plan to add support for polarity and similiar
> settings, at least to the generic part of your changes?
>

Hello Matthias,

sorry for answering with delay, I somehow overlooked your email.
Yes, I plan to add some basic platform data properties (like polarity)
in the generic part.

> In the TI DP83867, there are 2 separate settings for each LED:
>
> - Trigger event
> - Polarity or override (active-high/active-low/force-high/force-low -
> the latter two would be used for led_brightness_set)
> - (There is also a 3rd register that defines the blink frequency, but
> as it allows only a single setting for all LEDs, I would ignore it for
> now)

I think that blink frequency should be in the generic part as well.

>
> At least the per-LED polarity setting would be essential to have for
> this feature to be useful for our TQ-Systems mainboards with TI PHYs.
>

I will try to send new version next week (starting 7th September).

Marek

>
> Kind regards,
> Matthias
>
>
>
> >
> > Changes since v3:
> > - addressed some of Andrew's suggestions
> > - phy_hw_led_mode.c renamed to phy_led.c
> > - the DT reading code is now also generic, moved to phy_led.c and
> > called
> > from phy_probe
> > - the function registering the phydev-hw-mode trigger is now called
> > from
> > phy_device.c function phy_init before registering genphy drivers
> > - PHY LED functionality now depends on CONFIG_LEDS_TRIGGERS
> >
> > Changes since v2:
> > - to share code with other drivers which may want to also offer PHY
> > HW
> > control of LEDs some of the code was refactored and now resides in
> > phy_hw_led_mode.c. This code is compiled in when config option
> > LED_TRIGGER_PHY_HW is enabled. Drivers wanting to offer PHY HW
> > control
> > of LEDs should depend on this option.
> > - the "hw-control" trigger is renamed to "phydev-hw-mode" and is
> > registered by the code in phy_hw_led_mode.c
> > - the "hw_control" sysfs file is renamed to "hw_mode"
> > - struct phy_driver is extended by three methods to support PHY HW
> > LED
> > control
> > - I renamed the various HW control modes offeret by Marvell PHYs to
> > conform to other Linux mode names, for example the
> > "1000/100/10/else"
> > mode was renamed to "1Gbps/100Mbps/10Mbps", or "recv/else" was
> > renamed
> > to "rx" (this is the name of the mode in netdev trigger).
> >
> > Marek
> >
> >
> > Marek Behún (2):
> > net: phy: add API for LEDs controlled by PHY HW
> > net: phy: marvell: add support for PHY LEDs via LED class
> >
> > drivers/net/phy/Kconfig | 4 +
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/marvell.c | 287
> > +++++++++++++++++++++++++++++++++++
> > drivers/net/phy/phy_device.c | 25 ++-
> > drivers/net/phy/phy_led.c | 176 +++++++++++++++++++++
> > include/linux/phy.h | 51 +++++++
> > 6 files changed, 537 insertions(+), 7 deletions(-)
> > create mode 100644 drivers/net/phy/phy_led.c
> >
>

\
 
 \ /
  Last update: 2020-08-31 00:57    [W:0.124 / U:4.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site