Re: [PATCH] net: phy: dp83867: Add support for hardware blinking LEDs

Andrew Lunn
2023-09-09T13:27:09Z

On Sat, Sep 09, 2023 at 08:15:37AM +0100, Russell King (Oracle) wrote:
> On Sat, Sep 09, 2023 at 04:27:31AM +0200, Andrew Lunn wrote:
> > > This works as intended so far. Unfortunately this driver and the PHY LED
> > > framework do not support active-low LEDs (yet).
> >
> > Polarity is something which i've seen a few PHY devices have. It also
> > seems like a core LED concept, not something specific to PHY LEDs. So
> > i think this needs to be partially addressed in the LED core.
>
> However, doesn't the LED layer deals with LED brightness, not by logic
> state? It certainly looks that way, and it's left up to the drivers
> themselves to deal with any polarity inversion - which makes sense if
> the core is just concerned about brightness.
>
> Introducing inversion in the core means drivers will be passed a
> brightness of "100" for off and "0" for on which, do you not think,
> starts to get rather silly?

Documentation/devicetree/bindings/leds/leds-pwm.yaml has:

active-low:
description:
For PWMs where the LED is wired to supply rather than ground.
type: boolean

leds-pwm-multicolor.yaml and leds-bcm6358.txt uses the same
property. There is also one case of: irled/ir-spi-led.yaml

led-active-low:
type: boolean
description:
Output is negated with a NOT gate.

The implementation of the properties is repeated in each driver. So we
probably need to add the parsing of this property in phy_device.c, and
add a led_config() op to struct phy_device to pass polarity
information to the driver.

Andrew