Stephen,
On Thu, Sep 13, 2012 at 1:02 PM, Stephen Warren <swar...@wwwdotorg.org> wrote:
> On 09/12/2012 04:10 PM, Tom Warren wrote:
>
> Patch descriptions would be nice.
Sure, sorry. Not sure how much more info I can add beyond what's in
the commit msg, though, at least for this patch.
>
>> diff --git a/arch/arm/cpu/arm720t/tegra30/cpu.c
>> b/arch/arm/cpu/arm720t/tegra30/cpu.c
>
> There's quite a bit of Tegra20-support in this file. Can this file be
> shared with Tegra20 rather than forked and enhanced?
>
>> +/* Returns 1 if the current CPU executing is a Cortex-A9, else 0 */
>> +int cpu_is_cortexa9(void)
>> +{
>> + u32 id = readb(NV_PA_PG_UP_BASE + PG_UP_TAG_0);
>> + return id == (PG_UP_TAG_0_PID_CPU & 0xff);
>> +}
>
> Hmm. Given this is support for the AVP/COP running SPL, shouldn't this
> always be true? I thought Allen's SPL patches had cleaned this up.
Copied from tegra20/cpu.c - didn't notice it never gets called (same
for Tegra20). So it's vestigial and can be removed.
>
>> +static void enable_cpu_power_rail(void)
>> +{
>> + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
>> + u32 reg;
>> +
>> + debug("enable_cpu_power_rail entry\n");
>> + reg = readl(&pmc->pmc_cntrl);
>> + reg |= CPUPWRREQ_OE;
>> + writel(reg, &pmc->pmc_cntrl);
>> +
>> + /*
>> + * Pulse PWRREQ via I2C. We need to find out what this is
>> + * doing, tidy up the code and maybe find a better place for it.
>> + */
>> + tegra_i2c_ll_write_addr(0x005a, 0x0002);
>> + tegra_i2c_ll_write_data(0x2328, 0x0a02);
>> + udelay(1000);
>> + tegra_i2c_ll_write_data(0x0127, 0x0a02);
>> + udelay(10 * 1000);
>
> Those functions access the DVC I2C controller's register space, so
> presumably they're doing I2C accesses. Not all boards use the same PMIC,
> so it seems like we really do need to factor this out.
It's in the original internal T30 repo for Cardhu, with a comment from
Simon Glass that I edited somewhat. I haven't tried removing it to see
if the board still boots.
>
>> + /*
>> + * 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). We use 0xf00 as
>> + * is is ARM-friendly (can fit in a single ARMv4T mov immmediate
>> + * instruction).
>> + */
>> + udelay(3840);
>
> The Cardhu board at least does not use the TPS65861. At the very least
> the comment isn't quite right. Is this code needed?
>
No idea - it's in our internal T30 bringup repo, as well as Simon's. I
can try removing it and see if we power up consistently.
>> diff --git a/arch/arm/cpu/arm720t/tegra30/spl.c
>> b/arch/arm/cpu/arm720t/tegra30/spl.c
>
>> +void board_init_f(ulong dummy)
>> +{
>> + board_init_uart_f();
>> +
>> + /* Initialize periph GPIOs */
>> +#ifdef CONFIG_SPI_UART_SWITCH
>> + gpio_early_init_uart();
>> +#else
>> + gpio_config_uart();
>> +#endif
>
> Didn't we have patches to get rid of that mess and just use the same
> function consistently across all boards, or was that only discussed and
> never actually implemented?
Someone on the list talked about a cleanup (Lucas? Thierry?), but,
AFAIK, that never happened, or I would have put it in tegra/next. At
the very least, it's not needed in Tegra30/spl.c, so I'll remove
it/clean it up.
Thanks,
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
