[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16aac887-232a-7141-cc65-eab19c532592@linaro.org>
Date: Tue, 25 Oct 2022 10:27:50 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Camel Guo <camel.guo@...s.com>, Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Florian Fainelli <f.fainelli@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Russell King <linux@...linux.org.uk>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, Rob Herring <robh@...nel.org>,
kernel@...s.com
Subject: Re: [RFC net-next 1/2] dt-bindings: net: dsa: add bindings for GSW
Series switches
On 25/10/2022 09:52, Camel Guo wrote:
> Add documentation and an example for Maxlinear's GSW Series Ethernet
> switches.
>
> Signed-off-by: Camel Guo <camel.guo@...s.com>
> ---
> .../devicetree/bindings/net/dsa/mxl,gsw.yaml | 140 ++++++++++++++++++
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> MAINTAINERS | 6 +
> 3 files changed, 148 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml b/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
> new file mode 100644
> index 000000000000..8e124b7ec58c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
Filename based on compatible, so mxl,gsw145-mdio.yaml. But see below.
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/mxl,gsw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxlinear GSW Series Switch Device Tree Bindings
Drop "Device Tree Bindings"
> +
> +allOf:
> + - $ref: dsa.yaml#
> +
> +maintainers:
> + - Camel Guo <camel.guo@...s.com>
> +
> +description:
> + The Maxlinear's GSW Series Ethernet Switch is a highly integrated, low-power,
> + non-blocking Gigabit Ethernet Switch.
> +
> +properties:
> + compatible:
> + oneOf:
You do not have multiple choices, so no need for oneOf
> + - enum:
> + - mxl,gsw145-mdio
Why "mdio" suffix?
> +
> + reg:
> + maxItems: 1
> +
> + mdio:
> + type: object
> +
> + description:
> + Container of ethernet phy devices on the MDIO bus of GSW switch
> +
> + properties:
> + '#address-cells':
> + const: 1
> + '#size-cells':
> + const: 0
> +
> + allOf:
No need for allOf
> + - $ref: "http://devicetree.org/schemas/net/ethernet-phy.yaml#"
That's not an URL. Open other schemas using ethernet-phy and check how
they are doing.
You miss:
unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - mdio
> +
> +additionalProperties: true
This cannot be true. Again - open existing bindings and do like they are
doing, not differently.
You wanted here unevaluatedProperties: false.
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + mdio {
Hmmm... switch with MDIO is part of MDIO?
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + switch@0 {
> + compatible = "mxl,gsw145-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x0>;
reg is a second property
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists