[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f67b9a1f-73a5-e03f-a935-9bfb55a6b845@linaro.org>
Date: Wed, 28 Sep 2022 19:20:40 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Jerry Ray <jerry.ray@...rochip.com>, Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
UNGLinuxDriver@...rochip.com, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next][PATCH v3] dt-bindings: dsa: lan9303: Add lan9303 yaml
On 28/09/2022 18:21, Jerry Ray wrote:
> Adding the dt binding yaml for the lan9303 3-port ethernet switch.
> The microchip lan9354 3-port ethernet switch will also use the
> same binding.
>
> Signed-off-by: Jerry Ray <jerry.ray@...rochip.com>
> ---
> v2->v3:
> - removed cpu labels
> - now patching against latest net-next
> v1->v2:
> - fixed dt_binding_check warning
> - added max-speed setting on the switches 10/100 ports.
> ---
> .../devicetree/bindings/net/dsa/lan9303.txt | 99 +-------------
> .../bindings/net/dsa/microchip,lan9303.yaml | 129 ++++++++++++++++++
> MAINTAINERS | 8 ++
> 3 files changed, 139 insertions(+), 97 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/lan9303.txt b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
> index 46a732087f5c..8c8fedd1f00e 100644
> --- a/Documentation/devicetree/bindings/net/dsa/lan9303.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/lan9303.txt
> @@ -1,100 +1,5 @@
> SMSC/MicroChip LAN9303 three port ethernet switch
> -------------------------------------------------
Old file should be entirely removed.
>
> -Required properties:
> -
> -- compatible: should be
> - - "smsc,lan9303-i2c" for I2C managed mode
> - or
> - - "smsc,lan9303-mdio" for mdio managed mode
> -
> -Optional properties:
> -
> -- reset-gpios: GPIO to be used to reset the whole device
> -- reset-duration: reset duration in milliseconds, defaults to 200 ms
> -
> -Subnodes:
> -
> -The integrated switch subnode should be specified according to the binding
> -described in dsa/dsa.txt. The CPU port of this switch is always port 0.
> -
> -Note: always use 'reg = <0/1/2>;' for the three DSA ports, even if the device is
> -configured to use 1/2/3 instead. This hardware configuration will be
> -auto-detected and mapped accordingly.
> -
> -Example:
> -
> -I2C managed mode:
> -
> - master: masterdevice@X {
> -
> - fixed-link { /* RMII fixed link to LAN9303 */
> - speed = <100>;
> - full-duplex;
> - };
> - };
> -
> - switch: switch@a {
> - compatible = "smsc,lan9303-i2c";
> - reg = <0xa>;
> - reset-gpios = <&gpio7 6 GPIO_ACTIVE_LOW>;
> - reset-duration = <200>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 { /* RMII fixed link to master */
> - reg = <0>;
> - ethernet = <&master>;
> - };
> -
> - port@1 { /* external port 1 */
> - reg = <1>;
> - label = "lan1";
> - };
> -
> - port@2 { /* external port 2 */
> - reg = <2>;
> - label = "lan2";
> - };
> - };
> - };
> -
> -MDIO managed mode:
> -
> - master: masterdevice@X {
> - phy-handle = <&switch>;
> -
> - mdio {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - switch: switch-phy@0 {
> - compatible = "smsc,lan9303-mdio";
> - reg = <0>;
> - reset-gpios = <&gpio7 6 GPIO_ACTIVE_LOW>;
> - reset-duration = <100>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - reg = <0>;
> - ethernet = <&master>;
> - };
> -
> - port@1 { /* external port 1 */
> - reg = <1>;
> - label = "lan1";
> - };
> -
> - port@2 { /* external port 2 */
> - reg = <2>;
> - label = "lan2";
> - };
> - };
> - };
> - };
> - };
> +See Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml for the
> +device tree documentation covering the LAN9303 and LAN9354 devices.
No, just remove it.
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
> new file mode 100644
> index 000000000000..818770092a2c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/microchip,lan9303.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LAN9303 Ethernet Switch Series Tree Bindings
Drop "Tree Bindings"
> +
> +allOf:
> + - $ref: "dsa.yaml#"
Drop quotes.
> +
> +maintainers:
> + - UNGLinuxDriver@...rochip.com
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - smsc,lan9303-mdio
> + - microchip,lan9354-mdio
> + - enum:
> + - smsc,lan9303-i2c
oneOf does not make sense. It's just one enum.
> +
> + reg:
> + maxItems: 1
> +
> + reset-gpios:
> + description: Optional gpio specifier for a reset line
Drop "gpio specifier for a"
> + maxItems: 1
> +
> + reset-duration:
> + description: Reset duration in milliseconds, defaults to 200 ms
This does not look like standard type or unit suffix, so you need here
type. Don't you have warnings for this?
default: 200
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + //Ethernet switch connected via mdio to the host
Missing space before comments. Use Linux style comments, not your own.
> + ethernet0 {
Drop "0".
> + #address-cells = <1>;
> + #size-cells = <0>;
> + phy-handle = <&lan9303switch>;
> + phy-mode = "rmii";
> + fixed-link {
> + speed = <100>;
> + full-duplex;
> + };
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + lan9303switch: switch@0 {
> + compatible = "smsc,lan9303-mdio";
> + dsa,member = <0 0>;
> + reg = <0>;
reg goes after compatible.
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + phy-mode = "rmii";
> + ethernet = <ðernet>;
> + fixed-link {
> + speed = <100>;
> + full-duplex;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + max-speed = <100>;
> + label = "lan1";
> + };
> + port@2 {
> + reg = <2>;
> + max-speed = <100>;
> + label = "lan2";
> + };
> + };
> + };
> + };
> + };
> +
> + //Ethernet switch connected via i2c to the host
> + ethernet1 {
Just ethernet (and make it a second example)
Best regards,
Krzysztof
Powered by blists - more mailing lists