All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH] ARM: dts: ventana: fix eth1 pci dev node
Date: Fri, 14 Mar 2014 21:28:06 +0800	[thread overview]
Message-ID: <20140314132805.GD813@S2101-09.ap.freescale.net> (raw)
In-Reply-To: <1394747064-4106-1-git-send-email-tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>

On Thu, Mar 13, 2014 at 02:44:24PM -0700, Tim Harvey wrote:
> Properly add the PCI device node for the 2nd GigE port so that the device
> driver can get its MAC from DT.  Note that the Ventana bootloader uses
> the ethernet1 alias to populate the MAC address by adding the local-mac-address
> property.  Also remove the unnecesssary 'sky2' alias.
> 
> This is based on Shawn's for-next branch

This line shouldn't be necessarily in the commit log.

> 
> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6q-gw5400-a.dts  | 40 +++++++++++++++++++++++++++++++----
>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 39 ++++++++++++++++++++++++++++++----
>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 39 ++++++++++++++++++++++++++++++----
>  3 files changed, 106 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> index 902f983..5d2b912 100644
> --- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
> +++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> @@ -16,7 +16,7 @@
>  	model = "Gateworks Ventana GW5400-A";
>  	compatible = "gw,imx6q-gw5400-a", "gw,ventana", "fsl,imx6q";
>  
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		ethernet0 = &fec;
>  		ethernet1 = &eth1;
> @@ -26,7 +26,7 @@
>  		led0 = &led0;
>  		led1 = &led1;
>  		led2 = &led2;
> -		sky2 = &eth1;
> +

No need to add a new line.

>  		ssi0 = &ssi1;
>  		spi0 = &ecspi1;
>  		usb0 = &usbh1;
> @@ -496,8 +496,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2@8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";

So this was just a placeholder and did not actually work in any way,
right?

> +	pcie@0,0 {

Is this whole bridge/switch hierarchy binding documented somewhere or is
this just something that work for you?

> +		/* 00:00.0 host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		/*
> +		 * GigE PCI dev node needs to be defined so that enet driver
> +		 * can use it to obtain its boot-loader specified MAC
> +		 */
> +		pcie@0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie@8,0 {

What's the naming schema for all these pcie nodes?  Generally, we should
have the numbers encoded in the node name coming from the address cells
in 'reg' property.

Shawn

> +				/* 02:08.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x4000 0 0 0 0>;
> +				eth1: pcie@0,0 {
> +					/* 08:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> index c8e5ae0..46a8582 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> @@ -10,7 +10,7 @@
>   */
>  
>  / {
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		can0 = &can1;
>  		ethernet0 = &fec;
> @@ -19,7 +19,6 @@
>  		led1 = &led1;
>  		led2 = &led2;
>  		nand = &gpmi;
> -		sky2 = &eth1;
>  		ssi0 = &ssi1;
>  		usb0 = &usbh1;
>  		usb1 = &usbotg;
> @@ -503,8 +502,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2@8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";
> +	/*
> +	 * GigE PCI dev node needs to be defined so that enet driver
> +	 * can use it to obtain its boot-loader specified MAC
> +	 */
> +	pcie@0,0 {
> +		/* 00:00.0 root host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		pcie@0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie@4,0 {
> +				/* 02:04.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x2000 0 0 0 0>;
> +				eth1: pcie@0,0 {
> +					/* 04:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> index 2795dfc..697aa67 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> @@ -10,7 +10,7 @@
>   */
>  
>  / {
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		can0 = &can1;
>  		ethernet0 = &fec;
> @@ -19,7 +19,6 @@
>  		led1 = &led1;
>  		led2 = &led2;
>  		nand = &gpmi;
> -		sky2 = &eth1;
>  		ssi0 = &ssi1;
>  		usb0 = &usbh1;
>  		usb1 = &usbotg;
> @@ -525,8 +524,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2@8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";
> +	/*
> +	 * GigE PCI dev node needs to be defined so that enet driver
> +	 * can use it to obtain its boot-loader specified MAC
> +	 */
> +	pcie@0,0 {
> +		/* 00:00.0 root host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		pcie@0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie@8,0 {
> +				/* 02:08.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x4000 0 0 0 0>;
> +				eth1: pcie@0,0 {
> +					/* 08:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> -- 
> 1.8.3.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: shawn.guo@freescale.com (Shawn Guo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: dts: ventana: fix eth1 pci dev node
Date: Fri, 14 Mar 2014 21:28:06 +0800	[thread overview]
Message-ID: <20140314132805.GD813@S2101-09.ap.freescale.net> (raw)
In-Reply-To: <1394747064-4106-1-git-send-email-tharvey@gateworks.com>

On Thu, Mar 13, 2014 at 02:44:24PM -0700, Tim Harvey wrote:
> Properly add the PCI device node for the 2nd GigE port so that the device
> driver can get its MAC from DT.  Note that the Ventana bootloader uses
> the ethernet1 alias to populate the MAC address by adding the local-mac-address
> property.  Also remove the unnecesssary 'sky2' alias.
> 
> This is based on Shawn's for-next branch

This line shouldn't be necessarily in the commit log.

> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  arch/arm/boot/dts/imx6q-gw5400-a.dts  | 40 +++++++++++++++++++++++++++++++----
>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 39 ++++++++++++++++++++++++++++++----
>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 39 ++++++++++++++++++++++++++++++----
>  3 files changed, 106 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> index 902f983..5d2b912 100644
> --- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
> +++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> @@ -16,7 +16,7 @@
>  	model = "Gateworks Ventana GW5400-A";
>  	compatible = "gw,imx6q-gw5400-a", "gw,ventana", "fsl,imx6q";
>  
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		ethernet0 = &fec;
>  		ethernet1 = &eth1;
> @@ -26,7 +26,7 @@
>  		led0 = &led0;
>  		led1 = &led1;
>  		led2 = &led2;
> -		sky2 = &eth1;
> +

No need to add a new line.

>  		ssi0 = &ssi1;
>  		spi0 = &ecspi1;
>  		usb0 = &usbh1;
> @@ -496,8 +496,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";

So this was just a placeholder and did not actually work in any way,
right?

> +	pcie at 0,0 {

Is this whole bridge/switch hierarchy binding documented somewhere or is
this just something that work for you?

> +		/* 00:00.0 host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		/*
> +		 * GigE PCI dev node needs to be defined so that enet driver
> +		 * can use it to obtain its boot-loader specified MAC
> +		 */
> +		pcie at 0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie at 8,0 {

What's the naming schema for all these pcie nodes?  Generally, we should
have the numbers encoded in the node name coming from the address cells
in 'reg' property.

Shawn

> +				/* 02:08.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x4000 0 0 0 0>;
> +				eth1: pcie at 0,0 {
> +					/* 08:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> index c8e5ae0..46a8582 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> @@ -10,7 +10,7 @@
>   */
>  
>  / {
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		can0 = &can1;
>  		ethernet0 = &fec;
> @@ -19,7 +19,6 @@
>  		led1 = &led1;
>  		led2 = &led2;
>  		nand = &gpmi;
> -		sky2 = &eth1;
>  		ssi0 = &ssi1;
>  		usb0 = &usbh1;
>  		usb1 = &usbotg;
> @@ -503,8 +502,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";
> +	/*
> +	 * GigE PCI dev node needs to be defined so that enet driver
> +	 * can use it to obtain its boot-loader specified MAC
> +	 */
> +	pcie at 0,0 {
> +		/* 00:00.0 root host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		pcie at 0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie at 4,0 {
> +				/* 02:04.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x2000 0 0 0 0>;
> +				eth1: pcie at 0,0 {
> +					/* 04:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> index 2795dfc..697aa67 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> @@ -10,7 +10,7 @@
>   */
>  
>  / {
> -	/* these are used by bootloader for disabling nodes */
> +	/* these are used by bootloader for configuring nodes */
>  	aliases {
>  		can0 = &can1;
>  		ethernet0 = &fec;
> @@ -19,7 +19,6 @@
>  		led1 = &led1;
>  		led2 = &led2;
>  		nand = &gpmi;
> -		sky2 = &eth1;
>  		ssi0 = &ssi1;
>  		usb0 = &usbh1;
>  		usb1 = &usbotg;
> @@ -525,8 +524,40 @@
>  	reset-gpio = <&gpio1 29 0>;
>  	status = "okay";
>  
> -	eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
> -		compatible = "marvell,sky2";
> +	/*
> +	 * GigE PCI dev node needs to be defined so that enet driver
> +	 * can use it to obtain its boot-loader specified MAC
> +	 */
> +	pcie at 0,0 {
> +		/* 00:00.0 root host-bridge */
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		reg = <0x0 0 0 0 0>;
> +
> +		pcie at 0,0 {
> +			/* 01:00.0 PCIe switch */
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			reg = <0x0 0 0 0 0>;
> +
> +			pcie at 8,0 {
> +				/* 02:08.0 PCIe switch port */
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				reg = <0x4000 0 0 0 0>;
> +				eth1: pcie at 0,0 {
> +					/* 08:00.0 GigE */
> +					#address-cells = <3>;
> +					#size-cells = <2>;
> +					device_type = "pci";
> +					reg = <0x0 0 0 0 0>;
> +					compatible = "marvell,sky2";
> +				};
> +			};
> +		};
>  	};
>  };
>  
> -- 
> 1.8.3.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

  parent reply	other threads:[~2014-03-14 13:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-13 21:44 [PATCH] ARM: dts: ventana: fix eth1 pci dev node Tim Harvey
2014-03-13 21:44 ` Tim Harvey
     [not found] ` <1394747064-4106-1-git-send-email-tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
2014-03-14 13:28   ` Shawn Guo [this message]
2014-03-14 13:28     ` Shawn Guo
     [not found]     ` <20140314132805.GD813-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2014-03-18 20:02       ` Tim Harvey
2014-03-18 20:02         ` Tim Harvey
     [not found]         ` <CAJ+vNU2Av-n1-efbFbXNvO4SmoobL_WpQKApFRS1Zjy0egLLzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-03-18 20:15           ` Jason Gunthorpe
2014-03-18 20:15             ` Jason Gunthorpe
     [not found]             ` <20140318201519.GA8637-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2014-03-22  6:25               ` Shawn Guo
2014-03-22  6:25                 ` Shawn Guo

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=20140314132805.GD813@S2101-09.ap.freescale.net \
    --to=shawn.guo-kzfg59tc24xl57midrcfdg@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.