All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
@ 2013-05-16  6:29 Kevin Hao
  2013-05-28 22:45 ` Scott Wood
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Hao @ 2013-05-16  6:29 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc

All these boards use the same configuration file p1_p2_rdb_pc.h in
u-boot. So they have the same pci bus address set by the u-boot.
But in some of these boards the bus address set in dtb don't match
the one used by u-boot. And this will trigger a kernel bug in 32bit
kernel and cause the pci device malfunction. For example, on a
p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
and cpu address for one pci controller and then assign bus address
such as 0xa00004000 to some pci device. But in the kernel, the dtb
set the bus address to 0xe0000000 and the cpu address to 0xa0000000.
The kernel assumes mistakenly the assigned bus address 0xa0004000
in pci device is correct and keep it unchanged. This will definitely
cause the pci device malfunction. I have made two patches to fix
this in the pci subsystem.
http://patchwork.ozlabs.org/patch/243702/
http://patchwork.ozlabs.org/patch/243703/

But I still think it makes sense to set these bus address to match
with the u-boot. This issue can't be reproduced on 36bit kernel.
But I also tweak the 36bit dtb for the above reason.

The cpu address for the pci controller seems also not right in
p1025rdb_32b/36b.dts. So fix it at the same time.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
I just tested this on a p2020rdb-pca board. All others just passed build test.

 arch/powerpc/boot/dts/p1020mbg-pc_32b.dts | 4 ++--
 arch/powerpc/boot/dts/p1020mbg-pc_36b.dts | 4 ++--
 arch/powerpc/boot/dts/p1020utm-pc_32b.dts | 4 ++--
 arch/powerpc/boot/dts/p1020utm-pc_36b.dts | 4 ++--
 arch/powerpc/boot/dts/p1024rdb_32b.dts    | 4 ++--
 arch/powerpc/boot/dts/p1024rdb_36b.dts    | 4 ++--
 arch/powerpc/boot/dts/p1025rdb_32b.dts    | 4 ++--
 arch/powerpc/boot/dts/p1025rdb_36b.dts    | 4 ++--
 arch/powerpc/boot/dts/p2020rdb-pc_32b.dts | 4 ++--
 arch/powerpc/boot/dts/p2020rdb-pc_36b.dts | 4 ++--
 10 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts b/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
index ab8f076..042bda5 100644
--- a/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1020mbg-pc_36b.dts b/arch/powerpc/boot/dts/p1020mbg-pc_36b.dts
index 9e9f401..3afbd1f 100644
--- a/arch/powerpc/boot/dts/p1020mbg-pc_36b.dts
+++ b/arch/powerpc/boot/dts/p1020mbg-pc_36b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@fffe0a000 {
 		reg = <0xf 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0xc 0x00000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1020utm-pc_32b.dts b/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
index 4bfdd89..27576eb 100644
--- a/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1020utm-pc_36b.dts b/arch/powerpc/boot/dts/p1020utm-pc_36b.dts
index abec535..52eabf9c 100644
--- a/arch/powerpc/boot/dts/p1020utm-pc_36b.dts
+++ b/arch/powerpc/boot/dts/p1020utm-pc_36b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@fffe0a000 {
 		reg = <0xf 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0xc 0x00000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1024rdb_32b.dts b/arch/powerpc/boot/dts/p1024rdb_32b.dts
index 90e803e..ede1af6 100644
--- a/arch/powerpc/boot/dts/p1024rdb_32b.dts
+++ b/arch/powerpc/boot/dts/p1024rdb_32b.dts
@@ -53,7 +53,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -68,7 +68,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			reg = <0x0 0x0 0x0 0x0 0x0>;
diff --git a/arch/powerpc/boot/dts/p1024rdb_36b.dts b/arch/powerpc/boot/dts/p1024rdb_36b.dts
index 3656825..7642783 100644
--- a/arch/powerpc/boot/dts/p1024rdb_36b.dts
+++ b/arch/powerpc/boot/dts/p1024rdb_36b.dts
@@ -53,7 +53,7 @@
 
 	pci0: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -68,7 +68,7 @@
 
 	pci1: pcie@fffe0a000 {
 		reg = <0xf 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0xc 0x00000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			reg = <0x0 0x0 0x0 0x0 0x0>;
diff --git a/arch/powerpc/boot/dts/p1025rdb_32b.dts b/arch/powerpc/boot/dts/p1025rdb_32b.dts
index ac5729c..6ba7ddd 100644
--- a/arch/powerpc/boot/dts/p1025rdb_32b.dts
+++ b/arch/powerpc/boot/dts/p1025rdb_32b.dts
@@ -54,7 +54,7 @@
 	};
 
 	pci0: pcie@ffe09000 {
-		ranges = <0x2000000 0x0 0xe0000000 0 0xe0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
 		reg = <0 0xffe09000 0 0x1000>;
 		pcie@0 {
@@ -70,7 +70,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0xe0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1025rdb_36b.dts b/arch/powerpc/boot/dts/p1025rdb_36b.dts
index 06deb6f..4659e2d 100644
--- a/arch/powerpc/boot/dts/p1025rdb_36b.dts
+++ b/arch/powerpc/boot/dts/p1025rdb_36b.dts
@@ -55,7 +55,7 @@
 
 	pci0: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xe 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -70,7 +70,7 @@
 
 	pci1: pcie@fffe0a000 {
 		reg = <0xf 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0xc 0x00000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts b/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
index 57573bd..4ab21f8 100644
--- a/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
@@ -63,7 +63,7 @@
 
 	pci1: pcie@ffe09000 {
 		reg = <0 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -78,7 +78,7 @@
 
 	pci0: pcie@ffe0a000 {
 		reg = <0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p2020rdb-pc_36b.dts b/arch/powerpc/boot/dts/p2020rdb-pc_36b.dts
index 470247e..488f1ad 100644
--- a/arch/powerpc/boot/dts/p2020rdb-pc_36b.dts
+++ b/arch/powerpc/boot/dts/p2020rdb-pc_36b.dts
@@ -63,7 +63,7 @@
 
 	pci1: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -78,7 +78,7 @@
 
 	pci0: pcie@fffe0a000 {
 		reg = <0xf 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0xc 0x00000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-16  6:29 [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards Kevin Hao
@ 2013-05-28 22:45 ` Scott Wood
  2013-05-30  3:25   ` Kevin Hao
  2013-05-30 14:21   ` [PATCH] " Kumar Gala
  0 siblings, 2 replies; 8+ messages in thread
From: Scott Wood @ 2013-05-28 22:45 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc

On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
> All these boards use the same configuration file p1_p2_rdb_pc.h in
> u-boot. So they have the same pci bus address set by the u-boot.
> But in some of these boards the bus address set in dtb don't match
> the one used by u-boot. And this will trigger a kernel bug in 32bit
> kernel and cause the pci device malfunction. For example, on a
> p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
> and cpu address for one pci controller and then assign bus address
> such as 0xa00004000 to some pci device. But in the kernel, the dtb
> set the bus address to 0xe0000000 and the cpu address to 0xa0000000.
> The kernel assumes mistakenly the assigned bus address 0xa0004000
> in pci device is correct and keep it unchanged. This will definitely
> cause the pci device malfunction. I have made two patches to fix
> this in the pci subsystem.
> http://patchwork.ozlabs.org/patch/243702/
> http://patchwork.ozlabs.org/patch/243703/
>=20
> But I still think it makes sense to set these bus address to match
> with the u-boot. This issue can't be reproduced on 36bit kernel.
> But I also tweak the 36bit dtb for the above reason.

IIRC the reason for using 0xe0000000 on all PCIe roots is to maximize =20
the memory that is DMA-addressable without involving swiotlb.

Maybe U-Boot should be fixed?

-Scott=

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-28 22:45 ` Scott Wood
@ 2013-05-30  3:25   ` Kevin Hao
  2013-05-30 18:57     ` Scott Wood
  2013-05-30 14:21   ` [PATCH] " Kumar Gala
  1 sibling, 1 reply; 8+ messages in thread
From: Kevin Hao @ 2013-05-30  3:25 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc

[-- Attachment #1: Type: text/plain, Size: 2526 bytes --]

On Tue, May 28, 2013 at 05:45:56PM -0500, Scott Wood wrote:
> On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
> >All these boards use the same configuration file p1_p2_rdb_pc.h in
> >u-boot. So they have the same pci bus address set by the u-boot.
> >But in some of these boards the bus address set in dtb don't match
> >the one used by u-boot. And this will trigger a kernel bug in 32bit
> >kernel and cause the pci device malfunction. For example, on a
> >p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
> >and cpu address for one pci controller and then assign bus address
> >such as 0xa00004000 to some pci device. But in the kernel, the dtb
> >set the bus address to 0xe0000000 and the cpu address to 0xa0000000.
> >The kernel assumes mistakenly the assigned bus address 0xa0004000
> >in pci device is correct and keep it unchanged. This will definitely
> >cause the pci device malfunction. I have made two patches to fix
> >this in the pci subsystem.
> >http://patchwork.ozlabs.org/patch/243702/
> >http://patchwork.ozlabs.org/patch/243703/
> >
> >But I still think it makes sense to set these bus address to match
> >with the u-boot. This issue can't be reproduced on 36bit kernel.
> >But I also tweak the 36bit dtb for the above reason.
> 
> IIRC the reason for using 0xe0000000 on all PCIe roots is to
> maximize the memory that is DMA-addressable without involving
> swiotlb.

OK, this sounds reasonable. I can drop the changes for the 36bit dts. But for
the 32bit dts, it does cause the kernel hang on my p2020rdb-pca board when the
SiI3132 driver probe the on-board pcie to sata controller. I think this issue
should apply to all these boards if it has a pci device plugged. So we should
fix them ASAP.

> 
> Maybe U-Boot should be fixed?

Maybe. I have created patch for kernel to detect this kind of mismatch between
kernel and bootloader and then try to reassign the bus address automatically.
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=next&id=cf4d1cf5ac5e7d2b886af6ed906ea0dcdc5b6855

So with this patch the kernel should just work even without this patch and
the fix for u-boot. But this patch is just queued for 3.11. So I wish we can
tweak the 32bit dts to accommodate to the u-boot now so that we can make sure
that these boards are at least bootable for 3.10 or previous kernel. Then we
can revert this patch for more DMA address space once the pci patch are
merged into mainline.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-28 22:45 ` Scott Wood
  2013-05-30  3:25   ` Kevin Hao
@ 2013-05-30 14:21   ` Kumar Gala
  2013-05-30 14:43     ` Scott Wood
  1 sibling, 1 reply; 8+ messages in thread
From: Kumar Gala @ 2013-05-30 14:21 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc, Kevin Hao


On May 28, 2013, at 5:45 PM, Scott Wood wrote:

> On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
>> All these boards use the same configuration file p1_p2_rdb_pc.h in
>> u-boot. So they have the same pci bus address set by the u-boot.
>> But in some of these boards the bus address set in dtb don't match
>> the one used by u-boot. And this will trigger a kernel bug in 32bit
>> kernel and cause the pci device malfunction. For example, on a
>> p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
>> and cpu address for one pci controller and then assign bus address
>> such as 0xa00004000 to some pci device. But in the kernel, the dtb
>> set the bus address to 0xe0000000 and the cpu address to 0xa0000000.
>> The kernel assumes mistakenly the assigned bus address 0xa0004000
>> in pci device is correct and keep it unchanged. This will definitely
>> cause the pci device malfunction. I have made two patches to fix
>> this in the pci subsystem.
>> http://patchwork.ozlabs.org/patch/243702/
>> http://patchwork.ozlabs.org/patch/243703/
>> But I still think it makes sense to set these bus address to match
>> with the u-boot. This issue can't be reproduced on 36bit kernel.
>> But I also tweak the 36bit dtb for the above reason.
>=20
> IIRC the reason for using 0xe0000000 on all PCIe roots is to maximize =
the memory that is DMA-addressable without involving swiotlb.
>=20
> Maybe U-Boot should be fixed?
>=20
> -Scott

I feel that u-boot was the way it is to allow accessing each bus from =
the command line in u-boot w/o big changes for >32-bit addressing.

Linux was able to handle the PCI bus addresses all being the same.

- k=

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-30 14:21   ` [PATCH] " Kumar Gala
@ 2013-05-30 14:43     ` Scott Wood
  0 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2013-05-30 14:43 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc, Kevin Hao

On 05/30/2013 09:21:19 AM, Kumar Gala wrote:
>=20
> On May 28, 2013, at 5:45 PM, Scott Wood wrote:
>=20
> > On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
> >> All these boards use the same configuration file p1_p2_rdb_pc.h in
> >> u-boot. So they have the same pci bus address set by the u-boot.
> >> But in some of these boards the bus address set in dtb don't match
> >> the one used by u-boot. And this will trigger a kernel bug in 32bit
> >> kernel and cause the pci device malfunction. For example, on a
> >> p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
> >> and cpu address for one pci controller and then assign bus address
> >> such as 0xa00004000 to some pci device. But in the kernel, the dtb
> >> set the bus address to 0xe0000000 and the cpu address to =20
> 0xa0000000.
> >> The kernel assumes mistakenly the assigned bus address 0xa0004000
> >> in pci device is correct and keep it unchanged. This will =20
> definitely
> >> cause the pci device malfunction. I have made two patches to fix
> >> this in the pci subsystem.
> >> http://patchwork.ozlabs.org/patch/243702/
> >> http://patchwork.ozlabs.org/patch/243703/
> >> But I still think it makes sense to set these bus address to match
> >> with the u-boot. This issue can't be reproduced on 36bit kernel.
> >> But I also tweak the 36bit dtb for the above reason.
> >
> > IIRC the reason for using 0xe0000000 on all PCIe roots is to =20
> maximize the memory that is DMA-addressable without involving swiotlb.
> >
> > Maybe U-Boot should be fixed?
> >
> > -Scott
>=20
> I feel that u-boot was the way it is to allow accessing each bus from =20
> the command line in u-boot w/o big changes for >32-bit addressing.
>=20
> Linux was able to handle the PCI bus addresses all being the same.

It's a bit of a hack though, in that you're using the device tree to =20
indicate how you want the hardware programmed rather than to describe =20
the hardware or even what U-Boot's done to it, and in that you can't =20
arbitrarily change what U-Boot chose -- it only works because you're =20
picking an address that U-Boot used for one of the PCIe controllers and =20
thus U-Boot covered it with a LAW.

-Scott=

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-30  3:25   ` Kevin Hao
@ 2013-05-30 18:57     ` Scott Wood
  2013-05-31  7:53       ` Kevin Hao
  0 siblings, 1 reply; 8+ messages in thread
From: Scott Wood @ 2013-05-30 18:57 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc

On 05/29/2013 10:25:25 PM, Kevin Hao wrote:
> On Tue, May 28, 2013 at 05:45:56PM -0500, Scott Wood wrote:
> > On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
> > >All these boards use the same configuration file p1_p2_rdb_pc.h in
> > >u-boot. So they have the same pci bus address set by the u-boot.
> > >But in some of these boards the bus address set in dtb don't match
> > >the one used by u-boot. And this will trigger a kernel bug in 32bit
> > >kernel and cause the pci device malfunction. For example, on a
> > >p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
> > >and cpu address for one pci controller and then assign bus address
> > >such as 0xa00004000 to some pci device. But in the kernel, the dtb
> > >set the bus address to 0xe0000000 and the cpu address to =20
> 0xa0000000.
> > >The kernel assumes mistakenly the assigned bus address 0xa0004000
> > >in pci device is correct and keep it unchanged. This will =20
> definitely
> > >cause the pci device malfunction. I have made two patches to fix
> > >this in the pci subsystem.
> > >http://patchwork.ozlabs.org/patch/243702/
> > >http://patchwork.ozlabs.org/patch/243703/
> > >
> > >But I still think it makes sense to set these bus address to match
> > >with the u-boot. This issue can't be reproduced on 36bit kernel.
> > >But I also tweak the 36bit dtb for the above reason.
> >
> > IIRC the reason for using 0xe0000000 on all PCIe roots is to
> > maximize the memory that is DMA-addressable without involving
> > swiotlb.
>=20
> OK, this sounds reasonable. I can drop the changes for the 36bit dts. =20
> But for
> the 32bit dts, it does cause the kernel hang on my p2020rdb-pca board =20
> when the
> SiI3132 driver probe the on-board pcie to sata controller. I think =20
> this issue
> should apply to all these boards if it has a pci device plugged. So =20
> we should
> fix them ASAP.

Is this what your 3.11 patch fixes, or does it hang even with that?

> > Maybe U-Boot should be fixed?
>=20
> Maybe. I have created patch for kernel to detect this kind of =20
> mismatch between
> kernel and bootloader and then try to reassign the bus address =20
> automatically.
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=3D=
next&id=3Dcf4d1cf5ac5e7d2b886af6ed906ea0dcdc5b6855
>=20
> So with this patch the kernel should just work even without this =20
> patch and
> the fix for u-boot. But this patch is just queued for 3.11. So I wish =20
> we can
> tweak the 32bit dts to accommodate to the u-boot now so that we can =20
> make sure
> that these boards are at least bootable for 3.10 or previous kernel. =20
> Then we
> can revert this patch for more DMA address space once the pci patch =20
> are
> merged into mainline.

Is this a regression, or has it been broken for a while?

-Scott=

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-30 18:57     ` Scott Wood
@ 2013-05-31  7:53       ` Kevin Hao
  2013-06-01 12:12         ` [PATCH v2] " Kevin Hao
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Hao @ 2013-05-31  7:53 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc

[-- Attachment #1: Type: text/plain, Size: 3790 bytes --]

On Thu, May 30, 2013 at 01:57:42PM -0500, Scott Wood wrote:
> On 05/29/2013 10:25:25 PM, Kevin Hao wrote:
> >On Tue, May 28, 2013 at 05:45:56PM -0500, Scott Wood wrote:
> >> On 05/16/2013 01:29:45 AM, Kevin Hao wrote:
> >> >All these boards use the same configuration file p1_p2_rdb_pc.h in
> >> >u-boot. So they have the same pci bus address set by the u-boot.
> >> >But in some of these boards the bus address set in dtb don't match
> >> >the one used by u-boot. And this will trigger a kernel bug in 32bit
> >> >kernel and cause the pci device malfunction. For example, on a
> >> >p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
> >> >and cpu address for one pci controller and then assign bus address
> >> >such as 0xa00004000 to some pci device. But in the kernel, the dtb
> >> >set the bus address to 0xe0000000 and the cpu address to
> >0xa0000000.
> >> >The kernel assumes mistakenly the assigned bus address 0xa0004000
> >> >in pci device is correct and keep it unchanged. This will
> >definitely
> >> >cause the pci device malfunction. I have made two patches to fix
> >> >this in the pci subsystem.
> >> >http://patchwork.ozlabs.org/patch/243702/
> >> >http://patchwork.ozlabs.org/patch/243703/
> >> >
> >> >But I still think it makes sense to set these bus address to match
> >> >with the u-boot. This issue can't be reproduced on 36bit kernel.
> >> >But I also tweak the 36bit dtb for the above reason.
> >>
> >> IIRC the reason for using 0xe0000000 on all PCIe roots is to
> >> maximize the memory that is DMA-addressable without involving
> >> swiotlb.
> >
> >OK, this sounds reasonable. I can drop the changes for the 36bit
> >dts. But for
> >the 32bit dts, it does cause the kernel hang on my p2020rdb-pca
> >board when the
> >SiI3132 driver probe the on-board pcie to sata controller. I think
> >this issue
> >should apply to all these boards if it has a pci device plugged.
> >So we should
> >fix them ASAP.
> 
> Is this what your 3.11 patch fixes,

Yes, the patch in the pci next tree fix this issue in a more general
level.

> or does it hang even with that?

No.

> 
> >> Maybe U-Boot should be fixed?
> >
> >Maybe. I have created patch for kernel to detect this kind of
> >mismatch between
> >kernel and bootloader and then try to reassign the bus address
> >automatically.
> >https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=next&id=cf4d1cf5ac5e7d2b886af6ed906ea0dcdc5b6855
> >
> >So with this patch the kernel should just work even without this
> >patch and
> >the fix for u-boot. But this patch is just queued for 3.11. So I
> >wish we can
> >tweak the 32bit dts to accommodate to the u-boot now so that we
> >can make sure
> >that these boards are at least bootable for 3.10 or previous
> >kernel. Then we
> >can revert this patch for more DMA address space once the pci
> >patch are
> >merged into mainline.
> 
> Is this a regression, or has it been broken for a while?

It seems that the p2020rdb-pc_32b.dts was not bootable since it
was first introduced into the mainline kernel. The reason is that
the following patch change the method of doing the translation of the
bus->resource and resource->bus and then disclose this mismatch
between bootloader and kernel.

commit 6c5705fec63d83eeb165fe61e34adc92ecc2ce75
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Feb 23 20:19:03 2012 -0700

    powerpc/PCI: get rid of device resource fixups
    
    Tell the PCI core about host bridge address translation so it can take
    care of bus-to-resource conversion for us.
    
    CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>


Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards
  2013-05-31  7:53       ` Kevin Hao
@ 2013-06-01 12:12         ` Kevin Hao
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Hao @ 2013-06-01 12:12 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc

All these boards use the same configuration file p1_p2_rdb_pc.h in
u-boot. So they have the same pci bus address set by the u-boot.
But in some of these boards the bus address set in dtb don't match
the one used by u-boot. And this will trigger a kernel bug in 32bit
kernel and cause the pci device malfunction. For example, on a
p2020rdb-pc board the u-boot use the 0xa0000000 as both bus address
and cpu address for one pci controller and then assign bus address
such as 0xa00004000 to some pci device. But in the kernel, the dtb
set the bus address to 0xe0000000 and the cpu address to 0xa0000000.
The kernel assumes mistakenly the assigned bus address 0xa0004000
in pci device is correct and keep it unchanged. This will definitely
cause the pci device malfunction. I have made a patch to fix this
in the pci subsystem.
  https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=next&id=cf4d1cf5ac5e7d2b886af6ed906ea0dcdc5b6855

Since it changed the general code and will need more time to merge
into upstream. So I would like to tweak the 32bit dts of these boards
first and make it available to kernel ASAP.

The cpu address for the pci controller seems also not right in
p1025rdb_32b/36b.dts. So fix it at the same time.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
Cc: stable@vger.kernel.org
---
Hi Kumar,

I would like this to be merged into 3.10 since some boards which has
an integrated PCIe device (such as p2020rdb-pca) is not bootable at all
without this fix.

v2:
  * Remove the parts for the 36bit dts.
  * Add stable to the CC.

 arch/powerpc/boot/dts/p1020mbg-pc_32b.dts | 4 ++--
 arch/powerpc/boot/dts/p1020utm-pc_32b.dts | 4 ++--
 arch/powerpc/boot/dts/p1024rdb_32b.dts    | 4 ++--
 arch/powerpc/boot/dts/p1025rdb_32b.dts    | 4 ++--
 arch/powerpc/boot/dts/p1025rdb_36b.dts    | 2 +-
 arch/powerpc/boot/dts/p2020rdb-pc_32b.dts | 4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts b/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
index ab8f076..042bda5 100644
--- a/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p1020mbg-pc_32b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1020utm-pc_32b.dts b/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
index 4bfdd89..27576eb 100644
--- a/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p1020utm-pc_32b.dts
@@ -56,7 +56,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -71,7 +71,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0x0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1024rdb_32b.dts b/arch/powerpc/boot/dts/p1024rdb_32b.dts
index 90e803e..ede1af6 100644
--- a/arch/powerpc/boot/dts/p1024rdb_32b.dts
+++ b/arch/powerpc/boot/dts/p1024rdb_32b.dts
@@ -53,7 +53,7 @@
 
 	pci0: pcie@ffe09000 {
 		reg = <0x0 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -68,7 +68,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0x0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0x0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0x0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0x0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			reg = <0x0 0x0 0x0 0x0 0x0>;
diff --git a/arch/powerpc/boot/dts/p1025rdb_32b.dts b/arch/powerpc/boot/dts/p1025rdb_32b.dts
index ac5729c..6ba7ddd 100644
--- a/arch/powerpc/boot/dts/p1025rdb_32b.dts
+++ b/arch/powerpc/boot/dts/p1025rdb_32b.dts
@@ -54,7 +54,7 @@
 	};
 
 	pci0: pcie@ffe09000 {
-		ranges = <0x2000000 0x0 0xe0000000 0 0xe0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
 		reg = <0 0xffe09000 0 0x1000>;
 		pcie@0 {
@@ -70,7 +70,7 @@
 
 	pci1: pcie@ffe0a000 {
 		reg = <0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0xe0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p1025rdb_36b.dts b/arch/powerpc/boot/dts/p1025rdb_36b.dts
index 06deb6f..f7a9cf8 100644
--- a/arch/powerpc/boot/dts/p1025rdb_36b.dts
+++ b/arch/powerpc/boot/dts/p1025rdb_36b.dts
@@ -55,7 +55,7 @@
 
 	pci0: pcie@fffe09000 {
 		reg = <0xf 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0xe 0x20000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
diff --git a/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts b/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
index 57573bd..4ab21f8 100644
--- a/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
+++ b/arch/powerpc/boot/dts/p2020rdb-pc_32b.dts
@@ -63,7 +63,7 @@
 
 	pci1: pcie@ffe09000 {
 		reg = <0 0xffe09000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0xa0000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
@@ -78,7 +78,7 @@
 
 	pci0: pcie@ffe0a000 {
 		reg = <0 0xffe0a000 0 0x1000>;
-		ranges = <0x2000000 0x0 0xe0000000 0 0x80000000 0x0 0x20000000
+		ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
 		pcie@0 {
 			ranges = <0x2000000 0x0 0xe0000000
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-06-01 12:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-16  6:29 [PATCH] powerpc/mpc85xx: match with the pci bus address used by u-boot for all p1_p2_rdb_pc boards Kevin Hao
2013-05-28 22:45 ` Scott Wood
2013-05-30  3:25   ` Kevin Hao
2013-05-30 18:57     ` Scott Wood
2013-05-31  7:53       ` Kevin Hao
2013-06-01 12:12         ` [PATCH v2] " Kevin Hao
2013-05-30 14:21   ` [PATCH] " Kumar Gala
2013-05-30 14:43     ` Scott Wood

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.