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: <c1b64758-219b-9251-cea8-d5301f01ee7f@linaro.org>
Date:   Mon, 3 Oct 2022 19:48:56 +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 v4] dt-bindings: dsa: Add lan9303 yaml

On 03/10/2022 18:46, 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>
> ---
> v3->v4:
>  - Addressed v3 community feedback
> 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   | 100 -------------
>  .../bindings/net/dsa/microchip,lan9303.yaml   | 134 ++++++++++++++++++
>  MAINTAINERS                                   |   8 ++
>  3 files changed, 142 insertions(+), 100 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/lan9303.txt
>  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
> deleted file mode 100644
> index 46a732087f5c..000000000000
> --- a/Documentation/devicetree/bindings/net/dsa/lan9303.txt
> +++ /dev/null
> @@ -1,100 +0,0 @@
> -SMSC/MicroChip LAN9303 three port ethernet switch
> --------------------------------------------------
> -
> -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";
> -					};
> -				};
> -			};
> -		};
> -	};
> 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..ca6cbe83ba75
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
> @@ -0,0 +1,134 @@
> +# 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
> +
> +allOf:
> +  - $ref: dsa.yaml#
> +
> +maintainers:
> +  - UNGLinuxDriver@...rochip.com
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - smsc,lan9303-mdio
> +          - microchip,lan9354-mdio
> +      - enum:
> +          - smsc,lan9303-i2c
> +          - microchip,lan9354-i2c

This still does not make sense. It is one enum. Drop oneOf.

> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: Optional reset line
> +    maxItems: 1
> +
> +  reset-duration:
> +    description: Reset duration in milliseconds
> +    default: 200

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +
> +required:
> +  - compatible
> +  - reg
> +
Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ