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

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:

quoted

quoted

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