netdev - Re: [PATCH] dt-bindings: net: renesas,ethertsn: Add bindings for Ethernet TSN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2ab74479-f1fb-4faf-b223-ae750b4c08ce@linaro.org>
Date: Tue, 21 Nov 2023 08:45:29 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Niklas Söderlund
 <niklas.soderlund+renesas@...natech.se>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 devicetree@...r.kernel.org
Cc: netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: net: renesas,ethertsn: Add bindings for
 Ethernet TSN

On 20/11/2023 17:07, Niklas Söderlund wrote:
> Add bindings for Renesas R-Car Ethernet TSN End-station IP. The RTSN
> device provides Ethernet network.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>

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.

Why do you decide to skip some maintainers?

> ---
>  .../bindings/net/renesas,ethertsn.yaml        | 133 ++++++++++++++++++
>  1 file changed, 133 insertions(+)

A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.

>  create mode 100644 Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml b/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
> new file mode 100644
> index 000000000000..255c8f3a5a3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/renesas,ethertsn.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Ethernet TSN End-station
> +
> +maintainers:
> +  - Niklas Söderlund <niklas.soderlund@...natech.se>
> +
> +description:
> +  The RTSN device provides Ethernet network using a 10 Mbps, 100 Mbps, or 1
> +  Gbps full-duplex link via MII/GMII/RMII/RGMII. Depending on the connected PHY.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:

Drop items.

I assume you have oneOf above because you predict this will grow with
entries with fallbacks? If not, drop.

> +          - enum:
> +              - renesas,ethertsn-r8a779g0      # R-Car V4H
> +
> +  reg:
> +    items:
> +      - description: TSN End Station target
> +      - description: generalized Precision Time Protocol target
> +
> +  reg-names:
> +    items:
> +      - const: tsnes
> +      - const: gptp
> +
> +  interrupts:
> +    items:
> +      - description: TX data interrupt
> +      - description: RX data interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: tx_data

tx

> +      - const: rx_data

rx

> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  phy-mode:
> +    contains:
> +      enum:
> +        - mii
> +        - rgmii
> +
> +  phy-handle:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Specifies a reference to a node representing a PHY device.

You miss top-level $ref to ethernet controller

> +
> +  renesas,rx-internal-delay:
> +    type: boolean
> +    description:
> +      Enable internal Rx clock delay, typically 1.8ns.

Why this is bool, not delay in ns?
Why this is property of a board (not SoC)?

> +
> +  renesas,tx-internal-delay:
> +    type: boolean
> +    description:
> +      Enable internal Tx clock delay, typically 2.0ns.

Same questions.

> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^ethernet-phy@[0-9a-f]$":
> +    type: object
> +    $ref: ethernet-phy.yaml#

Missing unevaluatedProperties. Open existing bindings and look how it is
done there. Don't create something different.



Best regards,
Krzysztof