| Message ID | 20231214201442.660447-5-tobias@waldekranz.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | net: phy: marvell10g: Firmware loading and LED support for 88X3310 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 14/12/2023 21:14, Tobias Waldekranz wrote: > Hardware supports multiple ways of driving attached LEDs, but this is > not configurable via any sample-at-reset pins - rather it must be set > via software. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > .../bindings/net/marvell,marvell10g.yaml | 60 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/marvell,marvell10g.yaml > > diff --git a/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml b/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml > new file mode 100644 > index 000000000000..37ff7fdfdd3d > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml > @@ -0,0 +1,60 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/marvell,marvell10g.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell Alaska X 10G Ethernet PHY > + > +maintainers: > + - Tobias Waldekranz <tobias@waldekranz.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Bindings for Marvell Alaska X 10G Ethernet PHYs Drop Bindings for and describe the hardware. You are repeating title, so it is useless. > + > +allOf: > + - $ref: ethernet-phy.yaml# > + > +properties: How is this schema selected/applied? I guess you have exactly the same problem as recently talked about other ethernet PHY bindings. See: https://lore.kernel.org/linux-devicetree/20231209014828.28194-1-ansuelsmth@gmail.com/ > + leds: > + type: object > + > + properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + patternProperties: > + '^led@[a-f0-9]+$': > + $ref: /schemas/leds/common.yaml# Are you sure you need to repeat all this? > + > + properties: > + marvell,polarity: > + description: | > + Electrical polarity and drive type for this LED. In the > + active state, hardware may drive the pin either low or > + high. In the inactive state, the pin can either be > + driven to the opposite logic level, or be tristated. > + $ref: /schemas/types.yaml#/definitions/string > + enum: > + - active-low > + - active-high > + - active-low-tristate > + - active-high-tristate > + > +unevaluatedProperties: false > + > +examples: > + - | > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethernet-phy@0 { > + reg = <0>; > + > + marvell,polarity = "active-low-tristate"; It is clearly visible here that your schema is an no-op. You do not allow such property in the phy, but in leds! Best regards, Krzysztof
> + properties: > + marvell,polarity: > + description: | > + Electrical polarity and drive type for this LED. In the > + active state, hardware may drive the pin either low or > + high. In the inactive state, the pin can either be > + driven to the opposite logic level, or be tristated. > + $ref: /schemas/types.yaml#/definitions/string > + enum: > + - active-low > + - active-high > + - active-low-tristate > + - active-high-tristate Christian is working on adding a generic active-low property, which any PHY LED could use. The assumption being if the bool property is not present, it defaults to active-high. So we should consider, how popular are these two tristate values? Is this a Marvell only thing, or do other PHYs also have them? Do we want to make them part of the generic PHY led binding? Also, is an enum the correct representation? Maybe tristate should be another bool property? Hi/Low and tristate seem to be orthogonal, so maybe two properties would make it cleaner with respect to generic properties? Please work with Christian on this. Thanks Andrew
diff --git a/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml b/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml new file mode 100644 index 000000000000..37ff7fdfdd3d --- /dev/null +++ b/Documentation/devicetree/bindings/net/marvell,marvell10g.yaml @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/marvell,marvell10g.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell Alaska X 10G Ethernet PHY + +maintainers: + - Tobias Waldekranz <tobias@waldekranz.com> + +description: | + Bindings for Marvell Alaska X 10G Ethernet PHYs + +allOf: + - $ref: ethernet-phy.yaml# + +properties: + leds: + type: object + + properties: + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + patternProperties: + '^led@[a-f0-9]+$': + $ref: /schemas/leds/common.yaml# + + properties: + marvell,polarity: + description: | + Electrical polarity and drive type for this LED. In the + active state, hardware may drive the pin either low or + high. In the inactive state, the pin can either be + driven to the opposite logic level, or be tristated. + $ref: /schemas/types.yaml#/definitions/string + enum: + - active-low + - active-high + - active-low-tristate + - active-high-tristate + +unevaluatedProperties: false + +examples: + - | + mdio { + #address-cells = <1>; + #size-cells = <0>; + + ethernet-phy@0 { + reg = <0>; + + marvell,polarity = "active-low-tristate"; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index a151988646fe..2def66789f9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12849,6 +12849,7 @@ M: Russell King <linux@armlinux.org.uk> M: Marek Behún <kabel@kernel.org> L: netdev@vger.kernel.org S: Maintained +F: Documentation/devicetree/bindings/net/marvell,marvell10g.yaml F: drivers/net/phy/marvell10g.c MARVELL MVEBU THERMAL DRIVER
Hardware supports multiple ways of driving attached LEDs, but this is not configurable via any sample-at-reset pins - rather it must be set via software. Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- .../bindings/net/marvell,marvell10g.yaml | 60 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 61 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/marvell,marvell10g.yaml