[U-Boot] [PATCH v2 7/7] dm: x86: baytrail: Correct PCI region 3 when driver model is used

[Andrew Bradford]

Hi Simon,

Probably I'm not fully understanding the startup flow and how PCI fits
into it yet, so can you please help me to understand this patch a little
more?

On 06/25 11:55, Simon Glass wrote:
> Commit afbbd413a fixed this for non-driver-model. Make sure that the driver
> model code handles this also.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2:
> - Only limit the PCI system memory region on x86 machines
> 
>  arch/x86/cpu/cpu.c                | 1 +
>  common/board_f.c                  | 4 ++++
>  drivers/pci/pci-uclass.c          | 8 ++++++--
>  include/asm-generic/global_data.h | 1 +
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index d108ee5..936b6ee 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -351,6 +351,7 @@ int x86_cpu_init_f(void)
>  
>  		gd->arch.has_mtrr = has_mtrr();
>  	}
> +	gd->pci_ram_top = 0x80000000U;

So this sets gd->pci_ram_top to 2 GiB, then...

>  
>  	return 0;
>  }
> diff --git a/common/board_f.c b/common/board_f.c
> index 21be26f..cb85382 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -350,6 +350,10 @@ static int setup_dest_addr(void)
>  		debug("Reserving MP boot page to %08lx\n", gd->relocaddr);
>  	}
>  #endif
> +#ifdef CONFIG_PCI
> +	gd->pci_ram_top = gd->ram_top;

This can set gd->pci_ram_top to be the actual size of memory later in
boot, which on E3800 with 4 GiB of SDRAM will be 4 GiB (right?), then...

> +#endif
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index edec93f..5b91fe3 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -444,6 +444,7 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>  {
>  	int pci_addr_cells, addr_cells, size_cells;
>  	int cells_per_record;
> +	phys_addr_t addr;
>  	const u32 *prop;
>  	int len;
>  	int i;
> @@ -494,8 +495,11 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>  	}
>  
>  	/* Add a region for our local memory */
> -	pci_set_region(hose->regions + hose->region_count++, 0, 0,
> -		       gd->ram_size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +	addr = gd->ram_size;
> +	if (gd->pci_ram_top && gd->pci_ram_top < addr)
> +		addr = gd->pci_ram_top;

This sets addr to be the lesser of gd->ram_size and gd->pci_ram_top when
we enumerate a PCI device.  Right?

What is the flow if, for example, a Baytrail E3800 has 4 GiB of RAM (and
the physical memory hole from 2 GiB to 4 GiB exists), then does this
code here actually set addr to be 2 GiB like it should?

I don't fear that memory sizes of 2 GiB or less will work fine but I
don't quite understand if this patch works how I expect for memory sizes
larger than 2 GiB if the processor has a physical memory hole like E3800
does.

Why wouldn't the assignment in board_f.c interfere and set
gd->pci_ram_top to be 4 GiB instead of 2 GiB like it should be?

Thanks,
Andrew