Message ID | 20231215212244.1658-2-ansuelsmth@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: phy: generic polarity + LED support for qca808x | expand |
On Fri, Dec 15, 2023 at 10:22:41PM +0100, Christian Marangi wrote: > Document new LEDs polarity property to define what mode the LED needs to > be put to turn it on. > > Currently supported modes are: > > - active-low > - active-high > - active-low-tristate > - active-high-tristate Why is having a polarity unique to LEDs on ethernet PHYs? It's not. We already have 'active-low' established on several LED bindings. Please move the definition to leds/common.yaml and extend it. I would simply add an 'inactive-tristate' boolean property (if there's an actual user). I do worry this continues to evolve until we've re-created the pinctrl binding... Rob
On Wed, Dec 20, 2023 at 09:22:09AM -0600, Rob Herring wrote: > On Fri, Dec 15, 2023 at 10:22:41PM +0100, Christian Marangi wrote: > > Document new LEDs polarity property to define what mode the LED needs to > > be put to turn it on. > > > > Currently supported modes are: > > > > - active-low > > - active-high > > - active-low-tristate > > - active-high-tristate > > Why is having a polarity unique to LEDs on ethernet PHYs? It's not. We > already have 'active-low' established on several LED bindings. Please > move the definition to leds/common.yaml and extend it. I would simply > add an 'inactive-tristate' boolean property (if there's an actual user). > Should I also drop the active-low from the current schema that have it? Also we have led-active-low. (should we support both?) On the marvell10g series we are discussing of using tristate or not. We notice tristate might be confusing, would it be better to use inactive-high-impedance ? > I do worry this continues to evolve until we've re-created the pinctrl > binding... >
> I do worry this continues to evolve until we've re-created the pinctrl > binding... Hi Rob What is you opinion of the pinctrl binding? Should we just copy parts of it? Andrew
> On the marvell10g series we are discussing of using tristate or not. We > notice tristate might be confusing, would it be better to use > inactive-high-impedance ? The pincfg-node.yaml binding has: drive-open-drain: oneOf: - type: boolean - $ref: /schemas/types.yaml#/definitions/uint32 const: 1 # No known cases of 0 deprecated: true description: drive with open drain drive-open-source: type: boolean description: drive with open source I'm not sure what the deprecated means. Is it that a value is deprecated, not the property as a whole? Andrew
On Thu, Dec 21, 2023 at 10:43:17AM +0100, Andrew Lunn wrote: > > On the marvell10g series we are discussing of using tristate or not. We > > notice tristate might be confusing, would it be better to use > > inactive-high-impedance ? > > The pincfg-node.yaml binding has: > > drive-open-drain: > oneOf: > - type: boolean > - $ref: /schemas/types.yaml#/definitions/uint32 > const: 1 # No known cases of 0 > deprecated: true > description: drive with open drain > > drive-open-source: > type: boolean > description: drive with open source > > I'm not sure what the deprecated means. Is it that a value is > deprecated, not the property as a whole? Yeah, it means that only the boolean form of this property should be used going forward. The comment suggests that the value had no meaning in the first place, and that testing for presence alone has been sufficient all along.
On Thu, Dec 21, 2023 at 10:34:41AM +0100, Andrew Lunn wrote: > > I do worry this continues to evolve until we've re-created the pinctrl > > binding... > > Hi Rob > > What is you opinion of the pinctrl binding? Should we just copy parts > of it? > Hi, I have the new series ready but I'm not sure pincfg-node have useful property. What we should use from there? From what I can see only output-low would be useful to us. I didn't find a way to handle the inactive mode. Should I send the new series so we can continue the discussion there or better to wait for Rob feedback?
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index 8fb2a6ee7e5b..282bf18f50fd 100644 --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml @@ -225,6 +225,17 @@ properties: driver dependent and required for ports that define multiple LED for the same port. + polarity: + description: | + Electrical polarity and drive type for the LED to turn it + on. + $ref: /schemas/types.yaml#/definitions/string + enum: + - active-low + - active-high + - active-low-tristate + - active-high-tristate + required: - reg
Document new LEDs polarity property to define what mode the LED needs to be put to turn it on. Currently supported modes are: - active-low - active-high - active-low-tristate - active-high-tristate Mode is optional and if it's not defined, a default value is chosed by the driver. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- Changes v4: - Drop support for global active-low - Rework to polarity option (for marvell10g series support) Changes v3: - Out of RFC Changes v2: - Add this patch .../devicetree/bindings/net/ethernet-phy.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+)