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

[Matt Porter <mporter@linaro.org>]

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