[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-02-07 at 00:28 +0100, Alexander Graf wrote:
> On 06.02.2014, at 23:55, Scott Wood <scottwood at freescale.com> wrote:
> 
> > On Thu, 2014-02-06 at 13:48 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 03:19, Scott Wood <scottwood at freescale.com> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> +void pci_init_board(void)
> >>>> +{
> >>>> +	struct fsl_pci_info pci_info;
> >>>> +	const void *fdt = get_fdt();
> >>>> +	int pci_node;
> >>>> +
> >>>> +	puts("\n");
> >>>> +
> >>>> +	pci_node = fdt_path_offset(fdt, "/pci");
> >>>> +	if (pci_node < 0) {
> >>>> +		printf("PCI: disabled\n\n");
> >>>> +		return;
> >>>> +	}
> >>>> +
> >>>> +	SET_STD_PCI_INFO(pci_info, 1);
> >>>> +
> >>>> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
> >>>> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> >>>> +		pci_info.regs);
> >>> 
> >>> Why hardcode these things in a message?  Just don't print anything if
> >>> you don't have the info.
> >> 
> >> To make it look more akin to a real e500 board. But I'll change it.
> > 
> > It's a paravirt target...
> 
> So? It should still look "normal" to someone who runs it.

Not to the extent of making up info that isn't there.

> >>> How do you know you're not creating an overlapping TLB entry?  You
> >>> should map this to a fixed virtual address that you know is safe.
> >> 
> >> Ok. I'll map it behind CCSRBAR.
> > 
> > I'd give it its own CONFIG symbol to be explicitly located in the header
> > file.
> 
> It's only a temporary map that is alive for a few dozen instructions. I
> think a new CONFIG symbol is excessive here :).

It's address space that should be visible in the map to avoid potential
conflicts.  Whether you want to call it something with FDT in the name,
or call it something that indicates a multi-purpose temporary mapping
location, is up to you.

-Scott