[U-Boot] [PATCH 1/4] mips: Add basic MediaTek MT7620/88 support

[Daniel Schwierzeck <daniel.schwierzeck@gmail.com>]

Am Do., 9. Aug. 2018 um 12:46 Uhr schrieb Stefan Roese <sr@denx.de>:
>
> Hi Daniel,
>
> On 07.08.2018 17:44, Daniel Schwierzeck wrote:
> > 2018-08-06 17:11 GMT+02:00 Stefan Roese <sr@denx.de>:
> >> This patch adds basic support for the MediaTek MT7620/88 SoCs. Parts of
> >> the code is copied from the MediaTek GitHub repository:
> >>
> >> https://github.com/MediaTek-Labs/linkit-smart-uboot.git
> >>
> >> Support for the LinkIt Smart 7688 module and the Gardena Smart Gateway
> >> both based on the MT7688 will be added in further patches.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >> ---
> >>   arch/mips/Kconfig                     |  15 ++
> >>   arch/mips/Makefile                    |   1 +
> >>   arch/mips/mach-mt7620/Kconfig         | 130 ++++++++++
> >>   arch/mips/mach-mt7620/Makefile        |   8 +
> >>   arch/mips/mach-mt7620/cpu.c           |  66 +++++
> >>   arch/mips/mach-mt7620/ddr_calibrate.c | 331 +++++++++++++++++++++++++
> >>   arch/mips/mach-mt7620/lowlevel_init.S | 337 ++++++++++++++++++++++++++
> >>   arch/mips/mach-mt7620/mt76xx.h        |  39 +++
> >>   8 files changed, 927 insertions(+)
> >>   create mode 100644 arch/mips/mach-mt7620/Kconfig
> >>   create mode 100644 arch/mips/mach-mt7620/Makefile
> >>   create mode 100644 arch/mips/mach-mt7620/cpu.c
> >>   create mode 100644 arch/mips/mach-mt7620/ddr_calibrate.c
> >>   create mode 100644 arch/mips/mach-mt7620/lowlevel_init.S
> >>   create mode 100644 arch/mips/mach-mt7620/mt76xx.h
> >>
> >> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> >> index 31b622ff51..af791c320d 100644
> >> --- a/arch/mips/Kconfig
> >> +++ b/arch/mips/Kconfig
> >> @@ -87,6 +87,20 @@ config ARCH_BMIPS
> >>          select SYSRESET
> >>          imply CMD_DM
> >>
> >> +config ARCH_MT7620
> >> +       bool "Support MT7620/7688 SoCs"
> >> +       select SUPPORTS_LITTLE_ENDIAN
> >> +       select SUPPORTS_CPU_MIPS32_R1
> >> +       select SUPPORTS_CPU_MIPS32_R2
> >> +       select ROM_EXCEPTION_VECTORS
> >> +       select MIPS_TUNE_4KC
> >
> > MT7620 has a MIPS 24kc core, thus you could optimize for that
>
> Okay.
>
> >> +       select OF_CONTROL
> >> +       select DM
> >> +       select DM_SERIAL
> >> +       select DM_SPI
> >> +       select DM_SPI_FLASH
> >> +       select DISPLAY_CPUINFO
> >> +
> >
> > could you sort this in alphabetic order?
>
> Okay, will do in v2.
>
> > There were some patches recently
> > which did the sorting tree wide. Also drop the SPI options until you add a
> > SPI driver.
>
> The SPI driver is also on the list:
>
> http://patchwork.ozlabs.org/patch/954365/

ok, I didn't received this one. But since SPI is not strictly
required, you should prefer "imply" instead of a hard "select".

>
> >>   config MACH_PIC32
> >>          bool "Support Microchip PIC32"
> >>          select DM
> >> @@ -141,6 +155,7 @@ source "board/qemu-mips/Kconfig"
> >>   source "arch/mips/mach-ath79/Kconfig"
> >>   source "arch/mips/mach-bmips/Kconfig"
> >>   source "arch/mips/mach-pic32/Kconfig"
> >> +source "arch/mips/mach-mt7620/Kconfig"
> >>
> >>   if MIPS
> >>
> >> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> >> index 5deec9a202..cc8ea5d7d4 100644
> >> --- a/arch/mips/Makefile
> >> +++ b/arch/mips/Makefile
> >> @@ -15,6 +15,7 @@ machine-$(CONFIG_SOC_AU1X00) += au1x00
> >>   machine-$(CONFIG_ARCH_ATH79) += ath79
> >>   machine-$(CONFIG_ARCH_BMIPS) += bmips
> >>   machine-$(CONFIG_MACH_PIC32) += pic32
> >> +machine-$(CONFIG_ARCH_MT7620) += mt7620
> >>
> >>   machdirs := $(patsubst %,arch/mips/mach-%/,$(machine-y))
> >>   libs-y += $(machdirs)
> >> diff --git a/arch/mips/mach-mt7620/Kconfig b/arch/mips/mach-mt7620/Kconfig
> >> new file mode 100644
> >> index 0000000000..40e15b8a31
> >> --- /dev/null
> >> +++ b/arch/mips/mach-mt7620/Kconfig
> >> @@ -0,0 +1,130 @@
> >> +menu "MediaTek MIPS platforms"
> >> +       depends on ARCH_MT7620
> >> +
> >> +config SYS_MALLOC_F_LEN
> >> +       default 0x1000
> >> +
> >> +config SYS_SOC
> >> +       default "mt7620" if SOC_MT7620
> >> +
> >> +choice
> >> +       prompt "MediaTek MIPS SoC select"
> >> +
> >> +config SOC_MT7620
> >> +       bool "MT7620/8"
> >> +       select MIPS_L1_CACHE_SHIFT_5
> >> +       help
> >> +         This supports MediaTek MIPS MT7620 family.
> >> +
> >> +endchoice
> >> +
> >> +choice
> >> +       prompt "Board select"
> >> +
> >> +config BOARD_LINKIT_SMART_7688
> >> +       bool "LinkIt Smart 7688"
> >> +       depends on SOC_MT7620
> >> +       select SUPPORTS_BOOT_RAM
> >> +       help
> >> +         Seeed LinkIt Smart 7688 boards have a MT7688 SoC with 128 MiB of RAM
> >> +         and 32 MiB of flash (SPI).
> >> +         Between its different peripherals there's an integrated switch with 4
> >> +         ethernet ports, 1 USB port, 1 UART, GPIO buttons and LEDs, and
> >> +         a MT7688 (PCIe).
> >
> > could you add this part with patch 3/4?
>
> Ah, yes. This slipped in too early.
>
> >> +
> >> +endchoice
> >> +
> >> +choice
> >> +       prompt "Boot mode"
> >> +
> >> +config BOOT_RAM
> >> +       bool "RAM boot"
> >> +       depends on SUPPORTS_BOOT_RAM
> >> +       select SKIP_LOWLEVEL_INIT
> >> +       help
> >> +         This builds an image that is linked to a RAM address. It can be used
> >> +         for booting from CFE via TFTP using an ELF image, but it can also be
> >> +         booted from RAM by other bootloaders using a BIN image.
> >> +
> >> +config BOOT_ROM
> >> +       bool "ROM boot"
> >> +       depends on SUPPORTS_BOOT_RAM
> >> +       help
> >> +         This builds an image that is linked to a ROM address. It can be
> >> +         used as main bootloader image which is programmed onto the onboard
> >> +         flash storage (SPI NOR).
> >> +
> >> +endchoice
> >> +
> >> +choice
> >> +       prompt "DDR2 size"
> >> +
> >> +config ONBOARD_DDR2_SIZE_256MBIT
> >> +       bool "256MBit (32MByte) total size"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 256MBit (32MByte) of DDR total size
> >> +
> >> +config ONBOARD_DDR2_SIZE_512MBIT
> >> +       bool "512MBit (64MByte) total size"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 512MBit (64MByte) of DDR total size
> >> +
> >> +config ONBOARD_DDR2_SIZE_1024MBIT
> >> +       bool "1024MBit (128MByte) total size"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 1024MBit (128MByte) of DDR total size
> >> +
> >> +config ONBOARD_DDR2_SIZE_2048MBIT
> >> +       bool "2048MBit (256MByte) total size"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 2048MBit (256MByte) of DDR total size
> >> +
> >> +endchoice
> >> +
> >> +choice
> >> +       prompt "DDR2 chip width"
> >> +
> >> +config ONBOARD_DDR2_CHIP_WIDTH_8BIT
> >> +       bool "8bit DDR chip width"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use DDR chips with 8bit width
> >> +
> >> +config ONBOARD_DDR2_CHIP_WIDTH_16BIT
> >> +       bool "16bit DDR chip width"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use DDR chips with 16bit width
> >> +
> >> +endchoice
> >> +
> >> +choice
> >> +       prompt "DDR2 bus width"
> >> +
> >> +config ONBOARD_DDR2_BUS_WIDTH_16BIT
> >> +       bool "16bit DDR bus width"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 16bit DDR bus width
> >> +
> >> +config ONBOARD_DDR2_BUS_WIDTH_32BIT
> >> +       bool "32bit DDR bus width"
> >> +       depends on BOOT_ROM
> >> +       help
> >> +         Use 32bit DDR bus width
> >> +
> >> +endchoice
> >> +
> >> +config SUPPORTS_BOOT_RAM
> >> +       bool
> >> +
> >> +config SKIP_LOWLEVEL_INIT
> >> +       bool
> >
> > this is a legacy config option, thus you have to define it in your
> > board config header file.
> > MIPS shouldn't add a Kconfig symbol for that. This have to be migrated
> > to Kconfig tree-wide.
>
> Will move to config header for now.
>
> >> +
> >> +source "board/seeed/linkit-smart-7688/Kconfig"
> >
> > could you add this part with patch 3/4?
>
> Sure.
>
> >> +
> >> +endmenu
> >> diff --git a/arch/mips/mach-mt7620/Makefile b/arch/mips/mach-mt7620/Makefile
> >> new file mode 100644
> >> index 0000000000..1f3e65e8a5
> >> --- /dev/null
> >> +++ b/arch/mips/mach-mt7620/Makefile
> >> @@ -0,0 +1,8 @@
> >> +# SPDX-License-Identifier: GPL-2.0+
> >> +
> >> +obj-y += cpu.o
> >> +
> >> +ifndef CONFIG_SKIP_LOWLEVEL_INIT
> >> +obj-y += ddr_calibrate.o
> >> +obj-y += lowlevel_init.o
> >> +endif
> >> diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c
> >> new file mode 100644
> >> index 0000000000..0b22956499
> >> --- /dev/null
> >> +++ b/arch/mips/mach-mt7620/cpu.c
> >> @@ -0,0 +1,66 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (C) 2018 Stefan Roese <sr@denx.de>
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <dm.h>
> >> +#include <ram.h>
> >> +#include <asm/io.h>
> >> +#include <linux/io.h>
> >> +#include <linux/sizes.h>
> >> +#include "mt76xx.h"
> >> +
> >> +#define STR_LEN                        6
> >> +
> >> +#ifdef CONFIG_BOOT_ROM
> >> +int mach_cpu_init(void)
> >> +{
> >> +       void (*ptr)(void);
> >> +
> >> +       /*
> >> +        * DDR calibration routine needs to be called very early. This
> >> +        * function also configures the clock to run at full speed.
> >> +        */
> >> +       ptr = (void *)CKSEG0ADDR(ddr_calibrate);
> >> +       (*ptr)();
> >
> > what is the purpose of forcing the function symbol to KSEG0?
>
> Its copied from the original MediaTek code. I just tested it without
> forcing the execution into KSEG0 and the DDR calibration is extremely
> slow then, taking a few minutes to complete.
>
> I have to admit that I am not 100% sure, if the caches are configured
> 100% correctly / optimally for this SoC. Perhaps you (or someone else)
> has some improvements here.

I guess the BootROM does some XiP magic with the SPI flash controller
and runs in the uncached KSEG1 segment.
That would explain why the pre-relocation code runs so slowly. But
this shift from KSEG1 to KSEG0 could be done
generically in start.S after the cache initialization is complete. I
will have a look at it.

>
> >> +
> >> +       return 0;
> >> +}
> >> +#endif
> >> +
> >> +int print_cpuinfo(void)
> >> +{
> >> +       static const char * const boot_str[] = { "PLL (3-Byte SPI Addr)",
> >> +                                                "PLL (4-Byte SPI Addr)",
> >> +                                                "XTAL (3-Byte SPI Addr)",
> >> +                                                "XTAL (4-Byte SPI Addr)" };
> >> +       char *chr = (char *)MT76XX_CHIPID;
> >> +       char buf[STR_LEN + 1];
> >> +       u32 val;
> >> +
> >> +       snprintf(buf, STR_LEN + 1, "%s", chr);
> >> +       val = readl((void *)MT76XX_CHIP_REV_ID);
> >> +       printf("CPU:   %-*s Rev %ld.%ld - ", STR_LEN, buf,
> >> +              (val & GENMASK(11, 8)) >> 8, val & GENMASK(3, 0));
> >> +
> >> +       val = (readl((void *)MT76XX_SYSCFG0) & GENMASK(3, 1)) >> 1;
> >> +       printf("Boot from %s\n", boot_str[val]);
> >
> > could you try to create a CPU driver like on BMIPS?
>
> Do you really think this is necessary just for printing some SoC
> information here?

according to the datasheet those registers are in the memory space of
the system controller block. Thus you should create at least a syscon
driver. Also the device tree you're going to add references this
syscon device.

>
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +void _machine_restart(void)
> >> +{
> >> +       writel(0x01, (void *)MT76XX_RSTCTL);
> >> +
> >> +       while (1)
> >> +               ;       /* NOP */
> >
> > could you try to create a reset controller driver? Then you could also
> > use the generic syscon-reboot driver.
>
> Yes, I also thought about this. Will do in v2.
>
> >> +}
> >> +
> >> +int dram_init(void)
> >> +{
> >> +       gd->ram_size = get_ram_size((void *)CONFIG_SYS_SDRAM_BASE, SZ_256M);
> >
> > shouldn't you return the real configured DRAM size here?
>
> I always like using get_ram_size() as it automatically detects the
> RAM size. This might be different (smaller) than the one configured,
> if HW problems with the DDR interface exist.
>
> We might think about printing a warning, if the detected size does not
> match the configured one.
>
> >> +
> >> +       return 0;
> >> +}
> >> diff --git a/arch/mips/mach-mt7620/ddr_calibrate.c b/arch/mips/mach-mt7620/ddr_calibrate.c
> >> new file mode 100644
> >> index 0000000000..6fd136674e
> >> --- /dev/null
> >> +++ b/arch/mips/mach-mt7620/ddr_calibrate.c
> >> @@ -0,0 +1,331 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (C) 2018 Stefan Roese <sr@denx.de>
> >> + *
> >> + * This code is mostly based on the code extracted from this MediaTek
> >> + * github repository:
> >> + *
> >> + * https://github.com/MediaTek-Labs/linkit-smart-uboot.git
> >> + *
> >> + * I was not able to find a specific license or other developers
> >> + * copyrights here, so I can't add them here.
> >> + *
> >> + * Most functions in this file are copied from the MediaTek U-Boot
> >> + * repository. Without any documentation, it was impossible to really
> >> + * implement this differently. So its mostly a cleaned-up version of
> >> + * the original code, with only support for the MT7628 / MT7688 SoC.
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <linux/io.h>
> >> +#include <asm/cacheops.h>
> >> +#include <asm/io.h>
> >> +#include "mt76xx.h"
> >> +
> >> +#define NUM_OF_CACHELINE       64
> >> +#define MIN_START              6
> >> +#define MIN_FINE_START         0xf
> >> +#define MAX_START              7
> >> +#define MAX_FINE_START         0x0
> >> +
> >> +#define CPU_FRAC_DIV           1
> >> +
> >> +#if defined(CONFIG_ONBOARD_DDR2_SIZE_256MBIT)
> >> +#define DRAM_BUTTOM 0x02000000
> >> +#endif
> >> +#if defined(CONFIG_ONBOARD_DDR2_SIZE_512MBIT)
> >> +#define DRAM_BUTTOM 0x04000000
> >> +#endif
> >> +#if defined(CONFIG_ONBOARD_DDR2_SIZE_1024MBIT)
> >> +#define DRAM_BUTTOM 0x08000000
> >> +#endif
> >> +#if defined(CONFIG_ONBOARD_DDR2_SIZE_2048MBIT)
> >> +#define DRAM_BUTTOM 0x10000000
> >> +#endif
> >> +
> >> +static inline void cal_memcpy(void *src, void *dst, u32 size)
> >> +{
> >> +       u8 *psrc = (u8 *)src;
> >> +       u8 *pdst = (u8 *)dst;
> >> +       int i;
> >> +
> >> +       for (i = 0; i < size; i++, psrc++, pdst++)
> >> +               *pdst = *psrc;
> >> +}
> >> +
> >> +static inline void cal_memset(void *src, u8 pat, u32 size)
> >> +{
> >> +       u8 *psrc = (u8 *)src;
> >> +       int i;
> >> +
> >> +       for (i = 0; i < size; i++, psrc++)
> >> +               *psrc = pat;
> >> +}
> >> +
> >> +#define pref_op(hint, addr)                                            \
> >> +       __asm__ __volatile__(                                           \
> >> +               ".set   push\n"                                         \
> >> +               ".set   noreorder\n"                                    \
> >> +               "pref   %0, %1\n"                                       \
> >> +               ".set   pop\n"                                          \
> >> +               :                                                       \
> >> +               : "i" (hint), "R" (*(u8 *)(addr)))
> >> +
> >> +#define cache_op(op, addr)                                             \
> >> +       __asm__ __volatile__(                                           \
> >> +               ".set   push\n"                                         \
> >> +               ".set   noreorder\n"                                    \
> >> +               ".set   mips3\n"                                        \
> >> +               "cache  %0, %1\n"                                       \
> >> +               ".set   pop\n"                                          \
> >> +               :                                                       \
> >> +               : "i" (op), "R" (*(u8 *)(addr)))
> >
> > cache_op() is already available in arch/mips/include/asm/cacheops.h.
>
> Hmm, Where exactly?

sorry, I meant mips_cache()

>
> > Maybe you could add pref_op in the same way to that file.
>
> Could make sense, but please see above. Again, I'm still pretty new to
> MIPS and especially the caches.

I noticed prefetch() is already defined in
arch/mips/include/asm/processor.h. You could utilize it if you add a
boolean Kconfig option CONFIG_CPU_HAS_PREFETCH in arch/mips/Kconfig
and select it for MT7620.

>
> >> +
> >> +static inline void cal_invalidate_dcache_range(u32 start_addr, u32 stop)
> >> +{
> >> +       u32 lsize = CONFIG_SYS_CACHELINE_SIZE;
> >> +       u32 addr = start_addr & ~(lsize - 1);
> >> +       u32 aend = (stop - 1) & ~(lsize - 1);
> >> +
> >> +       while (1) {
> >> +               cache_op(HIT_INVALIDATE_D, addr);
> >> +               if (addr == aend)
> >> +                       break;
> >> +               addr += lsize;
> >> +       }
> >> +}
> >
> > you should use the generic MIPS implementation instead
>
> Yes, makes sense.
>
> Thanks,
> Stefan



--
- Daniel