From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Sat, 26 Dec 2015 08:28:52 +0100
Subject: [U-Boot] [PATCH v4 3/8] mips: add base support for atheros
	ath79 based SOCs
In-Reply-To: <BLU436-SMTP105B083068B85A93D641E40FFF80@phx.gbl>
References: <1451069788-6786-1-git-send-email-wills.wang@live.com>
 <BLU436-SMTP105B083068B85A93D641E40FFF80@phx.gbl>
Message-ID: <201512260828.52502.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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().

> 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@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.

[...]

> 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 .

[...]

> 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@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 ...

[...]