[U-Boot] [PATCH v5 2/5] mips: add support for QCA/Atheros ath79 SOCs

[Wills Wang]

On 12/30/2015 03:09 AM, Daniel Schwierzeck wrote:
> diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h
> index 2d9a0c9..3799e98 100644
> --- a/arch/mips/include/asm/global_data.h
> +++ b/arch/mips/include/asm/global_data.h
> @@ -20,6 +20,12 @@ struct arch_global_data {
>   	unsigned long tbl;
>   	unsigned long lastinc;
>   #endif
> +#ifdef CONFIG_ARCH_ATH79
> +	unsigned long id;
> +	unsigned long soc;
> +	unsigned long rev;
> +	unsigned long ver;
> +#endif
> do you really need this at this location? global_data is only intended for variables needed before and after relocation and should be kept as small as possible. You can always read SoC info values directly from the hardware registers if you need them. There is no value in caching those infos in global_data.
Yes, these values ware write in function  "print_cpuinfo", then u-boot 
run at ROM space, have no writable memory.
Their can't be acquired directly by read register, need code to make 
judgments.
> diff --git a/arch/mips/mach-ath79/ar933x/board.c b/arch/mips/mach-ath79/ar933x/board.c
> new file mode 100644
> index 0000000..b4a76e8
> --- /dev/null
> +++ b/arch/mips/mach-ath79/ar933x/board.c
> @@ -0,0 +1,42 @@
> +/*
> + * (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 <mach/ar71xx_regs.h>
> +#include <mach/ddr.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void uart_pin_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));
> +}
> +
> +
> +#ifdef CONFIG_BOARD_EARLY_INIT_F
> +int board_early_init_f(void)
> +{
> +	uart_pin_init();
> +	ddr_init();
> +	return 0;
> +}
> +#endif
> the content of this file should be moved to your board directory. As Marek already mentioned, PIN setup is board-specific. Probably not all AR933x based boards need to setup that UART pin.
For ar933x, all board need to call ddr_init, or board don't boot up.
ar933x have only one UART, we can't see any printout if uart_pin_init is 
missed.
>> diff --git a/arch/mips/mach-ath79/ar933x/clk.c b/arch/mips/mach-ath79/ar933x/clk.c
>> new file mode 100644
>> index 0000000..cfc4bb9
>> --- /dev/null
>> +++ b/arch/mips/mach-ath79/ar933x/clk.c
>> @@ -0,0 +1,86 @@
>> +/*
>> + * (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 <mach/ar71xx_regs.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static u32 ar933x_get_xtal(void)
>> +{
>> +	u32 val;
>> +
>> +	val = readl(KSEG1ADDR(AR71XX_RESET_BASE + AR933X_RESET_REG_BOOTSTRAP));
> now that you have implemented map_phymem(), you should always use it.
>
> const void __iomem *regs = map_physmem(AR71XX_RESET_BASE, AR71XX_RESET_SIZE, MAP_NOCACHE);
> ...
> val = readl(regs + AR933X_RESET_REG_BOOTSTRAP);
>
>
> I'm going to sync asm/io.h and others with the kernel. Thus your current code would not compile without warnings any more. Please fix all locations in your code.
>
>
> Apart from this you read the bootstrap settings in several places. You could create something like <mach/reset.h> and add a function like this
>
> static inline u32 ar933x_get_bootstrap(void)
> {
>      const void __iomem *regs = map_physmem(AR71XX_RESET_BASE, AR71XX_RESET_SIZE, MAP_NOCACHE);
>
>      return readl(regs + AR933X_RESET_REG_BOOTSTRAP);
> }
Ok.
>> +	if (val & AR933X_BOOTSTRAP_REF_CLK_40)
>> +		return 40000000;
>> +	else
>> +		return 25000000;
>> +}
>> +
>> +int get_serial_clock(void)
>> +{
>> +	return ar933x_get_xtal();
>> +}
>> +
>> +int get_clocks(void)
>> +{
>> +	u32 val, xtal, pll, div;
>> +
>> +	xtal = ar933x_get_xtal();
>> +	val = readl(KSEG1ADDR(AR71XX_PLL_BASE + AR933X_PLL_CPU_CONFIG_REG));
> readl(regs + ...) as mentioned above
Ok.
>> +
>> +	/* VCOOUT = XTAL * DIV_INT */
>> +	div = (val >> AR933X_PLL_CPU_CONFIG_REFDIV_SHIFT)
>> +			& AR933X_PLL_CPU_CONFIG_REFDIV_MASK;
>> +	pll = xtal / div;
>> +
>> +	/* PLLOUT = VCOOUT * (1/2^OUTDIV) */
>> +	div = (val >> AR933X_PLL_CPU_CONFIG_DIVINT_SHIFT)
>> +			& AR933X_PLL_CPU_CONFIG_DIVINT_MASK;
>> +	pll *= div;
>> +	div = (val >> AR933X_PLL_CPU_CONFIG_OUTDIV_SHIFT)
>> +			& AR933X_PLL_CPU_CONFIG_OUTDIV_MASK;
>> +	if (!div)
>> +		div = 1;
>> +	pll >>= div;
>> +
>> +	val = readl(KSEG1ADDR(AR71XX_PLL_BASE + AR933X_PLL_CLOCK_CTRL_REG));
> readl(regs + ...) as mentioned above
Ok.
> diff --git a/arch/mips/mach-ath79/ar933x/ddr.c b/arch/mips/mach-ath79/ar933x/ddr.c
> new file mode 100644
> index 0000000..3985299
> --- /dev/null
> +++ b/arch/mips/mach-ath79/ar933x/ddr.c
> @@ -0,0 +1,215 @@
> +/*
> + * (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 <mach/ar71xx_regs.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void ddr_init(void)
> +{
> +	void __iomem *regs = (void *)KSEG1ADDR(AR71XX_DDR_CTRL_BASE);
> map_physmem()
>
Ok.
>> +	u32 val;
>> +
>> +	writel(0x7fbc8cd0, regs + AR71XX_DDR_REG_CONFIG);
>> +	writel(0x9dd0e6a8, regs + AR71XX_DDR_REG_CONFIG2);
> do not use magic values. These should be defined somewhere with descriptive names.
>
Ok.
>> +
>> +	val = readl(KSEG1ADDR(AR71XX_RESET_BASE) +
>> +			AR933X_RESET_REG_BOOTSTRAP);
> use the suggested ar933x_get_bootstrap() function
Ok.
> +
> +void ddr_tap_tunning(void)
> do you mean ddr_tap_tuning() ?
Ok.
>
>> +{
>> +	void __iomem *regs = (void *)KSEG1ADDR(AR71XX_DDR_CTRL_BASE);
> map_physmem()
Ok.
> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
> new file mode 100644
> index 0000000..c03480d
> --- /dev/null
> +++ b/arch/mips/mach-ath79/cpu.c
> @@ -0,0 +1,173 @@
> +/*
> + * (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 = "????";
> const char *chip
Ok.
> diff --git a/arch/mips/mach-ath79/dram.c b/arch/mips/mach-ath79/dram.c
> new file mode 100644
> index 0000000..dbd56d2
> --- /dev/null
> +++ b/arch/mips/mach-ath79/dram.c
> @@ -0,0 +1,17 @@
> +/*
> + * (C) Copyright 2015
> + * Wills Wang, <wills.wang at live.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <linux/sizes.h>
> +#include <asm/addrspace.h>
> +#include <mach/ddr.h>
> +
> +phys_size_t initdram(int board_type)
> +{
> +	ddr_tap_tunning();
> +	return get_ram_size((void *)KSEG1, SZ_256M);
> is that information available in memory controller registers?
No.

-- 
Best Regards
Wills