Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Simon, On Tue, Sep 18, 2012 at 12:37 PM, Simon Glass wrote: > Hi Tom, > > On Wed, Sep 12, 2012 at 3:10 PM, Tom Warren wrote: >> Signed-off-by: Tom Warren >> --- >> arch/arm/cpu/arm720t/tegra30/Makefile | 48 +++ >> arch/arm/cpu/arm720t/tegra30/board.h | 25 ++ >> arch/arm/cpu/arm720t/tegra30/config.mk | 26 ++ >> arch/arm/cpu/arm720t/tegra30/cpu.c | 570 >> >> arch/arm/cpu/arm720t/tegra30/cpu.h | 65 >> arch/arm/cpu/arm720t/tegra30/spl.c | 132 > > It certainly has complicated your work, with the AVP arm720t refactor > going in before these patches. In some ways, yes. In others, it made it easy to first get a SPL (AVP) U-Boot up and printing its sign-on, and then having a base to work from/debug A9 code for the second, CPU-init half. > > I feel that quite a bit of the code here should perhaps go to > arch/arm/cpu/arm720t/tegra-common or similar, so that you can share it > with tegra30. WIP. There'll be an arch/arm/cpu/arm720t/tegra-common, an arch/arm/cpu/armv7/tegra-common, and even an arch/arm/cpu/tegra-common, plus an arch/arm/include/asm/arch-tegra to hold common code & include files. How those changes will be captured in a patchset (or 2) that show the movement of the original Tegra20 files, then the copy/edit for Tegra30 is still to be seen. Thanks, Tom > > Regards, > Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Hi Tom, On Wed, Sep 12, 2012 at 3:10 PM, Tom Warren wrote: > Signed-off-by: Tom Warren > --- > arch/arm/cpu/arm720t/tegra30/Makefile | 48 +++ > arch/arm/cpu/arm720t/tegra30/board.h | 25 ++ > arch/arm/cpu/arm720t/tegra30/config.mk | 26 ++ > arch/arm/cpu/arm720t/tegra30/cpu.c | 570 > > arch/arm/cpu/arm720t/tegra30/cpu.h | 65 > arch/arm/cpu/arm720t/tegra30/spl.c | 132 It certainly has complicated your work, with the AVP arm720t refactor going in before these patches. I feel that quite a bit of the code here should perhaps go to arch/arm/cpu/arm720t/tegra-common or similar, so that you can share it with tegra30. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Lucas,
On Thu, Sep 13, 2012 at 2:47 PM, Lucas Stach wrote:
> Hi Tom,
>
> Am Donnerstag, den 13.09.2012, 14:00 -0700 schrieb Tom Warren:
> [...]
>>
>> >> 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.
>
> I did the cleanup and you in fact missed it in the last round of tegra/next.
> Patch is called "tegra20: rework UART GPIO handling" and is already
> acked-by Simon Glass.
Thanks, I see it. Now that I've re-read it, I'm remembering that I was
waiting on Allen Martin to answer Stephen's question:
" .. it looks like both SPL and non-SPL end up calling
gpio_early_init_uart(); is that duplication correct or problematic?"
I'll apply it to /next regardless, since Simon seems to think it's OK.
BTW, I'm not sure how patchwork is supposed to operate, but it appears
that the originator of the patch needs to assign it to me, mark it as
under review, etc. I don't seem to have that power for patches I
haven't written. Is that how it works for other custodians? It would
really help me keep track of Tegra patches if they could be
marked/assigned to me so I can sort on 'em. Right now, I just use a
filter with 'tegra' in it, which doesn't always find all the patches I
need to know about.
Thanks,
Tom
>
> Thanks,
> Lucas
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Hi Tom,
Am Donnerstag, den 13.09.2012, 14:00 -0700 schrieb Tom Warren:
[...]
>
> >> 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.
I did the cleanup and you in fact missed it in the last round of tegra/next.
Patch is called "tegra20: rework UART GPIO handling" and is already
acked-by Simon Glass.
Thanks,
Lucas
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Stephen,
On Thu, Sep 13, 2012 at 1:02 PM, Stephen Warren 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
Re: [U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
On 09/12/2012 04:10 PM, Tom Warren wrote:
Patch descriptions would be nice.
> 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.
> +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.
> + /*
> + * 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?
> 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?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/9] Tegra: T30: Add AVP (arm720t) files
Signed-off-by: Tom Warren --- arch/arm/cpu/arm720t/tegra30/Makefile | 48 +++ arch/arm/cpu/arm720t/tegra30/board.h | 25 ++ arch/arm/cpu/arm720t/tegra30/config.mk | 26 ++ arch/arm/cpu/arm720t/tegra30/cpu.c | 570 arch/arm/cpu/arm720t/tegra30/cpu.h | 65 arch/arm/cpu/arm720t/tegra30/spl.c | 132 6 files changed, 866 insertions(+), 0 deletions(-) create mode 100644 arch/arm/cpu/arm720t/tegra30/Makefile create mode 100644 arch/arm/cpu/arm720t/tegra30/board.h create mode 100644 arch/arm/cpu/arm720t/tegra30/config.mk create mode 100644 arch/arm/cpu/arm720t/tegra30/cpu.c create mode 100644 arch/arm/cpu/arm720t/tegra30/cpu.h create mode 100644 arch/arm/cpu/arm720t/tegra30/spl.c diff --git a/arch/arm/cpu/arm720t/tegra30/Makefile b/arch/arm/cpu/arm720t/tegra30/Makefile new file mode 100644 index 000..96e722c --- /dev/null +++ b/arch/arm/cpu/arm720t/tegra30/Makefile @@ -0,0 +1,48 @@ +# +# (C) Copyright 2010-2012 Nvidia Corporation. +# +# (C) Copyright 2000-2008 +# Wolfgang Denk, DENX Software Engineering, w...@denx.de. +# +# See file CREDITS for list of people who contributed to this +# project. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307 USA +# + +include $(TOPDIR)/config.mk + +LIB= $(obj)lib$(SOC).o + +COBJS-y+= cpu.o +COBJS-$(CONFIG_SPL_BUILD) += spl.o + +SRCS := $(COBJS-y:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS-y)) + +all: $(obj).depend $(LIB) + +$(LIB):$(OBJS) + $(call cmd_link_o_target, $(OBJS)) + +# + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +# diff --git a/arch/arm/cpu/arm720t/tegra30/board.h b/arch/arm/cpu/arm720t/tegra30/board.h new file mode 100644 index 000..fc11a7b --- /dev/null +++ b/arch/arm/cpu/arm720t/tegra30/board.h @@ -0,0 +1,25 @@ +/* + * (C) Copyright 2010-2012 + * NVIDIA Corporation + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +void board_init_uart_f(void); +void gpio_config_uart(void); diff --git a/arch/arm/cpu/arm720t/tegra30/config.mk b/arch/arm/cpu/arm720t/tegra30/config.mk new file mode 100644 index 000..ca9c6ea --- /dev/null +++ b/arch/arm/cpu/arm720t/tegra30/config.mk @@ -0,0 +1,26 @@ +# +# (C) Copyright 2010-2012 +# NVIDIA Corporation +# +# (C) Copyright 2002 +# Gary Jennejohn, DENX Software Engineering, +# +# See file CREDITS for list of people who contributed to this +# project. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307 USA +# +USE_PRIVATE_LIBGCC = yes diff --git a/arch/arm/cpu/arm720t/tegra30/cpu.c b/arch/arm/cpu/arm720t/tegra30/cpu.c new file mode 100644 index 000..f7d9b87 --- /dev/null +++ b/arch/arm/cpu/arm720t/tegra30/cpu.c @@ -0,0 +1,570 @@ +/* + * (C) Copyright 2010-2012 + * NVID
