[PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Jingoo Han]

Exynos5440 has two PCIe controllers which can be used as root complex
for PCIe interface.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/exynos5440-ssdk5440.dts |    8 ++++++
 arch/arm/boot/dts/exynos5440.dtsi         |   40 ++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts b/arch/arm/boot/dts/exynos5440-ssdk5440.dts
index f32cd77..3d93804 100644
--- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
+++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts
@@ -65,4 +65,12 @@
 			clock-frequency = <50000000>;
 		};
 	};
+
+	pcie0@290000 {
+		reset-gpio = <&pin_ctrl 5 0>;
+	};
+
+	pcie1@2a0000 {
+		reset-gpio = <&pin_ctrl 22 0>;
+	};
 };
diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index 03d40c0..6295eda 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -126,7 +126,7 @@
 		clock-names = "spi", "spi_busclk0";
 	};
 
-	pinctrl {
+	pin_ctrl: pinctrl {
 		compatible = "samsung,exynos5440-pinctrl";
 		reg = <0xE0000 0x1000>;
 		interrupts = <0 37 0>, <0 38 0>, <0 39 0>, <0 40 0>,
@@ -230,4 +230,42 @@
 		clocks = <&clock 24>;
 		clock-names = "usbhost";
 	};
+
+	pcie0@290000 {
+		compatible = "samsung,exynos5440-pcie";
+		reg = <0x290000 0x1000
+			0x270000 0x1000
+			0x271000 0x40>;
+		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
+		clocks = <&clock 28>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0x0 0 &gic 53>;
+	};
+
+	pcie1@2a0000 {
+		compatible = "samsung,exynos5440-pcie";
+		reg = <0x2a0000 0x1000
+			0x272000 0x1000
+			0x271040 0x40>;
+		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
+		clocks = <&clock 29>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0x0 0 &gic 56>;
+	};
 };
-- 
1.7.10.4

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Thomas Petazzoni]

Dear Jingoo Han,

On Thu, 20 Jun 2013 16:12:24 +0900, Jingoo Han wrote:

> -	pinctrl {
> +	pin_ctrl: pinctrl {
>  		compatible = "samsung,exynos5440-pinctrl";

I know I'm nitpicking, but isn't this change completely unrelated to
PCIe support?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Jingoo Han]

On Thursday, June 20, 2013 4:49 PM, Thomas Petazzoni wrote:
> 
> Dear Jingoo Han,
> 
> On Thu, 20 Jun 2013 16:12:24 +0900, Jingoo Han wrote:
> 
> > -	pinctrl {
> > +	pin_ctrl: pinctrl {
> >  		compatible = "samsung,exynos5440-pinctrl";
> 
> I know I'm nitpicking, but isn't this change completely unrelated to
> PCIe support?

This change is related to PCIe support.
Without this, I cannot use gpio binding.

This change was guided by Thomas Abraham (Author of Samsung pinctrl).
Also, it was confirmed by Kukjin Kim (Maintainer of Samsung SoC).

Thank you for your caring. :)

Best regards,
Jingoo Han

> 
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Thomas Petazzoni]

Dear Jingoo Han,

On Thu, 20 Jun 2013 16:57:32 +0900, Jingoo Han wrote:

> > > -	pinctrl {
> > > +	pin_ctrl: pinctrl {
> > >  		compatible = "samsung,exynos5440-pinctrl";
> > 
> > I know I'm nitpicking, but isn't this change completely unrelated to
> > PCIe support?
> 
> This change is related to PCIe support.
> Without this, I cannot use gpio binding.
> 
> This change was guided by Thomas Abraham (Author of Samsung pinctrl).
> Also, it was confirmed by Kukjin Kim (Maintainer of Samsung SoC).
> 
> Thank you for your caring. :)

I mean, the change is fine for sure, but it should maybe part of a
separate patch as it is more a fix than really the introduction of the
PCIe controller node, as the patch title suggests. This would also for
example allow this fix to be merged right now (for 3.11), regardless of
what happens for the rest of your PCIe patches.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Tomasz Figa]

Hi Jingoo,

On Thursday 20 of June 2013 16:12:24 Jingoo Han wrote:
> Exynos5440 has two PCIe controllers which can be used as root complex
> for PCIe interface.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5440-ssdk5440.dts |    8 ++++++
>  arch/arm/boot/dts/exynos5440.dtsi         |   40
> ++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1
> deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> b/arch/arm/boot/dts/exynos5440-ssdk5440.dts index f32cd77..3d93804
> 100644
> --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> @@ -65,4 +65,12 @@
>  			clock-frequency = <50000000>;
>  		};
>  	};
> +
> +	pcie0@290000 {
> +		reset-gpio = <&pin_ctrl 5 0>;
> +	};
> +
> +	pcie1@2a0000 {
> +		reset-gpio = <&pin_ctrl 22 0>;
> +	};
>  };
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi
> b/arch/arm/boot/dts/exynos5440.dtsi index 03d40c0..6295eda 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -126,7 +126,7 @@
>  		clock-names = "spi", "spi_busclk0";
>  	};
> 
> -	pinctrl {
> +	pin_ctrl: pinctrl {
>  		compatible = "samsung,exynos5440-pinctrl";
>  		reg = <0xE0000 0x1000>;
>  		interrupts = <0 37 0>, <0 38 0>, <0 39 0>, <0 40 0>,
> @@ -230,4 +230,42 @@
>  		clocks = <&clock 24>;
>  		clock-names = "usbhost";
>  	};

I think this patch should be split into two:
 - patch adding just generlic PCIe nodes for Exynos5440,
 - patch adding label to the pinctrl node (which is a prerequisite) and 
board-specific properties of PCIe nodes.

> +
> +	pcie0@290000 {

Node naming looks incorrect here. The name should be as generic as 
possible, without any hardware-specific IDs, e.g. pcie, not pcie0. The 
@290000 suffix is enough to make the node unique.

> +		compatible = "samsung,exynos5440-pcie";
> +		reg = <0x290000 0x1000
> +			0x270000 0x1000
> +			0x271000 0x40>;
> +		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
> +		clocks = <&clock 28>, <&clock 27>;
> +		clock-names = "pcie", "pcie_bus";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   
/*
> configuration space */ +			  0x81000000 0 0	  
0x40001000 0 0x00010000  
> /* downstream I/O */ +			  0x82000000 0 0x40011000 
0x40011000 0
> 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells = 
<1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0x0 0 &gic 53>;
> +	};
> +
> +	pcie1@2a0000 {

Same here.

Best regards,
Tomasz

> +		compatible = "samsung,exynos5440-pcie";
> +		reg = <0x2a0000 0x1000
> +			0x272000 0x1000
> +			0x271040 0x40>;
> +		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
> +		clocks = <&clock 29>, <&clock 27>;
> +		clock-names = "pcie", "pcie_bus";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   
/*
> configuration space */ +			  0x81000000 0 0	  
0x60001000 0 0x00010000  
> /* downstream I/O */ +			  0x82000000 0 0x60011000 
0x60011000 0
> 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells = 
<1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0x0 0 &gic 56>;
> +	};
>  };

RE: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Jingoo Han]

On Thursday, June 20, 2013 5:04 PM, Tomasz Figa wrote:
> 
> Hi Jingoo,
> 
> On Thursday 20 of June 2013 16:12:24 Jingoo Han wrote:
> > Exynos5440 has two PCIe controllers which can be used as root complex
> > for PCIe interface.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  arch/arm/boot/dts/exynos5440-ssdk5440.dts |    8 ++++++
> >  arch/arm/boot/dts/exynos5440.dtsi         |   40
> > ++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1
> > deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > b/arch/arm/boot/dts/exynos5440-ssdk5440.dts index f32cd77..3d93804
> > 100644
> > --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > @@ -65,4 +65,12 @@
> >  			clock-frequency = <50000000>;
> >  		};
> >  	};
> > +
> > +	pcie0@290000 {
> > +		reset-gpio = <&pin_ctrl 5 0>;
> > +	};
> > +
> > +	pcie1@2a0000 {
> > +		reset-gpio = <&pin_ctrl 22 0>;
> > +	};
> >  };
> > diff --git a/arch/arm/boot/dts/exynos5440.dtsi
> > b/arch/arm/boot/dts/exynos5440.dtsi index 03d40c0..6295eda 100644
> > --- a/arch/arm/boot/dts/exynos5440.dtsi
> > +++ b/arch/arm/boot/dts/exynos5440.dtsi
> > @@ -126,7 +126,7 @@
> >  		clock-names = "spi", "spi_busclk0";
> >  	};
> >
> > -	pinctrl {
> > +	pin_ctrl: pinctrl {
> >  		compatible = "samsung,exynos5440-pinctrl";
> >  		reg = <0xE0000 0x1000>;
> >  		interrupts = <0 37 0>, <0 38 0>, <0 39 0>, <0 40 0>,
> > @@ -230,4 +230,42 @@
> >  		clocks = <&clock 24>;
> >  		clock-names = "usbhost";
> >  	};
> 
> I think this patch should be split into two:
>  - patch adding just generlic PCIe nodes for Exynos5440,
>  - patch adding label to the pinctrl node (which is a prerequisite) and
> board-specific properties of PCIe nodes.

Do you mean the following?

1. patch adding just generlic PCIe nodes for Exynos5440
    [PATCH] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

arch/arm/boot/dts/exynos5440.dtsi
+
+	pcie@290000 {
+		compatible = "samsung,exynos5440-pcie";
+		reg = <0x290000 0x1000
+			0x270000 0x1000
+			0x271000 0x40>;
+		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
+		clocks = <&clock 28>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0x0 0 &gic 53>;
+	};
+
+	pcie@2a0000 {
+		compatible = "samsung,exynos5440-pcie";
+		reg = <0x2a0000 0x1000
+			0x272000 0x1000
+			0x271040 0x40>;
+		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
+		clocks = <&clock 29>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0x0 0 &gic 56>;
+	};



2. patch adding label to the pinctrl node (which is a prerequisite) and
    board-specific properties of PCIe nodes.
   [PATCH] ARM: dts: Add pcie controller node for exynos5440-ssdk5440

arch/arm/boot/dts/exynos5440-ssdk5440.dts
+
+	pcie0@290000 {
+		reset-gpio = <&pin_ctrl 5 0>;
+	};
+
+	pcie1@2a0000 {
+		reset-gpio = <&pin_ctrl 22 0>;
+	};

arch/arm/boot/dts/exynos5440.dtsi
-	pinctrl {
+	pin_ctrl: pinctrl {



> 
> > +
> > +	pcie0@290000 {
> 
> Node naming looks incorrect here. The name should be as generic as
> possible, without any hardware-specific IDs, e.g. pcie, not pcie0. The
> @290000 suffix is enough to make the node unique.

I see. You are right.
I will fix it.


Best regards,
Jingoo Han

> 
> > +		compatible = "samsung,exynos5440-pcie";
> > +		reg = <0x290000 0x1000
> > +			0x270000 0x1000
> > +			0x271000 0x40>;
> > +		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
> > +		clocks = <&clock 28>, <&clock 27>;
> > +		clock-names = "pcie", "pcie_bus";
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		device_type = "pci";
> > +		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000
> /*
> > configuration space */ +			  0x81000000 0 0
> 0x40001000 0 0x00010000
> > /* downstream I/O */ +			  0x82000000 0 0x40011000
> 0x40011000 0
> > 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells =
> <1>;
> > +		interrupt-map-mask = <0 0 0 0>;
> > +		interrupt-map = <0x0 0 &gic 53>;
> > +	};
> > +
> > +	pcie1@2a0000 {
> 
> Same here.
> 
> Best regards,
> Tomasz
> 
> > +		compatible = "samsung,exynos5440-pcie";
> > +		reg = <0x2a0000 0x1000
> > +			0x272000 0x1000
> > +			0x271040 0x40>;
> > +		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
> > +		clocks = <&clock 29>, <&clock 27>;
> > +		clock-names = "pcie", "pcie_bus";
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		device_type = "pci";
> > +		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000
> /*
> > configuration space */ +			  0x81000000 0 0
> 0x60001000 0 0x00010000
> > /* downstream I/O */ +			  0x82000000 0 0x60011000
> 0x60011000 0
> > 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells =
> <1>;
> > +		interrupt-map-mask = <0 0 0 0>;
> > +		interrupt-map = <0x0 0 &gic 56>;
> > +	};
> >  };

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Jingoo Han]

On Thursday, June 20, 2013 5:00 PM, Thomas Petazzoni wrote:
> 
> Dear Jingoo Han,
> 
> On Thu, 20 Jun 2013 16:57:32 +0900, Jingoo Han wrote:
> 
> > > > -	pinctrl {
> > > > +	pin_ctrl: pinctrl {
> > > >  		compatible = "samsung,exynos5440-pinctrl";
> > >
> > > I know I'm nitpicking, but isn't this change completely unrelated to
> > > PCIe support?
> >
> > This change is related to PCIe support.
> > Without this, I cannot use gpio binding.
> >
> > This change was guided by Thomas Abraham (Author of Samsung pinctrl).
> > Also, it was confirmed by Kukjin Kim (Maintainer of Samsung SoC).
> >
> > Thank you for your caring. :)
> 
> I mean, the change is fine for sure, but it should maybe part of a
> separate patch as it is more a fix than really the introduction of the
> PCIe controller node, as the patch title suggests. This would also for
> example allow this fix to be merged right now (for 3.11), regardless of
> what happens for the rest of your PCIe patches.
> 
CC'ed Tomasz Figa


OK, I see.
I will separate this patch to two patches, as Tomasz Figa mentioned.
(http://www.spinics.net/lists/linux-samsung-soc/msg19639.html)
Thank you.


Best regards,
Jingoo Han

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Arnd Bergmann]

On Thursday 20 June 2013, Jingoo Han wrote:
> Exynos5440 has two PCIe controllers which can be used as root complex
> for PCIe interface.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Tomasz Figa]

On Thursday 20 of June 2013 17:25:12 Jingoo Han wrote:
> On Thursday, June 20, 2013 5:04 PM, Tomasz Figa wrote:
> > Hi Jingoo,
> > 
> > On Thursday 20 of June 2013 16:12:24 Jingoo Han wrote:
> > > Exynos5440 has two PCIe controllers which can be used as root complex
> > > for PCIe interface.
> > > 
> > > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > > ---
> > > 
> > >  arch/arm/boot/dts/exynos5440-ssdk5440.dts |    8 ++++++
> > >  arch/arm/boot/dts/exynos5440.dtsi         |   40
> > > 
> > > ++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1
> > > deletion(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > > b/arch/arm/boot/dts/exynos5440-ssdk5440.dts index f32cd77..3d93804
> > > 100644
> > > --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > > +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > > @@ -65,4 +65,12 @@
> > > 
> > >  			clock-frequency = <50000000>;
> > >  		
> > >  		};
> > >  	
> > >  	};
> > > 
> > > +
> > > +	pcie0@290000 {
> > > +		reset-gpio = <&pin_ctrl 5 0>;
> > > +	};
> > > +
> > > +	pcie1@2a0000 {
> > > +		reset-gpio = <&pin_ctrl 22 0>;
> > > +	};
> > > 
> > >  };
> > > 
> > > diff --git a/arch/arm/boot/dts/exynos5440.dtsi
> > > b/arch/arm/boot/dts/exynos5440.dtsi index 03d40c0..6295eda 100644
> > > --- a/arch/arm/boot/dts/exynos5440.dtsi
> > > +++ b/arch/arm/boot/dts/exynos5440.dtsi
> > > @@ -126,7 +126,7 @@
> > > 
> > >  		clock-names = "spi", "spi_busclk0";
> > >  	
> > >  	};
> > > 
> > > -	pinctrl {
> > > +	pin_ctrl: pinctrl {
> > > 
> > >  		compatible = "samsung,exynos5440-pinctrl";
> > >  		reg = <0xE0000 0x1000>;
> > >  		interrupts = <0 37 0>, <0 38 0>, <0 39 0>, <0 40 0>,
> > > 
> > > @@ -230,4 +230,42 @@
> > > 
> > >  		clocks = <&clock 24>;
> > >  		clock-names = "usbhost";
> > >  	
> > >  	};
> > 
> > I think this patch should be split into two:
> >  - patch adding just generlic PCIe nodes for Exynos5440,
> >  - patch adding label to the pinctrl node (which is a prerequisite) and
> > 
> > board-specific properties of PCIe nodes.
> 
> Do you mean the following?
> 
> 1. patch adding just generlic PCIe nodes for Exynos5440
>     [PATCH] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC
> 
> arch/arm/boot/dts/exynos5440.dtsi
> +
> +	pcie@290000 {
> +		compatible = "samsung,exynos5440-pcie";
> +		reg = <0x290000 0x1000
> +			0x270000 0x1000
> +			0x271000 0x40>;
> +		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
> +		clocks = <&clock 28>, <&clock 27>;
> +		clock-names = "pcie", "pcie_bus";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /*
> configuration space */ +			  0x81000000 0 0	  0x40001000 0 0x00010000  
> /* downstream I/O */ +			  0x82000000 0 0x40011000 0x40011000 0
> 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0x0 0 &gic 53>;
> +	};
> +
> +	pcie@2a0000 {
> +		compatible = "samsung,exynos5440-pcie";
> +		reg = <0x2a0000 0x1000
> +			0x272000 0x1000
> +			0x271040 0x40>;
> +		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
> +		clocks = <&clock 29>, <&clock 27>;
> +		clock-names = "pcie", "pcie_bus";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /*
> configuration space */ +			  0x81000000 0 0	  0x60001000 0 0x00010000  
> /* downstream I/O */ +			  0x82000000 0 0x60011000 0x60011000 0
> 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0x0 0 &gic 56>;
> +	};
> 
> 
> 
> 2. patch adding label to the pinctrl node (which is a prerequisite) and
>     board-specific properties of PCIe nodes.
>    [PATCH] ARM: dts: Add pcie controller node for exynos5440-ssdk5440
> 
> arch/arm/boot/dts/exynos5440-ssdk5440.dts
> +
> +	pcie0@290000 {
> +		reset-gpio = <&pin_ctrl 5 0>;
> +	};
> +
> +	pcie1@2a0000 {
> +		reset-gpio = <&pin_ctrl 22 0>;
> +	};
> 
> arch/arm/boot/dts/exynos5440.dtsi
> -	pinctrl {
> +	pin_ctrl: pinctrl {
> 

Exactly.

> > > +
> > > +	pcie0@290000 {
> > 
> > Node naming looks incorrect here. The name should be as generic as
> > possible, without any hardware-specific IDs, e.g. pcie, not pcie0. The
> > @290000 suffix is enough to make the node unique.
> 
> I see. You are right.
> I will fix it.

Thanks.

Best regards,
Tomasz

> Best regards,
> Jingoo Han
> 
> > > +		compatible = "samsung,exynos5440-pcie";
> > > +		reg = <0x290000 0x1000
> > > +			0x270000 0x1000
> > > +			0x271000 0x40>;
> > > +		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
> > > +		clocks = <&clock 28>, <&clock 27>;
> > > +		clock-names = "pcie", "pcie_bus";
> > > +		#address-cells = <3>;
> > > +		#size-cells = <2>;
> > > +		device_type = "pci";
> > > +		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000
> > 
> > /*
> > 
> > > configuration space */ +			  0x81000000 0 0
> > 
> > 0x40001000 0 0x00010000
> > 
> > > /* downstream I/O */ +			  0x82000000 0 0x40011000
> > 
> > 0x40011000 0
> > 
> > > 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells =
> > 
> > <1>;
> > 
> > > +		interrupt-map-mask = <0 0 0 0>;
> > > +		interrupt-map = <0x0 0 &gic 53>;
> > > +	};
> > > +
> > > +	pcie1@2a0000 {
> > 
> > Same here.
> > 
> > Best regards,
> > Tomasz
> > 
> > > +		compatible = "samsung,exynos5440-pcie";
> > > +		reg = <0x2a0000 0x1000
> > > +			0x272000 0x1000
> > > +			0x271040 0x40>;
> > > +		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
> > > +		clocks = <&clock 29>, <&clock 27>;
> > > +		clock-names = "pcie", "pcie_bus";
> > > +		#address-cells = <3>;
> > > +		#size-cells = <2>;
> > > +		device_type = "pci";
> > > +		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000
> > 
> > /*
> > 
> > > configuration space */ +			  0x81000000 0 0
> > 
> > 0x60001000 0 0x00010000
> > 
> > > /* downstream I/O */ +			  0x82000000 0 0x60011000
> > 
> > 0x60011000 0
> > 
> > > 0x1ffef000>; /* non-prefetchable memory */ +		#interrupt-cells =
> > 
> > <1>;
> > 
> > > +		interrupt-map-mask = <0 0 0 0>;
> > > +		interrupt-map = <0x0 0 &gic 56>;
> > > +	};
> > > 
> > >  };
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Arnd Bergmann]

On Thursday 20 June 2013, Jingoo Han wrote:
> 2. patch adding label to the pinctrl node (which is a prerequisite) and
>     board-specific properties of PCIe nodes.
>    [PATCH] ARM: dts: Add pcie controller node for exynos5440-ssdk5440
> 
> arch/arm/boot/dts/exynos5440-ssdk5440.dts
> +
> +       pcie0@290000 {
> +               reset-gpio = <&pin_ctrl 5 0>;
> +       };
> +
> +       pcie1@2a0000 {
> +               reset-gpio = <&pin_ctrl 22 0>;
> +       };
> 
> arch/arm/boot/dts/exynos5440.dtsi
> -       pinctrl {
> +       pin_ctrl: pinctrl {
> 

Note that you don't really have to use a label, you can also refer
to the pinctrl node by its full path, which is just "/pinctrl"
or "/pinctrl@e0000".

	Arnd

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Jingoo Han]

On Thursday, June 20, 2013 7:17 PM, Arnd Bergmann wrote:
> On Thursday 20 June 2013, Jingoo Han wrote:
> > 2. patch adding label to the pinctrl node (which is a prerequisite) and
> >     board-specific properties of PCIe nodes.
> >    [PATCH] ARM: dts: Add pcie controller node for exynos5440-ssdk5440
> >
> > arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > +
> > +       pcie0@290000 {
> > +               reset-gpio = <&pin_ctrl 5 0>;
> > +       };
> > +
> > +       pcie1@2a0000 {
> > +               reset-gpio = <&pin_ctrl 22 0>;
> > +       };
> >
> > arch/arm/boot/dts/exynos5440.dtsi
> > -       pinctrl {
> > +       pin_ctrl: pinctrl {
> >
> 
> Note that you don't really have to use a label, you can also refer
> to the pinctrl node by its full path, which is just "/pinctrl"
> or "/pinctrl@e0000".

I see.

But, I have a question.
Using phandle has been the usual way to referring to a node. 
Why do you want to do it differently?

Best regards,
Jingoo Han

> 
> 	Arnd

Re: [PATCH V6 3/3] ARM: dts: Add pcie controller node for Samsung EXYNOS5440 SoC

[Tomasz Figa]

On Thursday 20 of June 2013 20:04:47 Jingoo Han wrote:
> On Thursday, June 20, 2013 7:17 PM, Arnd Bergmann wrote:
> > On Thursday 20 June 2013, Jingoo Han wrote:
> > > 2. patch adding label to the pinctrl node (which is a prerequisite)
> > > and
> > > 
> > >     board-specific properties of PCIe nodes.
> > >    
> > >    [PATCH] ARM: dts: Add pcie controller node for exynos5440-ssdk5440
> > > 
> > > arch/arm/boot/dts/exynos5440-ssdk5440.dts
> > > +
> > > +       pcie0@290000 {
> > > +               reset-gpio = <&pin_ctrl 5 0>;
> > > +       };
> > > +
> > > +       pcie1@2a0000 {
> > > +               reset-gpio = <&pin_ctrl 22 0>;
> > > +       };
> > > 
> > > arch/arm/boot/dts/exynos5440.dtsi
> > > -       pinctrl {
> > > +       pin_ctrl: pinctrl {
> > 
> > Note that you don't really have to use a label, you can also refer
> > to the pinctrl node by its full path, which is just "/pinctrl"
> > or "/pinctrl@e0000".

Sure, you can, but this is ugly.

> 
> I see.
> 
> But, I have a question.
> Using phandle has been the usual way to referring to a node.
> Why do you want to do it differently?

Path is translated to a phandle as well.

--
Best regards,
Tomasz