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: <8e4aa981-7f41-047d-2101-370118b4f2c0@gmail.com>
Date:   Fri, 8 Sep 2017 11:48:12 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Tristram.Ha@...rochip.com, andrew@...n.ch, muvarov@...il.com,
        pavel@....cz, nathan.leigh.conrad@...il.com,
        vivien.didelot@...oirfairelinux.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Woojung.Huh@...rochip.com
Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new
 drivers can be added

On 09/07/2017 02:11 PM, Tristram.Ha@...rochip.com wrote:
> From: Tristram Ha <Tristram.Ha@...rochip.com>
> 
> Add other KSZ switches support so that patch check does not complain.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@...rochip.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 ++++++++++++----------
>  1 file changed, 62 insertions(+), 55 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> index 0ab8b39..34af0e0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
>  
>  Required properties:
>  
> -- compatible: For external switch chips, compatible string must be exactly one
> -  of: "microchip,ksz9477"
> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> +	      "microchip,ksz8795" for KSZ8795 chip,
> +	      "microchip,ksz8794" for KSZ8794 chip,
> +	      "microchip,ksz8765" for KSZ8765 chip,
> +	      "microchip,ksz8895" for KSZ8895 chip,
> +	      "microchip,ksz8864" for KSZ8864 chip,
> +	      "microchip,ksz8873" for KSZ8873 chip,
> +	      "microchip,ksz8863" for KSZ8863 chip,
> +	      "microchip,ksz8463" for KSZ8463 chip

It becomes pretty obvious there is a 1 to 1 mapping between the
compatible name and what you should be using it for so specifying
ksz8795 for KSZ8795 chip is really redundant.

You could just list all compatible strings that you support and change
the verbiage to be:

compatible: Should be one of:
		"microchip,ksz9477"
		...
		"microchip,ksz8463"
>  
>  See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional  required and optional properties.
> @@ -13,60 +20,60 @@ Examples:
>  
>  Ethernet switch connected via SPI to the host, CPU port wired to eth0:
>  
> -                             eth0: ethernet@...01000 {
> -                                             fixed-link {
> -                                                             speed = <1000>;
> -                                                             full-duplex;
> -                                             };
> -                             };
> +	eth0: ethernet@...01000 {
> +		fixed-link {
> +			speed = <1000>;
> +			full-duplex;
> +		};
> +	};

This is a good clean up, but it would probably belong in a separate
patch that you would do before adding compatible strings for instance.

>  
> -                             spi1: spi@...08000 {
> -                                             pinctrl-0 = <&pinctrl_spi_ksz>;
> -                                             cs-gpios = <&pioC 25 0>;
> -                                             id = <1>;
> -                                             status = "okay";
> +	spi1: spi@...08000 {
> +		cs-gpios = <&pioC 25 0>;
> +		id = <1>;
> +		status = "okay";
>  
> -                                             ksz9477: ksz9477@0 {
> -                                                             compatible = "microchip,ksz9477";
> -                                                             reg = <0>;
> +		ksz9477: ksz9477@0 {
> +			compatible = "microchip,ksz9477";
> +			reg = <0>;
>  
> -                                                             spi-max-frequency = <44000000>;
> -                                                             spi-cpha;
> -                                                             spi-cpol;
> +			spi-max-frequency = <44000000>;
> +			spi-cpha;
> +			spi-cpol;
> +
> +			status = "okay";
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				port@0 {
> +					reg = <0>;
> +					label = "lan1";
> +				};
> +				port@1 {
> +					reg = <1>;
> +					label = "lan2";
> +				};
> +				port@2 {
> +					reg = <2>;
> +					label = "lan3";
> +				};
> +				port@3 {
> +					reg = <3>;
> +					label = "lan4";
> +				};
> +				port@4 {
> +					reg = <4>;
> +					label = "lan5";
> +				};
> +				port@5 {
> +					reg = <5>;
> +					label = "cpu";
> +					ethernet = <&eth0>;
> +					fixed-link {
> +						speed = <1000>;
> +						full-duplex;
> +					};
> +				};
> +			};
> +		};
> +	};
>  
> -                                                             status = "okay";
> -                                                             ports {
> -                                                                             #address-cells = <1>;
> -                                                                             #size-cells = <0>;
> -                                                                             port@0 {
> -                                                                                             reg = <0>;
> -                                                                                             label = "lan1";
> -                                                                             };
> -                                                                             port@1 {
> -                                                                                             reg = <1>;
> -                                                                                             label = "lan2";
> -                                                                             };
> -                                                                             port@2 {
> -                                                                                             reg = <2>;
> -                                                                                             label = "lan3";
> -                                                                             };
> -                                                                             port@3 {
> -                                                                                             reg = <3>;
> -                                                                                             label = "lan4";
> -                                                                             };
> -                                                                             port@4 {
> -                                                                                             reg = <4>;
> -                                                                                             label = "lan5";
> -                                                                             };
> -                                                                             port@5 {
> -                                                                                             reg = <5>;
> -                                                                                             label = "cpu";
> -                                                                                             ethernet = <&eth0>;
> -                                                                                             fixed-link {
> -                                                                                                             speed = <1000>;
> -                                                                                                             full-duplex;
> -                                                                                             };
> -                                                                             };
> -                                                             };
> -                                             };
> -                             };
> 


-- 
Florian

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ