[U-Boot] [PATCH V4] arm: Tegra2: add support for A9 CPU init

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

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.

>>
>>> +               /*
>>> +                * 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.

Amicalement,
-- 
Albert.