lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 25 Aug 2020 10:13:59 +0200
From:   Matthias Schiffer <>
To:     Marek Behún <>
Cc:, Pavel Machek <>,, Dan Murphy <>,
        Ondřej Jirman <>,
        Russell King <>,
        Thomas Petazzoni <>,
        Gregory Clement <>,
        Andrew Lunn <>,,
Subject: Re: [PATCH RFC leds + net-next v4 0/2] Add support for LEDs on
 Marvell PHYs

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.
> 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?

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

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.

Kind regards,

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

Powered by blists - more mailing lists