[U-Boot] [PATCH v4 3/8] mips: add base support for atheros ath79 based SOCs

[Wills Wang]

On 12/26/2015 03:28 PM, Marek Vasut wrote:
> On Friday, December 25, 2015 at 07:56:23 PM, Wills Wang wrote:
>> This patch enable work for ar933x SOC, tested on ar9331 board.
>>
>> Signed-off-by: Wills Wang <wills.wang at live.com>
>> ---
> [...]
>
>> +int arch_cpu_init(void)
>> +{
>> +	u32 val;
>> +
>> +	/*
>> +	 * Set GPIO10 (UART_SO) as output and enable UART,
>> +	 * BIT(15) in GPIO_FUNCTION_1 register must be written with 1
>> +	 */
>> +	val = readl(KSEG1ADDR(AR71XX_GPIO_BASE + AR71XX_GPIO_REG_OE));
>> +	val |= BIT(10);
>> +	writel(val, KSEG1ADDR(AR71XX_GPIO_BASE + AR71XX_GPIO_REG_OE));
>> +
>> +	val = readl(KSEG1ADDR(AR71XX_GPIO_BASE + AR71XX_GPIO_REG_FUNC));
>> +	val |= (AR933X_GPIO_FUNC_UART_EN | BIT(15));
>> +	writel(val, KSEG1ADDR(AR71XX_GPIO_BASE + AR71XX_GPIO_REG_FUNC));
>> +	return 0;
>> +}
> Pinmux should be done on a per-board basis, not per-CPU. Also, please use
> setbits_le32().
I don't find other more fit entry for pinctrl initialization.
We must set these IO pin before serial initialization.
Put it into board directory?
>> diff --git a/arch/mips/mach-ath79/ar933x/ddr_tap.S
>> b/arch/mips/mach-ath79/ar933x/ddr_tap.S new file mode 100644
>> index 0000000..18c57de
>> --- /dev/null
>> +++ b/arch/mips/mach-ath79/ar933x/ddr_tap.S
>> @@ -0,0 +1,268 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Wills Wang, <wills.wang at live.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +#include <asm/asm.h>
>> +#include <asm/regdef.h>
>> +#include <asm/mipsregs.h>
>> +#include <asm/addrspace.h>
>> +#include <asm/arch/ar71xx_regs.h>
>> +
>> +#define DRAM_K0(x)      KSEG0ADDR(x)
>> +#define DRAM_K1(x)      KSEG1ADDR(x)
>> +
>> +	.text
>> +	.set noreorder
>> +
>> +LEAF(ddr_tap_init)
>> +	/* Tap settings for the DDR */
>> +	li      t0, 0xffffffff
>> +	li      t1, DRAM_K0(0x500000)
>> +	sw      t0, 0x0(t1)
>> +	sw      t0, 0x4(t1)
>> +	sw      t0, 0x8(t1)
>> +	sw      t0, 0xc(t1)
>> +	nop
>> +	nop
> This should be C code, pretty please.
Must this be change to c code?
I think there should be no problem.
>
> [...]
>
>> diff --git a/arch/mips/mach-ath79/ar933x/lowlevel_init.S
>> b/arch/mips/mach-ath79/ar933x/lowlevel_init.S new file mode 100644
>> index 0000000..72509ca
>> --- /dev/null
>> +++ b/arch/mips/mach-ath79/ar933x/lowlevel_init.S
> lowlevel_init.S should be C code too, I don't see anything which would
> require this to be ASM .
I don't find SRAM in this chip, we need DDR memory to handle C runtime 
statck.
> [...]
>
>> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
>> new file mode 100644
>> index 0000000..681127c
>> --- /dev/null
>> +++ b/arch/mips/mach-ath79/cpu.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Wills Wang, <wills.wang at live.com>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/addrspace.h>
>> +#include <asm/types.h>
>> +#include <asm/arch/ath79.h>
>> +#include <asm/arch/ar71xx_regs.h>
>> +
>> +int print_cpuinfo(void)
>> +{
>> +	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
>> +	char *chip = "????";
>> +	u32 id, major, minor;
>> +	u32 rev = 0;
>> +	u32 ver = 1;
>> +
>> +	id = readl(KSEG1ADDR(AR71XX_RESET_BASE + AR71XX_RESET_REG_REV_ID));
>> +	major = id & REV_ID_MAJOR_MASK;
>> +
>> +	switch (major) {
>> +	case REV_ID_MAJOR_AR71XX:
>> +		minor = id & AR71XX_REV_ID_MINOR_MASK;
>> +		rev = id >> AR71XX_REV_ID_REVISION_SHIFT;
>> +		rev &= AR71XX_REV_ID_REVISION_MASK;
>> +		switch (minor) {
> I did review this already and my suggestions were ignored :-( I stop here ...
Sorry, I forget this.
This code inherit from kernel, to change would maintain it much more 
difficult.
>
> [...]
>
>