[U-Boot] [PATCH v7 1/7] mips: add base support for QCA/Atheros ath79 SOCs

[Marek Vasut]

On Saturday, January 23, 2016 at 02:31:31 AM, Wills Wang wrote:
> On Friday, January 22, 2016 10:44 PM, Marek Vasut wrote:
> > On Friday, January 22, 2016 at 10:02:18 AM, Wills Wang wrote:
> >> On Sunday, January 17, 2016 03:19 AM, Marek Vasut wrote:
> >>> On Saturday, January 16, 2016 at 07:13:47 PM, Wills Wang wrote:
> >>> 
> >>> Commit message is missing.
> >>> 
> >>>> Signed-off-by: Wills Wang <wills.wang at live.com>
> >>>> ---
> >>>> 
> >>>> Changes in v7:
> >>>> - Use setbits_32
> >>>> - Fix include path for SoC specific headers
> >>>> 
> >>>> Changes in v6:
> >>>> - Move ar933x as separate patch
> >>>> - Add get_bootstrap in reset.c
> >>>> - Use map_physmem instead of KSEG1ADDR
> >>>> - Add arch_cpu_init for detect SOC type for early
> >>> 
> >>> [...]
> >>> 
> >>>> diff --git a/arch/mips/mach-ath79/Kconfig
> >>>> b/arch/mips/mach-ath79/Kconfig new file mode 100644
> >>>> index 0000000..df84876
> >>>> --- /dev/null
> >>>> +++ b/arch/mips/mach-ath79/Kconfig
> >>>> @@ -0,0 +1,10 @@
> >>>> +menu "QCA/Athroes 7xxx/9xxx platforms"
> >>>> +	depends on ARCH_ATH79
> >>>> +
> >>>> +config SYS_VENDOR
> >>>> +	default "ath79"
> >>> 
> >>> Vendor should be atheros I believe.
> >> 
> >> Now atheros was merged into qualcomm, ap121 and ap143, one belongs
> >> to atheros and the other to qualcomm.
> > 
> > So write qualcomm/atheros ? What's the problem ?
> 
> SYS_VENDOR is used to specify the folder

directory, this is not windows.

> name under board, i will change it into atheros-qualcomm or qca.

qca then.

> >>>> +config SYS_SOC
> >>>> +	default "ath79"
> >>>> +
> >>>> +endmenu
> >>>> diff --git a/arch/mips/mach-ath79/Makefile
> >>>> b/arch/mips/mach-ath79/Makefile new file mode 100644
> >>>> index 0000000..6203cf0
> >>>> --- /dev/null
> >>>> +++ b/arch/mips/mach-ath79/Makefile
> >>>> @@ -0,0 +1,7 @@
> >>>> +#
> >>>> +# SPDX-License-Identifier:	GPL-2.0+
> >>>> +#
> >>>> +
> >>>> +obj-y += reset.o
> >>>> +obj-y += cpu.o
> >>>> +obj-y += dram.o
> >>>> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
> >>>> new file mode 100644
> >>>> index 0000000..d8910a0
> >>>> --- /dev/null
> >>>> +++ b/arch/mips/mach-ath79/cpu.c
> >>>> @@ -0,0 +1,203 @@
> >>>> +/*
> >>>> + * Copyright (C) 2015-2016 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/ath79.h>
> >>>> +#include <mach/ar71xx_regs.h>
> >>>> +
> >>>> +struct ath79_soc_desc {
> >>>> +	enum ath79_soc_type soc;
> >>>> +	const char *chip;
> >>>> +};
> >>>> +
> >>>> +static struct ath79_soc_desc desc[] = {
> >>>> +	{ATH79_SOC_AR7130,      "7130"},
> >>>> +	{ATH79_SOC_AR7141,      "7141"},
> >>>> +	{ATH79_SOC_AR7161,      "7161"},
> >>> 
> >>> Just curious, were 7161 chips ever tested ?
> >> 
> >> These id rules are verified by kernel and openwrt project,
> >> Of course, we can also remove support for these chips that faded out the
> >> market.
> > 
> > That's not answering my question, I am asking if the code was tested on
> > those chips at all or not.
> 
> Not test.

OK, which SoCs were tested ?

> >>>> +	{ATH79_SOC_AR7240,      "7240"},
> >>>> +	{ATH79_SOC_AR7242,      "7242"},
> >>>> +	{ATH79_SOC_AR9130,      "9130"},
> >>>> +	{ATH79_SOC_AR9132,      "9132"},
> >>>> +	{ATH79_SOC_AR9330,      "9330"},
> >>>> +	{ATH79_SOC_AR9331,      "9331"},
> >>>> +	{ATH79_SOC_AR9341,      "9341"},
> >>>> +	{ATH79_SOC_AR9342,      "9342"},
> >>>> +	{ATH79_SOC_AR9344,      "9344"},
> >>>> +	{ATH79_SOC_QCA9533,     "9533"},
> >>>> +	{ATH79_SOC_QCA9556,     "9556"},
> >>>> +	{ATH79_SOC_QCA9558,     "9558"},
> >>>> +	{ATH79_SOC_TP9343,      "9343"},
> >>>> +	{ATH79_SOC_QCA9561,     "9561"},
> >>>> +};
> >>>> +
> >>>> +int arch_cpu_init(void)
> >>>> +{
> >>>> +	void __iomem *base;
> >>>> +	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
> >>>> +	u32 id, major, minor;
> >>>> +	u32 rev = 0;
> >>>> +	u32 ver = 1;
> >>>> +
> >>>> +	base = map_physmem(AR71XX_RESET_BASE, AR71XX_RESET_SIZE,
> >>>> +			   MAP_NOCACHE);
> >>>> +
> >>>> +	id = readl(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) {
> >>>> +		case AR71XX_REV_ID_MINOR_AR7130:
> >>>> +			soc = ATH79_SOC_AR7130;
> >>>> +			break;
> >>>> +
> >>>> +		case AR71XX_REV_ID_MINOR_AR7141:
> >>>> +			soc = ATH79_SOC_AR7141;
> >>>> +			break;
> >>>> +
> >>>> +		case AR71XX_REV_ID_MINOR_AR7161:
> >>>> +			soc = ATH79_SOC_AR7161;
> >>>> +			break;
> >>>> +		}
> >>>> +		break;
> >>> 
> >>> This could easily be a lookup-table instead of such big switch
> >>> statement.
> >> 
> >> This has already been explained in v4, i have tried to use lookup-table,
> >> but the code is not more beautiful than now, the bit filed of id is not
> >> regular.
> >> 
> >> The shift bits of minor and revision is different for same chips, same
> >> chips
> >> should ignore the minor bits,  the lookup table must store these shift
> >> information and which was ignored in common, so the table will become
> >> more complex if this code want to be compatible with many other SOCs.
> > 
> > I don't see a problem with storing offset and mask in the table.
> > 
> >> If people want to add other QCA's SoC, need spend a great deal of time
> >> to debug this code.
> > 
> > Why? They'd just add the necessary entry into the table.
> 
> The bit filed of id register is not regular, they need modify this lookup
> table sometimes.

Major seems pretty regular to me, minor might be different.

> >>>> +	case REV_ID_MAJOR_AR7240:
> >>>> +		soc = ATH79_SOC_AR7240;
> >>>> +		rev = id & AR71XX_REV_ID_REVISION_MASK;
> >>>> +		break;
> >>> 
> >>> [...]
> >>> 
> >>>> diff --git a/arch/mips/mach-ath79/include/mach/ar71xx_regs.h
> >>>> b/arch/mips/mach-ath79/include/mach/ar71xx_regs.h new file mode 100644
> >>>> index 0000000..5e80eaf
> >>>> --- /dev/null
> >>>> +++ b/arch/mips/mach-ath79/include/mach/ar71xx_regs.h
> >>>> @@ -0,0 +1,1187 @@
> >>>> +/*
> >>>> + * Atheros AR71XX/AR724X/AR913X SoC register definitions
> >>>> + *
> >>>> + * Copyright (C) 2015-2016 Wills Wang <wills.wang at live.com>
> >>>> + * Copyright (C) 2010-2011 Jaiganesh Narayanan
> >>>> <jnarayanan at atheros.com> + * Copyright (C) 2008-2010 Gabor Juhos
> >>>> <juhosg at openwrt.org>
> >>>> + * Copyright (C) 2008 Imre Kaloz <kaloz at openwrt.org>
> >>>> + *
> >>>> + * SPDX-License-Identifier:     GPL-2.0+
> >>>> + */
> >>>> +
> >>>> +#ifndef __ASM_MACH_AR71XX_REGS_H
> >>>> +#define __ASM_MACH_AR71XX_REGS_H
> >>>> +
> >>>> +#ifndef __ASSEMBLY__
> >>>> +#include <linux/bitops.h>
> >>>> +#else
> >>>> +#ifndef BIT
> >>>> +#define BIT(nr)                 (1 << (nr))
> >>> 
> >>> Linux defines the macro as (1ul << (nr))
> >> 
> >> This header is also used by assembler,  the assembler don't recognize
> >> label 1ul.
> > 
> > In that case, this should be fixed in bitops.h and brought up in the
> > kernel mailing list. Otherwise, we will develop unnecessary boilerplate
> > code soon.
> > 
> >>>> +#endif
> >>>> +#endif
> >>> 
> >>> [...]