netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Tristram.Ha@microchip.com, andrew@lunn.ch, muvarov@gmail.com,
	pavel@ucw.cz, nathan.leigh.conrad@gmail.com,
	vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Woojung.Huh@microchip.com
Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
Date: Fri, 8 Sep 2017 11:48:12 -0700	[thread overview]
Message-ID: <8e4aa981-7f41-047d-2101-370118b4f2c0@gmail.com> (raw)
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A5B@CHN-SV-EXMX02.mchp-main.com>

On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Add other KSZ switches support so that patch check does not complain.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.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@10001000 {
> -                                             fixed-link {
> -                                                             speed = <1000>;
> -                                                             full-duplex;
> -                                             };
> -                             };
> +	eth0: ethernet@10001000 {
> +		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@f8008000 {
> -                                             pinctrl-0 = <&pinctrl_spi_ksz>;
> -                                             cs-gpios = <&pioC 25 0>;
> -                                             id = <1>;
> -                                             status = "okay";
> +	spi1: spi@f8008000 {
> +		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

  parent reply	other threads:[~2017-09-08 18:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 21:11 [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added Tristram.Ha
2017-09-07 21:54 ` Andrew Lunn
2017-09-08 13:32   ` Maxim Uvarov
2017-09-08 14:12     ` Andrew Lunn
2017-09-08 18:40       ` Tristram.Ha
2017-09-08 18:45         ` Florian Fainelli
2017-09-08 19:01         ` Andrew Lunn
2017-09-08 19:05           ` Florian Fainelli
2017-09-08 21:50           ` Pavel Machek
2017-09-08 21:53         ` Pavel Machek
2017-09-08  9:04 ` Pavel Machek
2017-09-08 18:48 ` Florian Fainelli [this message]
2017-09-08 19:00   ` Maxim Uvarov
2017-09-08 19:48     ` Tristram.Ha
2017-09-08 19:54       ` Florian Fainelli
2017-09-08 20:07         ` Tristram.Ha
2017-09-08 20:58           ` Florian Fainelli
2017-09-11  7:53       ` Maxim Uvarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e4aa981-7f41-047d-2101-370118b4f2c0@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=Tristram.Ha@microchip.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=linux-kernel@vger.kernel.org \
    --cc=muvarov@gmail.com \
    --cc=nathan.leigh.conrad@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=vivien.didelot@savoirfairelinux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).