[U-Boot] [PATCH] powerpc: add support for the Freescale P1022DS reference board

Add basic suport for the Freescale P1022DS reference board.
Signed-off-by: Timur Tabi timur@freescale.com ---
This patch requires the following two patches to be applied first:
fsl/85xx: add clkdvdr and pmuxcr2 to global utilities structure definition fsl: rename 'dma' to 'brdcfg1' in the ngPIXIS structure
Makefile | 3 + arch/powerpc/cpu/mpc8xxx/pci_cfg.c | 26 ++ board/freescale/p1022ds/Makefile | 40 +++ board/freescale/p1022ds/config.mk | 14 + board/freescale/p1022ds/ddr.c | 108 +++++++ board/freescale/p1022ds/law.c | 27 ++ board/freescale/p1022ds/p1022ds.c | 369 ++++++++++++++++++++++++ board/freescale/p1022ds/p1022ds_diu.c | 151 ++++++++++ board/freescale/p1022ds/tlb.c | 76 +++++ include/configs/P1022DS.h | 505 +++++++++++++++++++++++++++++++++ 10 files changed, 1319 insertions(+), 0 deletions(-) create mode 100644 board/freescale/p1022ds/Makefile create mode 100644 board/freescale/p1022ds/config.mk create mode 100644 board/freescale/p1022ds/ddr.c create mode 100644 board/freescale/p1022ds/law.c create mode 100644 board/freescale/p1022ds/p1022ds.c create mode 100644 board/freescale/p1022ds/p1022ds_diu.c create mode 100644 board/freescale/p1022ds/tlb.c create mode 100644 include/configs/P1022DS.h
diff --git a/Makefile b/Makefile index 1445e8b..583a576 100644 --- a/Makefile +++ b/Makefile @@ -2493,6 +2493,9 @@ P2020DS_36BIT_config \ P2020DS_config: unconfig @$(MKCONFIG) -t $(@:_config=) P2020DS powerpc mpc85xx p2020ds freescale
+P1022DS_config: unconfig + @$(MKCONFIG) -t $(@:_config=) P1022DS powerpc mpc85xx p1022ds freescale + P1011RDB_config \ P1011RDB_NAND_config \ P1011RDB_SDCARD_config \ diff --git a/arch/powerpc/cpu/mpc8xxx/pci_cfg.c b/arch/powerpc/cpu/mpc8xxx/pci_cfg.c index 85995ca..fc79fe1 100644 --- a/arch/powerpc/cpu/mpc8xxx/pci_cfg.c +++ b/arch/powerpc/cpu/mpc8xxx/pci_cfg.c @@ -186,6 +186,32 @@ static struct pci_info pci_config_info[] = (1 << 0x16) | (1 << 0x17) | (1 << 0x18) | (1 << 0x1c), }, }; +#elif defined(CONFIG_P1022) +static struct pci_info pci_config_info[] = +{ + [LAW_TRGT_IF_PCIE_1] = { + .agent = (1 << 0) | (1 << 1), + .cfg = (1 << 6) | (1 << 7) | (1 << 9) | (1 << 0xa) | + (1 << 0xb) | (1 << 0xd) | (1 << 0xe) | + (1 << 0xf) | (1 << 0x15) | (1 << 0x16) | + (1 << 0x17) | (1 << 0x18) | (1 << 0x19) | + (1 << 0x1a) | (1 << 0x1b) | (1 << 0x1c) | + (1 << 0x1d) | (1 << 0x1e) | (1 << 0x1f), + }, + [LAW_TRGT_IF_PCIE_2] = { + .agent = (1 << 0) | (1 << 2), + .cfg = (1 << 0) | (1 << 1) | (1 << 6) | (1 << 7) | + (1 << 9) | (1 << 0xa) | (1 << 0xb) | (1 << 0xd) | + (1 << 0x15) | (1 << 0x16) | (1 << 0x17) | + (1 << 0x18) | (1 << 0x1c), + }, + [LAW_TRGT_IF_PCIE_3] = { + .agent = (1 << 0) | (1 << 3), + .cfg = (1 << 6) | (1 << 7) | (1 << 9) | (1 << 0xd) | + (1 << 0x15) | (1 << 0x16) | (1 << 0x17) | (1 << 0x18) | + (1 << 0x19) | (1 << 0x1a) | (1 << 0x1b), + }, +}; #elif defined(CONFIG_P2010) || defined(CONFIG_P2020) static struct pci_info pci_config_info[] = { diff --git a/board/freescale/p1022ds/Makefile b/board/freescale/p1022ds/Makefile new file mode 100644 index 0000000..4596c35 --- /dev/null +++ b/board/freescale/p1022ds/Makefile @@ -0,0 +1,40 @@ +# +# Copyright 2010 Freescale Semiconductor, Inc. +# +# 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. +# + +include $(TOPDIR)/config.mk + +LIB = $(obj)lib$(BOARD).a + +COBJS-y += $(BOARD).o +COBJS-y += ddr.o +COBJS-y += law.o +COBJS-y += tlb.o +COBJS-${CONFIG_FSL_DIU_FB} += p1022ds_diu.o + +SRCS := $(SOBJS:.o=.S) $(COBJS-y:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS-y)) +SOBJS := $(addprefix $(obj),$(SOBJS)) + +$(LIB): $(obj).depend $(OBJS) $(SOBJS) + $(AR) $(ARFLAGS) $@ $(OBJS) + +clean: + rm -f $(OBJS) $(SOBJS) + +distclean: clean + rm -f $(LIB) core *.bak .depend + +######################################################################### + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +######################################################################### diff --git a/board/freescale/p1022ds/config.mk b/board/freescale/p1022ds/config.mk new file mode 100644 index 0000000..4581d20 --- /dev/null +++ b/board/freescale/p1022ds/config.mk @@ -0,0 +1,14 @@ +# +# Copyright 2010 Freescale Semiconductor, Inc. +# +# 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. +# + +ifndef TEXT_BASE +TEXT_BASE = 0xeff80000 +endif + +RESET_VECTOR_ADDRESS = 0xeffffffc diff --git a/board/freescale/p1022ds/ddr.c b/board/freescale/p1022ds/ddr.c new file mode 100644 index 0000000..c43c902 --- /dev/null +++ b/board/freescale/p1022ds/ddr.c @@ -0,0 +1,108 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * Authors: Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#include <common.h> +#include <i2c.h> + +#include <asm/fsl_ddr_sdram.h> +#include <asm/fsl_ddr_dimm_params.h> + +unsigned int fsl_ddr_get_mem_data_rate(void) +{ + return get_ddr_freq(0); +} + +void fsl_ddr_get_spd(ddr3_spd_eeprom_t *ctrl_dimms_spd, unsigned int ctrl_num) +{ + int ret; + + /* The P1022 has only one DDR controller, and the board has only one + DIMM slot. */ + ret = i2c_read(SPD_EEPROM_ADDRESS1, 0, 1, (u8 *)ctrl_dimms_spd, + sizeof(ddr3_spd_eeprom_t)); + if (ret) { + debug("DDR: failed to read SPD from address %u\n", i2c_address); + memset(ctrl_dimms_spd, 0, sizeof(ddr3_spd_eeprom_t)); + } +} + +typedef struct { + u32 datarate_mhz_low; + u32 datarate_mhz_high; + u32 n_ranks; + u32 clk_adjust; + u32 cpo; + u32 write_data_delay; + u32 force_2T; +} board_specific_parameters_t; + +/* ranges for parameters: + * wr_data_delay = 0-6 + * clk adjust = 0-8 + * cpo 2-0x1E (30) + */ + +static const board_specific_parameters_t bsp[] = { +/* memory controller 0 */ +/* lo| hi| num| clk| cpo|wrdata|2T */ +/* mhz| mhz|ranks|adjst| | delay| */ + { 0, 333, 1, 5, 31, 3, 0}, + {334, 400, 1, 5, 31, 3, 0}, + {401, 549, 1, 5, 31, 3, 0}, + {550, 680, 1, 5, 31, 5, 0}, + {681, 850, 1, 5, 31, 5, 0}, + { 0, 333, 2, 5, 31, 3, 0}, + {334, 400, 2, 5, 31, 3, 0}, + {401, 549, 2, 5, 31, 3, 0}, + {550, 680, 2, 5, 31, 5, 0}, + {681, 850, 2, 5, 31, 5, 0}, +}; + +void fsl_ddr_board_options(memctl_options_t *popts, dimm_params_t *pdimm, + unsigned int ctrl_num) +{ + unsigned long ddr_freq; + unsigned int i; + + /* set odt_rd_cfg and odt_wr_cfg. */ + for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) { + popts->cs_local_opts[i].odt_rd_cfg = 0; + popts->cs_local_opts[i].odt_wr_cfg = 1; + } + + /* Get clk_adjust, cpo, write_data_delay,2T, according to the board ddr + * freqency and n_banks specified in board_specific_parameters table. + */ + ddr_freq = get_ddr_freq(0) / 1000000; + for (i = 0; i < ARRAY_SIZE(bsp); i++) { + if (ddr_freq >= bsp[i].datarate_mhz_low && + ddr_freq <= bsp[i].datarate_mhz_high && + pdimm->n_ranks == bsp[i].n_ranks) { + popts->clk_adjust = bsp[i].clk_adjust; + popts->cpo_override = bsp[i].cpo; + popts->write_data_delay = bsp[i].write_data_delay; + popts->twoT_en = bsp[i].force_2T; + break; + } + } + + popts->half_strength_driver_enable = 1; + + /* Per AN4039, enable ZQ calibration. */ + popts->zq_en = 1; + + + /* + * For wake-up on ARP, we need auto self refresh enabled + */ + popts->auto_self_refresh_en = 1; + popts->sr_it = 0xb; +} diff --git a/board/freescale/p1022ds/law.c b/board/freescale/p1022ds/law.c new file mode 100644 index 0000000..f465635 --- /dev/null +++ b/board/freescale/p1022ds/law.c @@ -0,0 +1,27 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * Authors: Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#include <common.h> +#include <asm/fsl_law.h> +#include <asm/mmu.h> + +struct law_entry law_table[] = { + SET_LAW(CONFIG_SYS_FLASH_BASE_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_LBC), + 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), + SET_LAW(PIXIS_BASE_PHYS, LAW_SIZE_4K, LAW_TRGT_IF_LBC), +}; + +int num_law_entries = ARRAY_SIZE(law_table); diff --git a/board/freescale/p1022ds/p1022ds.c b/board/freescale/p1022ds/p1022ds.c new file mode 100644 index 0000000..083d195 --- /dev/null +++ b/board/freescale/p1022ds/p1022ds.c @@ -0,0 +1,369 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * Authors: Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#include <common.h> +#include <command.h> +#include <pci.h> +#include <asm/processor.h> +#include <asm/mmu.h> +#include <asm/cache.h> +#include <asm/immap_85xx.h> +#include <asm/fsl_pci.h> +#include <asm/fsl_ddr_sdram.h> +#include <asm/io.h> +#include <libfdt.h> +#include <fdt_support.h> +#include <tsec.h> +#include <asm/fsl_law.h> +#include <asm/mp.h> +#include <netdev.h> +#include <i2c.h> + +#include "../common/ngpixis.h" + +DECLARE_GLOBAL_DATA_PTR; + +int board_early_init_f(void) +{ + ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR; + + /* Set pmuxcr to allow both i2c1 and i2c2 */ + setbits_be32(&gur->pmuxcr, 0x1000); + in_be32(&gur->pmuxcr); + + /* Set muxing for TSEC2 + * Clear bits 11:16 in PMUXCR2 to enable etsec2 + */ + clrbits_be32(&gur->pmuxcr2, 0x1F8000); + + return 0; +} + +int checkboard(void) +{ + u8 sw; + + puts("Board: P1022DS "); +#ifdef CONFIG_PHYS_64BIT + puts("(36-bit addrmap) "); +#endif + + printf("Sys ID: 0x%02x, Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", + in_8(&pixis->id), in_8(&pixis->arch), in_8(&pixis->scver)); + + sw = in_8(&PIXIS_SW(PIXIS_LBMAP_SWITCH)); + + switch ((sw & PIXIS_LBMAP_MASK) >> PIXIS_LBMAP_SHIFT) { + case 0: + case 1: + printf ("vBank: %u\n", ((sw & 0x30) >> 4)); + break; + case 2: + case 3: + puts ("Promjet\n"); + break; + } + + return 0; +} + +phys_size_t initdram(int board_type) +{ + phys_size_t dram_size = 0; + + puts("Initializing....\n"); + + dram_size = fsl_ddr_sdram(); + dram_size = setup_ddr_tlbs(dram_size / 0x100000); + dram_size *= 0x100000; + + puts(" DDR: "); + return dram_size; +} + +int misc_init_r(void) +{ + u8 temp; + + /* Enable the TFP410 Encoder (I2C address 0x38) + */ + + temp = 0xBF; + if (i2c_write(0x38, 0x08, 1, &temp, sizeof(temp)) < 0) + return -1; + + /* Verify if enabled */ + temp = 0; + if (i2c_read(0x38, 0x08, 1, &temp, sizeof(temp)) < 0) + return -1; + + debug("DVI Encoder Read: 0x%02lx\n", temp); + + temp = 0x10; + if (i2c_write(0x38, 0x0A, 1, &temp, sizeof(temp)) < 0) + return -1; + + /* Verify if enabled */ + temp = 0; + if (i2c_read(0x38, 0x0A, 1, &temp, sizeof(temp)) < 0) + return -1; + + debug("DVI Encoder Read: 0x%02lx\n",temp); + + return 0; +} + + +#ifdef CONFIG_PCIE1 +static struct pci_controller pcie1_hose; +#endif + +#ifdef CONFIG_PCIE2 +static struct pci_controller pcie2_hose; +#endif + +#ifdef CONFIG_PCIE3 +static struct pci_controller pcie3_hose; +#endif + +#ifdef CONFIG_PCI +void pci_init_board(void) +{ + volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); + struct fsl_pci_info pci_info[3]; + u32 devdisr, pordevsr, io_sel; + int first_free_busno = 0; + int num = 0; + + int pcie_ep, pcie_configured; + + devdisr = in_be32(&gur->devdisr); + pordevsr = in_be32(&gur->pordevsr); + io_sel = (pordevsr & MPC85xx_PORDEVSR_IO_SEL) >> 18; + + debug (" pci_init_board: devdisr=%x, io_sel=%x\n", devdisr, io_sel); + + if (!(pordevsr & MPC85xx_PORDEVSR_SGMII2_DIS)) + printf(" eTSEC2 is in sgmii mode.\n"); + if (!(pordevsr & MPC85xx_PORDEVSR_SGMII3_DIS)) + printf(" eTSEC3 is in sgmii mode.\n"); + + puts("\n"); + +#ifdef CONFIG_PCIE2 + pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_2, io_sel); + + if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE2)) { + SET_STD_PCIE_INFO(pci_info[num], 2); + pcie_ep = fsl_setup_hose(&pcie2_hose, pci_info[num].regs); + printf(" PCIE2 connected to Slot 3 as %s (base addr %lx)\n", + pcie_ep ? "Endpoint" : "Root Complex", + pci_info[num].regs); + first_free_busno = fsl_pci_init_port(&pci_info[num++], + &pcie2_hose, first_free_busno); + + } else { + printf(" PCIE2: disabled\n"); + } + puts("\n"); +#else + setbits_be32(&gur->devdisr, MPC85xx_DEVDISR_PCIE2); /* disable */ +#endif + +#ifdef CONFIG_PCIE3 + pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_3, io_sel); + + if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE3)) { + SET_STD_PCIE_INFO(pci_info[num], 3); + pcie_ep = fsl_setup_hose(&pcie3_hose, pci_info[num].regs); + printf(" PCIE3 connected to Slot 2 as %s (base addr %lx)\n", + pcie_ep ? "Endpoint" : "Root Complex", + pci_info[num].regs); + first_free_busno = fsl_pci_init_port(&pci_info[num++], + &pcie3_hose, first_free_busno); + } else { + printf(" PCIE3: disabled\n"); + } + puts("\n"); +#else + setbits_be32(&gur->devdisr, MPC85xx_DEVDISR_PCIE3); /* disable */ +#endif + +#ifdef CONFIG_PCIE1 + pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_1, io_sel); + + if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE)) { + SET_STD_PCIE_INFO(pci_info[num], 1); + pcie_ep = fsl_setup_hose(&pcie1_hose, pci_info[num].regs); + printf(" PCIE1 connected to Slot 1 as %s (base addr %lx)\n", + pcie_ep ? "Endpoint" : "Root Complex", + pci_info[num].regs); + first_free_busno = fsl_pci_init_port(&pci_info[num++], + &pcie1_hose, first_free_busno); + } else { + printf(" PCIE1: disabled\n"); + } + puts("\n"); +#else + setbits_be32(&gur->devdisr, MPC85xx_DEVDISR_PCIE); /* disable */ +#endif +} +#endif + +int board_early_init_r(void) +{ + const unsigned int flashbase = CONFIG_SYS_FLASH_BASE; + const u8 flash_esel = find_tlb_idx((void *)flashbase, 1); + + /* + * Remap Boot flash + PROMJET region to caching-inhibited + * so that flash can be erased properly. + */ + + /* Flush d-cache and invalidate i-cache of any FLASH data */ + flush_dcache(); + invalidate_icache(); + + /* invalidate existing TLB entry for flash + promjet */ + disable_tlb(flash_esel); + + set_tlb(1, flashbase, CONFIG_SYS_FLASH_BASE_PHYS, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, flash_esel, BOOKE_PAGESZ_256M, 1); + + return 0; +} + +#ifdef CONFIG_GET_CLK_FROM_ICS307 + +/* decode S[0-2] to Output Divider (OD) */ +static u8 ics307_S_to_OD[] = { + 10, 2, 8, 4, 5, 7, 3, 6 +}; + +/* Calculate frequency being generated by ICS307-02 clock chip based upon + * the control bytes being programmed into it. */ +/* XXX: This function should probably go into a common library */ +static unsigned long ics307_clk_freq(unsigned char cw0, unsigned char cw1, + unsigned char cw2) +{ + const unsigned long InputFrequency = CONFIG_ICS307_REFCLK_HZ; + unsigned long VDW = ((cw1 << 1) & 0x1FE) + ((cw2 >> 7) & 1); + unsigned long RDW = cw2 & 0x7F; + unsigned long OD = ics307_S_to_OD[cw0 & 0x7]; + unsigned long freq; + + /* CLK1Frequency = InputFrequency * 2 * (VDW + 8) / ((RDW + 2) * OD) */ + + /* cw0: C1 C0 TTL F1 F0 S2 S1 S0 + * cw1: V8 V7 V6 V5 V4 V3 V2 V1 + * cw2: V0 R6 R5 R4 R3 R2 R1 R0 + * + * R6:R0 = Reference Divider Word (RDW) + * V8:V0 = VCO Divider Word (VDW) + * S2:S0 = Output Divider Select (OD) + * F1:F0 = Function of CLK2 Output + * TTL = duty cycle + * C1:C0 = internal load capacitance for cyrstal + */ + + /* Adding 1 to get a "nicely" rounded number, but this needs + * more tweaking to get a "properly" rounded number. */ + + freq = 1 + (InputFrequency * 2 * (VDW + 8) / ((RDW + 2) * OD)); + + debug("ICS307: CW[0-2]: %02X %02X %02X => %lu Hz\n", cw0, cw1, cw2, + freq); + return freq; +} + +unsigned long calculate_board_sys_clk(void) +{ + ulong val; + + val = ics307_clk_freq(in_8(&pixis->sclk[0]), in_8(&pixis->sclk[1]), + in_8(&pixis->sclk[2])); + debug("sysclk val = %lu\n", val); + return val; +} + +unsigned long calculate_board_ddr_clk(void) +{ + ulong val; + + val = ics307_clk_freq(in_8(&pixis->dclk[0]), in_8(&pixis->dclk[1]), + in_8(&pixis->dclk[2])); + debug("ddrclk val = %lu\n", val); + return val; +} + +#endif + +/* #ifdef CONFIG_TSEC_ENET */ +int board_eth_init(bd_t *bis) +{ + struct tsec_info_struct tsec_info[4]; + int num = 0; + +#ifdef CONFIG_TSEC1 + SET_STD_TSEC_INFO(tsec_info[num], 1); + num++; +#endif +#ifdef CONFIG_TSEC2 + SET_STD_TSEC_INFO(tsec_info[num], 2); + num++; +#endif + if (!num) { + printf("No TSECs initialized\n"); + + return 0; + } + + tsec_eth_init(bis, tsec_info, num); + + return pci_eth_init(bis); +} +/* #endif */ + +#if defined(CONFIG_OF_BOARD_SETUP) +void ft_board_setup(void *blob, bd_t *bd) +{ + phys_addr_t base; + phys_size_t size; + + ft_cpu_setup(blob, bd); + + base = getenv_bootm_low(); + size = getenv_bootm_size(); + + fdt_fixup_memory(blob, (u64)base, (u64)size); + +#ifdef CONFIG_PCIE3 + ft_fsl_pci_setup(blob, "pci2", &pcie3_hose); +#endif +#ifdef CONFIG_PCIE2 + ft_fsl_pci_setup(blob, "pci0", &pcie2_hose); +#endif +#ifdef CONFIG_PCIE1 + ft_fsl_pci_setup(blob, "pci1", &pcie1_hose); +#endif +#ifdef CONFIG_FSL_SGMII_RISER + fsl_sgmii_riser_fdt_fixup(blob); +#endif +} +#endif + +#ifdef CONFIG_MP +void board_lmb_reserve(struct lmb *lmb) +{ + cpu_mp_lmb_reserve(lmb); +} +#endif diff --git a/board/freescale/p1022ds/p1022ds_diu.c b/board/freescale/p1022ds/p1022ds_diu.c new file mode 100644 index 0000000..ae00184 --- /dev/null +++ b/board/freescale/p1022ds/p1022ds_diu.c @@ -0,0 +1,151 @@ +/* + * Copyright 2007-2010 Freescale Semiconductor, Inc. + * Authors: York Sun yorksun@freescale.com + * Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#include <common.h> +#include <command.h> +#include <asm/io.h> + +#include "../common/ngpixis.h" +#include "../common/fsl_diu_fb.h" + +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE) +#include <stdio_dev.h> +#include <video_fb.h> +#endif + +extern unsigned int FSL_Logo_BMP[]; + +static unsigned int xres, yres; + +void diu_set_pixel_clock(unsigned int pixclock) +{ + ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR; + unsigned long speed_ccb, temp, pixval; + + speed_ccb = get_bus_freq(0); + temp = 1000000000UL / pixclock; + temp *= 1000; + pixval = speed_ccb / temp; + debug("DIU pixval = %lu\n", pixval); + + /* Modify PXCLK in GUTS CLKDVDR */ + debug("DIU: Current value of CLKDVDR = 0x%08x\n", in_be32(&gur->clkdvdr)); + clrbits_be32(&gur->clkdvdr, 0xdfff0000); /* turn off clock */ + setbits_be32(&gur->clkdvdr, 0x80000000 | ((pixval & 0x1F) << 16)) + debug("DIU: Modified value of CLKDVDR = 0x%08x\n", in_be32(&gur->clkdvdr)); +} + +void p1022ds_diu_init(void) +{ + char *monitor_port; + volatile ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR; + + monitor_port = getenv("monitor"); + + if (!strcmp(monitor_port, "1")) { /* 1 - LVDS */ + xres = 1024; + yres = 768; + clrsetbits_8(&pixis->brdcfg1, 0x08, 0x40 | 0x20); + } else { /* 0, or uninitialized env var */ + xres = 1280; + yres = 1024; + setbits_8(&pixis->brdcfg1, 0x80); + } + + /* Set BRDCFG0[ELBC_DIU] */ + clrsetbits_8(&pixis->brdcfg0, 0xC2, 2); + /* we must do the dummy read from eLBC to sync the write as above */ + in_8(&pixis->brdcfg0); + + /* Setting PMUXCR to switch to DVI from ELBC */ + /* Set pmuxcr to allow both i2c1 and i2c2 */ + clrsetbits_be32(&gur->pmuxcr, 0xc0000000, 0x40000000); + in_be32(&gur->pmuxcr); + + fsl_diu_init(xres, 0x88883316, 0, (unsigned char *)FSL_Logo_BMP); +} + +int p1022diu_init_show_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned int addr; + + if (argc < 2) { + cmd_usage(cmdtp); + return 1; + } + + if (!strncmp(argv[1], "init",4)) { +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE) + fsl_diu_clear_screen(); + drv_video_init(); +#else + p1022ds_diu_init(); +#endif + } else { + addr = simple_strtoul(argv[1], NULL, 16); + fsl_diu_clear_screen(); + fsl_diu_display_bmp((unsigned char *)addr, 0, 0, 0); + } + + return 0; +} + +U_BOOT_CMD( + diufb, CONFIG_SYS_MAXARGS, 1, p1022diu_init_show_bmp, + "Init or Display BMP file", + "init\n - initialize DIU\n" + "addr\n - display bmp at address 'addr'" +); + + +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE) + +/* + * The Graphic Device + */ +static GraphicDevice *video_hw_init(void) +{ + static GraphicDevice ctfb; + struct fb_info *info; + + p1022ds_diu_init(); + + /* fill in Graphic device struct */ + sprintf(pGD->modeIdent, "%dx%dx%d %ldkHz %ldHz", xres, yres, 32, 64, 60); + + ctfb.frameAdrs = (unsigned int)fsl_fb_open(&info); + ctfb.winSizeX = xres; + ctfb.winSizeY = yres - info->logo_height; + ctfb.plnSizeX = ctfb.winSizeX; + ctfb.plnSizeY = ctfb.winSizeY; + + ctfb.gdfBytesPP = 4; + ctfb.gdfIndex = GDF_32BIT_X888RGB; + + ctfb.isaBase = 0; + ctfb.pciBase = 0; + ctfb.memSize = info->screen_size - info->logo_size; + + /* Cursor Start Address */ + ctfb.dprBase = 0; + ctfb.vprBase = 0; + ctfb.cprBase = 0; + + return &pGD; +} + +void video_set_lut(unsigned int index, unsigned char r, unsigned char g, + unsigned char b) +{ +} + +#endif diff --git a/board/freescale/p1022ds/tlb.c b/board/freescale/p1022ds/tlb.c new file mode 100644 index 0000000..e620112 --- /dev/null +++ b/board/freescale/p1022ds/tlb.c @@ -0,0 +1,76 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * Authors: Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#include <common.h> +#include <asm/mmu.h> + +struct fsl_e_tlb_entry tlb_table[] = { + /* TLB 0 - for temp stack in cache */ + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR, CONFIG_SYS_INIT_RAM_ADDR, + MAS3_SX|MAS3_SW|MAS3_SR, 0, + 0, 0, BOOKE_PAGESZ_4K, 0), + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024, + CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024, + MAS3_SX|MAS3_SW|MAS3_SR, 0, + 0, 0, BOOKE_PAGESZ_4K, 0), + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024, + CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024, + MAS3_SX|MAS3_SW|MAS3_SR, 0, + 0, 0, BOOKE_PAGESZ_4K, 0), + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024, + CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024, + MAS3_SX|MAS3_SW|MAS3_SR, 0, + 0, 0, BOOKE_PAGESZ_4K, 0), + + /* TLB 1 */ + /* *I*** - Covers boot page */ + SET_TLB_ENTRY(1, 0xfffff000, 0xfffff000, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I, + 0, 0, BOOKE_PAGESZ_4K, 1), + + /* *I*G* - CCSRBAR */ + SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 1, BOOKE_PAGESZ_1M, 1), + + /* W**G* - Flash/promjet, localbus */ + /* This will be changed to *I*G* after relocation to RAM. */ + SET_TLB_ENTRY(1, CONFIG_SYS_FLASH_BASE, CONFIG_SYS_FLASH_BASE_PHYS, + MAS3_SX|MAS3_SR, MAS2_W|MAS2_G, + 0, 2, BOOKE_PAGESZ_256M, 1), + + /* *I*G* - PCI */ + SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT, CONFIG_SYS_PCIE3_MEM_PHYS, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 3, BOOKE_PAGESZ_1G, 1), + + /* *I*G* - PCI */ + SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT + 0x40000000, + CONFIG_SYS_PCIE3_MEM_PHYS + 0x40000000, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 4, BOOKE_PAGESZ_256M, 1), + + SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT + 0x50000000, + CONFIG_SYS_PCIE3_MEM_PHYS + 0x50000000, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 5, BOOKE_PAGESZ_256M, 1), + + /* *I*G* - PCI I/O */ + SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_IO_VIRT, CONFIG_SYS_PCIE3_IO_PHYS, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 6, BOOKE_PAGESZ_256K, 1), + + SET_TLB_ENTRY(1, PIXIS_BASE, PIXIS_BASE_PHYS, + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, 7, BOOKE_PAGESZ_4K, 1), +}; + +int num_tlb_entries = ARRAY_SIZE(tlb_table); diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h new file mode 100644 index 0000000..65a1265 --- /dev/null +++ b/include/configs/P1022DS.h @@ -0,0 +1,505 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * Authors: Srikanth Srinivasan srikanth.srinivasan@freescale.com + * Timur Tabi timur@freescale.com + * + * 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. + */ + +#ifndef __CONFIG_H +#define __CONFIG_H + +/* High Level Configuration Options */ +#define CONFIG_BOOKE /* BOOKE */ +#define CONFIG_E500 /* BOOKE e500 family */ +#define CONFIG_MPC85xx /* MPC8540/60/55/41/48 */ +#define CONFIG_P1022 +#define CONFIG_P1022DS +#define CONFIG_MP /* support multiple processors */ + +#define CONFIG_FSL_ELBC /* Has Enhanced localbus controller */ +#define CONFIG_PCI /* Enable PCI/PCIE */ +#define CONFIG_PCIE1 /* PCIE controler 1 (slot 1) */ +#define CONFIG_PCIE2 /* PCIE controler 2 (slot 2) */ +#define CONFIG_PCIE3 /* PCIE controler 3 (ULI bridge) */ +#define CONFIG_FSL_PCI_INIT /* Use common FSL init code */ +#define CONFIG_FSL_PCIE_RESET /* need PCIe reset errata */ +#define CONFIG_SYS_PCI_64BIT /* enable 64-bit PCI resources */ + +#define CONFIG_PHYS_64BIT +#define CONFIG_ENABLE_36BIT_PHYS +#define CONFIG_ADDR_MAP +#define CONFIG_SYS_NUM_ADDR_MAP 16 /* number of TLB1 entries */ + +#define CONFIG_FSL_LAW /* Use common FSL init code */ + +#define CONFIG_TSEC_ENET /* tsec ethernet support */ +#define CONFIG_ENV_OVERWRITE + +#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif +#define CONFIG_SYS_CLK_FREQ calculate_board_sys_clk() /* sysclk for MPC85xx */ +#define CONFIG_DDR_CLK_FREQ calculate_board_ddr_clk() /* ddrclk for MPC85xx */ +#define CONFIG_ICS307_REFCLK_HZ 33333000 /* ICS307 clock chip ref freq */ +#define CONFIG_GET_CLK_FROM_ICS307 /* decode sysclk and ddrclk freq + from ICS307 instead of switches */ + +/* + * These can be toggled for performance analysis, otherwise use default. + */ +#define CONFIG_L2_CACHE /* toggle L2 cache */ +#define CONFIG_BTB /* toggle branch predition */ + +#define CONFIG_SYS_MEMTEST_START 0x00000000 /* memtest works on */ +#define CONFIG_SYS_MEMTEST_END 0x7fffffff + +/* + * Base addresses -- Note these are effective addresses where the + * actual resources get mapped (not physical addresses) + */ +#define CONFIG_SYS_CCSRBAR_DEFAULT 0xff700000 /* CCSRBAR Default */ +#define CONFIG_SYS_CCSRBAR 0xffe00000 /* relocated CCSRBAR */ +#define CONFIG_SYS_CCSRBAR_PHYS 0xfffe00000ull /* physical addr of CCSRBAR */ +#define CONFIG_SYS_IMMR CONFIG_SYS_CCSRBAR /* PQII uses CONFIG_SYS_IMMR */ + +#define CONFIG_SYS_PCIE3_ADDR (CONFIG_SYS_CCSRBAR+0xb000) +#define CONFIG_SYS_PCIE2_ADDR (CONFIG_SYS_CCSRBAR+0x9000) +#define CONFIG_SYS_PCIE1_ADDR (CONFIG_SYS_CCSRBAR+0xa000) + +/* DDR Setup */ +#define CONFIG_DDR_SPD +#define CONFIG_SYS_DDR_TLB_START 9 +#define CONFIG_VERY_BIG_RAM +#define CONFIG_FSL_DDR3 + +/* ECC will be enabled based on perf_mode environment variable */ +//#define CONFIG_DDR_ECC + +#define CONFIG_ECC_INIT_VIA_DDRCONTROLLER +#define CONFIG_MEM_INIT_VALUE 0xdeadbeef + +#define CONFIG_SYS_DDR_SDRAM_BASE 0x00000000 +#define CONFIG_SYS_SDRAM_BASE CONFIG_SYS_DDR_SDRAM_BASE + +#define CONFIG_NUM_DDR_CONTROLLERS 1 +#define CONFIG_DIMM_SLOTS_PER_CTLR 1 +#define CONFIG_CHIP_SELECTS_PER_CTRL (2 * CONFIG_DIMM_SLOTS_PER_CTLR) + +/* I2C addresses of SPD EEPROMs */ +#define CONFIG_SYS_SPD_BUS_NUM 1 /* SPD EEPROM located on I2C bus 1 */ +#define SPD_EEPROM_ADDRESS1 0x51 /* CTLR 0 DIMM 0 */ + +/* + * Memory map + * + * 0x0000_0000 0x7fff_ffff DDR 2G Cacheable + * 0x8000_0000 0xbfff_ffff PCI Express Mem 1G non-cacheable + * 0xc000_0000 0xdfff_ffff PCI 512M non-cacheable + * 0xe100_0000 0xe3ff_ffff PCI IO range 4M non-cacheable + * + * Localbus cacheable (TBD) + * 0xXXXX_XXXX 0xXXXX_XXXX SRAM YZ M Cacheable + * + * Localbus non-cacheable + * 0xe000_0000 0xe80f_ffff Promjet/free 128M non-cacheable + * 0xe800_0000 0xefff_ffff FLASH 128M non-cacheable + * 0xffa0_0000 0xffaf_ffff NAND 1M non-cacheable + * 0xffdf_0000 0xffdf_7fff PIXIS 32K non-cacheable TLB0 + * 0xffd0_0000 0xffd0_3fff L1 for stack 16K Cacheable TLB0 + * 0xffe0_0000 0xffef_ffff CCSR 1M non-cacheable + */ + +/* + * Local Bus Definitions + */ +#define CONFIG_SYS_FLASH_BASE 0xe0000000 /* start of FLASH 128M */ +#define CONFIG_SYS_FLASH_BASE_PHYS 0xfe0000000ull + +#define CONFIG_FLASH_BR_PRELIM (BR_PHYS_ADDR((CONFIG_SYS_FLASH_BASE_PHYS + 0x8000000)) | BR_PS_16 | BR_V) +#define CONFIG_FLASH_OR_PRELIM (OR_AM_128MB | 0xff7) + +#define CONFIG_SYS_BR0_PRELIM CONFIG_FLASH_BR_PRELIM /* NOR Base Address */ +#define CONFIG_SYS_OR0_PRELIM CONFIG_FLASH_OR_PRELIM /* NOR Options */ + +#define CONFIG_SYS_BR1_PRELIM (BR_PHYS_ADDR(CONFIG_SYS_FLASH_BASE_PHYS) | BR_PS_16 | BR_V) +#define CONFIG_SYS_OR1_PRELIM CONFIG_FLASH_OR_PRELIM + +#define CONFIG_SYS_FLASH_BANKS_LIST {CONFIG_SYS_FLASH_BASE_PHYS + 0x8000000, CONFIG_SYS_FLASH_BASE_PHYS} +#define CONFIG_SYS_FLASH_QUIET_TEST +#define CONFIG_FLASH_SHOW_PROGRESS 45 /* count down from 45/5: 9..1 */ + +#define CONFIG_SYS_MAX_FLASH_BANKS 2 /* number of banks */ +#define CONFIG_SYS_MAX_FLASH_SECT 1024 /* sectors per device */ +#define CONFIG_SYS_FLASH_ERASE_TOUT 60000 /* Flash Erase Timeout (ms) */ +#define CONFIG_SYS_FLASH_WRITE_TOUT 500 /* Flash Write Timeout (ms) */ + +#define CONFIG_SYS_MONITOR_BASE TEXT_BASE /* start of monitor */ + +#define CONFIG_FLASH_CFI_DRIVER +#define CONFIG_SYS_FLASH_CFI +#define CONFIG_SYS_FLASH_EMPTY_INFO +#define CONFIG_SYS_FLASH_AMD_CHECK_DQ7 + +#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_EARLY_INIT_R +#define CONFIG_MISC_INIT_R + +#define CONFIG_FSL_NGPIXIS +#define PIXIS_BASE 0xffdf0000 /* PIXIS registers */ +#define PIXIS_BASE_PHYS 0xfffdf0000ull + +#define CONFIG_SYS_BR2_PRELIM (BR_PHYS_ADDR(PIXIS_BASE_PHYS) | BR_PS_8 | BR_V) +#define CONFIG_SYS_OR2_PRELIM (OR_AM_32KB | 0x6ff7) + +#define PIXIS_LBMAP_SWITCH 7 +#define PIXIS_LBMAP_MASK 0xc0 +#define PIXIS_LBMAP_SHIFT 6 +#define PIXIS_LBMAP_ALTBANK 0x20 + +#define CONFIG_SYS_INIT_RAM_LOCK +#define CONFIG_SYS_INIT_RAM_ADDR 0xffd00000 /* Initial L1 address */ +#define CONFIG_SYS_INIT_RAM_END 0x00004000 /* End of used area in RAM */ + +#define CONFIG_SYS_GBL_DATA_SIZE 128 /* num bytes initial data */ +#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_INIT_RAM_END - CONFIG_SYS_GBL_DATA_SIZE) +#define CONFIG_SYS_INIT_SP_OFFSET CONFIG_SYS_GBL_DATA_OFFSET + +#define CONFIG_SYS_MONITOR_LEN (512 * 1024) /* Reserve 256 kB for Mon */ +#define CONFIG_SYS_MALLOC_LEN (6 * 1024 * 1024) /* Reserved for malloc */ + +/* Serial Port - controlled on board with jumper J8 + * open - index 2 + * shorted - index 1 + */ +#define CONFIG_CONS_INDEX 1 +#define CONFIG_SYS_NS16550 +#define CONFIG_SYS_NS16550_SERIAL +#define CONFIG_SYS_NS16550_REG_SIZE 1 +#define CONFIG_SYS_NS16550_CLK get_bus_freq(0) + +#define CONFIG_SYS_BAUDRATE_TABLE \ + {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200} + +#define CONFIG_SYS_NS16550_COM1 (CONFIG_SYS_CCSRBAR+0x4500) +#define CONFIG_SYS_NS16550_COM2 (CONFIG_SYS_CCSRBAR+0x4600) + +/* Use the HUSH parser */ +#define CONFIG_SYS_HUSH_PARSER +#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " + +#define CONFIG_FSL_DIU_FB /* FSL DIU */ +#define CONFIG_SYS_DIU_ADDR (CONFIG_SYS_CCSRBAR + 0x10000) +/*DIU Configuration*/ +#define DIU_CONNECT_TO_DVI /* DIU controller connects to DVI encoder*/ + +/* video */ +/* #define CONFIG_VIDEO */ +#ifdef CONFIG_VIDEO +#define CONFIG_CFB_CONSOLE +#define CONFIG_VGA_AS_SINGLE_DEVICE +#endif + +/* + * Pass open firmware flat tree + */ +#define CONFIG_OF_LIBFDT +#define CONFIG_OF_BOARD_SETUP +#define CONFIG_OF_STDOUT_VIA_ALIAS + +/* new uImage format support */ +#define CONFIG_FIT +#define CONFIG_FIT_VERBOSE /* enable fit_format_{error,warning}() */ + +/* I2C */ +#define CONFIG_FSL_I2C /* Use FSL common I2C driver */ +#define CONFIG_HARD_I2C /* I2C with hardware support */ +#define CONFIG_I2C_MULTI_BUS +#define CONFIG_SYS_I2C_SPEED 400000 /* I2C speed and slave address */ +#define CONFIG_SYS_I2C_EEPROM_ADDR 0x57 +#define CONFIG_SYS_I2C_SLAVE 0x7F +#define CONFIG_SYS_I2C_NOPROBES {{0,0x29}} /* Don't probe these addrs */ +#define CONFIG_SYS_I2C_OFFSET 0x3000 +#define CONFIG_SYS_I2C2_OFFSET 0x3100 + +/* + * I2C2 EEPROM + */ +#define CONFIG_ID_EEPROM +#define CONFIG_SYS_I2C_EEPROM_NXID +#define CONFIG_SYS_I2C_EEPROM_ADDR 0x57 +#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 1 +#define CONFIG_SYS_EEPROM_BUS_NUM 1 + +/* + * General PCI + * Memory space is mapped 1-1, but I/O space must start from 0. + */ + +/* controller 3, Slot 1, tgtid 3, Base address b000 */ +#define CONFIG_SYS_PCIE3_MEM_VIRT 0x80000000 +#define CONFIG_SYS_PCIE3_MEM_BUS 0xe0000000 +#define CONFIG_SYS_PCIE3_MEM_PHYS 0xc00000000ull +#define CONFIG_SYS_PCIE3_MEM_SIZE 0x20000000 /* 512M */ +#define CONFIG_SYS_PCIE3_IO_VIRT 0xffc00000 +#define CONFIG_SYS_PCIE3_IO_BUS 0x00000000 +#define CONFIG_SYS_PCIE3_IO_PHYS 0xfffc00000ull +#define CONFIG_SYS_PCIE3_IO_SIZE 0x00010000 /* 64k */ + +/* controller 2, direct to uli, tgtid 2, Base address 9000 */ +#define CONFIG_SYS_PCIE2_MEM_VIRT 0xa0000000 +#define CONFIG_SYS_PCIE2_MEM_BUS 0xe0000000 +#define CONFIG_SYS_PCIE2_MEM_PHYS 0xc20000000ull +#define CONFIG_SYS_PCIE2_MEM_SIZE 0x20000000 /* 512M */ +#define CONFIG_SYS_PCIE2_IO_VIRT 0xffc10000 +#define CONFIG_SYS_PCIE2_IO_BUS 0x00000000 +#define CONFIG_SYS_PCIE2_IO_PHYS 0xfffc10000ull +#define CONFIG_SYS_PCIE2_IO_SIZE 0x00010000 /* 64k */ + +/* controller 1, Slot 2, tgtid 1, Base address a000 */ +#define CONFIG_SYS_PCIE1_MEM_VIRT 0xc0000000 +#define CONFIG_SYS_PCIE1_MEM_BUS 0xe0000000 +#define CONFIG_SYS_PCIE1_MEM_PHYS 0xc40000000ull +#define CONFIG_SYS_PCIE1_MEM_SIZE 0x20000000 /* 512M */ +#define CONFIG_SYS_PCIE1_IO_VIRT 0xffc20000 +#define CONFIG_SYS_PCIE1_IO_BUS 0x00000000 +#define CONFIG_SYS_PCIE1_IO_PHYS 0xfffc20000ull +#define CONFIG_SYS_PCIE1_IO_SIZE 0x00010000 /* 64k */ + +#ifdef CONFIG_PCI + +#define CONFIG_NET_MULTI +#define CONFIG_PCI_PNP /* do pci plug-and-play */ + +#ifndef CONFIG_PCI_PNP + #define PCI_ENET0_IOADDR CONFIG_SYS_PCIE3_IO_BUS + #define PCI_ENET0_MEMADDR CONFIG_SYS_PCIE3_IO_BUS + #define PCI_IDSEL_NUMBER 0x11 /* IDSEL = AD11 */ +#endif + +#define CONFIG_PCI_SCAN_SHOW /* show pci devices on startup */ + +#endif /* CONFIG_PCI */ + +/* SATA */ +#define CONFIG_LIBATA +#define CONFIG_FSL_SATA + +#define CONFIG_SYS_SATA_MAX_DEVICE 2 +#define CONFIG_SATA1 +#define CONFIG_SYS_SATA1 CONFIG_SYS_MPC85xx_SATA1_ADDR +#define CONFIG_SYS_SATA1_FLAGS FLAGS_DMA +#define CONFIG_SATA2 +#define CONFIG_SYS_SATA2 CONFIG_SYS_MPC85xx_SATA2_ADDR +#define CONFIG_SYS_SATA2_FLAGS FLAGS_DMA + +#ifdef CONFIG_FSL_SATA +#define CONFIG_LBA48 +#define CONFIG_CMD_SATA +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_EXT2 +#endif + +#define CONFIG_MMC + +#ifdef CONFIG_MMC +#define CONFIG_CMD_MMC +#define CONFIG_FSL_ESDHC +#define CONFIG_GENERIC_MMC +#define CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_MPC85xx_ESDHC_ADDR +#endif + +#if defined(CONFIG_MMC) || defined(CONFIG_USB_EHCI) +#define CONFIG_CMD_EXT2 +#define CONFIG_CMD_FAT +#define CONFIG_DOS_PARTITION +#endif + + +#ifdef CONFIG_TSEC_ENET + +/* TSECV2 */ +#define CONFIG_TSECV2 + +#ifndef CONFIG_NET_MULTI +#define CONFIG_NET_MULTI +#endif + +#define CONFIG_MII /* MII PHY management */ +#define CONFIG_TSEC1 1 +#define CONFIG_TSEC1_NAME "eTSEC1" +#define CONFIG_TSEC2 1 +#define CONFIG_TSEC2_NAME "eTSEC2" + +#define TSEC1_PHY_ADDR 1 +#define TSEC2_PHY_ADDR 2 + +#define TSEC1_FLAGS (TSEC_GIGABIT | TSEC_REDUCED) +#define TSEC2_FLAGS (TSEC_GIGABIT | TSEC_REDUCED) + +#define TSEC1_PHYIDX 0 +#define TSEC2_PHYIDX 0 + +#define CONFIG_ETHPRIME "eTSEC1" + +#define CONFIG_PHY_GIGE /* Include GbE speed/duplex detection */ +#endif + +/* + * Environment + */ +#define CONFIG_ENV_IS_IN_FLASH +#define CONFIG_ENV_ADDR (CONFIG_SYS_MONITOR_BASE - CONFIG_ENV_SECT_SIZE) +#define CONFIG_ENV_SIZE 0x2000 +#define CONFIG_ENV_SECT_SIZE 0x20000 /* 128K (one sector) */ + +#define CONFIG_LOADS_ECHO /* echo on for serial download */ +#define CONFIG_SYS_LOADS_BAUD_CHANGE /* allow baudrate change */ + +/* + * Command line configuration. + */ +#include <config_cmd_default.h> + +#define CONFIG_CMD_IRQ +#define CONFIG_CMD_PING +#define CONFIG_CMD_I2C +#define CONFIG_CMD_MII +#define CONFIG_CMD_ELF +#define CONFIG_CMD_IRQ +#define CONFIG_CMD_SETEXPR + +#ifdef CONFIG_PCI +#define CONFIG_CMD_PCI +#define CONFIG_CMD_NET +#endif + +/* + * USB + */ + +#define CONFIG_USB_EHCI + +#ifdef CONFIG_USB_EHCI +#define CONFIG_CMD_USB +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET +#define CONFIG_USB_EHCI_FSL +#define CONFIG_USB_STORAGE +#define CONFIG_CMD_FAT +#endif + +/* + * Miscellaneous configurable options + */ +#define CONFIG_SYS_LONGHELP /* undef to save memory */ +#define CONFIG_CMDLINE_EDITING /* Command-line editing */ +#define CONFIG_SYS_LOAD_ADDR 0x2000000 /* default load address */ +#define CONFIG_SYS_PROMPT "=> " /* Monitor Command Prompt */ +#ifdef CONFIG_CMD_KGDB +#define CONFIG_SYS_CBSIZE 1024 /* Console I/O Buffer Size */ +#else +#define CONFIG_SYS_CBSIZE 256 /* Console I/O Buffer Size */ +#endif +/* Print Buffer Size */ +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16) +#define CONFIG_SYS_MAXARGS 16 /* max number of command args */ +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE /* Boot Argument Buffer Size */ +#define CONFIG_SYS_HZ 1000 /* decrementer freq: 1ms ticks */ + +/* + * For booting Linux, the board info and command line data + * have to be in the first 16 MB of memory, since this is + * the maximum mapped by the Linux kernel during initialization. + */ +#define CONFIG_SYS_BOOTMAPSZ (16 << 20) /* Initial Memory map for Linux*/ + +/* + * Internal Definitions + * + * Boot Flags + */ +#define BOOTFLAG_COLD 0x01 /* Normal Power-On: Boot from FLASH */ +#define BOOTFLAG_WARM 0x02 /* Software reboot */ + +#ifdef CONFIG_CMD_KGDB +#define CONFIG_KGDB_BAUDRATE 230400 /* speed to run kgdb serial port */ +#define CONFIG_KGDB_SER_INDEX 2 /* which serial port to use */ +#endif + +/* + * Environment Configuration + */ + +/* The mac addresses for all ethernet interface */ +#ifdef CONFIG_TSEC_ENET +#define CONFIG_HAS_ETH0 +#define CONFIG_HAS_ETH1 +#endif + +#define CONFIG_HOSTNAME p1022ds +#define CONFIG_ROOTPATH /opt/nfsroot +#define CONFIG_BOOTFILE uImage +#define CONFIG_UBOOTPATH u-boot.bin /* U-Boot image on TFTP server */ + +/* default location for tftp and bootm */ +#define CONFIG_LOADADDR 1000000 + +#define CONFIG_BOOTDELAY 10 /* -1 disables auto-boot */ +#undef CONFIG_BOOTARGS /* the boot command will set bootargs */ + +#define CONFIG_BAUDRATE 115200 + +#define CONFIG_EXTRA_ENV_SETTINGS \ + "perf_mode=stable\0" \ + "memctl_intlv_ctl=2\0" \ + "netdev=eth0\0" \ + "uboot=" MK_STR(CONFIG_UBOOTPATH) "\0" \ + "tftpflash=tftpboot $loadaddr $uboot; " \ + "protect off " MK_STR(TEXT_BASE) " +$filesize; " \ + "erase " MK_STR(TEXT_BASE) " +$filesize; " \ + "cp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize; " \ + "protect on " MK_STR(TEXT_BASE) " +$filesize; " \ + "cmp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize\0" \ + "consoledev=ttyS0\0" \ + "ramdiskaddr=2000000\0" \ + "ramdiskfile=uramdisk\0" \ + "fdtaddr=c00000\0" \ + "fdtfile=p1022ds.dtb\0" \ + "bdev=sda3\0" \ + "diuregs=md e002c000 1d\0" \ + "dium=mw e002c01c\0" \ + "diuerr=md e002c014 1\0" \ + "othbootargs=diufb=15M video=fslfb:1280x1024-32@60,monitor=0 tty0 debug\0" \ + "monitor=0-DVI\0" + +#define CONFIG_HDBOOT \ + "setenv bootargs root=/dev/$bdev rw " \ + "console=$consoledev,$baudrate $othbootargs;" \ + "tftp $loadaddr $bootfile;" \ + "tftp $fdtaddr $fdtfile;" \ + "bootm $loadaddr - $fdtaddr" + +#define CONFIG_NFSBOOTCOMMAND \ + "setenv bootargs root=/dev/nfs rw " \ + "nfsroot=$serverip:$rootpath " \ + "ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:$netdev:off " \ + "console=$consoledev,$baudrate $othbootargs;" \ + "tftp $loadaddr $bootfile;" \ + "tftp $fdtaddr $fdtfile;" \ + "bootm $loadaddr - $fdtaddr" + +#define CONFIG_RAMBOOTCOMMAND \ + "setenv bootargs root=/dev/ram rw " \ + "console=$consoledev,$baudrate $othbootargs;" \ + "tftp $ramdiskaddr $ramdiskfile;" \ + "tftp $loadaddr $bootfile;" \ + "tftp $fdtaddr $fdtfile;" \ + "bootm $loadaddr $ramdiskaddr $fdtaddr" + +#define CONFIG_BOOTCOMMAND CONFIG_RAMBOOTCOMMAND + +#endif /* __CONFIG_H */

Dear Timur Tabi,
In message 1274392909-16422-1-git-send-email-timur@freescale.com you wrote:
Add basic suport for the Freescale P1022DS reference board.
Signed-off-by: Timur Tabi timur@freescale.com
This patch requires the following two patches to be applied first:
fsl/85xx: add clkdvdr and pmuxcr2 to global utilities structure definition fsl: rename 'dma' to 'brdcfg1' in the ngPIXIS structure
Makefile | 3 + arch/powerpc/cpu/mpc8xxx/pci_cfg.c | 26 ++ board/freescale/p1022ds/Makefile | 40 +++ board/freescale/p1022ds/config.mk | 14 + board/freescale/p1022ds/ddr.c | 108 +++++++ board/freescale/p1022ds/law.c | 27 ++ board/freescale/p1022ds/p1022ds.c | 369 ++++++++++++++++++++++++ board/freescale/p1022ds/p1022ds_diu.c | 151 ++++++++++ board/freescale/p1022ds/tlb.c | 76 +++++ include/configs/P1022DS.h | 505 +++++++++++++++++++++++++++++++++ 10 files changed, 1319 insertions(+), 0 deletions(-) create mode 100644 board/freescale/p1022ds/Makefile create mode 100644 board/freescale/p1022ds/config.mk create mode 100644 board/freescale/p1022ds/ddr.c create mode 100644 board/freescale/p1022ds/law.c create mode 100644 board/freescale/p1022ds/p1022ds.c create mode 100644 board/freescale/p1022ds/p1022ds_diu.c create mode 100644 board/freescale/p1022ds/tlb.c create mode 100644 include/configs/P1022DS.h
Entries to MAKEALL and MAINTAINERS missing.
diff --git a/Makefile b/Makefile index 1445e8b..583a576 100644 --- a/Makefile +++ b/Makefile @@ -2493,6 +2493,9 @@ P2020DS_36BIT_config \ P2020DS_config: unconfig @$(MKCONFIG) -t $(@:_config=) P2020DS powerpc mpc85xx p2020ds freescale
+P1022DS_config: unconfig
- @$(MKCONFIG) -t $(@:_config=) P1022DS powerpc mpc85xx p1022ds freescale
P1011RDB_config \ P1011RDB_NAND_config \ P1011RDB_SDCARD_config \
Please keep lists sorted / sort them.
diff --git a/board/freescale/p1022ds/ddr.c b/board/freescale/p1022ds/ddr.c new file mode 100644 index 0000000..c43c902 --- /dev/null +++ b/board/freescale/p1022ds/ddr.c
...
+void fsl_ddr_get_spd(ddr3_spd_eeprom_t *ctrl_dimms_spd, unsigned int ctrl_num) +{
- int ret;
- /* The P1022 has only one DDR controller, and the board has only one
DIMM slot. */
Incorrect multiline comment style.
...
+/* ranges for parameters:
- wr_data_delay = 0-6
- clk adjust = 0-8
- cpo 2-0x1E (30)
- */
Incorrect multiline comment style.
+static const board_specific_parameters_t bsp[] = { +/* memory controller 0 */ +/* lo| hi| num| clk| cpo|wrdata|2T */ +/* mhz| mhz|ranks|adjst| | delay| */
Incorrect multiline comment style.
Please fix globally!
- { 0, 333, 1, 5, 31, 3, 0},
- {334, 400, 1, 5, 31, 3, 0},
- {401, 549, 1, 5, 31, 3, 0},
- {550, 680, 1, 5, 31, 5, 0},
- {681, 850, 1, 5, 31, 5, 0},
- { 0, 333, 2, 5, 31, 3, 0},
- {334, 400, 2, 5, 31, 3, 0},
- {401, 549, 2, 5, 31, 3, 0},
- {550, 680, 2, 5, 31, 5, 0},
- {681, 850, 2, 5, 31, 5, 0},
Please use TABs for vertical alignment.
+void fsl_ddr_board_options(memctl_options_t *popts, dimm_params_t *pdimm,
unsigned int ctrl_num)
+{
...
- /* Per AN4039, enable ZQ calibration. */
- popts->zq_en = 1;
- /*
Drop one of these empty lines, please.
+int board_early_init_f(void) +{
- ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR;
- /* Set pmuxcr to allow both i2c1 and i2c2 */
- setbits_be32(&gur->pmuxcr, 0x1000);
- in_be32(&gur->pmuxcr);
What is this in_be32() needed for? Either add a comment why it's needed, or remove it.
...
+phys_size_t initdram(int board_type) +{
- phys_size_t dram_size = 0;
- puts("Initializing....\n");
- dram_size = fsl_ddr_sdram();
- dram_size = setup_ddr_tlbs(dram_size / 0x100000);
- dram_size *= 0x100000;
- puts(" DDR: ");
- return dram_size;
How about using get_ram_size() for autosizing / testing?
- /* Enable the TFP410 Encoder (I2C address 0x38)
- */
Please use a symbolic name instead of the hard-wired constant.
+void pci_init_board(void) +{
- volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
- struct fsl_pci_info pci_info[3];
- u32 devdisr, pordevsr, io_sel;
- int first_free_busno = 0;
- int num = 0;
- int pcie_ep, pcie_configured;
- devdisr = in_be32(&gur->devdisr);
- pordevsr = in_be32(&gur->pordevsr);
- io_sel = (pordevsr & MPC85xx_PORDEVSR_IO_SEL) >> 18;
- debug (" pci_init_board: devdisr=%x, io_sel=%x\n", devdisr, io_sel);
- if (!(pordevsr & MPC85xx_PORDEVSR_SGMII2_DIS))
printf(" eTSEC2 is in sgmii mode.\n");
- if (!(pordevsr & MPC85xx_PORDEVSR_SGMII3_DIS))
printf(" eTSEC3 is in sgmii mode.\n");
- puts("\n");
Drop that.
+#ifdef CONFIG_PCIE2
- pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_2, io_sel);
- if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE2)) {
SET_STD_PCIE_INFO(pci_info[num], 2);
pcie_ep = fsl_setup_hose(&pcie2_hose, pci_info[num].regs);
printf(" PCIE2 connected to Slot 3 as %s (base addr %lx)\n",
pcie_ep ? "Endpoint" : "Root Complex",
pci_info[num].regs);
first_free_busno = fsl_pci_init_port(&pci_info[num++],
&pcie2_hose, first_free_busno);
- } else {
printf(" PCIE2: disabled\n");
- }
- puts("\n");
Ditto.
+#else
- setbits_be32(&gur->devdisr, MPC85xx_DEVDISR_PCIE2); /* disable */
+#endif
+#ifdef CONFIG_PCIE3
- pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_3, io_sel);
- if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE3)) {
SET_STD_PCIE_INFO(pci_info[num], 3);
pcie_ep = fsl_setup_hose(&pcie3_hose, pci_info[num].regs);
printf(" PCIE3 connected to Slot 2 as %s (base addr %lx)\n",
pcie_ep ? "Endpoint" : "Root Complex",
pci_info[num].regs);
first_free_busno = fsl_pci_init_port(&pci_info[num++],
&pcie3_hose, first_free_busno);
- } else {
printf(" PCIE3: disabled\n");
- }
- puts("\n");
Ditto.
+#else
- setbits_be32(&gur->devdisr, MPC85xx_DEVDISR_PCIE3); /* disable */
+#endif
+#ifdef CONFIG_PCIE1
- pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_1, io_sel);
- if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE)) {
SET_STD_PCIE_INFO(pci_info[num], 1);
pcie_ep = fsl_setup_hose(&pcie1_hose, pci_info[num].regs);
printf(" PCIE1 connected to Slot 1 as %s (base addr %lx)\n",
pcie_ep ? "Endpoint" : "Root Complex",
pci_info[num].regs);
first_free_busno = fsl_pci_init_port(&pci_info[num++],
&pcie1_hose, first_free_busno);
- } else {
printf(" PCIE1: disabled\n");
- }
- puts("\n");
Ditto.
Hm... looks as if you were repeating the same code 3 times. Please make this a function.
+#ifdef CONFIG_GET_CLK_FROM_ICS307
This CONFIG_GET_CLK_FROM_ICS307 is documented?
+/* decode S[0-2] to Output Divider (OD) */ +static u8 ics307_S_to_OD[] = {
- 10, 2, 8, 4, 5, 7, 3, 6
+};
+/* Calculate frequency being generated by ICS307-02 clock chip based upon
- the control bytes being programmed into it. */
+/* XXX: This function should probably go into a common library */ +static unsigned long ics307_clk_freq(unsigned char cw0, unsigned char cw1,
unsigned char cw2)
+{
- const unsigned long InputFrequency = CONFIG_ICS307_REFCLK_HZ;
- unsigned long VDW = ((cw1 << 1) & 0x1FE) + ((cw2 >> 7) & 1);
- unsigned long RDW = cw2 & 0x7F;
- unsigned long OD = ics307_S_to_OD[cw0 & 0x7];
- unsigned long freq;
Please do not use any CamelCase or UPPER CASE identifiers.
Please fix globally.
+#ifdef CONFIG_PCIE3
- ft_fsl_pci_setup(blob, "pci2", &pcie3_hose);
+#endif +#ifdef CONFIG_PCIE2
- ft_fsl_pci_setup(blob, "pci0", &pcie2_hose);
+#endif +#ifdef CONFIG_PCIE1
- ft_fsl_pci_setup(blob, "pci1", &pcie1_hose);
+#endif
Is this intentional? Looks weird to me...
PCIE1 => pci1 => pcie1_hose PCIE2 => pci0 => pcie2_hose PCIE3 => pci2 => pcie3_hose
Weird, weird...
+++ b/board/freescale/p1022ds/p1022ds_diu.c @@ -0,0 +1,151 @@
This should probably go to drivers/video/ ?
- Copyright 2007-2010 Freescale Semiconductor, Inc.
- Authors: York Sun yorksun@freescale.com
Srikanth Srinivasan <srikanth.srinivasan@freescale.com>
Timur Tabi <timur@freescale.com>
- 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.
- */
+#include <common.h> +#include <command.h> +#include <asm/io.h>
+#include "../common/ngpixis.h" +#include "../common/fsl_diu_fb.h"
+#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE) +#include <stdio_dev.h> +#include <video_fb.h> +#endif
+extern unsigned int FSL_Logo_BMP[];
+static unsigned int xres, yres;
+void diu_set_pixel_clock(unsigned int pixclock) +{
- ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR;
- unsigned long speed_ccb, temp, pixval;
- speed_ccb = get_bus_freq(0);
- temp = 1000000000UL / pixclock;
- temp *= 1000;
- pixval = speed_ccb / temp;
- debug("DIU pixval = %lu\n", pixval);
- /* Modify PXCLK in GUTS CLKDVDR */
- debug("DIU: Current value of CLKDVDR = 0x%08x\n", in_be32(&gur->clkdvdr));
- clrbits_be32(&gur->clkdvdr, 0xdfff0000); /* turn off clock */
- setbits_be32(&gur->clkdvdr, 0x80000000 | ((pixval & 0x1F) << 16))
- debug("DIU: Modified value of CLKDVDR = 0x%08x\n", in_be32(&gur->clkdvdr));
+}
Hmmm... this looks pretty much similar to board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c;
I think you should merge these two into one driver.
+int p1022diu_init_show_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- unsigned int addr;
...while doing so, please clean up to use the standard bmp command instead.
+U_BOOT_CMD(
- diufb, CONFIG_SYS_MAXARGS, 1, p1022diu_init_show_bmp,
- "Init or Display BMP file",
- "init\n - initialize DIU\n"
- "addr\n - display bmp at address 'addr'"
+);
This should go away.
diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h new file mode 100644 index 0000000..65a1265 --- /dev/null +++ b/include/configs/P1022DS.h
...
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
+/*
- Base addresses -- Note these are effective addresses where the
- actual resources get mapped (not physical addresses)
- */
+#define CONFIG_SYS_CCSRBAR_DEFAULT 0xff700000 /* CCSRBAR Default */ +#define CONFIG_SYS_CCSRBAR 0xffe00000 /* relocated CCSRBAR */ +#define CONFIG_SYS_CCSRBAR_PHYS 0xfffe00000ull /* physical addr of CCSRBAR */ +#define CONFIG_SYS_IMMR CONFIG_SYS_CCSRBAR /* PQII uses CONFIG_SYS_IMMR */
Lines too long. Please fix globally.
+/* ECC will be enabled based on perf_mode environment variable */ +//#define CONFIG_DDR_ECC
No C++ comments. Please fix globally.
+/* video */ +/* #define CONFIG_VIDEO */
Please do not add dead code. Please fix globally.
+#define CONFIG_BOOTDELAY 10 /* -1 disables auto-boot */ +#undef CONFIG_BOOTARGS /* the boot command will set bootargs */
Do not undefine what is not defined anyway.
+#define CONFIG_EXTRA_ENV_SETTINGS \
- "perf_mode=stable\0" \
- "memctl_intlv_ctl=2\0" \
- "netdev=eth0\0" \
- "uboot=" MK_STR(CONFIG_UBOOTPATH) "\0" \
- "tftpflash=tftpboot $loadaddr $uboot; " \
- "protect off " MK_STR(TEXT_BASE) " +$filesize; " \
- "erase " MK_STR(TEXT_BASE) " +$filesize; " \
- "cp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize; " \
- "protect on " MK_STR(TEXT_BASE) " +$filesize; " \
- "cmp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize\0" \
Please indent by TABs only.
Best regards,
Wolfgang Denk

On May 20, 2010, at 5:33 PM, Wolfgang Denk wrote:
...
+phys_size_t initdram(int board_type) +{
- phys_size_t dram_size = 0;
- puts("Initializing....\n");
- dram_size = fsl_ddr_sdram();
- dram_size = setup_ddr_tlbs(dram_size / 0x100000);
- dram_size *= 0x100000;
- puts(" DDR: ");
- return dram_size;
How about using get_ram_size() for autosizing / testing?
Why, the board is SPD based?
- k

Dear Kumar Gala,
In message AD79CE60-A61F-4976-A1CE-9D96171797F7@kernel.crashing.org you wrote:
How about using get_ram_size() for autosizing / testing?
Why, the board is SPD based?
Testing memory has always been an excellent idea - it's pretty useful information to know that the RAM is not only physically present, but actually working. Also, it helps to detect problems with the SPD detection code.
Best regards,
Wolfgang Denk

On Thu, May 20, 2010 at 5:33 PM, Wolfgang Denk wd@denx.de wrote:
Entries to MAKEALL and MAINTAINERS missing.
Ok.
+P1022DS_config: Â Â Â Â Â Â Â unconfig
- @$(MKCONFIG) -t $(@:_config=) P1022DS powerpc mpc85xx p1022ds freescale
P1011RDB_config    \  P1011RDB_NAND_config \  P1011RDB_SDCARD_config \
Please keep lists sorted / sort them.
Ok.
+void fsl_ddr_get_spd(ddr3_spd_eeprom_t *ctrl_dimms_spd, unsigned int ctrl_num) +{
- int ret;
- /* The P1022 has only one DDR controller, and the board has only one
- DIMM slot. */
Incorrect multiline comment style.
Ok.
...
+/* ranges for parameters:
- wr_data_delay = 0-6
- clk adjust = 0-8
- cpo 2-0x1E (30)
- */
Incorrect multiline comment style.
Sorry, what's wrong with this, specifically? Should it look like this:
/* * ranges for parameters: * wr_data_delay = 0-6 * clk adjust = 0-8 * cpo 2-0x1E (30) */
Please keep in mind that a lot of this code is taken from the existing p2020ds support that's already in your repository, so many of your comments are really criticisms of code that you have already accepted.
- { Â 0, 333, Â Â 1, Â Â 5, Â 31, Â Â 3, Â 0},
- {334, 400, Â Â 1, Â Â 5, Â 31, Â Â 3, Â 0},
- {401, 549, Â Â 1, Â Â 5, Â 31, Â Â 3, Â 0},
- {550, 680, Â Â 1, Â Â 5, Â 31, Â Â 5, Â 0},
- {681, 850, Â Â 1, Â Â 5, Â 31, Â Â 5, Â 0},
- { Â 0, 333, Â Â 2, Â Â 5, Â 31, Â Â 3, Â 0},
- {334, 400, Â Â 2, Â Â 5, Â 31, Â Â 3, Â 0},
- {401, 549, Â Â 2, Â Â 5, Â 31, Â Â 3, Â 0},
- {550, 680, Â Â 2, Â Â 5, Â 31, Â Â 5, Â 0},
- {681, 850, Â Â 2, Â Â 5, Â 31, Â Â 5, Â 0},
Please use TABs for vertical alignment.
I thought I did.
+void fsl_ddr_board_options(memctl_options_t *popts, dimm_params_t *pdimm,
- unsigned int ctrl_num)
+{
...
- /* Per AN4039, enable ZQ calibration. */
- popts->zq_en = 1;
- /*
Drop one of these empty lines, please.
Ok. I don't know how that got in there.
+int board_early_init_f(void) +{
- ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR;
- /* Set pmuxcr to allow both i2c1 and i2c2 */
- setbits_be32(&gur->pmuxcr, 0x1000);
- in_be32(&gur->pmuxcr);
What is this in_be32() needed for? Either add a comment why it's needed, or remove it.
Ok. It's for serializing the I/O. The PIXIS won't complete the read until after the write is finished.
+phys_size_t initdram(int board_type) +{
- phys_size_t dram_size = 0;
- puts("Initializing....\n");
- dram_size = fsl_ddr_sdram();
- dram_size = setup_ddr_tlbs(dram_size / 0x100000);
- dram_size *= 0x100000;
- puts(" Â Â DDR: ");
- return dram_size;
How about using get_ram_size() for autosizing / testing?
That's what fsl_ddr_sdram() does. It returns the size based on SPD.
- /* Â Enable the TFP410 Encoder (I2C address 0x38)
- */
Please use a symbolic name instead of the hard-wired constant.
Ok.
+void pci_init_board(void) +{
- volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
- struct fsl_pci_info pci_info[3];
- u32 devdisr, pordevsr, io_sel;
- int first_free_busno = 0;
- int num = 0;
- int pcie_ep, pcie_configured;
- devdisr = in_be32(&gur->devdisr);
- pordevsr = in_be32(&gur->pordevsr);
- io_sel = (pordevsr & MPC85xx_PORDEVSR_IO_SEL) >> 18;
- debug (" Â pci_init_board: devdisr=%x, io_sel=%x\n", devdisr, io_sel);
- if (!(pordevsr & MPC85xx_PORDEVSR_SGMII2_DIS))
- printf(" Â Â eTSEC2 is in sgmii mode.\n");
- if (!(pordevsr & MPC85xx_PORDEVSR_SGMII3_DIS))
- printf(" Â Â eTSEC3 is in sgmii mode.\n");
- puts("\n");
Drop that.
This code is identical to the code in the p2020ds.c, so I'm just mirroring it. The only difference is the names of the slots in the printf. I would prefer to keep this new code as close as possible to our existing code. I suspect that we'll be unifying the PCI code in the future, and keeping it similar will make it easier.
+#ifdef CONFIG_PCIE1
- pcie_configured = is_fsl_pci_cfg(LAW_TRGT_IF_PCIE_1, io_sel);
- if (pcie_configured && !(devdisr & MPC85xx_DEVDISR_PCIE)) {
- SET_STD_PCIE_INFO(pci_info[num], 1);
- pcie_ep = fsl_setup_hose(&pcie1_hose, pci_info[num].regs);
- printf(" Â Â PCIE1 connected to Slot 1 as %s (base addr %lx)\n",
- pcie_ep ? "Endpoint" : "Root Complex",
- pci_info[num].regs);
- first_free_busno = fsl_pci_init_port(&pci_info[num++],
- &pcie1_hose, first_free_busno);
- } else {
- printf(" Â Â PCIE1: disabled\n");
- }
- puts("\n");
Ditto.
Hm... looks as if you were repeating the same code 3 times. Please make this a function.
The code isn't really the same. I would need to pass a lot of parameters to this function: the hose, the devdisr mask, the slot name, the slot number, the bus number, and so on.
+#ifdef CONFIG_GET_CLK_FROM_ICS307
This CONFIG_GET_CLK_FROM_ICS307 is documented?
We've been using it for years. Now you complain? It's the same code in almost all of our recent boards.
+static unsigned long ics307_clk_freq(unsigned char cw0, unsigned char cw1,
- unsigned char cw2)
+{
- const unsigned long InputFrequency = CONFIG_ICS307_REFCLK_HZ;
- unsigned long VDW = ((cw1 << 1) & 0x1FE) + ((cw2 >> 7) & 1);
- unsigned long RDW = cw2 & 0x7F;
- unsigned long OD = ics307_S_to_OD[cw0 & 0x7];
- unsigned long freq;
Please do not use any CamelCase or UPPER CASE identifiers.
Again, this is the same code as always. You've accepted this code a dozen times over. Why are you picking on me about it now?
+#ifdef CONFIG_PCIE3
- ft_fsl_pci_setup(blob, "pci2", &pcie3_hose);
+#endif +#ifdef CONFIG_PCIE2
- ft_fsl_pci_setup(blob, "pci0", &pcie2_hose);
+#endif +#ifdef CONFIG_PCIE1
- ft_fsl_pci_setup(blob, "pci1", &pcie1_hose);
+#endif
Is this intentional? Looks weird to me...
PCIE1 => pci1 => pcie1_hose PCIE2 => pci0 => pcie2_hose PCIE3 => pci2 => pcie3_hose
Weird, weird...
I asked Kumar about it, but he didn't really have much to say, so I left it.
+++ b/board/freescale/p1022ds/p1022ds_diu.c @@ -0,0 +1,151 @@
This should probably go to drivers/video/ ?
It's p1022-specific, so I don't see why.
Hmmm... this looks pretty much similar to board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c;
I think you should merge these two into one driver.
And one day I will. But not today. It's going to take me a long time to analyze both code and test it on both boards. I can't solve every problem at once.
+int p1022diu_init_show_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- unsigned int addr;
...while doing so, please clean up to use the standard bmp command instead.
I didn't know there was a standard bmp command.
+U_BOOT_CMD(
- diufb, CONFIG_SYS_MAXARGS, 1, p1022diu_init_show_bmp,
- "Init or Display BMP file",
- "init\n   - initialize DIU\n"
- "addr\n   - display bmp at address 'addr'"
+);
This should go away.
What's wrong with it?
diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h new file mode 100644 index 0000000..65a1265 --- /dev/null +++ b/include/configs/P1022DS.h
...
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
OMG, every single one of our header files does this!!!! We've been doing this for years!
I really do believe you're picking on me now.
+/*
- Base addresses -- Note these are effective addresses where the
- actual resources get mapped (not physical addresses)
- */
+#define CONFIG_SYS_CCSRBAR_DEFAULT  0xff700000    /* CCSRBAR Default */ +#define CONFIG_SYS_CCSRBAR      0xffe00000    /* relocated CCSRBAR */ +#define CONFIG_SYS_CCSRBAR_PHYS        0xfffe00000ull  /* physical addr of CCSRBAR */ +#define CONFIG_SYS_IMMR            CONFIG_SYS_CCSRBAR    /* PQII uses CONFIG_SYS_IMMR */
Lines too long. Please fix globally.
Ok.
+/* ECC will be enabled based on perf_mode environment variable */ +//#define   CONFIG_DDR_ECC
No C++ comments. Please fix globally.
Oops, I forgot to delete that one.
+/* video */ +/* #define CONFIG_VIDEO */
Please do not add dead code. Please fix globally.
Ok.
+#define CONFIG_BOOTDELAY 10  /* -1 disables auto-boot */ +#undef  CONFIG_BOOTARGS        /* the boot command will set bootargs */
Do not undefine what is not defined anyway.
Sorry, I don't know how that got in there. My eyes saw #define and not #undef.

Dear Timur Tabi,
In message AANLkTinbCYBbwksm5XX5psWlYqpB_dZ6-DaOuhuKkM-S@mail.gmail.com you wrote: ...
+/* ranges for parameters:
- =A0wr_data_delay =3D 0-6
- =A0clk adjust =3D 0-8
- =A0cpo 2-0x1E (30)
- */
Incorrect multiline comment style.
Sorry, what's wrong with this, specifically? Should it look like this:
/*
- ranges for parameters:
- wr_data_delay =3D 0-6
- clk adjust =3D 0-8
- cpo 2-0x1E (30)
*/
Yes,indeed. Please see the Coding Style document in case of further questions.
Please use TABs for vertical alignment.
I thought I did.
You did not.
/* Set pmuxcr to allow both i2c1 and i2c2 */
setbits_be32(&gur->pmuxcr, 0x1000);
in_be32(&gur->pmuxcr);
What is this in_be32() needed for? Either add a comment why it's needed, or remove it.
Ok. It's for serializing the I/O. The PIXIS won't complete the read until after the write is finished.
Please add such a comment, then.
dram_size = fsl_ddr_sdram();
dram_size = setup_ddr_tlbs(dram_size / 0x100000);
dram_size *= 0x100000;
puts(" DDR: ");
return dram_size;
How about using get_ram_size() for autosizing / testing?
That's what fsl_ddr_sdram() does. It returns the size based on SPD.
fsl_ddr_sdram() does NOT the same as get_ram_size(). It does not perform any actual testing, and will not detect errors in the SPD information or in the code processing it.
...
if (!(pordevsr & MPC85xx_PORDEVSR_SGMII2_DIS))
printf(" eTSEC2 is in sgmii mode.\n");
if (!(pordevsr & MPC85xx_PORDEVSR_SGMII3_DIS))
printf(" eTSEC3 is in sgmii mode.\n");
puts("\n");
Drop that.
I should have been more specific here. Hiere, at the first repetition of this code, I only meant you should drop the ``puts("\n");'' line.
This code is identical to the code in the p2020ds.c, so I'm just mirroring it. The only difference is the names of the slots in the printf. I would prefer to keep this new code as close as possible to our existing code. I suspect that we'll be unifying the PCI code in the future, and keeping it similar will make it easier.
Thanks for pointing this out. so we have even more occurrences of this block of code - time has come to factor this out into a common function that is used for this board and for p2020ds as well.
Hm... looks as if you were repeating the same code 3 times. Please make this a function.
The code isn't really the same. I would need to pass a lot of parameters to this function: the hose, the devdisr mask, the slot name, the slot number, the bus number, and so on.
Actually it is not that many arguments.
In any case, there are so many occurrences of this block of code that it really should be turned into a function.
+#ifdef CONFIG_GET_CLK_FROM_ICS307
This CONFIG_GET_CLK_FROM_ICS307 is documented?
We've been using it for years. Now you complain? It's the same code in almost all of our recent boards.
Please document it.
Please do not use any CamelCase or UPPER CASE identifiers.
Again, this is the same code as always. You've accepted this code a dozen times over. Why are you picking on me about it now?
I complain about things when I see them.
Do you really expect me to review all patches submitted on this list in scrupulous detail? I skim through most of them. I read many of them more carefully. I try to review as many patches as time permits, but I never claimed 100% coverage.
Maybe this slept through in the past.
Now it got caught.
It would be nice if you also fixed all the other dozen places where such code has crept in.
+#ifdef CONFIG_PCIE3
ft_fsl_pci_setup(blob, "pci2", &pcie3_hose);
+#endif +#ifdef CONFIG_PCIE2
ft_fsl_pci_setup(blob, "pci0", &pcie2_hose);
+#endif +#ifdef CONFIG_PCIE1
ft_fsl_pci_setup(blob, "pci1", &pcie1_hose);
+#endif
Is this intentional? Looks weird to me...
PCIE1 => pci1 => pcie1_hose PCIE2 => pci0 => pcie2_hose PCIE3 => pci2 => pcie3_hose
Weird, weird...
I asked Kumar about it, but he didn't really have much to say, so I left it.
You mean neither of you knows if this is correct or wrong ???
+++ b/board/freescale/p1022ds/p1022ds_diu.c @@ -0,0 +1,151 @@
This should probably go to drivers/video/ ?
It's p1022-specific, so I don't see why.
No, it's not. It is largely the same as mpc8610hpcd_diu.c; common video driver code belongs into drivers/video/
Hmmm... this looks pretty much similar to board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c;
I think you should merge these two into one driver.
And one day I will. But not today. It's going to take me a long time to analyze both code and test it on both boards. I can't solve every problem at once.
It's only about 200 LOC, and large parts look pretty similar. Please do not duplicate all this, instead factor our the common parts.
+int p1022diu_init_show_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *> argv[]) +{
unsigned int addr;
...while doing so, please clean up to use the standard bmp command instead.
I didn't know there was a standard bmp command.
You know it know.
+U_BOOT_CMD(
diufb, CONFIG_SYS_MAXARGS, 1, p1022diu_init_show_bmp,
"Init or Display BMP file",
"init\n - initialize DIU\n"
"addr\n - display bmp at address 'addr'"
+);
This should go away.
What's wrong with it?
It's duplicating existing code ("bmp" command).
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
OMG, every single one of our header files does this!!!! We've been doing this for years!
I really do believe you're picking on me now.
I'm not. I'm just reviewing a patch.
It would be great if you cleaned up the other places as well.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
fsl_ddr_sdram() does NOT the same as get_ram_size(). It does not perform any actual testing, and will not detect errors in the SPD information or in the code processing it.
Ok, I'll add it.
This code is identical to the code in the p2020ds.c, so I'm just mirroring it. The only difference is the names of the slots in the printf. I would prefer to keep this new code as close as possible to our existing code. I suspect that we'll be unifying the PCI code in the future, and keeping it similar will make it easier.
Thanks for pointing this out. so we have even more occurrences of this block of code - time has come to factor this out into a common function that is used for this board and for p2020ds as well.
Kumar just posted some patches for p2020ds that changes this, so I'll incorporate those.
This CONFIG_GET_CLK_FROM_ICS307 is documented?
We've been using it for years. Now you complain? It's the same code in almost all of our recent boards.
Please document it.
Looks like Kumar posted patches for this, too, so I'll add those as well.
I complain about things when I see them.
Do you really expect me to review all patches submitted on this list in scrupulous detail?
No, apparently just mine.
It would be nice if you also fixed all the other dozen places where such code has crept in.
Can I do one thing at a time? I agree that a lot of those could be optimized, and if you look at the git log for our stuff, you'll see we've been doing that a lot in the past year. But in general, fixes like these usually work better if the board support is already present in your repository.
PCIE1 => pci1 => pcie1_hose PCIE2 => pci0 => pcie2_hose PCIE3 => pci2 => pcie3_hose
Weird, weird...
I asked Kumar about it, but he didn't really have much to say, so I left it.
You mean neither of you knows if this is correct or wrong ???
I guess I should clarify. Kumar said it was based on the order of the addresses of the PCI devices within CCSR space, but he didn't offer any insight as to whether it should be fixed or how.
+++ b/board/freescale/p1022ds/p1022ds_diu.c @@ -0,0 +1,151 @@
This should probably go to drivers/video/ ?
It's p1022-specific, so I don't see why.
No, it's not. It is largely the same as mpc8610hpcd_diu.c; common video driver code belongs into drivers/video/
Ok, will you allow me to merge those two functions into a single driver at a later time, when I have the opportunity to examine the code and test it on both boards?
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
OMG, every single one of our header files does this!!!! We've been doing this for years!
I really do believe you're picking on me now.
I'm not. I'm just reviewing a patch.
It would be great if you cleaned up the other places as well.
Well, this particular change would probably need to be made later, because I would probably need to change all the board header files at once. It's not trivial.

Dear Timur Tabi,
In message 4BF68E6A.4070801@freescale.com you wrote:
Do you really expect me to review all patches submitted on this list in scrupulous detail?
No, apparently just mine.
Unless you pay me for this (or FSL) you should not even rely on this.
It would be nice if you also fixed all the other dozen places where such code has crept in.
Can I do one thing at a time? I agree that a lot of those could be optimized, and if you look at the git log for our stuff, you'll see we've been doing that a lot in the past year. But in general, fixes like these usually work better if the board support is already present in your repository.
Well, there are things and things.
When we discover that crap has piled up here and there, it's a good idea to perform a clean up.
When the first one adds some feature or new architecture or similar, it is pretty normal that he cannot yet always decide correctly what needs to go where, or which parts are specific to CPU or board and which are common. But when more boards get added, this usually becomes clear pretty quickly.
It has always been more efficient to prevent code that is known to need such cleanup to prevent from going into mainline in the first place, than to allow it to go in and hope that somebody, sometime will find time and resources and zest for a clean-up.
Is it easier for you to convince your manager that you need N more hours to get that code accepted for mainline, or to get allownce for N hours to perform some cleanup in U-Boot that is not related to your current project?
[I have somemore tasks at hand if the latter should be the case :-) ]
PCIE1 => pci1 => pcie1_hose PCIE2 => pci0 => pcie2_hose PCIE3 => pci2 => pcie3_hose
Weird, weird...
I asked Kumar about it, but he didn't really have much to say, so I left it.
You mean neither of you knows if this is correct or wrong ???
I guess I should clarify. Kumar said it was based on the order of the addresses of the PCI devices within CCSR space, but he didn't offer any insight as to whether it should be fixed or how.
So please check this again. It looks broken to me.
No, it's not. It is largely the same as mpc8610hpcd_diu.c; common video driver code belongs into drivers/video/
Ok, will you allow me to merge those two functions into a single driver at a later time, when I have the opportunity to examine the code and test it on both boards?
Please let's do it now, instead of adding code that is known to need fixing. See above for my reasoning.
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
...
Well, this particular change would probably need to be made later, because I would probably need to change all the board header files at once. It's not trivial.
What exactly is not trivial in moving some prototype declarations to another header file? Please elucidate. The most difficult part is probably determining where they actually belong to.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
Unless you pay me for this (or FSL) you should not even rely on this.
Heh.
Well, there are things and things.
When we discover that crap has piled up here and there, it's a good idea to perform a clean up.
I agree 100%.
When the first one adds some feature or new architecture or similar, it is pretty normal that he cannot yet always decide correctly what needs to go where, or which parts are specific to CPU or board and which are common. But when more boards get added, this usually becomes clear pretty quickly.
Ok.
It has always been more efficient to prevent code that is known to need such cleanup to prevent from going into mainline in the first place, than to allow it to go in and hope that somebody, sometime will find time and resources and zest for a clean-up.
I'm not sure I agree with that completely. In many cases, it's more efficient to work on tasks in bite-size chunks.
Is it easier for you to convince your manager that you need N more hours to get that code accepted for mainline, or to get allownce for N hours to perform some cleanup in U-Boot that is not related to your current project?
Definitely the latter. Cleaning up U-Boot (and Linux) code so that it's overall easier to maintain and more efficient is one of the primary tasks of my department.
[I have somemore tasks at hand if the latter should be the case :-) ]
Please, send me a list. We have an internal bug tracking system, and I would be more than happy to log all the items that concern you (as long as they affect Freescale PowerPC products in some way).
I guess I should clarify. Kumar said it was based on the order of the addresses of the PCI devices within CCSR space, but he didn't offer any insight as to whether it should be fixed or how.
So please check this again. It looks broken to me.
Ok.
Ok, will you allow me to merge those two functions into a single driver at a later time, when I have the opportunity to examine the code and test it on both boards?
Please let's do it now, instead of adding code that is known to need fixing. See above for my reasoning.
And see my above response for why I'd prefer not to solve this now.
I'll try to fix it anyway, though.
+#ifndef __ASSEMBLY__ +extern unsigned long calculate_board_sys_clk(void); +extern unsigned long calculate_board_ddr_clk(void); +#endif
Please move to appropriate header file.
...
Well, this particular change would probably need to be made later, because I would probably need to change all the board header files at once. It's not trivial.
What exactly is not trivial in moving some prototype declarations to another header file? Please elucidate. The most difficult part is probably determining where they actually belong to.
Well, I'll take a closer look, but my concern is that there is no good header file to move these into that wouldn't require modifying dozens of files. Since I'm trying to add support for one particular board, I don't want to modify every other board in the same patch set.
I'll try to fix it anyway, though.

Wolfgang Denk wrote:
> >>>> +#ifndef __ASSEMBLY__ > >>>> +extern unsigned long calculate_board_sys_clk(void); > >>>> +extern unsigned long calculate_board_ddr_clk(void); > >>>> +#endif >>> >>> Please move to appropriate header file.
...
Well, this particular change would probably need to be made later, because I would probably need to change all the board header files at once. It's not trivial.
What exactly is not trivial in moving some prototype declarations to another header file? Please elucidate. The most difficult part is probably determining where they actually belong to.
Well, I'm having a hard time finding a good header file.
For the P1022DS, I just need to have this macros known to speed.c, so I could add these lines to the top of /arch/powerpc/cpu/mpc85xx/speed.c:
#include <common.h> #include <ppc_asm.tmpl> #include <asm/processor.h> #include <asm/io.h>
/* Clock frequency */ unsigned long calculate_board_sys_clk(void); unsigned long calculate_board_ddr_clk(void);
DECLARE_GLOBAL_DATA_PTR;
But I suspect this will not be sufficient for all of the other 85xx boards.
Another option is arch/powerpc/include/asm/config.h, but that header file doesn't have any function prototypes today.
There's also include/common.h, near this block:
#if defined(CONFIG_MPC85xx) typedef MPC85xx_SYS_INFO sys_info_t; void get_sys_info ( sys_info_t * ); ulong get_ddr_freq (ulong); #endif #if defined(CONFIG_MPC86xx) typedef MPC86xx_SYS_INFO sys_info_t; void get_sys_info ( sys_info_t * ); #endif
Suggestions?

Wolfgang Denk wrote:
Hm... looks as if you were repeating the same code 3 times. Please make this a function.
The code isn't really the same. I would need to pass a lot of parameters to this function: the hose, the devdisr mask, the slot name, the slot number, the bus number, and so on.
Actually it is not that many arguments.
It is. Here's the prototype I came up with;
static int configure_pci(enum srds_prtcl pci, const char *name, const char *target, int endpoint, int first_free_busno, phys_addr_t mem_addr, enum law_size mem_size, phys_addr_t io_addr, enum law_size io_size, struct fsl_pci_info *pci_info, struct pci_controller *hose);
Are you sure you would really rather see this as its own function?

On 05/26/2010 01:12 PM, Timur Tabi wrote:
Wolfgang Denk wrote:
Hm... looks as if you were repeating the same code 3 times. Please make this a function.
The code isn't really the same. I would need to pass a lot of parameters to this function: the hose, the devdisr mask, the slot name, the slot number, the bus number, and so on.
Actually it is not that many arguments.
It is. Here's the prototype I came up with;
static int configure_pci(enum srds_prtcl pci, const char *name, const char *target, int endpoint, int first_free_busno, phys_addr_t mem_addr, enum law_size mem_size, phys_addr_t io_addr, enum law_size io_size, struct fsl_pci_info *pci_info, struct pci_controller *hose);
Are you sure you would really rather see this as its own function?
Perhaps (most of) this information could be put in a data structure to which you point?
-Scott

Scott Wood wrote:
Perhaps (most of) this information could be put in a data structure to which you point?
That doesn't change the amount of information that needs to be passed, it only makes the prototype have fewer lines.
The point I'm trying to make is that the code in question is not the same code copy/pasted three times. Every line is different. Moving that code into a function is not going to simplify anything.

On 05/26/2010 01:19 PM, Timur Tabi wrote:
Scott Wood wrote:
Perhaps (most of) this information could be put in a data structure to which you point?
That doesn't change the amount of information that needs to be passed, it only makes the prototype have fewer lines.
Which is relevant, given that you're whipping out a big scary-looking prototype as a reason to avoid refactoring. :-)
The point I'm trying to make is that the code in question is not the same code copy/pasted three times. Every line is different. Moving that code into a function is not going to simplify anything.
They are not identical lines, but they're doing the same basic thing -- making it data driven would be a proper separation of concerns.
-Scott

Scott Wood wrote:
Which is relevant, given that you're whipping out a big scary-looking prototype as a reason to avoid refactoring. :-)
So instead of this:
configure_pci(PCIE1, "PCIE1", "Slot 1", pcie_ep, num, LAW_TRGT_IF_PCIE_1, CONFIG_SYS_PCIE1_MEM_PHYS, LAW_SIZE_512M, CONFIG_SYS_PCIE1_IO_PHYS, LAW_SIZE_64K, &pci_info[num], &pcie1_hose);
You want this instead:
struct { enum srds_prtcl pci; const char *name; const char *target; int endpoint; int first_free_busno; enum law_trgt_if law; phys_addr_t mem_addr; enum law_size mem_size; phys_addr_t io_addr; enum law_size io_size; struct fsl_pci_info *pci_info; struct pci_controller *hose; } x;
x.pci = PCIE1; x.name = "PCIE1"; x.target = "Slot 1"; x.endpoint =- pcie_ep; x.first_free_busno = num; x.law = LAW_TRGT_IF_PCIE_1; x.mem_addr = CONFIG_SYS_PCIE1_MEM_PHYS; x.mem_size = LAW_SIZE_512M; x.io_addr = CONFIG_SYS_PCIE1_IO_PHYS; x.io_size = LAW_SIZE_64K; x.pci_info = &pci_info[num]; x.hose = &pcie1_hose;
configure_pci(&x);
I don't see how this is an improvement.

On 05/26/2010 02:34 PM, Timur Tabi wrote:
Scott Wood wrote:
Which is relevant, given that you're whipping out a big scary-looking prototype as a reason to avoid refactoring. :-)
So instead of this:
configure_pci(PCIE1, "PCIE1", "Slot 1", pcie_ep, num, LAW_TRGT_IF_PCIE_1, CONFIG_SYS_PCIE1_MEM_PHYS, LAW_SIZE_512M, CONFIG_SYS_PCIE1_IO_PHYS, LAW_SIZE_64K,&pci_info[num],&pcie1_hose);
You want this instead:
struct { enum srds_prtcl pci; const char *name; const char *target; int endpoint; int first_free_busno; enum law_trgt_if law; phys_addr_t mem_addr; enum law_size mem_size; phys_addr_t io_addr; enum law_size io_size; struct fsl_pci_info *pci_info; struct pci_controller *hose; } x;
x.pci = PCIE1; x.name = "PCIE1"; x.target = "Slot 1"; x.endpoint =- pcie_ep; x.first_free_busno = num; x.law = LAW_TRGT_IF_PCIE_1; x.mem_addr = CONFIG_SYS_PCIE1_MEM_PHYS; x.mem_size = LAW_SIZE_512M; x.io_addr = CONFIG_SYS_PCIE1_IO_PHYS; x.io_size = LAW_SIZE_64K; x.pci_info =&pci_info[num]; x.hose =&pcie1_hose;
configure_pci(&x);
I don't see how this is an improvement.
Much of that struct could be defined statically, with the board or soc file providing an array of PCI info structs. Look at how the 83xx PCI code does it -- you'll need more than just the generic u-boot pci_region struct, though you could have it as a member of the hw-specific struct.
A few of those things don't belong there -- I think first_free_busno should be a static variable inside the pci setup function, for example (does Linux still even need separate bus number spaces on different hoses?). pcie_ep should just be a local variable. The hose could maybe just be an uninitialized member of the struct instead of a pointer to an arbitrary other symbol.
-Scott

Scott Wood wrote:
A few of those things don't belong there -- I think first_free_busno should be a static variable inside the pci setup function, for example (does Linux still even need separate bus number spaces on different hoses?). pcie_ep should just be a local variable. The hose could maybe just be an uninitialized member of the struct instead of a pointer to an arbitrary other symbol.
I'm not sure what you mean about the 'hose'. How about this:
static void configure_pcie(enum srds_prtcl pci, u32 devdisr_mask, const char *target, enum law_trgt_if law, phys_addr_t mem_addr, enum law_size mem_size, phys_addr_t io_addr, enum law_size io_size, struct pci_controller *hose) { volatile ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR; struct fsl_pci_info pci_info; u32 devdisr; static int first_free_busno = 0; int is_endpoint; int num;
devdisr = in_be32(&gur->devdisr);
if (is_serdes_configured(pci) && !(devdisr & devdisr_mask)) { set_next_law(mem_addr, mem_size, law); set_next_law(io_addr, io_size, law);
switch (pci) { #ifdef CONFIG_PCIE1 case PCIE1: num = 1; SET_STD_PCIE_INFO(pci_info, 1); break; #endif #ifdef CONFIG_PCIE2 case PCIE2: num = 2; SET_STD_PCIE_INFO(pci_info, 2); break; #endif #ifdef CONFIG_PCIE3 case PCIE3: num = 3; SET_STD_PCIE_INFO(pci_info, 3); break; #endif #ifdef CONFIG_PCIE4 case PCIE4: num = 4; SET_STD_PCIE_INFO(pci_info, 4); break; #endif default: break; }
is_endpoint = fsl_setup_hose(hose, pci_info.regs); printf(" PCIE%u: connected to %s as %s (base addr %lx)\n", num, target, is_endpoint ? "Endpoint" : "Root Complex", pci_info.regs);
first_free_busno = fsl_pci_init_port(&pci_info, hose, first_free_busno); } else { printf(" PCIE%u: disabled\n", num); } }

Wolfgang Denk wrote:
- { 0, 333, 1, 5, 31, 3, 0},
- {334, 400, 1, 5, 31, 3, 0},
- {401, 549, 1, 5, 31, 3, 0},
- {550, 680, 1, 5, 31, 5, 0},
- {681, 850, 1, 5, 31, 5, 0},
- { 0, 333, 2, 5, 31, 3, 0},
- {334, 400, 2, 5, 31, 3, 0},
- {401, 549, 2, 5, 31, 3, 0},
- {550, 680, 2, 5, 31, 5, 0},
- {681, 850, 2, 5, 31, 5, 0},
Please use TABs for vertical alignment.
Ok, I understand now what you mean. However, the columns are right-justified. You can't use tabs to align right-justified columns.
+phys_size_t initdram(int board_type) +{
- phys_size_t dram_size = 0;
- puts("Initializing....\n");
- dram_size = fsl_ddr_sdram();
- dram_size = setup_ddr_tlbs(dram_size / 0x100000);
- dram_size *= 0x100000;
- puts(" DDR: ");
- return dram_size;
How about using get_ram_size() for autosizing / testing?
It appears get_ram_size() has never been used on PowerPC before. This function writes data to memory in blocks and reads it back. In my experience, attempting to access memory that doesn't exist will generate a machine check and cause U-Boot to hang.
In addition, we only create a TLB to map the lower 2GB, no matter how much memory is in the system. But the initdram() function returns the full size of RAM, no matter how big it is. So won't get_ram_size() always fail if I have more than 2GB of RAM?

Dear Timur Tabi,
In message 4BFC1736.5030902@freescale.com you wrote:
- { 0, 333, 1, 5, 31, 3, 0},
- {334, 400, 1, 5, 31, 3, 0},
- {401, 549, 1, 5, 31, 3, 0},
- {550, 680, 1, 5, 31, 5, 0},
- {681, 850, 1, 5, 31, 5, 0},
- { 0, 333, 2, 5, 31, 3, 0},
- {334, 400, 2, 5, 31, 3, 0},
- {401, 549, 2, 5, 31, 3, 0},
- {550, 680, 2, 5, 31, 5, 0},
- {681, 850, 2, 5, 31, 5, 0},
Please use TABs for vertical alignment.
Ok, I understand now what you mean. However, the columns are right-justified. You can't use tabs to align right-justified columns.
There is exactly two entries in that table where it would make any difference, and if that's really that important to you, adding two spaces after the TAB would work wonders.
How about using get_ram_size() for autosizing / testing?
It appears get_ram_size() has never been used on PowerPC before. This
It appears you really haven't bothered checking. We have been using it on PowerPC since day 1 - for well over 10 years by now. On many, many systems. Get a clue!
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
There is exactly two entries in that table where it would make any difference, and if that's really that important to you, adding two spaces after the TAB would work wonders.
The column headers would be unaligned:
static const board_specific_parameters_t bsp[] = { /* * memory controller 0 * lo| hi| num| clk| cpo|wrdata|2T * mhz| mhz|ranks|adjst| | delay| */ { 0, 333, 1, 5, 31, 3, 0}, {334, 400, 1, 5, 31, 3, 0}, {401, 549, 1, 5, 31, 3, 0}, {550, 680, 1, 5, 31, 5, 0}, {681, 850, 1, 5, 31, 5, 0}, { 0, 333, 2, 5, 31, 3, 0}, {334, 400, 2, 5, 31, 3, 0}, {401, 549, 2, 5, 31, 3, 0}, {550, 680, 2, 5, 31, 5, 0}, {681, 850, 2, 5, 31, 5, 0}, };
How about using get_ram_size() for autosizing / testing?
It appears get_ram_size() has never been used on PowerPC before. This
It appears you really haven't bothered checking. We have been using it on PowerPC since day 1 - for well over 10 years by now. On many, many systems. Get a clue!
Sorry, I must have used grep wrong or something. I see what you're talking about now.

Wolfgang Denk wrote:
It appears you really haven't bothered checking. We have been using it on PowerPC since day 1 - for well over 10 years by now. On many, many systems. Get a clue!
I've looked at get_ram_size(), and I don't think I can use it.
We use phys_addr_t to represent DDR sizes, which is a 64-bit integer on the P1022DS. get_ram_size() takes a 'long' and returns a 'long', so it's not capable of handling the amount of memory that we support on the board.
I can use get_ram_size() to test the lower 2GB of DDR, but I can't use it to verify the amount of RAM in the system.

Dear Timur Tabi,
In message 4BFD837D.2040508@freescale.com you wrote:
Wolfgang Denk wrote:
It appears you really haven't bothered checking. We have been using it on PowerPC since day 1 - for well over 10 years by now. On many, many systems. Get a clue!
I've looked at get_ram_size(), and I don't think I can use it.
Then fix it, please.
We use phys_addr_t to represent DDR sizes, which is a 64-bit integer on the P1022DS. get_ram_size() takes a 'long' and returns a 'long', so it's not capable of handling the amount of memory that we support on the board.
I can use get_ram_size() to test the lower 2GB of DDR, but I can't use it to verify the amount of RAM in the system.
Do you really have > 4 GB RAM on this board?
If this turns out to be a real problem, we have to fix get_ram_size to handle bigger memory sizes as well. I guess this will needed for more and more systems.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
I've looked at get_ram_size(), and I don't think I can use it.
Then fix it, please.
Let me guess, you won't accept my patch until I fix get_ram_size() first and use it, right?
We use phys_addr_t to represent DDR sizes, which is a 64-bit integer on the P1022DS. get_ram_size() takes a 'long' and returns a 'long', so it's not capable of handling the amount of memory that we support on the board.
I can use get_ram_size() to test the lower 2GB of DDR, but I can't use it to verify the amount of RAM in the system.
Do you really have > 4 GB RAM on this board?
Actually, anything more than 2GB would cause the problem, since the maximum value of a long is 2GB. On this particular board, there is only 2GB, but all of our new boards are expected to support more than 4GB of RAM.
If this turns out to be a real problem, we have to fix get_ram_size to handle bigger memory sizes as well. I guess this will needed for more and more systems.
Yes.

Dear Timur Tabi,
In message 4BFE823A.1080409@freescale.com you wrote:
I've looked at get_ram_size(), and I don't think I can use it.
Then fix it, please.
Let me guess, you won't accept my patch until I fix get_ram_size() first and use it, right?
Do you think this is an unreasonable request? A quick fix takes me less than it takes to write this email (see following patch).
Do you really have > 4 GB RAM on this board?
Actually, anything more than 2GB would cause the problem, since the maximum value of a long is 2GB. On this particular board, there is only 2GB, but all of our new boards are expected to support more than 4GB of RAM.
All the more reasons to fix this.
If this turns out to be a real problem, we have to fix get_ram_size to handle bigger memory sizes as well. I guess this will needed for more and more systems.
Yes.
Well, didn't you say you were looking for a list of task to imrpove U-Boot for your boards?
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
Do you think this is an unreasonable request? A quick fix takes me less than it takes to write this email (see following patch).
Fixing get_ram_size() is not a quick fix. We would need to implement a temporary TLB mechanism for accessing memory above 2GB, and a machine check handler for handling memory access to non-existent RAM.
If this turns out to be a real problem, we have to fix get_ram_size to handle bigger memory sizes as well. I guess this will needed for more and more systems.
Yes.
Well, didn't you say you were looking for a list of task to imrpove U-Boot for your boards?
True. But I also want to get real work done in a timely manner. It's taking a lot longer than I expected to get support for this board approved.

On Thu, May 27, 2010 at 01:25:38PM -0500, Timur Tabi wrote:
Wolfgang Denk wrote:
Do you think this is an unreasonable request? A quick fix takes me less than it takes to write this email (see following patch).
Fixing get_ram_size() is not a quick fix. We would need to implement a temporary TLB mechanism for accessing memory above 2GB, and a machine check handler for handling memory access to non-existent RAM.
Passing the actual, known size of RAM (why guess when we know?) as "maxsize" should eliminate the machine check problem[1] -- you'd just be using it as a not particularly exhaustive memory tester. I don't see why it should be mandatory.
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
-Scott
[1] It's worse than machine checks, what if some I/O device is mapped directly after RAM? IIRC people have run into this sort of problem doing this type of memory sizing on PCs.

Scott Wood wrote:
Passing the actual, known size of RAM (why guess when we know?) as "maxsize" should eliminate the machine check problem[1] -- you'd just be using it as a not particularly exhaustive memory tester. I don't see why it should be mandatory.
The purpose of get_ram_size() is to verify the "known size of RAM". That is, once you think you know how much RAM there is, you ask get_ram_size() to verify that claim. The return value is the true, validated amount of RAM.
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
Ah, that's a serious limitation.

On 05/27/2010 02:07 PM, Timur Tabi wrote:
Scott Wood wrote:
Passing the actual, known size of RAM (why guess when we know?) as "maxsize" should eliminate the machine check problem[1] -- you'd just be using it as a not particularly exhaustive memory tester. I don't see why it should be mandatory.
The purpose of get_ram_size() is to verify the "known size of RAM". That is, once you think you know how much RAM there is, you ask get_ram_size() to verify that claim.
It would still (sort of) verify that you don't have less than you claim. Attempting to experimentally determine whether you have more than you claim is dangerous.
-Scott

Dear Scott Wood,
In message 4BFEC39E.7040302@freescale.com you wrote:
Attempting to experimentally determine whether you have more than you claim is dangerous.
It's not. Actually it allows to detect certain classes of errors.
Best regards,
Wolfgang Denk

Dear Timur Tabi,
In message 4BFEC2FD.5050103@freescale.com you wrote:
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
Ah, that's a serious limitation.
No, it is not. Testing is done per bank.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
Ah, that's a serious limitation.
No, it is not. Testing is done per bank.
Ok, I get it now. I'm supposed to call get_ram_size() on each DIMM I locate. That's going to be complicated because we don't have any TLBs set up when we do that.
And what if we have a 4GB DIMM? Because virtual addresses are still 32-bits, we would need to change get_ram_size() to modify a temporary TLB while it's reading/writing memory. So we'd need to have an e300 version of get_ram_size(), an e500 version, an e600 version, and so on.
So I have a question: are you still going to require me to use get_ram_size() in my P1022DS board patch?

Dear Timur Tabi,
In message 4BFED1EA.5060104@freescale.com you wrote:
So I have a question: are you still going to require me to use get_ram_size() in my P1022DS board patch?
I never *reqired* this.
All I wrote was:
| > + puts(" DDR: "); | > + return dram_size; | | How about using get_ram_size() for autosizing / testing?
Best regards,
Wolfgang Denk

Dear Scott Wood,
In message 20100527190340.GA5915@schlenkerla.am.freescale.net you wrote:
Passing the actual, known size of RAM (why guess when we know?) as "maxsize" should eliminate the machine check problem[1] -- you'd just be using it as a not particularly exhaustive memory tester. I don't see why it should be mandatory.
Typically we chose "maxsize" to b twice the actual possible maximum to allow for real testing.
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
Such configurations are usually set up of from several differently sized banks of memory, and get_ram_size() is always run per bank. So as long as chip manufacturers continue to make RAM chips with power-of-two sizes only, everything should be fine.
[1] It's worse than machine checks, what if some I/O device is mapped directly after RAM? IIRC people have run into this sort of problem doing this type of memory sizing on PCs.
Well, let's call this a bug in setting up the memory map for the system ;-)
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
Such configurations are usually set up of from several differently sized banks of memory, and get_ram_size() is always run per bank. So as long as chip manufacturers continue to make RAM chips with power-of-two sizes only, everything should be fine.
What if the board has two DIMM slots, one of which has a 1GB DIMM and the other has a 512MB DIMM?
[1] It's worse than machine checks, what if some I/O device is mapped directly after RAM? IIRC people have run into this sort of problem doing this type of memory sizing on PCs.
Well, let's call this a bug in setting up the memory map for the system ;-)
I thought get_ram_size() was supposed to safely determine how much RAM is actually in the system? Otherwise, it should be called verify_ram_size().

Dear Timur Tabi,
In message 4BFED01F.1040703@freescale.com you wrote:
Such configurations are usually set up of from several differently sized banks of memory, and get_ram_size() is always run per bank. So as long as chip manufacturers continue to make RAM chips with power-of-two sizes only, everything should be fine.
What if the board has two DIMM slots, one of which has a 1GB DIMM and the other has a 512MB DIMM?
Then you run get_ram_size() twice, and it will find 1 GiB and 512 MiB of memory - what's the problem?
I thought get_ram_size() was supposed to safely determine how much RAM is actually in the system? Otherwise, it should be called verify_ram_size().
Indeed it is intended to determine tha actual RAM size - and does so fine in dozens of board configurations.
Best regards,
Wolfgang Denk

On 05/27/2010 02:53 PM, Wolfgang Denk wrote:
Dear Scott Wood,
In message20100527190340.GA5915@schlenkerla.am.freescale.net you wrote:
Passing the actual, known size of RAM (why guess when we know?) as "maxsize" should eliminate the machine check problem[1] -- you'd just be using it as a not particularly exhaustive memory tester. I don't see why it should be mandatory.
Typically we chose "maxsize" to b twice the actual possible maximum to allow for real testing.
If you set maxsize beyond what you expect to find, how are you going to constrain it to operating on one bank?
It also doesn't handle non-power-of-two sized memory -- don't rely on the value it returns.
Such configurations are usually set up of from several differently sized banks of memory, and get_ram_size() is always run per bank. So as long as chip manufacturers continue to make RAM chips with power-of-two sizes only, everything should be fine.
So it's not the board code at all that should be calling this, it's the SDRAM code? Which is already in u-boot, and not in this patch (other than some board-specific tweaks)?
[1] It's worse than machine checks, what if some I/O device is mapped directly after RAM? IIRC people have run into this sort of problem doing this type of memory sizing on PCs.
Well, let's call this a bug in setting up the memory map for the system ;-)
Let's not. It can be crowded enough as is, we don't need more restrictions coming from u-boot wanting to do questionable and unnecessary things.
-Scott

Dear Scott Wood,
In message 4BFED090.1040305@freescale.com you wrote:
If you set maxsize beyond what you expect to find, how are you going to constrain it to operating on one bank?
Maybe it helps if you read section "System Initialization" in teh README; this explains the original design when I implemented this a decade ago.
Such configurations are usually set up of from several differently sized banks of memory, and get_ram_size() is always run per bank. So as long as chip manufacturers continue to make RAM chips with power-of-two sizes only, everything should be fine.
So it's not the board code at all that should be calling this, it's the SDRAM code? Which is already in u-boot, and not in this patch (other than some board-specific tweaks)?
I don't know where you initialize the RAM.
Let's not. It can be crowded enough as is, we don't need more restrictions coming from u-boot wanting to do questionable and unnecessary things.
Well, what seems unnecessary to you is actually pretty important to others.
Best regards,
Wolfgang Denk

Dear Timur Tabi,
In message 4BFEB922.9040106@freescale.com you wrote:
Do you think this is an unreasonable request? A quick fix takes me less than it takes to write this email (see following patch).
Fixing get_ram_size() is not a quick fix. We would need to implement a temporary TLB mechanism for accessing memory above 2GB, and a machine check handler for handling memory access to non-existent RAM.
We don't have machine check handlers on the other PPC systems either. We kust make sure that the mapping is big enough for the maximal RAM size we are going to test.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
We don't have machine check handlers on the other PPC systems either. We kust make sure that the mapping is big enough for the maximal RAM size we are going to test.
What if the mapping is too big? Isn't the whole point of get_ram_size() that there is less RAM than we say there is? That is, if there's only 1GB of RAM, and we do this:
dram_size = get_ram_size(0, 2GB);
That get_ram_size() will return 1GB?

Dear Timur Tabi,
In message 4BFECDF7.4070405@freescale.com you wrote:
What if the mapping is too big? Isn't the whole point of get_ram_size() that there is less RAM than we say there is? That is, if there's only 1GB of RAM, and we do this:
get_ram_size() will also detect a number of hardware problems list stuck / swapped / mirrored address lines etc. (actually a pretty large percentage of the real world hardware problems).
Best regards,
Wolfgang Denk

On 05/27/2010 03:00 PM, Wolfgang Denk wrote:
Dear Timur Tabi,
In message4BFECDF7.4070405@freescale.com you wrote:
What if the mapping is too big? Isn't the whole point of get_ram_size() that there is less RAM than we say there is? That is, if there's only 1GB of RAM, and we do this:
get_ram_size() will also detect a number of hardware problems list stuck / swapped / mirrored address lines etc. (actually a pretty large percentage of the real world hardware problems).
How about splitting out the testing bit into something that will just test a specified range, and report any errors, rather than quietly pretend you have less RAM if the test fails?
-Scott

Add basic suport for the Freescale P1022DS reference board.
<snip>
+#elif defined(CONFIG_P1022) +static struct pci_info pci_config_info[] = +{
- [LAW_TRGT_IF_PCIE_1] = {
.agent = (1 << 0) | (1 << 1),
.cfg = (1 << 6) | (1 << 7) | (1 << 9) | (1 << 0xa) |
(1 << 0xb) | (1 << 0xd) | (1 << 0xe) |
(1 << 0xf) | (1 << 0x15) | (1 << 0x16) |
(1 << 0x17) | (1 << 0x18) | (1 << 0x19) |
(1 << 0x1a) | (1 << 0x1b) | (1 << 0x1c) |
(1 << 0x1d) | (1 << 0x1e) | (1 << 0x1f),
- },
Tiimur,
Did you grab the patch from our latest BSP tree?
It seems like your patch doesn't base on latest upstream tree, IIRC, the PCI stuff in mpc8xxx/pci_cfg.c is ready in upstream tree.
Did you have a test the patch on V2 board? The V2 board has big change from V1 board. Anyway I would like we have sufficient test on V2 board before it is merged to main tree.
Thanks, Dave

Liu Dave-R63238 wrote:
Did you grab the patch from our latest BSP tree?
No, I've been using the ltib patches. I guess I should use the repository on git.ap instead.
It seems like your patch doesn't base on latest upstream tree, IIRC, the PCI stuff in mpc8xxx/pci_cfg.c is ready in upstream tree.
Ah, I was using the P2020 as a basis, not the 8536. And I see Kumar just posted patches for the P2020.
Did you have a test the patch on V2 board? The V2 board has big change from V1 board. Anyway I would like we have sufficient test on V2 board before it is merged to main tree.
When I power on the board, it says this:
Board: P1022DS Sys ID: 0x19, Sys Ver: 0x02, FPGA Ver: 0x04, vBank: 0
How can I tell if I have a rev2 board? There's a sticker that says, "sch-26191 rev b"

On May 20, 2010, at 5:01 PM, Timur Tabi wrote:
diff --git a/arch/powerpc/cpu/mpc8xxx/pci_cfg.c b/arch/powerpc/cpu/mpc8xxx/pci_cfg.c index 85995ca..fc79fe1 100644 --- a/arch/powerpc/cpu/mpc8xxx/pci_cfg.c +++ b/arch/powerpc/cpu/mpc8xxx/pci_cfg.c @@ -186,6 +186,32 @@ static struct pci_info pci_config_info[] = (1 << 0x16) | (1 << 0x17) | (1 << 0x18) | (1 << 0x1c), }, }; +#elif defined(CONFIG_P1022) +static struct pci_info pci_config_info[] = +{
- [LAW_TRGT_IF_PCIE_1] = {
.agent = (1 << 0) | (1 << 1),
.cfg = (1 << 6) | (1 << 7) | (1 << 9) | (1 << 0xa) |
(1 << 0xb) | (1 << 0xd) | (1 << 0xe) |
(1 << 0xf) | (1 << 0x15) | (1 << 0x16) |
(1 << 0x17) | (1 << 0x18) | (1 << 0x19) |
(1 << 0x1a) | (1 << 0x1b) | (1 << 0x1c) |
(1 << 0x1d) | (1 << 0x1e) | (1 << 0x1f),
- },
- [LAW_TRGT_IF_PCIE_2] = {
.agent = (1 << 0) | (1 << 2),
.cfg = (1 << 0) | (1 << 1) | (1 << 6) | (1 << 7) |
(1 << 9) | (1 << 0xa) | (1 << 0xb) | (1 << 0xd) |
(1 << 0x15) | (1 << 0x16) | (1 << 0x17) |
(1 << 0x18) | (1 << 0x1c),
- },
- [LAW_TRGT_IF_PCIE_3] = {
.agent = (1 << 0) | (1 << 3),
.cfg = (1 << 6) | (1 << 7) | (1 << 9) | (1 << 0xd) |
(1 << 0x15) | (1 << 0x16) | (1 << 0x17) | (1 << 0x18) |
(1 << 0x19) | (1 << 0x1a) | (1 << 0x1b),
- },
+}; #elif defined(CONFIG_P2010) || defined(CONFIG_P2020) static struct pci_info pci_config_info[] = {
This is the old style that we want to move away from. I posted patches for the P2020 DS.
- k
participants (6)
-
Kumar Gala
-
Liu Dave-R63238
-
Scott Wood
-
Timur Tabi
-
Timur Tabi
-
Wolfgang Denk