Re: [PATCH 1/2] dt-bindings: dp83867: define ti,ledX-active-low properties

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Nov 04, 2022 at 08:17:44AM +0100, Rasmus Villemoes wrote:
> On 03/11/2022 23.17, Andrew Lunn wrote:
> > On Thu, Nov 03, 2022 at 03:31:17PM +0100, Rasmus Villemoes wrote:
> >> The dp83867 has three LED_X pins that can be used to drive LEDs. They
> >> are by default driven active high, but on some boards the reverse is
> >> needed. Add bindings to allow a board to specify that they should be
> >> active low.
> > 
> > Somebody really does need to finish the PHY LEDs via /sys/class/leds.
> > It looks like this would then be a reasonable standard property:
> > active-low, not a vendor property.
> > 
> > Please help out with the PHY LEDs patches.
> 
> So how do you imagine this to work in DT? Should the dp83867 phy node
> grow a subnode like this?
> 
>   leds {
>     #address-cells = <1>;
>     #size-cells = <0>;
> 
>     led@0 {
>       reg = <0>;
>       active-low;
>     };
>     led@2 {
>       reg = <2>;
>       active-low;
>     };
>   };

Yes, something like that. They should follow the DT binding for LEDs.
Documentation/devicetree/bindings/leds/common.yaml

> 
> Since the phy drives the leds automatically based on (by default)
> link/activity, there's not really any need for a separate LED driver nor
> do I see what would be gained by somehow listing the LEDs in
> /sys/class/leds. Please expand.

The PHY driver would be the LED driver, hopefully with some shared
code in phylib. You can then use the standard Linux LED way of
configuring what the LED means, using triggers. Those triggers get
offloaded to the hardware when possible, or done in software when not.
The DT binding would then follow the common LED binding.

What i want to get away from is that there is no consistent DT binding
for PHY leds. In fact, there are at least four different ways to
configure PHY leds, and you want to add a fifth.

	  Andrew