Hi Bin, On 06/01 20:58, Bin Meng wrote: > Hi Andrew, > > On Mon, Jun 1, 2015 at 8:24 PM, Andrew Bradford > <and...@bradfordembedded.com> wrote: > > Hi Bin, > > > > On 05/31 14:10, Bin Meng wrote: > >> Hi Andrew, > >> > >> On Sat, May 30, 2015 at 2:27 AM, Andrew Bradford > >> <and...@bradfordembedded.com> wrote: > >> > Hi Bin, > >> > > >> > On 05/29 13:00, Bin Meng wrote: > >> >> Hi Andrew, > >> >> > >> >> On Wed, May 27, 2015 at 7:53 PM, Andrew Bradford > >> >> <and...@bradfordembedded.com> wrote: > >> >> > Hi Bin, > >> >> > > >> >> > On 05/27 12:21, Bin Meng wrote: > >> >> >> Hi Andrew, > >> >> >> > >> >> >> On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford > >> >> >> <and...@bradfordembedded.com> wrote: > >> >> >> > Hi Bin, > >> >> >> > > >> >> >> > On 05/23 23:50, Bin Meng wrote: > >> >> >> >> +Simon. > >> >> >> >> > >> >> >> >> Hi Andrew, > >> >> >> >> > >> >> >> >> On Sat, May 23, 2015 at 3:09 AM, <and...@bradfordembedded.com> > >> >> >> >> wrote: > >> >> >> >> > From: Andrew Bradford <andrew.bradf...@kodakalaris.com> > >> >> >> >> > > >> >> >> >> > PCI on Intel Baytrail is mapped to 0x80000000, which is not > >> >> >> >> > always at > >> >> >> >> > the end of SDRAM, such as when running with 4 GiB of SDRAM. > >> >> >> >> > The PCI bus > >> >> >> >> > memory mapping must stay within low memory and so when running > >> >> >> >> > with > > >> >> >> >> > 2 GiB of SDRAM, there is a hole in the SDRAM between 2 GiB and > >> >> >> >> > 4 GiB for > >> >> >> >> > memory mapped IO, such as PCI. > >> >> >> >> > >> >> >> >> Are you saying that if we mount 4GB DDR DIMM on the MinnowMax > >> >> >> >> board, > >> >> >> >> the Intel FSP will only put 0~2G as system RAM space, and leave > >> >> >> >> 2G~4G > >> >> >> >> as PCI address and other I/Os? > >> >> >> > > >> >> >> > Yes. If you mount 4 GiB of SDRAM onto an E3800 processor, then > >> >> >> > physical > >> >> >> > addresses from 0 to just below 2 GiB will be SDRAM (as per the > >> >> >> > HOBs) and > >> >> >> > also from 4 GiB to 6 GiB (also verified via the HOBs). The space > >> >> >> > from 2 > >> >> >> > GiB to 4 GiB will be mapped as various other regions. > >> >> >> > >> >> >> Ah, that's exactly the information I want :) > >> >> >> > >> >> >> > If you see section 4.1.1.1 (page 71 in the January 2015, Revision > >> >> >> > 3.6) > >> >> >> > E3800 datasheet, it shows that from 2 GiB to 4 GiB is mapped for > >> >> >> > PCI, > >> >> >> > Abort Page, Local APIC, and the Boot Vector. There's a lot of > >> >> >> > space in > >> >> >> > this area which appears unused, so I'm unsure as to why the area > >> >> >> > is so > >> >> >> > large. > >> >> >> > > >> >> >> > I have an Intel Valley Island board with E3825 and a 4 GiB SODIMM. > >> >> >> > I'm > >> >> >> > working on getting patches ready for this board but found that if I > >> >> >> > enabled all 4 GiB of SDRAM that the PCI bus would not function > >> >> >> > correctly. With this patch then the PCI bus functions (I'm able > >> >> >> > to do > >> >> >> > network operations with the RTL8111 Ethernet adapter). > >> >> >> > > >> >> >> >> I see from minnowmax.h, the PCI address starts from 0xd0000000. > >> >> >> >> > >> >> >> >> #define CONFIG_PCI_MEM_BUS 0xd0000000 > >> >> >> >> #define CONFIG_PCI_MEM_PHYS CONFIG_PCI_MEM_BUS > >> >> >> >> #define CONFIG_PCI_MEM_SIZE 0x10000000 > >> >> >> >> > >> >> >> >> #define CONFIG_PCI_PREF_BUS 0xc0000000 > >> >> >> >> #define CONFIG_PCI_PREF_PHYS CONFIG_PCI_PREF_BUS > >> >> >> >> #define CONFIG_PCI_PREF_SIZE 0x10000000 > >> >> >> > > >> >> >> > I see that hose->regions+0 is set to CONFIG_PCI_MEM_BUS and > >> >> >> > hose->regions+2 is set to CONFIG_PCI_PREF_BUS. However I'm > >> >> >> > modifying > >> >> >> > hose->regions+3. So the values from minnowmax.h *are* being used. > >> >> >> > I'm > >> >> >> > not yet that familiar with PCI configuration, so likely I'm not > >> >> >> > fully > >> >> >> > understanding how u-boot sets this up. > >> >> >> > > >> >> >> > >> >> >> The regions+3 is used by the inbound access, eg: PCI device access to > >> >> >> system memory. > >> >> >> > >> >> >> > Possibly my address of 0x80000000 is not correct, even though it > >> >> >> > works > >> >> >> > for me. But 0x80000000 is where it was being placed before, since > >> >> >> > it > >> >> >> > was going at the end of SDRAM (2GiB on minnowmax). > >> >> >> > > >> >> >> > >> >> >> You understanding is correct. We should only open 2GiB inbound memory > >> >> >> window for PCI devices. > >> >> > > >> >> > But, if you have less than 2 GiB of memory, my patch in theory *could* > >> >> > break things, right?. It seems like a better approach would be to > >> >> > limit > >> >> > the size to the lesser of 0x80000000 and gd->ram_size. Does that make > >> >> > sense? > >> >> > > >> >> > >> >> I think 2GB is a safe value and won't break things. Region 0 and > >> >> region 3 should not overlap. > >> >> > >> >> >> > If I artificially limit the amount of SDRAM by setting the fsp > >> >> >> > configuration to memory-down and then setting the DRAM > >> >> >> > configuration > >> >> >> > values such that I mimmic 1 GiB or 2 GiB of SDRAM, having my patch > >> >> >> > still > >> >> >> > provides access to the PCI bus, so with my patch there should be no > >> >> >> > adverse affects on E3800 systems that have less than 4 GiB of > >> >> >> > SDRAM. > >> >> >> > But without my patch, when running with >=4 GiB of SDRAM, PCI > >> >> >> > accesses > >> >> >> > end up returning "pci_hose_bus_to_phys: invalid physical address" > >> >> >> > errors. > >> >> > >> >> Can you add some printf to show all of the pci_hose_bus_to_phys() > >> >> parameters' value here when 4GB RAM is mounted? I'd like to understand > >> >> how the message "pci_hose_bus_to_phys: invalid physical address" is > >> >> produced. > >> > > >> > Patch of my changes to enable reporting of physical addresses being > >> > used looks like: > >> > > >> > diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c > >> > index b9ff23f..3babcb7 100644 > >> > --- a/drivers/pci/pci_common.c > >> > +++ b/drivers/pci/pci_common.c > >> > @@ -205,7 +205,7 @@ int __pci_hose_bus_to_phys(struct pci_controller > >> > *hose, > >> > return 0; > >> > } > >> > } > >> > - > >> > + printf("__pci_hose_bus_to_phys() failed!\n"); > >> > return 1; > >> > } > >> > > >> > @@ -237,6 +237,7 @@ phys_addr_t pci_hose_bus_to_phys(struct > >> > pci_controller *hose, > >> > if (ret) > >> > puts("pci_hose_bus_to_phys: invalid physical address\n"); > >> > > >> > + printf("bus_addr: 0x%x \t flags: 0x%lx \t phys_addr: %llx\n", > >> > bus_addr, flags, phys_addr); > >> > return phys_addr; > >> > } > >> > > >> > > >> > When I only configure 2GB of RAM, this prints bus and physical addresses > >> > in the 0x7adc0000 range when interfacing to the RTL8111. Everything > >> > works as expected, and I get only 1 copy of my "failed" message for each > >> > printing of my bus_addr, flags, and phys_addr data. > >> > > >> > When I switch to 4 GB of RAM configured, now __pci_hose_bus_to_phys() > >> > will not perform the modification to the physical address that gets > >> > passed to it. Then I see accesses to phys_addr to 0x7adc0000 range but > >> > phys_addresses are printed as 0 and I get 2 copies of my "failed" > >> > message for each printing of my bus_addr, flags, and phys_addr data. > >> > Ethernet does not work in this case. > >> > > >> > When I change to 1 GB of RAM, everything works and the phys and bus > >> > addresses are in the 0x3adc0000 range and I only get 1 "failed" message > >> > for each printing of bus_addr, flags, and phys_addr data. Just like in > >> > the 2GB case. > >> > > >> > Once I apply my patch to change region+3 to 0x80000000, then both 1GB > >> > and 2 GB work the same as before, but now with 4 GB things work and the > >> > output from my new printf()s matches the 2GB operation with phys and bus > >> > addresses in the 0x7adc0000 range. > >> > >> What's the value of gd->ram_size when you have 4GB RAM mounted? My > >> read of __pci_hose_bus_to_phys() logic is that it should still return > >> 0x7adc0000 with the logic below. > >> > >> if (bus_addr >= res->bus_start && (bus_addr - res->bus_start) > >> < res->size) { > >> *pa = (bus_addr - res->bus_start + res->phys_start); > >> return 0; > > > > gd->ram_size is 0x100000000 when I have 4 GB of SDRAM. dram_init() in > > Ah, I did not realize gd->ram_size can be exactly 4GiB on the > MinnowMax board. My previous assumption was it should be something > like 0xffxxxxxx as there has to be some memory hole configured by the > FSP, as was seen on my Crown Bay board (it has 1GiB memory but > gd->ram_size reports 1023MiB). So this explains the failure you saw. > > > fsp_dram.c sets the gd->ram_size by simply summing the sizes of all of > > the HOBs that get returned as resource descriptors of type system or > > reserved memory. > > > > Without patch [1] the meminfo command will report 0 bytes of RAM, but > > with patch [1] then the meminfo command reports 4 GB of RAM. > > > > [1]:http://git.denx.de/?p=u-boot.git;a=commitdiff;h=ea11b401b5ca10b5991e7c65832cfb7db54996c1 > > > > Without the patch from this thread (not [1]), and with 4 GB of RAM, hose > > region 3 is bus_start=0, phys_start=0, and size=0. And > > __pci_hose_bus_to_phys() is going through regions 0, 1, 2, and 3. It's > > region 3 that's failing. Regions 0, 1, and 2 seem fine with or without > > my patch from this thread. > > > > With the patch from this thread, then the size for region 3 is > > 0x80000000 and regions 0, 1, and 2 have the same values as without the > > patch. With the patch, __pci_hose_bus_to_phys() works the same for > > configurations of 1, 2, or 4 GB of RAM. > > > > I'm still concerned about the less than 2 GB of RAM case, as with my > > patch then region 3 will always be 0x80000000 in size and we may not > > actually have that much RAM. Likely this won't actually cause a > > problem, but it still worries me. > > > > This should not matter. The region 3 is just by U-Boot to translate > the bus address. It is not configured to any hardware registers. But > if you worry that too much, you can configure region 3 as: > > pci_set_region(hose->regions + 3, > 0, > 0, > - gd->ram_size, > + gd->ram_size < 0x80000000 ? gd->ram_size : 0x80000000;, > PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
Yes, this would make me worry less :) I can submit a v2. > > My test case that fails is trying to run the 'dhcp' command to fetch the > > Linux kernel over the Ethernet network connected to the RTL8111. > > > > Is your test case fails because of this region3 size equals to 2GiB? I > don't think so. My test case failes because region 3 shows a size of 0, without my patch when you have 4 GB of RAM. > >> > Sorry I'm relaying information rather than giving direct output from the > >> > board, but I don't yet have a UART setup on my Valley Island from within > >> > u-boot, I'm doing all my work via the vesa console and a USB keyboard. > >> > > >> > >> I thought you just wanted to enable the early debug UART on the Valley > >> Island board before to debug FSP, but it looks to me that you did not > >> get U-Boot's console on any (legacy or PCI) serial port when U-Boot is > >> up? Yes? Since you mentioned that Valley Island board has the PCI UART > >> connected from the BayTrail SoC, I think you can refer to Crown Bay's > >> implementation (arch/x86/dts/crownbay.dts) to add PCI UART in the > >> board's device tree so you can have a working U-Boot console over a > >> PCI UART. > >> > >> chosen { > >> /* > >> * By default the legacy superio serial port is used as the > >> * U-Boot serial console. If we want to use UART from > >> Topcliff > >> * PCH as the console, change this property to &pciuart#. > >> * > >> * For example, stdout-path = &pciuart0 will use the first > >> * UART on Topcliff PCH. > >> */ > >> stdout-path = "/serial"; > >> }; > > > > OK, I'll give this a shot soon, thanks for the pointers! :) Thanks, Andrew _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot