On Fri, Feb 14, 2014 at 10:36:20AM +0800, FengHua wrote: > > > > > -----Original Messages----- > > From: "Wolfgang Denk" <w...@denx.de> > > Sent Time: 2014-02-14 00:30:24 (Friday) > > To: feng...@phytium.com.cn > > Cc: u-boot@lists.denx.de, tr...@ti.com, albert.u.b...@aribaud.net > > Subject: Re: [PATCH][RFC v2] add pci 64 bit prefechable mem support > > > > Dear feng...@phytium.com.cn, > > > > In message <1392282108-56485-1-git-send-email-feng...@phytium.com.cn> you > > wrote: > > > From: David Feng <feng...@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 at 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot