[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[andrew at bradfordembedded.com]

From: Andrew Bradford <andrew.bradford@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.

Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
---
 arch/x86/cpu/baytrail/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
index 6c291f9..0a87890 100644
--- a/arch/x86/cpu/baytrail/pci.c
+++ b/arch/x86/cpu/baytrail/pci.c
@@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
 	pci_set_region(hose->regions + 3,
 		       0,
 		       0,
-		       gd->ram_size,
+		       0x80000000,
 		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
 
 	hose->region_count = 4;
-- 
1.9.1

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Bin Meng]

+Simon.

Hi Andrew,

On Sat, May 23, 2015 at 3:09 AM,  <andrew@bradfordembedded.com> wrote:
> From: Andrew Bradford <andrew.bradford@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?

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

> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> ---
>  arch/x86/cpu/baytrail/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> index 6c291f9..0a87890 100644
> --- a/arch/x86/cpu/baytrail/pci.c
> +++ b/arch/x86/cpu/baytrail/pci.c
> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>         pci_set_region(hose->regions + 3,
>                        0,
>                        0,
> -                      gd->ram_size,
> +                      0x80000000,
>                        PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>
>         hose->region_count = 4;
> --

Regards,
Bin

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Andrew Bradford]

Hi Bin,

On 05/23 23:50, Bin Meng wrote:
> +Simon.
> 
> Hi Andrew,
> 
> On Sat, May 23, 2015 at 3:09 AM,  <andrew@bradfordembedded.com> wrote:
> > From: Andrew Bradford <andrew.bradford@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.

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.

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).

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.

> 
> > Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> > ---
> >  arch/x86/cpu/baytrail/pci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> > index 6c291f9..0a87890 100644
> > --- a/arch/x86/cpu/baytrail/pci.c
> > +++ b/arch/x86/cpu/baytrail/pci.c
> > @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
> >         pci_set_region(hose->regions + 3,
> >                        0,
> >                        0,
> > -                      gd->ram_size,
> > +                      0x80000000,
> >                        PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> >
> >         hose->region_count = 4;
> > --
> 
> Regards,
> Bin

Thanks,
Andrew

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Bin Meng]

Hi Andrew,

On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford
<andrew@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@bradfordembedded.com> wrote:
>> > From: Andrew Bradford <andrew.bradford@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.

> 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.
>

Yes, this all makes sense, so

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Andrew Bradford]

Hi Bin,

On 05/27 12:21, Bin Meng wrote:
> Hi Andrew,
> 
> On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford
> <andrew@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@bradfordembedded.com> wrote:
> >> > From: Andrew Bradford <andrew.bradford@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?

> > 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.
> >
> 
> Yes, this all makes sense, so
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Thanks!
-Andrew

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Bin Meng]

Hi Andrew,

On Wed, May 27, 2015 at 7:53 PM, Andrew Bradford
<andrew@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@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@bradfordembedded.com> wrote:
>> >> > From: Andrew Bradford <andrew.bradford@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.

Regards,
Bin

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Andrew Bradford]

Hi Bin,

On 05/29 13:00, Bin Meng wrote:
> Hi Andrew,
> 
> On Wed, May 27, 2015 at 7:53 PM, Andrew Bradford
> <andrew@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@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@bradfordembedded.com> wrote:
> >> >> > From: Andrew Bradford <andrew.bradford@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.

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.

Thanks,
Andrew

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Bin Meng]

Hi Andrew,

On Sat, May 30, 2015 at 2:27 AM, Andrew Bradford
<andrew@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@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@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@bradfordembedded.com> wrote:
>> >> >> > From: Andrew Bradford <andrew.bradford@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

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Andrew Bradford]

Hi Bin, 

On 05/31 14:10, Bin Meng wrote:
> Hi Andrew,
> 
> On Sat, May 30, 2015 at 2:27 AM, Andrew Bradford
> <andrew@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@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@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@bradfordembedded.com> wrote:
> >> >> >> > From: Andrew Bradford <andrew.bradford@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] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Bin Meng]

Hi Andrew,

On Mon, Jun 1, 2015 at 8:24 PM, Andrew Bradford
<andrew@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
>> <andrew@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@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@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@bradfordembedded.com> wrote:
>> >> >> >> > From: Andrew Bradford <andrew.bradford@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);

> 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.

>> > 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! :)
>

Regards,
Bin

[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram

[Andrew Bradford]

Hi Bin,

On 06/01 20:58, Bin Meng wrote:
> Hi Andrew,
> 
> On Mon, Jun 1, 2015 at 8:24 PM, Andrew Bradford
> <andrew@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
> >> <andrew@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@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@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@bradfordembedded.com> wrote:
> >> >> >> >> > From: Andrew Bradford <andrew.bradford@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] [PATCH v2] x86: baytrail: pci region 3 is not always mapped to end of ram

[andrew at bradfordembedded.com]

From: Andrew Bradford <andrew.bradford@kodakalaris.com>

Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
and additional SDRAM is mapped from 0x100000000 and up.  There is a
physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
Because of this, PCI region 3 should only try to use up to the amount of
SDRAM or 0x80000000, which ever is less.

Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
---
v2: limit maximum size to lesser of SDRAM or 0x80000000
---
 arch/x86/cpu/baytrail/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
index 6c291f9..53475e8 100644
--- a/arch/x86/cpu/baytrail/pci.c
+++ b/arch/x86/cpu/baytrail/pci.c
@@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
 	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);
 
 	hose->region_count = 4;
-- 
1.9.1

[U-Boot] [PATCH v2] x86: baytrail: pci region 3 is not always mapped to end of ram

[Andrew Bradford]

On 06/03 12:18, andrew at bradfordembedded.com wrote:
> From: Andrew Bradford <andrew.bradford@kodakalaris.com>
> 
> Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
> and additional SDRAM is mapped from 0x100000000 and up.  There is a
> physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
> Because of this, PCI region 3 should only try to use up to the amount of
> SDRAM or 0x80000000, which ever is less.
> 
> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> ---
> v2: limit maximum size to lesser of SDRAM or 0x80000000
> ---
>  arch/x86/cpu/baytrail/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> index 6c291f9..53475e8 100644
> --- a/arch/x86/cpu/baytrail/pci.c
> +++ b/arch/x86/cpu/baytrail/pci.c
> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>  	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);
>  
>  	hose->region_count = 4;

I'm very sorry, please disregard this!
It does not actually build.

I will fix and send a v3.

Thanks,
Andrew

[U-Boot] [PATCH v3] x86: baytrail: pci region 3 is not always mapped to end of ram

[andrew at bradfordembedded.com]

From: Andrew Bradford <andrew.bradford@kodakalaris.com>

Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
and additional SDRAM is mapped from 0x100000000 and up.  There is a
physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
Because of this, PCI region 3 should only try to use up to the amount of
SDRAM or 0x80000000, which ever is less.

Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
---
v3: Fix build breakage due to semicolon
v2: limit maximum size to lesser of SDRAM or 0x80000000
---
 arch/x86/cpu/baytrail/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
index 6c291f9..48409de 100644
--- a/arch/x86/cpu/baytrail/pci.c
+++ b/arch/x86/cpu/baytrail/pci.c
@@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
 	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);
 
 	hose->region_count = 4;
-- 
1.9.1

[U-Boot] [PATCH v3] x86: baytrail: pci region 3 is not always mapped to end of ram

[Bin Meng]

On Thu, Jun 4, 2015 at 12:37 AM,  <andrew@bradfordembedded.com> wrote:
> From: Andrew Bradford <andrew.bradford@kodakalaris.com>
>
> Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
> and additional SDRAM is mapped from 0x100000000 and up.  There is a
> physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
> Because of this, PCI region 3 should only try to use up to the amount of
> SDRAM or 0x80000000, which ever is less.
>
> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> ---
> v3: Fix build breakage due to semicolon
> v2: limit maximum size to lesser of SDRAM or 0x80000000
> ---
>  arch/x86/cpu/baytrail/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> index 6c291f9..48409de 100644
> --- a/arch/x86/cpu/baytrail/pci.c
> +++ b/arch/x86/cpu/baytrail/pci.c
> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>         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);
>
>         hose->region_count = 4;
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH v3] x86: baytrail: pci region 3 is not always mapped to end of ram

[Simon Glass]

On 3 June 2015 at 18:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Thu, Jun 4, 2015 at 12:37 AM,  <andrew@bradfordembedded.com> wrote:
>> From: Andrew Bradford <andrew.bradford@kodakalaris.com>
>>
>> Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
>> and additional SDRAM is mapped from 0x100000000 and up.  There is a
>> physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
>> Because of this, PCI region 3 should only try to use up to the amount of
>> SDRAM or 0x80000000, which ever is less.
>>
>> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
>> ---
>> v3: Fix build breakage due to semicolon
>> v2: limit maximum size to lesser of SDRAM or 0x80000000
>> ---
>>  arch/x86/cpu/baytrail/pci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
>> index 6c291f9..48409de 100644
>> --- a/arch/x86/cpu/baytrail/pci.c
>> +++ b/arch/x86/cpu/baytrail/pci.c
>> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>>         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);
>>
>>         hose->region_count = 4;
>> --
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3] x86: baytrail: pci region 3 is not always mapped to end of ram

[Simon Glass]

On 4 June 2015 at 02:47, Simon Glass <sjg@chromium.org> wrote:
> On 3 June 2015 at 18:15, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Thu, Jun 4, 2015 at 12:37 AM,  <andrew@bradfordembedded.com> wrote:
>>> From: Andrew Bradford <andrew.bradford@kodakalaris.com>
>>>
>>> Baytrail physically maps the first 2 GB of SDRAM from 0x0 to 0x7FFFFFFF
>>> and additional SDRAM is mapped from 0x100000000 and up.  There is a
>>> physical memory hole from 0x80000000 to 0xFFFFFFFF for other uses.
>>> Because of this, PCI region 3 should only try to use up to the amount of
>>> SDRAM or 0x80000000, which ever is less.
>>>
>>> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
>>> ---
>>> v3: Fix build breakage due to semicolon
>>> v2: limit maximum size to lesser of SDRAM or 0x80000000
>>> ---
>>>  arch/x86/cpu/baytrail/pci.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
>>> index 6c291f9..48409de 100644
>>> --- a/arch/x86/cpu/baytrail/pci.c
>>> +++ b/arch/x86/cpu/baytrail/pci.c
>>> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>>>         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);
>>>
>>>         hose->region_count = 4;
>>> --
>>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-x86, thanks!