diff mbox series

[net-next,4/4] dt-bindings: net: marvell10g: Document LED polarity

Message ID 20231214201442.660447-5-tobias@waldekranz.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: phy: marvell10g: Firmware loading and LED support for 88X3310 | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 2 maintainers not CCed: pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 67 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Tobias Waldekranz Dec. 14, 2023, 8:14 p.m. UTC
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

Comments

Krzysztof Kozlowski Dec. 15, 2023, 8:47 a.m. UTC | #1
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
Andrew Lunn Dec. 15, 2023, 11:19 a.m. UTC | #2
> +        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 mbox series

Patch

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