Albert, On Wed, Apr 13, 2011 at 1:30 PM, Albert ARIBAUD <albert.u.b...@aribaud.net> wrote: > Hi Tom, > > Le 13/04/2011 22:21, Tom Warren a écrit : > >>>> + >>>> + /* Wait for the power to come up */ >>>> + while (!is_cpu_powered()) >>>> + ; /* Do nothing */ >>> >>> What if power never comes up? >> >> Then the system is hung. I can put a printf here, if you'd like. > > Is the system hung? Can it really not proceed to the prompt? Anyway, at > least, yes, a printf would be welcome. It's hung because it's looping on the while ;. While it's possible we could limp along on the AVP only, that's not a viable config, and I don't know what a partially enabled CPU complex is going to do - interrupt storm, memory corruption, etc. I'll just put in a printf for the failure case. Thanks.
> >>> >>>> + /* >>>> + * Remove the I/O clamps from CPU power partition. >>>> + * Recommended only on a Warm boot, if the CPU partition >>>> gets >>>> + * power gated. Shouldn't cause any harm when called >>>> after >>>> a >>>> + * cold boot according to HW, probably just redundant. >>>> + */ >>>> + remove_cpu_io_clamps(); >>>> + } >>>> +} >>>> + >>>> +static void enable_cpu_power_rail(void) >>>> +{ >>>> + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE; >>>> + u32 reg; >>>> + >>>> + reg = readl(&pmc->pmc_cntrl); >>>> + reg |= CPUPWRREQ_OE; >>>> + writel(reg,&pmc->pmc_cntrl); >>>> + >>>> + /* >>>> + * The TI PMU65861C needs a 3.75ms delay between enabling >>>> + * the power rail and enabling the CPU clock. This delay >>>> + * between SM1EN and SM1 is for switching time + the ramp >>>> + * up of the voltage to the CPU (VDD_CPU from PMU). >>>> + */ >>>> + udelay(3750); >>>> +} >>>> + >>>> +static void reset_A9_cpu(int reset) >>>> +{ >>>> + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr >>>> *)NV_PA_CLK_RST_BASE; >>>> + u32 reg, cpu; >>>> + >>>> + /* >>>> + * NOTE: Regardless of whether the request is to hold the CPU in >>>> reset >>>> + * or take it out of reset, every processor in the CPU >>>> complex >>>> + * except the master (CPU 0) will be held in reset because >>>> the >>>> + * AVP only talks to the master. The AVP does not know >>>> that >>>> there >>>> + * are multiple processors in the CPU complex. >>>> + */ >>>> + >>>> + /* Hold CPU 1 in reset */ >>>> + cpu = SET_DBGRESET1 | SET_DERESET1 | SET_CPURESET1; >>>> + writel(cpu,&clkrst->crc_cpu_cmplx_set); >>>> + >>>> + reg = readl(&clkrst->crc_rst_dev_l); >>>> + if (reset) { >>>> + /* Now place CPU0 into reset */ >>>> + cpu |= SET_DBGRESET0 | SET_DERESET0 | SET_CPURESET0; >>>> + writel(cpu,&clkrst->crc_cpu_cmplx_set); >>>> + >>>> + /* Enable master CPU reset */ >>>> + reg |= SWR_CPU_RST; >>>> + } else { >>>> + /* Take CPU0 out of reset */ >>>> + cpu = CLR_DBGRESET0 | CLR_DERESET0 | CLR_CPURESET0; >>>> + writel(cpu,&clkrst->crc_cpu_cmplx_clr); >>>> + >>>> + /* Disable master CPU reset */ >>>> + reg&= ~SWR_CPU_RST; >>>> + } >>>> + >>>> + writel(reg,&clkrst->crc_rst_dev_l); >>>> +} >>>> + >>>> +static void clock_enable_coresight(int enable) >>>> +{ >>>> + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr >>>> *)NV_PA_CLK_RST_BASE; >>>> + u32 rst, clk, src; >>>> + >>>> + rst = readl(&clkrst->crc_rst_dev_u); >>>> + clk = readl(&clkrst->crc_clk_out_enb_u); >>>> + >>>> + if (enable) { >>>> + rst&= ~SWR_CSITE_RST; >>>> + clk |= CLK_ENB_CSITE; >>>> + } else { >>>> + rst |= SWR_CSITE_RST; >>>> + clk&= ~CLK_ENB_CSITE; >>>> + } >>>> + >>>> + writel(clk,&clkrst->crc_clk_out_enb_u); >>>> + writel(rst,&clkrst->crc_rst_dev_u); >>>> + >>>> + if (enable) { >>>> + /* >>>> + * Put CoreSight on PLLP_OUT0 (216 MHz) and divide it >>>> down >>>> by >>>> + * 1.5, giving an effective frequency of 144MHz. >>>> + * Set PLLP_OUT0 [bits31:30 = 00], and use a 7.1 divisor >>>> + * (bits 7:0), so 00000001b == 1.5 (n+1 + .5) >>>> + */ >>>> + src = CLK_DIVIDER(NVBL_PLLP_KHZ, 144000); >>>> + writel(src,&clkrst->crc_clk_src_csite); >>>> + >>>> + /* Unlock the CPU CoreSight interfaces */ >>>> + rst = 0xC5ACCE55; >>>> + writel(rst, CSITE_CPU_DBG0_LAR); >>>> + writel(rst, CSITE_CPU_DBG1_LAR); >>>> + } >>>> +} >>>> + >>>> +void start_cpu(u32 reset_vector) >>>> +{ >>>> + /* Enable VDD_CPU */ >>>> + enable_cpu_power_rail(); >>>> + >>>> + /* Hold the CPUs in reset */ >>>> + reset_A9_cpu(1); >>>> + >>>> + /* Disable the CPU clock */ >>>> + enable_cpu_clock(0); >>>> + >>>> + /* Enable CoreSight */ >>>> + clock_enable_coresight(1); >>>> + >>>> + /* >>>> + * Set the entry point for CPU execution from reset, >>>> + * if it's a non-zero value. >>>> + */ >>>> + if (reset_vector) >>>> + writel(reset_vector, EXCEP_VECTOR_CPU_RESET_VECTOR); >>>> + >>>> + /* Enable the CPU clock */ >>>> + enable_cpu_clock(1); >>>> + >>>> + /* If the CPU doesn't already have power, power it up */ >>>> + if (!is_cpu_powered()) >>>> + powerup_cpu(); >>> >>> For my education (I don't know Tegra2) haven't the AVP already enabled >>> the CPU power rail and waited 3.75 ms for it to come up? If so, what >>> could prevent the CPU from being powered now? >> >> True, this is just an additional check that was in the code I ported. >> I'll remove it as redundant. >> >>> >>>> + /* Take the CPU out of reset */ >>>> + reset_A9_cpu(0); >>>> +} >>>> + >>>> + >>>> +void halt_avp(void) >>>> +{ >>>> + for (;;) { >>>> + writel((HALT_COP_EVENT_JTAG | HALT_COP_EVENT_IRQ_1 \ >>>> + | HALT_COP_EVENT_FIQ_1 | (FLOW_MODE_STOP<<29)), >>>> + FLOW_CTLR_HALT_COP_EVENTS); >>>> + } >>> >>> Must the write be repeated indefinitely? Can it not be done once then >>> followed by an empty for(;;) ? >> >> IIRC, there was an additional infinite for (;;) in previous code, but >> that was removed to satisfy a reviewer. > > :) > >> I can change it if you insist, but I don't know if it's written this >> way (ported from legacy bootloader) on purpose, i.e. to keep the AVP >> from spontaneously waking up and executing code from SRAM, etc. If >> it's not a deal-breaker, I'd prefer to leave it as-is. > > That's ok, leave it as it is. Great. Thanks! > > Amicalement, > -- > Albert. > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot