[U-Boot] [PATCH][RFC v2] add pci 64 bit prefechable mem support

[U-Boot] [PATCH][RFC v2] add pci 64 bit prefechable mem support

[fenghua at phytium.com.cn]

From: David Feng <fenghua@phytium.com.cn>

u-boot did not program the upper 32 bits of prefetchable base and limit
in pci bridge config space. I think it's needed when 64 bit address space
is used.
Changes for v1:
  - use lowest bit of prefechable base(or limit) to determine whether
    the bridge support 64 bit prefechable address space.

Signed-off-by: David Feng <fenghua@phytium.com.cn>
---
 drivers/pci/pci_auto.c |   38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 86ba6b5..14652be 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -197,9 +197,12 @@ void pciauto_prescan_setup_bridge(struct pci_controller *hose,
 	struct pci_region *pci_mem = hose->pci_mem;
 	struct pci_region *pci_prefetch = hose->pci_prefetch;
 	struct pci_region *pci_io = hose->pci_io;
-	u16 cmdstat;
+	u16 cmdstat, prefechable_64;
 
 	pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat);
+	pci_hose_read_config_word(hose, dev, PCI_PREF_MEMORY_BASE,
+				&prefechable_64);
+	prefechable_64 &= 0x1;
 
 	/* Configure bus number registers */
 	pci_hose_write_config_byte(hose, dev, PCI_PRIMARY_BUS,
@@ -226,12 +229,26 @@ void pciauto_prescan_setup_bridge(struct pci_controller *hose,
 		/* Set up memory and I/O filter limits, assume 32-bit I/O space */
 		pci_hose_write_config_word(hose, dev, PCI_PREF_MEMORY_BASE,
 					(pci_prefetch->bus_lower & 0xfff00000) >> 16);
+		if (prefechable_64)
+#ifdef CONFIG_SYS_PCI_64BIT
+			pci_hose_write_config_dword(hose, dev,
+					PCI_PREF_BASE_UPPER32,
+					pci_prefetch->bus_lower >> 32);
+#else
+			pci_hose_write_config_dword(hose, dev,
+					PCI_PREF_BASE_UPPER32,
+					0x0);
+#endif
 
 		cmdstat |= PCI_COMMAND_MEMORY;
 	} else {
 		/* We don't support prefetchable memory for now, so disable */
 		pci_hose_write_config_word(hose, dev, PCI_PREF_MEMORY_BASE, 0x1000);
 		pci_hose_write_config_word(hose, dev, PCI_PREF_MEMORY_LIMIT, 0x0);
+		if (prefechable_64) {
+			pci_hose_write_config_word(hose, dev, PCI_PREF_BASE_UPPER32, 0x0);
+			pci_hose_write_config_word(hose, dev, PCI_PREF_LIMIT_UPPER32, 0x0);
+		}
 	}
 
 	if (pci_io) {
@@ -271,11 +288,28 @@ void pciauto_postscan_setup_bridge(struct pci_controller *hose,
 	}
 
 	if (pci_prefetch) {
+		u16 prefechable_64;
+
+		pci_hose_read_config_word(hose, dev,
+					PCI_PREF_MEMORY_LIMIT,
+					&prefechable_64);
+		prefechable_64 &= 0x1;
+
 		/* Round memory allocator to 1MB boundary */
 		pciauto_region_align(pci_prefetch, 0x100000);
 
 		pci_hose_write_config_word(hose, dev, PCI_PREF_MEMORY_LIMIT,
-				(pci_prefetch->bus_lower - 1) >> 16);
+					(pci_prefetch->bus_lower - 1) >> 16);
+		if (prefechable_64)
+#ifdef CONFIG_SYS_PCI_64BIT
+			pci_hose_write_config_dword(hose, dev,
+					PCI_PREF_LIMIT_UPPER32,
+					(pci_prefetch->bus_lower - 1) >> 32);
+#else
+			pci_hose_write_config_dword(hose, dev,
+					PCI_PREF_LIMIT_UPPER32,
+					0x0);
+#endif
 	}
 
 	if (pci_io) {
-- 
1.7.9.5

[U-Boot] [PATCH][RFC v2] add pci 64 bit prefechable mem support

[Wolfgang Denk]

Dear fenghua at phytium.com.cn,

In message <1392282108-56485-1-git-send-email-fenghua@phytium.com.cn> you wrote:
> From: David Feng <fenghua@phytium.com.cn>
> 
> u-boot did not program the upper 32 bits of prefetchable base and limit
> in pci bridge config space. I think it's needed when 64 bit address space
> is used.

You write "I think it's needed" - is it or not?

Do you have a specific test case that fails without your patch, and
works with it?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Substitute "damn" every time you're inclined to write "very"; your
editor will delete it and the writing will be just as it should be.
                - Mark Twain

[U-Boot] [PATCH][RFC v2] add pci 64 bit prefechable mem support

[FengHua]

> -----Original Messages-----
> From: "Wolfgang Denk" <wd@denx.de>
> Sent Time: 2014-02-14 00:30:24 (Friday)
> To: fenghua at phytium.com.cn
> Cc: u-boot at lists.denx.de, trini at ti.com, albert.u.boot at aribaud.net
> Subject: Re: [PATCH][RFC v2] add pci 64 bit prefechable mem support
> 
> Dear fenghua at phytium.com.cn,
> 
> In message <1392282108-56485-1-git-send-email-fenghua@phytium.com.cn> you wrote:
> > From: David Feng <fenghua@phytium.com.cn>
> > 
> > u-boot did not program the upper 32 bits of prefetchable base and limit
> > in pci bridge config space. I think it's needed when 64 bit address space
> > is used.
> 
> You write "I think it's needed" - is it or not?
> 
> Do you have a specific test case that fails without your patch, and
> works with it?
> 
> Best regards,
> 
> Wolfgang Denk. 
There's no test case now (maybe a few days later I could make a test).
"PCI-to-PCI Bridge Architecture Specification" require that the upper 32 bit of prefetchable space
must be initialized by configuration software. But usually the default value is zero already.
A board using 64 bit pci prefetchable memory space and a pci device with 64 bit prefetchable space
are needed. I think u-boot did not encounter this situation before.

Best Regards,
David

[U-Boot] [PATCH][RFC v2] add pci 64 bit prefechable mem support

[Matt Porter]

On Fri, Feb 14, 2014 at 10:36:20AM +0800, FengHua wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Wolfgang Denk" <wd@denx.de>
> > Sent Time: 2014-02-14 00:30:24 (Friday)
> > To: fenghua at phytium.com.cn
> > Cc: u-boot at lists.denx.de, trini at ti.com, albert.u.boot at aribaud.net
> > Subject: Re: [PATCH][RFC v2] add pci 64 bit prefechable mem support
> > 
> > Dear fenghua at phytium.com.cn,
> > 
> > In message <1392282108-56485-1-git-send-email-fenghua@phytium.com.cn> you wrote:
> > > From: David Feng <fenghua@phytium.com.cn>
> > > 
> > > u-boot did not program the upper 32 bits of prefetchable base and limit
> > > in pci bridge config space. I think it's needed when 64 bit address space
> > > is used.
> > 
> > You write "I think it's needed" - is it or not?
> > 
> > Do you have a specific test case that fails without your patch, and
> > works with it?
> > 
> > Best regards,
> > 
> > Wolfgang Denk. 
> There's no test case now (maybe a few days later I could make a test).
> "PCI-to-PCI Bridge Architecture Specification" require that the upper 32 bit of prefetchable space
> must be initialized by configuration software. But usually the default value is zero already.
> A board using 64 bit pci prefetchable memory space and a pci device with 64 bit prefetchable space
> are needed. I think u-boot did not encounter this situation before.

There's two things happening here.

1) You are adding support to explicitly program the upper32 prefetch
limit/base to zero (in the 64-bit prefetch memory <4GB case) which is
a completely theoretical fix. I can more or less confirm that this
doesn't cause a problem in practice for prefetch memory <4GB. When I
wrote the original code in-kernel, I had noticed this on the 21154
bridges and others when I was trying out WIP prefetch support (which
I never finished to upstream because we let the kernel subsystem fix
up prefetch later). If we look@the history of the prefetch support
added to the U-Boot version of pci_auto.c it's also proven on real h/w
that this is only a theoretical fix. To be fair, it is best to be safe,
but as Wolfgang points out it appears you are fixing something that's
not practically broken

2) 64-bit prefetch support for prefetch memory >4GB. It's up to the
maintainers, but given that this is untested code, I don't see a good
reason to merge it. I have reviewed it and the implementation looks
correct to me per spec. However, I believe that you should resubmit this
patch along with support for a platform that actually makes use of it as
you describe above. At that time, it would be appropriate to fix the
possible latent bug (for a not-yet-known p2p bridge that doesn't default
upper32 limit/base to 0) in the <4GB case just as part of handling the
>4GB case.

-Matt