diff mbox series

[2/4] dt-bindings: net: Add onsemi NCN26010 ethernet controller

Message ID ZQf1Mgb8lfHkB6rl@lopingdog.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: Add NCN26010 driver | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next, async
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: 9 this patch: 9
netdev/cc_maintainers warning 7 maintainers not CCed: robh+dt@kernel.org krzysztof.kozlowski+dt@linaro.org pabeni@redhat.com davem@davemloft.net edumazet@google.com conor+dt@kernel.org kuba@kernel.org
netdev/build_clang success Errors and warnings before: 9 this patch: 9
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: 9 this patch: 9
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jay Monkman Sept. 18, 2023, 6:58 a.m. UTC 
Add devicetree bindings for onsemi's NCN26010 10BASE-T1S
ethernet controller.

Signed-off-by: Jay Monkman <jtm@lopingdog.com>
---
 .../devicetree/bindings/net/onnn,macphy.yaml  | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/onnn,macphy.yaml

Comments

Krzysztof Kozlowski Sept. 18, 2023, 8:28 p.m. UTC | #1 
On 18/09/2023 08:58, Jay Monkman wrote:
> 

Drop stray blank line.

> Add devicetree bindings for onsemi's NCN26010 10BASE-T1S
> ethernet controller.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

> 
> Signed-off-by: Jay Monkman <jtm@lopingdog.com>
> ---
>  .../devicetree/bindings/net/onnn,macphy.yaml  | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/onnn,macphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/onnn,macphy.yaml b/Documentation/devicetree/bindings/net/onnn,macphy.yaml
> new file mode 100644
> index 000000000000..1813da81b95f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/onnn,macphy.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/onnn,macphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: onsemi 10BASE-T1S Ethernet Controller
> +
> +maintainers:
> +  - Jay Monkman <jtm@lopingdog.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Bindings for onsemi 10BASE-T1S ethernet controller.

Drop "bindings for" and instead describe the hardware.

> +
> +  Supported devices:
> +    ncn26010

Supported by what? By bindings? Or driver? Drop, instead, describe the
hardware.

> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: onnn,ncn26010
> +
> +  ctrl-protected:
> +    description: |

Do not need '|' unless you need to preserve formatting.

Is it a generic property? Does not look like. All non-standard
properties need vendor prefix.

> +      Enables control data read/write protection mode.

I don't understand this and it is very close to duplicate the property
name. Describe what it does in the hardware, not in the driver, so it
will be justified to have it in bindings in the first place.

> +    type: boolean
> +
> +  poll-interval:

Missing units.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Defines how often the MACPHY thread runs in milliseconds.

thread? So Linux kthread? Nope, not a bindings property.

> +      If not specified, a default value of 5 is used.
> +
> +  tx-fcs-calc:
> +    description: |
> +      Enables driver calculation of the FCS on transmitted frames.
> +    type: boolean

Driver? Not hardware? So drop the property.

> +
> +  rx-fcs-check:
> +    description: |
> +      Enables driver checking of the FCS on received frames
> +    type: boolean

Not a bindings property.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  mdio:
> +    $ref: mdio.yaml#
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible

Nothing else is required?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;

If this is a SPI device, then you miss spi-peripheral-props.

> +
> +      ethernet@0 {
> +        compatible = "onnn,ncn26010";
> +        reg = <0>; /* CE0 */

CE0? Isn't chip select obvious from the reg=0?


Best regards,
Krzysztof
Jay Monkman Sept. 19, 2023, 3:19 a.m. UTC | #2 
On Mon, Sep 18, 2023 at 10:28:47PM +0200, Krzysztof Kozlowski wrote:

> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.

Will do. Thanks for your other comments - I've got some work to do.

Jay
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/onnn,macphy.yaml b/Documentation/devicetree/bindings/net/onnn,macphy.yaml
new file mode 100644
index 000000000000..1813da81b95f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/onnn,macphy.yaml
@@ -0,0 +1,94 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/onnn,macphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: onsemi 10BASE-T1S Ethernet Controller
+
+maintainers:
+  - Jay Monkman <jtm@lopingdog.com>
+
+description: |
+  Bindings for onsemi 10BASE-T1S ethernet controller.
+
+  Supported devices:
+    ncn26010
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+properties:
+  compatible:
+    const: onnn,ncn26010
+
+  ctrl-protected:
+    description: |
+      Enables control data read/write protection mode.
+    type: boolean
+
+  poll-interval:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Defines how often the MACPHY thread runs in milliseconds.
+      If not specified, a default value of 5 is used.
+
+  tx-fcs-calc:
+    description: |
+      Enables driver calculation of the FCS on transmitted frames.
+    type: boolean
+
+  rx-fcs-check:
+    description: |
+      Enables driver checking of the FCS on received frames
+    type: boolean
+
+  interrupts:
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  mdio:
+    $ref: mdio.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      ethernet@0 {
+        compatible = "onnn,ncn26010";
+        reg = <0>; /* CE0 */
+        ctrl-protected;
+        tx-fcs-calc;
+        rx-fcs-check;
+        poll-interval = <5>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&eth1_pins>;
+        interrupt-parent = <&gpio>;
+        interrupts = <25 0x2>;
+
+        mdio {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          ephy0: ethernet-phy@0 {
+            compatible = "ethernet-phy-ieee802.3-c22";
+            reg = <0>;
+            dio0-fn = "gpio";
+            dio0-slew = "slow";
+            dio1-fn = "gpio";
+            dio1-slew = "slow";
+            eni-enable;
+          };
+        };
+      };
+    };