Hi, On Sun, Nov 16, 2014 at 1:17 AM, Kevin Cernekee <cernekee@xxxxxxxxx> wrote: > bmips_be_defconfig supports Linux running on the following CM and DSL > SoCs: > > - BCM3384 (BMIPS5000) cable modem application processor, BE, SMP > - BCM3384 (BMIPS4355) cable modem "spare CPU"*, BE > - BCM6328 (BMIPS4355) ADSL chip, BE Is BMIPS435*5* intentional? I would have assumed at least 6328 is also MIPS4350 like BCM6368. > - BCM6368 (BMIPS4350) ADSL chip, BE, SMP > > *experimental; most configurations will require changing CONFIG_PHYSICAL_START > > bmips_stb_defconfig supports Linux running on the (nominally LE) STB > chipsets: > > - BCM7125 (BMIPS4380) set-top box chip, LE, SMP > - BCM7346 (BMIPS5000) set-top box chip, LE, SMP > - BCM7360 (BMIPS3300) set-top box chip, LE > - BCM7420 (BMIPS5000) set-top box chip, LE, SMP > - BCM7425 (BMIPS5000) set-top box chip, LE, SMP > > serial8250 and bcm63xx_uart do not currently coexist. If/when this is > fixed, it will be also possible to boot the BE image on any supported STB > board configured for BE. For now, each defconfig can only pick one UART > driver, and the BE defconfig enables bcm63xx_uart. > > On these MIPS systems, endianness cannot be reconfigured at runtime. On > STB it is sometimes offered as a board jumper or 0-ohm resistor, and > sometimes hardwired to LE only. The CM and DSL systems always run BE. > > Device Tree is used to configure the following items: > > - UART, USB, GENET peripherals > - IRQ controllers > - Early console base address (bcm63xx_uart only) > - SMP or UP mode > - MIPS counter frequency > - Memory size / regions > - DMA remappings > - Kernel command line > > The DT-enabled bootloader and build instructions for 3384 are posted at > https://github.com/Broadcom/aeolus . The other chips use legacy non-DT > bootloaders, and so the kernel employs autodetection heuristics to select > a builtin DTB. > > Signed-off-by: Kevin Cernekee <cernekee@xxxxxxxxx> This looks nice, and I think I agree with a new target instead of trying to retrofit all the dtb/SoC support into bcm63xx. Some individual comments here because I'm too lazy to search for the introducing patches (more meant as general commentary than nitpicks). (snip) > diff --git a/Documentation/devicetree/bindings/mips/brcm/bmips.txt b/Documentation/devicetree/bindings/mips/brcm/bmips.txt > new file mode 100644 > index 0000000..4a8cd8f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mips/brcm/bmips.txt > @@ -0,0 +1,8 @@ > +* Broadcom MIPS (BMIPS) CPUs > + > +Required properties: > +- compatible: "brcm,bmips3300", "brcm,bmips4350", "brcm,bmips4380" > + "brcm,bmips5000" > + > +- mips-hpt-frequency: This is common to all CPUs in the system so it lives > + under the "cpus" node. Is it a good idea to hardcode this? Some SoC CPUs allow running with different frequencies, which will directly affect this. Also I would assume this would break once we add support for runtime clock changes for BMIPS; at least on the DSL platform you can change the clock between 1/4 (IIRC) and 1/1 for power saving. > diff --git a/Documentation/devicetree/bindings/mips/brcm/soc.txt b/Documentation/devicetree/bindings/mips/brcm/soc.txt > new file mode 100644 > index 0000000..1eedcb8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mips/brcm/soc.txt > @@ -0,0 +1,21 @@ > +* Broadcom cable/DSL/settop platforms > + > +SoCs: > + > +Required properties: > +- compatible: "brcm,bcm3384", "brcm,bcm33843" > + "brcm,bcm3384-viper", "brcm,bcm33843-viper" > + "brcm,bcm6328", "brcm,bcm6368", > + "brcm,bcm7125", "brcm,bcm7346", "brcm,bcm7360", > + "brcm,bcm7420", "brcm,bcm7425" > + > +Boards: > + > +Required properties: > +- compatible: "brcm,bcm93384wvg", "brcm,bcm93384wvg-viper" > + "brcm,bcm9ejtagprb", "brcm,bcm96368mvwg", > + "brcm,bcm97125cbmb", "brcm,bcm97346dbsmb", "brcm,bcm97360svmb", > + "brcm,bcm97420c", "brcm,bcm97425svmb" Should the list of supported boards really be hardcoded here/in the kernel? It doesn't match what the code does, as it (as far as I can tell) accepts dtbs without any of the board compatible ids when passed from bootloader. > + > +The experimental -viper variants are for running Linux on the 3384's > +BMIPS4355 cable modem CPU instead of the BMIPS5000 application processor. (snip) > diff --git a/arch/mips/bmips/irq.c b/arch/mips/bmips/irq.c > new file mode 100644 > index 0000000..14552e5 > --- /dev/null > +++ b/arch/mips/bmips/irq.c > @@ -0,0 +1,38 @@ > +/* > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > + * by the Free Software Foundation. > + * > + * Copyright (C) 2014 Broadcom Corporation > + * Author: Kevin Cernekee <cernekee@xxxxxxxxx> > + */ > + > +#include <linux/of.h> > +#include <linux/irqchip.h> > + > +#include <asm/bmips.h> > +#include <asm/irq.h> > +#include <asm/irq_cpu.h> > +#include <asm/time.h> > + > +unsigned int get_c0_compare_int(void) > +{ > + return CP0_LEGACY_COMPARE_IRQ; > +} > + > +void __init arch_init_irq(void) > +{ > + struct device_node *dn; > + > + /* Only the STB (bcm7038) controller supports SMP IRQ affinity */ > + dn = of_find_compatible_node(NULL, NULL, "brcm,bcm7038-l1-intc"); > + if (dn) > + of_node_put(dn); > + else > + bmips_tp1_irqs = 0; > + > + irqchip_init(); > +} > + > +OF_DECLARE_2(irqchip, mips_cpu_intc, "mti,cpu-interrupt-controller", > + mips_cpu_irq_of_init); > diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c > new file mode 100644 > index 0000000..c34601d > --- /dev/null > +++ b/arch/mips/bmips/setup.c > @@ -0,0 +1,249 @@ > +/* > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + * > + * Copyright (C) 2014 Broadcom Corporation > + * Author: Kevin Cernekee <cernekee@xxxxxxxxx> > + */ > + > +#include <linux/init.h> > +#include <linux/bitops.h> > +#include <linux/bootmem.h> > +#include <linux/clk-provider.h> > +#include <linux/ioport.h> > +#include <linux/kernel.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_fdt.h> > +#include <linux/of_platform.h> > +#include <linux/smp.h> > +#include <asm/addrspace.h> > +#include <asm/bmips.h> > +#include <asm/bootinfo.h> > +#include <asm/cpu-type.h> > +#include <asm/mipsregs.h> > +#include <asm/prom.h> > +#include <asm/smp-ops.h> > +#include <asm/time.h> > +#include <asm/traps.h> > +#include <asm/fw/cfe/cfe_api.h> > + > +#define CMT_LOCAL_TPID BIT(31) > +#define RELO_NORMAL_VEC BIT(18) > + > +#define REG_DSL_CHIP_ID ((void __iomem *)CKSEG1ADDR(0x10000000)) > +#define REG_STB_CHIP_ID ((void __iomem *)CKSEG1ADDR(0x10404000)) > + > +#define REG_BCM6328_OTP ((void __iomem *)CKSEG1ADDR(0x1000062c)) > +#define BCM6328_TP1_DISABLED BIT(9) > + > +static const unsigned long kbase = VMLINUX_LOAD_ADDRESS & 0xfff00000; > + > +struct bmips_board_list { > + void *dtb; > + u32 chip_id; > + const char *boardname; > + void (*quirk_fn)(void); > +}; > + > +extern char __dtb_bcm9ejtagprb_begin; > +extern char __dtb_bcm96368mvwg_begin; > +extern char __dtb_bcm97125cbmb_begin; > +extern char __dtb_bcm97346dbsmb_begin; > +extern char __dtb_bcm97360svmb_begin; > +extern char __dtb_bcm97420c_begin; > +extern char __dtb_bcm97425svmb_begin; I think it would be a good idea to have the embedded dtbs optional, especially if you already provide an interface for passing a dtb pointer. > + > +static void kbase_setup(void) > +{ > + __raw_writel(kbase | RELO_NORMAL_VEC, > + BMIPS_GET_CBR() + BMIPS_RELO_VECTOR_CONTROL_1); > + ebase = kbase; > +} > + > +static void bcm3384_quirks(void) > +{ > + /* > + * Some experimental CM boxes are set up to let CM own the Viper TP0 > + * and let Linux own TP1. This requires moving the kernel > + * load address to a non-conflicting region (e.g. via > + * CONFIG_PHYSICAL_START) and supplying an alternate DTB. > + * If we detect this condition, we need to move the MIPS exception > + * vectors up to an area that we own. > + * > + * This is distinct from the OTHER special case mentioned in > + * smp-bmips.c (boot on TP1, but enable SMP, then TP0 becomes our > + * logical CPU#1). For the Viper TP1 case, SMP is off limits. > + * > + * Also note that many BMIPS435x CPUs do not have a > + * BMIPS_RELO_VECTOR_CONTROL_1 register, so it isn't safe to just > + * write VMLINUX_LOAD_ADDRESS into that register on every SoC. > + */ > + if (current_cpu_type() == CPU_BMIPS4350 && > + kbase != CKSEG0 && > + read_c0_brcm_cmt_local() & CMT_LOCAL_TPID) { > + board_ebase_setup = &kbase_setup; > + bmips_smp_enabled = 0; > + } > +} > + > +static void bcm6328_quirks(void) > +{ > + /* Check OTP to see if CPU1 is enabled (it usually isn't) */ > + if (__raw_readl(REG_BCM6328_OTP) & BCM6328_TP1_DISABLED) > + bmips_smp_enabled = 0; > +} > + > +static void bcm6368_quirks(void) > +{ > + /* > + * The bootloader has set up the CPU1 reset vector at > + * 0xa000_0200. > + * This conflicts with the special interrupt vector (IV). > + * The bootloader has also set up CPU1 to respond to the wrong > + * IPI interrupt. > + * Here we will start up CPU1 in the background and ask it to > + * reconfigure itself then go back to sleep. > + */ > + memcpy((void *)0xa0000200, &bmips_smp_movevec, 0x20); > + __sync(); > + set_c0_cause(C_SW0); > + cpumask_set_cpu(1, &bmips_booted_mask); > +} > + > +static const struct bmips_board_list bmips_board_list[] = { > + { &__dtb_bcm9ejtagprb_begin, 0x6328, NULL, &bcm6328_quirks }, > + { &__dtb_bcm96368mvwg_begin, 0x6368, NULL, &bcm6368_quirks }, > + { &__dtb_bcm97125cbmb_begin, 0x7125, "BCM97125C0" }, > + { &__dtb_bcm97346dbsmb_begin, 0x7346, "BCM97346DBSMB" }, > + { &__dtb_bcm97360svmb_begin, 0x7360, "BCM97360SVMB" }, > + { &__dtb_bcm97420c_begin, 0x7420, "BCM97420C_DDR3" }, > + { &__dtb_bcm97425svmb_begin, 0x7425, "BCM97425SVMB" }, > + { }, > +}; > + > +void __init prom_init(void) > +{ > + register_bmips_smp_ops(); > +} > + > +void __init prom_free_prom_memory(void) > +{ > +} > + > +const char *get_system_type(void) > +{ > + return "BMIPS multiplatform kernel"; > +} > + > +void __init plat_time_init(void) > +{ > + struct device_node *np; > + u32 freq; > + > + np = of_find_node_by_name(NULL, "cpus"); > + if (!np) > + panic("missing 'cpus' DT node"); > + if (of_property_read_u32(np, "mips-hpt-frequency", &freq) < 0) > + panic("missing 'mips-hpt-frequency' property"); > + of_node_put(np); > + > + mips_hpt_frequency = freq; > +} > + > +static void __init *find_dtb(void) > +{ > + u32 chip_id; > + char boardname[64] = ""; > + const struct bmips_board_list *b; > + > + /* Intended to somewhat resemble ARM; see Documentation/arm/Booting */ > + if (fw_arg0 == 0 && fw_arg1 == 0xffffffff) > + return phys_to_virt(fw_arg2); I know a bit late, but how about using the OF_DT_MAGIC (0xd00dfeed) for indicating that there is a device tree in arg2? > + > + if (fw_arg1 != 0 || fw_arg3 != CFE_EPTSEAL || > + (fw_arg2 & 0xf0000000) != CKSEG0) > + panic("cannot identify chip"); > + > + /* > + * Unfortunately the CFE API doesn't seem to provide chip > + * identification, but we can check the entry point to see whether > + * the current platform is a DSL chip or STB chip. On STB, > + * CAE_STKSIZE = _regidx(13) = 13*8 = 104, so the first instruction is: > + * 0: 23bdff98 addi sp,sp,-104 > + */ > + if (__raw_readl((void *)fw_arg2) == 0x23bdff98) { > + chip_id = __raw_readl(REG_STB_CHIP_ID); > + cfe_init(fw_arg0, fw_arg2); > + cfe_getenv("CFE_BOARDNAME", boardname, sizeof(boardname)); > + } else { > + /* > + * This works on most modern chips, but will break on older > + * ones like 6358 > + */ > + chip_id = __raw_readl(REG_DSL_CHIP_ID); Unfortunately I don't know any good way to discriminate between the "old" and the "new" chips except by looking a the PRID and REV (BMIPS3300 <= 3.2 || BMIPS4350 <= 1.0 => 0xfff00000, BMIPS3300 >= 3.3 || BMIPS4350 >= 3.0 => 0xb0000000). > + } > + > + /* 4-digit parts use bits [31:16]; 5-digit parts use [27:8] */ This might be true for the STB chips, but the DSL 5-digit parts use [31:12]. And to add insult to insury, some 4-digit parts use [15:12] for the chip variant, so you can't just check [15:12] for 0 or != 0 either (I would assume 63381 and 6328 (variant 63281) will have both 1 at [15:12]. > + if (chip_id & 0xf0000000) > + chip_id >>= 16; > + else > + chip_id >>= 8; > + > + for (b = bmips_board_list; b->dtb; b++) { > + if (b->chip_id != chip_id) > + continue; > + if (b->boardname && strcmp(b->boardname, boardname)) > + continue; > + if (b->quirk_fn) > + b->quirk_fn(); Hmm, maybe move running the quirk out of here and into plat_mem_setup()? Currently e.g. a BCM6328 with a bootloader passed dtb won't have its quirk run. > + return b->dtb; > + } > + > + panic("no dtb found for current board"); > +} > + > +void __init plat_mem_setup(void) > +{ > + set_io_port_base(0); > + ioport_resource.start = 0; > + ioport_resource.end = ~0; > + > + __dt_setup_arch(find_dtb()); > + strlcpy(arcs_cmdline, boot_command_line, COMMAND_LINE_SIZE); > + > + /* BCM3384 DTB comes from the bootloader, not from bmips_board_list */ > + if (of_flat_dt_is_compatible(of_get_flat_dt_root(), > + "brcm,bcm3384-viper")) { > + bcm3384_quirks(); > + } > +} > + > +void __init device_tree_init(void) > +{ > + struct device_node *np; > + > + unflatten_and_copy_device_tree(); > + > + /* Disable SMP boot unless both CPUs are listed in DT and !disabled */ > + np = of_find_node_by_name(NULL, "cpus"); > + if (np && of_get_available_child_count(np) <= 1) > + bmips_smp_enabled = 0; > + of_node_put(np); > +} > + > +int __init plat_of_setup(void) > +{ > + return __dt_register_buses("brcm,bmips", "simple-bus"); Huh, "brcm,bmips" does not appear anywhere else. How does this work? Should this be a required compatible? (OT: "buses" irks me because in my native tongue the plural uses "ss", but I accept that using one "s" is also a valid spelling in english :P) > +} > + > +arch_initcall(plat_of_setup); > + > +static int __init plat_dev_init(void) > +{ > + of_clk_init(NULL); > + return 0; > +} > + > +device_initcall(plat_dev_init); > diff --git a/arch/mips/boot/dts/Makefile b/arch/mips/boot/dts/Makefile > index ca9c90e..ffae96b 100644 > --- a/arch/mips/boot/dts/Makefile > +++ b/arch/mips/boot/dts/Makefile > @@ -1,3 +1,12 @@ > +dtb-$(CONFIG_BMIPS_MULTIPLATFORM) += bcm93384wvg.dtb \ Minor stilistic nitpick: I would use dtb-$(CONFIG_BMIPS_MULTIPLATFORM) += \ bcm93384wvg.dtb \ so that adding a board before bcm963384wvg would only require an insert, not also a modification. > + bcm93384wvg_viper.dtb \ > + bcm96368mvwg.dtb \ > + bcm9ejtagprb.dtb \ > + bcm97125cbmb.dtb \ > + bcm97346dbsmb.dtb \ > + bcm97360svmb.dtb \ > + bcm97420c.dtb \ > + bcm97425svmb.dtb > dtb-$(CONFIG_CAVIUM_OCTEON_SOC) += octeon_3xxx.dtb octeon_68xx.dtb > dtb-$(CONFIG_DT_EASY50712) += easy50712.dtb > dtb-$(CONFIG_DT_XLP_EVP) += xlp_evp.dtb > diff --git a/arch/mips/boot/dts/bcm3384_common.dtsi b/arch/mips/boot/dts/bcm3384_common.dtsi > new file mode 100644 > index 0000000..448cb5b > --- /dev/null > +++ b/arch/mips/boot/dts/bcm3384_common.dtsi > @@ -0,0 +1,44 @@ > +/ { > + clocks { > + #address-cells = <1>; This does not really make sense, as there is no address used at all for the periph_clk. > + #size-cells = <0>; > + > + periph_clk: periph_clk@0 { Same for the @0 - there is no appropriate reg = <0>, so using an address here does not make sense. > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <54000000>; > + }; > + }; > + > + aliases { > + uart0 = &uart0; > + }; > + > + uart0: serial@14e00520 { > + compatible = "brcm,bcm6345-uart"; > + reg = <0x14e00520 0x18>; > + interrupt-parent = <&periph_intc>; > + interrupts = <2>; > + clocks = <&periph_clk>; > + status = "disabled"; > + }; > + > + ehci0: usb@15400300 { > + compatible = "brcm,bcm3384-ehci", "generic-ehci"; > + reg = <0x15400300 0x100>; > + big-endian; > + interrupt-parent = <&periph_intc>; > + interrupts = <41>; > + status = "disabled"; > + }; > + > + ohci0: usb@15400400 { > + compatible = "brcm,bcm3384-ohci", "generic-ohci"; > + reg = <0x15400400 0x100>; > + big-endian; > + no-big-frame-no; > + interrupt-parent = <&periph_intc>; > + interrupts = <40>; > + status = "disabled"; > + }; > +}; (snip) > diff --git a/arch/mips/boot/dts/bcm6328.dtsi b/arch/mips/boot/dts/bcm6328.dtsi > new file mode 100644 > index 0000000..a7e397f > --- /dev/null > +++ b/arch/mips/boot/dts/bcm6328.dtsi > @@ -0,0 +1,63 @@ > +/ { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "brcm,bcm6328"; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mips-hpt-frequency = <160000000>; > + > + cpu@0 { > + compatible = "brcm,bmips4350"; > + device_type = "cpu"; > + reg = <0>; > + }; Since there are SMP-enabled variants, maybe it should have its second thread documented here (but defaulting to "disabled")? > + }; > + > + clocks { > + #address-cells = <1>; > + #size-cells = <0>; > + > + periph_clk: periph_clk@0 { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <50000000>; > + }; > + }; > + > + aliases { > + uart0 = &uart0; > + }; > + > + cpu_intc: cpu_intc@0 { It does not have an address, so it should not have @0 in the node name I think. > + #address-cells = <0>; > + compatible = "mti,cpu-interrupt-controller"; > + > + interrupt-controller; > + #interrupt-cells = <1>; > + }; > + > + periph_intc: periph_intc@10000024 { > + compatible = "brcm,bcm7120-l2-intc"; > + reg = <0x10000024 0x4 0x1000002c 0x4 > + 0x10000020 0x4 0x10000028 0x4>; The "lowest" register address you use is 0x10000020, so the name should arguably be periph_intc@10000020, not periph_intc@10000024. I guess the second cpu block (10000030 - 1000003c) wired to hw irq 3 can be added later. This will easily translate to a lot of io(re)map calls in case of 63268/6318 when describing both cpu blocks ( a total of 16 "reg"s). Also I wonder how you properly describe the intc of 63381, where it has separate mask registers, but a shared status register. > + > + interrupt-controller; > + #interrupt-cells = <1>; > + > + interrupt-parent = <&cpu_intc>; > + interrupts = <2>; > + brcm,int-map-mask = <0xffffffff 0xffffffff>; > + }; > + > + uart0: serial@10000100 { > + compatible = "brcm,bcm6345-uart"; > + reg = <0x10000100 0x18>; > + interrupt-parent = <&periph_intc>; > + interrupts = <28>; > + clocks = <&periph_clk>; > + status = "disabled"; > + }; > +}; (snip) Jonas
