[U-Boot] [PATCH 07/23] pci: Handle MIPS systems with virtual CONFIG_SYS_SDRAM_BASE

[Paul Burton]

On Tuesday, 27 September 2016 12:01:32 BST Daniel Schwierzeck wrote:
> 2016-09-27 2:35 GMT+02:00 Simon Glass <sjg at chromium.org>:
> > Hi Paul,
> > 
> > On 26 September 2016 at 12:29, Paul Burton <paul.burton at imgtec.com> wrote:
> >> The decode_regions() function in the PCI code presumes that
> >> CONFIG_SYS_SDRAM_BASE is a physical address, which seems reasonable
> >> given that README states that it should be.
> >> 
> >> However there is also common code which expects CONFIG_SYS_SDRAM_BASE to
> >> be an address accessible by the CPU, ie. a valid virtual address -
> >> notably gd->ram_top is set to it & various pieces of data are located
> >> relative to that, and getenv_bootm_low() defaults to
> >> CONFIG_SYS_SDRAM_BASE as the lower bound on addresses to load into. Thus
> >> on MIPS CONFIG_SYS_SDRAM_BASE is a virtual address.
> >> 
> >> This patch takes the simple approach to fixing this & converts
> >> CONFIG_SYS_SDRAM_BASE to a physical address for use by the PCI code when
> >> built for MIPS.
> >> 
> >> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
> >> 
> >> ---
> >> 
> >>  drivers/pci/pci-uclass.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> >> index 415c632..26fe0f4 100644
> >> --- a/drivers/pci/pci-uclass.c
> >> +++ b/drivers/pci/pci-uclass.c
> >> @@ -848,7 +848,9 @@ static int decode_regions(struct pci_controller
> >> *hose, const void *blob,>> 
> >>         /* Add a region for our local memory */
> >>         size = gd->ram_size;
> >> 
> >> -#ifdef CONFIG_SYS_SDRAM_BASE
> >> +#if defined(CONFIG_MIPS)
> >> +       base = virt_to_phys((void *)CONFIG_SYS_SDRAM_BASE);
> >> +#elif defined(CONFIG_SYS_SDRAM_BASE)
> >> 
> >>         base = CONFIG_SYS_SDRAM_BASE;
> >>  
> >>  #endif
> >>  
> >>         if (gd->pci_ram_top && gd->pci_ram_top < base + size)
> >> 
> >> --
> >> 2.10.0
> > 
> > Can you find another way? We don't want arch- or board-specific
> > #ifdefs in uclasses or driver code.
> 
> I suggest that we fix all MIPS boards to configure
> CONFIG_SYS_SDRAM_BASE as physical address. I guess that would be
> simply 0 for all current supported MIPS boards. In "arch/mips/" there
> are only two users ("arch/mips/cpu/start.S" and
> "arch/mips/lib/bootm.c") which could be easily fixed to map
> CONFIG_SYS_SDRAM_BASE to KSEG0.

Hi Daniel & Simon,

So doing that led to the 27 patch series I just sent out. It's never simple :) It's not so much 
the 2 users in arch/mips/ as the generic uses which cause the trouble - and avoiding them 
needs something like phys_to_virt() so the bulk of that series is about providing that for all 
architectures.

FYI CONFIG_SYS_SDRAM_BASE is zero for all MIPS boards except the PIC32 one. I've tested 
the series on a 32 bit Malta & a 64 bit boston, and build tested a bunch of other boards. The 
only thing to really take note of is that for MIPS64 systems using a physical 
CONFIG_SYS_SDRAM_BASE and phys_to_virt we end up accessing memory through xkphys 
instead of ckseg0. Nothing seems to have exploded too badly from that (just the boston 
board_get_usable_ram_top() needed adjusting) but it's probably worth pointing out.

Thanks,
    Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161001/4229fa03/attachment.sig>