[U-Boot] [PATCH v7 1/7] mips: add base support for QCA/Atheros ath79 SOCs
Marek Vasut
marex at denx.de
Fri Jan 22 15:44:57 CET 2016
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 ?
> >> +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.
> >> + {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.
> >> + 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
> >
> > [...]
More information about the U-Boot
mailing list