[U-Boot] [PATCH v3 2/2] mpc85xx: support for Freescale COM Express P2020

[Ira W. Snyder]

On Sat, Nov 12, 2011 at 12:16:05PM -0600, Kumar Gala wrote:
> > 
> > This entire file is identical to board/freescale/p1_p2_rdb/ddr.c. In
> > fact, since this board only boots via the On-Chip ROM, the whole file is
> > useless: fixed_sdram() should just return the RAM size. We're running
> > from RAM when this function executes.
> > 
> > Is it ok with you if I replace the entire file with the following?
> > 
> > phys_size_t fixed_sdram(void)
> > {
> > 	return CONFIG_SYS_SDRAM_SIZE << 20;
> > }
> 
> If the board has SO-DIMMs than I'd expect SPD support.  Sounds like you're working on this w/Matt & York.
> 

I don't need fixed_sdram() anymore. I figured out how to get L2 SRAM
boot working this week. Also, I finally tracked down a bug in the
Freescale DDR SPD code, exposed due to invalid data in the SPD. Darn
hardware manufacturers!

> >>> diff --git a/board/freescale/p2020come/law.c b/board/freescale/p2020come/law.c
> >>> new file mode 100644
> >>> index 0000000..56508db
> >>> --- /dev/null
> >>> +++ b/board/freescale/p2020come/law.c
> >>> @@ -0,0 +1,36 @@
> >>> +/*
> >>> + * Copyright 2009 Freescale Semiconductor, Inc.
> >>> + *
> >>> + * See file CREDITS for list of people who contributed to this
> >>> + * project.
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU General Public License as
> >>> + * published by the Free Software Foundation; either version 2 of
> >>> + * the License, or (at your option) any later version.
> >>> + *
> >>> + * This program is distributed in the hope that it will be useful,
> >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the
> >>> + * GNU General Public License for more details.
> >>> + *
> >>> + * You should have received a copy of the GNU General Public License
> >>> + * along with this program; if not, write to the Free Software
> >>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> >>> + * MA 02111-1307 USA
> >>> + */
> >>> +
> >>> +#include <common.h>
> >>> +#include <asm/fsl_law.h>
> >>> +#include <asm/mmu.h>
> >>> +
> >>> +struct law_entry law_table[] = {
> >>> +	SET_LAW(CONFIG_SYS_PCIE1_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCIE_1),
> >>> +	SET_LAW(CONFIG_SYS_PCIE1_IO_PHYS, LAW_SIZE_64K, LAW_TRGT_IF_PCIE_1),
> >>> +	SET_LAW(CONFIG_SYS_PCIE2_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCIE_2),
> >>> +	SET_LAW(CONFIG_SYS_PCIE2_IO_PHYS, LAW_SIZE_64K, LAW_TRGT_IF_PCIE_2),
> >>> +	SET_LAW(CONFIG_SYS_PCIE3_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCIE_3),
> >>> +	SET_LAW(CONFIG_SYS_PCIE3_IO_PHYS, LAW_SIZE_64K, LAW_TRGT_IF_PCIE_3),
> >> 
> >> We normally set these up dynamically.
> >> 
> > 
> > This is a modified version of the code from
> > board/freescale/p1_p2_rdb/law.c. Can you suggest an in tree example of
> > the way you'd like the code to look? I copied what I assume is a good
> > example…
> 
> If you look at current board/freescale/p1_p2_rdb/law.c it doesn't have PCI LAWs anymore.  I think for your example you just need an empty data structure:
> 
> 	struct law_entry law_table[] = {
> 	};
> 
> this should hopefully make num_law_entries = 0;
> 
> 
> > 
> >>> +};
> >>> +
> >>> +int num_law_entries = ARRAY_SIZE(law_table);

Unfortunately, having a law_table with no entries causes a bug. A nasty
bug which was hard to track down.

When law_table is empty, and when num_law_entries = 0, both variables
exist in BSS only.

Both the law_table and num_law_entries are used in init_laws(), called
from cpu_init_early_f(). This happens before BSS is initialized. Also
before DDR is initialized.

And now you see the bug. The U-Boot hangs due to an invalid memory
access. This is before the console is initialized, making it hard to
track down.

How do you suggest I work around this? A single redundant entry in the
law_table works (the system boots).

Thanks,
Ira