[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram
Bin Meng
bmeng.cn at gmail.com
Sun May 31 08:10:23 CEST 2015
Hi Andrew,
On Sat, May 30, 2015 at 2:27 AM, Andrew Bradford
<andrew at 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
>> <andrew at 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
>> >> <andrew at 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, <andrew at bradfordembedded.com> wrote:
>> >> >> > From: Andrew Bradford <andrew.bradford at 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;
> 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";
};
Regards,
Bin
More information about the U-Boot
mailing list