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
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.
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.
> > 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! :)
-Andrew
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
