2014-07-21 21:08:57

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

Hi all,

I'm taking over the latest resubmission of this patch series.
There are a few moderate changes for v8 (noted below), but we
are waiting mostly for an Ack for the reboot driver.

This patchset contains the board support package for the
Broadcom BCM7445 ARM-based SoC [1]. These changes contain a
minimal set of code needed for a BCM7445-based board to boot
the Linux kernel.

These changes heavily leverage the OF/devicetree framework. The
machine is also built into the multi-platform ARMv7 image.

Changes are also available here:

https://github.com/brcm/linux/tree/brcmstb-v8
git://github.com/brcm/linux.git +brcmstb-v8

v8:
- rebase to v3.16-rc6
- convert SMP bringup to use CPU_METHOD_OF_DECLARE
- add MAINTAINERS entries
- select a few new drivers for the ARCH_BRCMSTB platform

v7 (https://lkml.org/lkml/2014/2/26/133):
- rebase to v3.14-rc4
- detect and apply ARM erratum 798181 to Brahma15 CPUs
- split-up bcm7445.dts into a common dtsi and board-specific dts

v6 (https://lkml.org/lkml/2014/2/3/493):
- rebased to v3.14-rc1
- utilize common APIs for handling CPU power-down
- drop deprecated __cpuinit attributes

v5 (https://lkml.org/lkml/2014/1/21/640):
- rebased to v3.13 tag
- make UART DT node a child of 'rdb' node
- fix ordering of debug UART entries

v4 (https://lkml.org/lkml/2014/1/17/455):
- make a reboot driver and put it in the drivers folder
- rework DT bindings to leverage 'syscon'
- rework BSP code to use 'syscon' for all register mappings
- misc. tweaks per suggestions from v3

v3 (https://lkml.org/lkml/2014/1/14/696):
- rebased to v3.13-rc8
- switched to using 'multi_v7_defconfig'
- eliminated dependence on compile-time peripheral register access
- moved DT node iomap out from 'init_early'
- misc. minor cleanups from mailing-list discussion for v2

v2 (https://lkml.org/lkml/2013/11/26/570):
- rebased to v3.13-rc1
- moved implementation to 'mach-bcm' folder
- added CPU init for B16RM

v1:
- initial submission

[1] http://www.broadcom.com/products/Cable/Cable-Set-Top-Box-Solutions/BCM7445

Brian Norris (2):
ARM: brcmstb: select GISB arbiter and interrupt drivers
MAINTAINERS: add entry for Broadcom ARM STB architecture

Gregory Fong (1):
ARM: Enable erratum 798181 for Broadcom Brahma-B15

Marc Carino (8):
ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs
power: reset: Add reboot driver for brcmstb
ARM: brcmstb: add debug UART for earlyprintk support
ARM: do CPU-specific init for Broadcom Brahma15 cores
ARM: brcmstb: add CPU binding for Broadcom Brahma15
ARM: brcmstb: add misc. DT bindings for brcmstb
ARM: brcmstb: gic: add compatible string for Broadcom Brahma15
ARM: brcmstb: dts: add a reference DTS for Broadcom 7445

.../devicetree/bindings/arm/brcm-brcmstb.txt | 95 ++++++
Documentation/devicetree/bindings/arm/cpus.txt | 2 +
Documentation/devicetree/bindings/arm/gic.txt | 1 +
MAINTAINERS | 9 +
arch/arm/Kconfig.debug | 16 +-
arch/arm/boot/dts/Makefile | 2 +
arch/arm/boot/dts/bcm7445-bcm97445svmb.dts | 14 +
arch/arm/boot/dts/bcm7445.dtsi | 111 +++++++
arch/arm/configs/multi_v7_defconfig | 1 +
arch/arm/kernel/smp_tlb.c | 20 +-
arch/arm/mach-bcm/Kconfig | 17 +
arch/arm/mach-bcm/Makefile | 5 +
arch/arm/mach-bcm/brcmstb.c | 28 ++
arch/arm/mach-bcm/brcmstb.h | 19 ++
arch/arm/mach-bcm/headsmp-brcmstb.S | 33 ++
arch/arm/mach-bcm/platsmp-brcmstb.c | 363 +++++++++++++++++++++
arch/arm/mm/proc-v7.S | 11 +
drivers/power/reset/Kconfig | 10 +
drivers/power/reset/Makefile | 1 +
drivers/power/reset/brcmstb-reboot.c | 120 +++++++
20 files changed, 869 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
create mode 100644 arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
create mode 100644 arch/arm/boot/dts/bcm7445.dtsi
create mode 100644 arch/arm/mach-bcm/brcmstb.c
create mode 100644 arch/arm/mach-bcm/brcmstb.h
create mode 100644 arch/arm/mach-bcm/headsmp-brcmstb.S
create mode 100644 arch/arm/mach-bcm/platsmp-brcmstb.c
create mode 100644 drivers/power/reset/brcmstb-reboot.c

--
1.9.1


2014-07-21 21:09:25

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 02/11] power: reset: Add reboot driver for brcmstb

From: Marc Carino <[email protected]>

Add support for reboot functionality on boards with ARM-based
Broadcom STB chipsets.

Signed-off-by: Marc Carino <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Cc: Dmitry Eremin-Solenikov <[email protected]>
Cc: David Woodhouse <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 1 +
drivers/power/reset/Kconfig | 10 +++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/brcmstb-reboot.c | 120 +++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 drivers/power/reset/brcmstb-reboot.c

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 0073633e7699..9782e8d80647 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -94,6 +94,7 @@ config ARCH_BRCMSTB
select MIGHT_HAVE_PCI
select HAVE_SMP
select HAVE_ARM_ARCH_TIMER
+ select POWER_RESET_BRCMSTB
help
Say Y if you intend to run the kernel on a Broadcom ARM-based STB
chipset.
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index bdcf5173e377..fcb9825debe5 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -20,6 +20,16 @@ config POWER_RESET_AXXIA

Say Y if you have an Axxia family SoC.

+config POWER_RESET_BRCMSTB
+ bool "Broadcom STB reset driver"
+ depends on POWER_RESET && ARCH_BRCMSTB
+ help
+ This driver provides restart support for ARM-based Broadcom STB
+ boards.
+
+ Say Y here if you have an ARM-based Broadcom STB board and you wish
+ to have restart support.
+
config POWER_RESET_GPIO
bool "GPIO power-off driver"
depends on OF_GPIO && POWER_RESET
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index dde2e8bbac53..7379818ca69d 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -1,5 +1,6 @@
obj-$(CONFIG_POWER_RESET_AS3722) += as3722-poweroff.o
obj-$(CONFIG_POWER_RESET_AXXIA) += axxia-reset.o
+obj-$(CONFIG_POWER_RESET_BRCMSTB) += brcmstb-reboot.o
obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
new file mode 100644
index 000000000000..3f236924742a
--- /dev/null
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright (C) 2013 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/smp.h>
+#include <linux/mfd/syscon.h>
+
+#include <asm/system_misc.h>
+
+#define RESET_SOURCE_ENABLE_REG 1
+#define SW_MASTER_RESET_REG 2
+
+static struct regmap *regmap;
+static u32 rst_src_en;
+static u32 sw_mstr_rst;
+
+static void brcmstb_reboot(enum reboot_mode mode, const char *cmd)
+{
+ int rc;
+ u32 tmp;
+
+ rc = regmap_write(regmap, rst_src_en, 1);
+ if (rc) {
+ pr_err("failed to write rst_src_en (%d)\n", rc);
+ return;
+ }
+
+ rc = regmap_read(regmap, rst_src_en, &tmp);
+ if (rc) {
+ pr_err("failed to read rst_src_en (%d)\n", rc);
+ return;
+ }
+
+ rc = regmap_write(regmap, sw_mstr_rst, 1);
+ if (rc) {
+ pr_err("failed to write sw_mstr_rst (%d)\n", rc);
+ return;
+ }
+
+ rc = regmap_read(regmap, sw_mstr_rst, &tmp);
+ if (rc) {
+ pr_err("failed to read sw_mstr_rst (%d)\n", rc);
+ return;
+ }
+
+ while (1)
+ ;
+}
+
+static int brcmstb_reboot_probe(struct platform_device *pdev)
+{
+ int rc;
+ struct device_node *np = pdev->dev.of_node;
+
+ regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+ if (IS_ERR(regmap)) {
+ pr_err("failed to get syscon phandle\n");
+ return -EINVAL;
+ }
+
+ rc = of_property_read_u32_index(np, "syscon", RESET_SOURCE_ENABLE_REG,
+ &rst_src_en);
+ if (rc) {
+ pr_err("can't get rst_src_en offset (%d)\n", rc);
+ return -EINVAL;
+ }
+
+ rc = of_property_read_u32_index(np, "syscon", SW_MASTER_RESET_REG,
+ &sw_mstr_rst);
+ if (rc) {
+ pr_err("can't get sw_mstr_rst offset (%d)\n", rc);
+ return -EINVAL;
+ }
+
+ arm_pm_restart = brcmstb_reboot;
+
+ return 0;
+}
+
+static const struct of_device_id of_match[] = {
+ { .compatible = "brcm,brcmstb-reboot", },
+ {},
+};
+
+static struct platform_driver brcmstb_reboot_driver = {
+ .probe = brcmstb_reboot_probe,
+ .driver = {
+ .name = "brcmstb-reboot",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match,
+ },
+};
+
+static int __init brcmstb_reboot_init(void)
+{
+ return platform_driver_probe(&brcmstb_reboot_driver,
+ brcmstb_reboot_probe);
+}
+subsys_initcall(brcmstb_reboot_init);
--
1.9.1

2014-07-21 21:09:44

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 08/11] ARM: brcmstb: gic: add compatible string for Broadcom Brahma15

From: Marc Carino <[email protected]>

Document the Broadcom Brahma B15 GIC implementation as compatible
with the ARM GIC standard.

Signed-off-by: Marc Carino <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
---
Documentation/devicetree/bindings/arm/gic.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 5573c08d3180..c7d2fa156678 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -16,6 +16,7 @@ Main node required properties:
"arm,cortex-a9-gic"
"arm,cortex-a7-gic"
"arm,arm11mp-gic"
+ "brcm,brahma-b15-gic"
- interrupt-controller : Identifies the node as an interrupt controller
- #interrupt-cells : Specifies the number of cells needed to encode an
interrupt source. The type shall be a <u32> and the value shall be 3.
--
1.9.1

2014-07-21 21:10:00

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 11/11] MAINTAINERS: add entry for Broadcom ARM STB architecture

Signed-off-by: Brian Norris <[email protected]>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 61a8f486306b..8e9cce53f618 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1969,6 +1969,15 @@ F: arch/arm/mach-bcm/bcm_5301x.c
F: arch/arm/boot/dts/bcm5301x.dtsi
F: arch/arm/boot/dts/bcm470*

+BROADCOM BCM7XXX ARM ARCHITECTURE
+M: Marc Carino <[email protected]>
+M: Brian Norris <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+T: git git://github.com/brcm/linux.git
+S: Maintained
+F: arch/arm/mach-bcm/*brcmstb*
+F: arch/arm/boot/dts/bcm7*.dts*
+
BROADCOM TG3 GIGABIT ETHERNET DRIVER
M: Nithin Nayak Sujir <[email protected]>
M: Michael Chan <[email protected]>
--
1.9.1

2014-07-21 21:09:46

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 10/11] ARM: brcmstb: select GISB arbiter and interrupt drivers

Signed-off-by: Brian Norris <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 9782e8d80647..58c01aed9752 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -95,6 +95,8 @@ config ARCH_BRCMSTB
select HAVE_SMP
select HAVE_ARM_ARCH_TIMER
select POWER_RESET_BRCMSTB
+ select BRCMSTB_GISB_ARB
+ select BRCMSTB_L2_IRQ
help
Say Y if you intend to run the kernel on a Broadcom ARM-based STB
chipset.
--
1.9.1

2014-07-21 21:09:42

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 05/11] ARM: Enable erratum 798181 for Broadcom Brahma-B15

From: Gregory Fong <[email protected]>

Broadcom Brahma-B15 (r0p0..r0p2) is also affected by Cortex-A15
erratum 798181, so enable the workaround for Brahma-B15.

Signed-off-by: Gregory Fong <[email protected]>
Acked-by: Marc Carino <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm/kernel/smp_tlb.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
index 95d063620b76..2e72be4f623e 100644
--- a/arch/arm/kernel/smp_tlb.c
+++ b/arch/arm/kernel/smp_tlb.c
@@ -92,15 +92,19 @@ void erratum_a15_798181_init(void)
unsigned int midr = read_cpuid_id();
unsigned int revidr = read_cpuid(CPUID_REVIDR);

- /* Cortex-A15 r0p0..r3p2 w/o ECO fix affected */
- if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2 ||
- (revidr & 0x210) == 0x210) {
- return;
- }
- if (revidr & 0x10)
- erratum_a15_798181_handler = erratum_a15_798181_partial;
- else
+ /* Brahma-B15 r0p0..r0p2 affected
+ * Cortex-A15 r0p0..r3p2 w/o ECO fix affected */
+ if ((midr & 0xff0ffff0) == 0x420f00f0 && midr <= 0x420f00f2)
erratum_a15_798181_handler = erratum_a15_798181_broadcast;
+ else if ((midr & 0xff0ffff0) == 0x410fc0f0 && midr <= 0x413fc0f2 &&
+ (revidr & 0x210) != 0x210) {
+ if (revidr & 0x10)
+ erratum_a15_798181_handler =
+ erratum_a15_798181_partial;
+ else
+ erratum_a15_798181_handler =
+ erratum_a15_798181_broadcast;
+ }
}
#endif

--
1.9.1

2014-07-21 21:10:42

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 09/11] ARM: brcmstb: dts: add a reference DTS for Broadcom 7445

From: Marc Carino <[email protected]>

Add a sample DTS which will allow bootup of a board populated
with the BCM7445 chip.

Signed-off-by: Marc Carino <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Cc: Matt Porter <[email protected]>
---
arch/arm/boot/dts/Makefile | 2 +
arch/arm/boot/dts/bcm7445-bcm97445svmb.dts | 14 ++++
arch/arm/boot/dts/bcm7445.dtsi | 111 +++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
create mode 100644 arch/arm/boot/dts/bcm7445.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index adb5ed9e269e..38439ca9b361 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -59,6 +59,8 @@ dtb-$(CONFIG_ARCH_BERLIN) += \
berlin2-sony-nsz-gs7.dtb \
berlin2cd-google-chromecast.dtb \
berlin2q-marvell-dmp.dtb
+dtb-$(CONFIG_ARCH_BRCMSTB) += \
+ bcm7445-bcm97445svmb.dtb
dtb-$(CONFIG_ARCH_DAVINCI) += da850-enbw-cmc.dtb \
da850-evm.dtb
dtb-$(CONFIG_ARCH_EFM32) += efm32gg-dk3750.dtb
diff --git a/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts b/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
new file mode 100644
index 000000000000..9eec2ac1112f
--- /dev/null
+++ b/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+#include "bcm7445.dtsi"
+
+/ {
+ model = "Broadcom STB (bcm7445), SVMB reference board";
+ compatible = "brcm,bcm7445", "brcm,brcmstb";
+
+ memory {
+ device_type = "memory";
+ reg = <0x00 0x00000000 0x00 0x40000000>,
+ <0x00 0x40000000 0x00 0x40000000>,
+ <0x00 0x80000000 0x00 0x40000000>;
+ };
+};
diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
new file mode 100644
index 000000000000..0ca0f4e523d0
--- /dev/null
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -0,0 +1,111 @@
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+#include "skeleton.dtsi"
+
+/ {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ model = "Broadcom STB (bcm7445)";
+ compatible = "brcm,bcm7445", "brcm,brcmstb";
+ interrupt-parent = <&gic>;
+
+ chosen {
+ bootargs = "console=ttyS0,115200 earlyprintk";
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ compatible = "brcm,brahma-b15";
+ device_type = "cpu";
+ enable-method = "brcm,brahma-b15";
+ reg = <0>;
+ };
+
+ cpu@1 {
+ compatible = "brcm,brahma-b15";
+ device_type = "cpu";
+ enable-method = "brcm,brahma-b15";
+ reg = <1>;
+ };
+
+ cpu@2 {
+ compatible = "brcm,brahma-b15";
+ device_type = "cpu";
+ enable-method = "brcm,brahma-b15";
+ reg = <2>;
+ };
+
+ cpu@3 {
+ compatible = "brcm,brahma-b15";
+ device_type = "cpu";
+ enable-method = "brcm,brahma-b15";
+ reg = <3>;
+ };
+ };
+
+ gic: interrupt-controller@ffd00000 {
+ compatible = "brcm,brahma-b15-gic", "arm,cortex-a15-gic";
+ reg = <0x00 0xffd01000 0x00 0x1000>,
+ <0x00 0xffd02000 0x00 0x2000>,
+ <0x00 0xffd04000 0x00 0x2000>,
+ <0x00 0xffd06000 0x00 0x2000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ };
+
+ timer {
+ compatible = "arm,armv7-timer";
+ interrupts = <GIC_PPI 13 (GIC_CPU_MASK_RAW(15) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14 (GIC_CPU_MASK_RAW(15) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11 (GIC_CPU_MASK_RAW(15) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10 (GIC_CPU_MASK_RAW(15) | IRQ_TYPE_LEVEL_LOW)>;
+ };
+
+ rdb {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ ranges = <0 0x00 0xf0000000 0x1000000>;
+
+ serial@40ab00 {
+ compatible = "ns16550a";
+ reg = <0x40ab00 0x20>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+ clock-frequency = <0x4d3f640>;
+ };
+
+ sun_top_ctrl: syscon@404000 {
+ compatible = "brcm,bcm7445-sun-top-ctrl",
+ "syscon";
+ reg = <0x404000 0x51c>;
+ };
+
+ hif_cpubiuctrl: syscon@3e2400 {
+ compatible = "brcm,bcm7445-hif-cpubiuctrl",
+ "syscon";
+ reg = <0x3e2400 0x5b4>;
+ };
+
+ hif_continuation: syscon@452000 {
+ compatible = "brcm,bcm7445-hif-continuation",
+ "syscon";
+ reg = <0x452000 0x100>;
+ };
+ };
+
+ smpboot {
+ compatible = "brcm,brcmstb-smpboot";
+ syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
+ syscon-cont = <&hif_continuation>;
+ };
+
+ reboot {
+ compatible = "brcm,brcmstb-reboot";
+ syscon = <&sun_top_ctrl 0x304 0x308>;
+ };
+};
--
1.9.1

2014-07-21 21:09:40

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 03/11] ARM: brcmstb: add debug UART for earlyprintk support

From: Marc Carino <[email protected]>

Add the UART definitions needed to support earlyprintk on brcmstb machines.

Signed-off-by: Marc Carino <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
---
arch/arm/Kconfig.debug | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 8f90595069a1..89c06a8e810c 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -130,6 +130,17 @@ choice
Say Y here if you want kernel low-level debugging support
on Marvell Berlin SoC based platforms.

+ config DEBUG_BRCMSTB_UART
+ bool "Use BRCMSTB UART for low-level debug"
+ depends on ARCH_BRCMSTB
+ select DEBUG_UART_8250
+ help
+ Say Y here if you want the debug print routines to direct
+ their output to the first serial port on these devices.
+
+ If you have a Broadcom STB chip and would like early print
+ messages to appear over the UART, select this option.
+
config DEBUG_CLPS711X_UART1
bool "Kernel low-level debugging messages via UART1"
depends on ARCH_CLPS711X
@@ -1088,6 +1099,7 @@ config DEBUG_UART_PHYS
default 0xd4018000 if DEBUG_MMP_UART3
default 0xe0000000 if ARCH_SPEAR13XX
default 0xf0000be0 if ARCH_EBSA110
+ default 0xf040ab00 if DEBUG_BRCMSTB_UART
default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
default 0xf1012000 if ARCH_DOVE || ARCH_KIRKWOOD || ARCH_MV78XX0 || \
ARCH_ORION5X
@@ -1133,6 +1145,7 @@ config DEBUG_UART_VIRT
default 0xfa71e000 if DEBUG_QCOM_UARTDM
default 0xfb009000 if DEBUG_REALVIEW_STD_PORT
default 0xfb10c000 if DEBUG_REALVIEW_PB1176_PORT
+ default 0xfc40ab00 if DEBUG_BRCMSTB_UART
default 0xfd000000 if ARCH_SPEAR3XX || ARCH_SPEAR6XX
default 0xfd000000 if ARCH_SPEAR13XX
default 0xfd012000 if ARCH_MV78XX0
@@ -1186,7 +1199,8 @@ config DEBUG_UART_8250_WORD
ARCH_KEYSTONE || \
DEBUG_DAVINCI_DMx_UART0 || DEBUG_DAVINCI_DA8XX_UART1 || \
DEBUG_DAVINCI_DA8XX_UART2 || \
- DEBUG_BCM_KONA_UART
+ DEBUG_BCM_KONA_UART || \
+ DEBUG_BRCMSTB_UART

config DEBUG_UART_8250_FLOW_CONTROL
bool "Enable flow control for 8250 UART"
--
1.9.1

2014-07-21 21:09:38

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 04/11] ARM: do CPU-specific init for Broadcom Brahma15 cores

From: Marc Carino <[email protected]>

Perform any CPU-specific initialization required on the
Broadcom Brahma-15 core.

Signed-off-by: Marc Carino <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Cc: Russell King <[email protected]>
---
arch/arm/mm/proc-v7.S | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3db2c2f04a30..c324c0e1aae9 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -216,6 +216,7 @@ __v7_cr7mp_setup:
__v7_ca7mp_setup:
__v7_ca12mp_setup:
__v7_ca15mp_setup:
+__v7_b15mp_setup:
__v7_ca17mp_setup:
mov r10, #0
1:
@@ -528,6 +529,16 @@ __v7_ca15mp_proc_info:
.size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info

/*
+ * Broadcom Corporation Brahma-B15 processor.
+ */
+ .type __v7_b15mp_proc_info, #object
+__v7_b15mp_proc_info:
+ .long 0x420f00f0
+ .long 0xff0ffff0
+ __v7_proc __v7_b15mp_setup, hwcaps = HWCAP_IDIV
+ .size __v7_b15mp_proc_info, . - __v7_b15mp_proc_info
+
+ /*
* ARM Ltd. Cortex A17 processor.
*/
.type __v7_ca17mp_proc_info, #object
--
1.9.1

2014-07-21 21:11:25

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 07/11] ARM: brcmstb: add misc. DT bindings for brcmstb

From: Marc Carino <[email protected]>

Document the bindings that the Broadcom STB platform needs
for proper bootup.

Signed-off-by: Marc Carino <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
---
.../devicetree/bindings/arm/brcm-brcmstb.txt | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/brcm-brcmstb.txt

diff --git a/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
new file mode 100644
index 000000000000..3c436cc4f35d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
@@ -0,0 +1,95 @@
+ARM Broadcom STB platforms Device Tree Bindings
+-----------------------------------------------
+Boards with Broadcom Brahma15 ARM-based BCMxxxx (generally BCM7xxx variants)
+SoC shall have the following DT organization:
+
+Required root node properties:
+ - compatible: "brcm,bcm<chip_id>", "brcm,brcmstb"
+
+example:
+/ {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ model = "Broadcom STB (bcm7445)";
+ compatible = "brcm,bcm7445", "brcm,brcmstb";
+
+Further, syscon nodes that map platform-specific registers used for general
+system control is required:
+
+ - compatible: "brcm,bcm<chip_id>-sun-top-ctrl", "syscon"
+ - compatible: "brcm,bcm<chip_id>-hif-cpubiuctrl", "syscon"
+ - compatible: "brcm,bcm<chip_id>-hif-continuation", "syscon"
+
+example:
+ rdb {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ ranges = <0 0x00 0xf0000000 0x1000000>;
+
+ sun_top_ctrl: syscon@404000 {
+ compatible = "brcm,bcm7445-sun-top-ctrl", "syscon";
+ reg = <0x404000 0x51c>;
+ };
+
+ hif_cpubiuctrl: syscon@3e2400 {
+ compatible = "brcm,bcm7445-hif-cpubiuctrl", "syscon";
+ reg = <0x3e2400 0x5b4>;
+ };
+
+ hif_continuation: syscon@452000 {
+ compatible = "brcm,bcm7445-hif-continuation", "syscon";
+ reg = <0x452000 0x100>;
+ };
+ };
+
+Lastly, nodes that allow for support of SMP initialization and reboot are
+required:
+
+smpboot
+-------
+Required properties:
+
+ - compatible
+ The string "brcm,brcmstb-smpboot".
+
+ - syscon-cpu
+ A phandle / integer array property which lets the BSP know the location
+ of certain CPU power-on registers.
+
+ The layout of the property is as follows:
+ o a phandle to the "hif_cpubiuctrl" syscon node
+ o offset to the base CPU power zone register
+ o offset to the base CPU reset register
+
+ - syscon-cont
+ A phandle pointing to the syscon node which describes the CPU boot
+ continuation registers.
+ o a phandle to the "hif_continuation" syscon node
+
+example:
+ smpboot {
+ compatible = "brcm,brcmstb-smpboot";
+ syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
+ syscon-cont = <&hif_continuation>;
+ };
+
+reboot
+-------
+Required properties
+
+ - compatible
+ The string property "brcm,brcmstb-reboot".
+
+ - syscon
+ A phandle / integer array that points to the syscon node which describes
+ the general system reset registers.
+ o a phandle to "sun_top_ctrl"
+ o offset to the "reset source enable" register
+ o offset to the "software master reset" register
+
+example:
+ reboot {
+ compatible = "brcm,brcmstb-reboot";
+ syscon = <&sun_top_ctrl 0x304 0x308>;
+ };
--
1.9.1

2014-07-21 21:11:46

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 06/11] ARM: brcmstb: add CPU binding for Broadcom Brahma15

From: Marc Carino <[email protected]>

Add the Broadcom Brahma B15 CPU to the DT CPU binding list.

Signed-off-by: Marc Carino <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
---
Documentation/devicetree/bindings/arm/cpus.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 1fe72a0778cd..06fd7bba830c 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -163,6 +163,7 @@ nodes to be present and contain the properties described below.
"arm,cortex-r4"
"arm,cortex-r5"
"arm,cortex-r7"
+ "brcm,brahma-b15"
"faraday,fa526"
"intel,sa110"
"intel,sa1100"
@@ -184,6 +185,7 @@ nodes to be present and contain the properties described below.
can be one of:
"allwinner,sun6i-a31"
"arm,psci"
+ "brcm,brahma-b15"
"marvell,armada-375-smp"
"marvell,armada-380-smp"
"marvell,armada-xp-smp"
--
1.9.1

2014-07-21 21:12:22

by Brian Norris

[permalink] [raw]
Subject: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

From: Marc Carino <[email protected]>

The BCM7xxx series of Broadcom SoCs are used primarily in set-top boxes.

This patch adds machine support for the ARM-based Broadcom SoCs.

Signed-off-by: Marc Carino <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 1 +
arch/arm/mach-bcm/Kconfig | 14 ++
arch/arm/mach-bcm/Makefile | 5 +
arch/arm/mach-bcm/brcmstb.c | 28 +++
arch/arm/mach-bcm/brcmstb.h | 19 ++
arch/arm/mach-bcm/headsmp-brcmstb.S | 33 ++++
arch/arm/mach-bcm/platsmp-brcmstb.c | 363 ++++++++++++++++++++++++++++++++++++
7 files changed, 463 insertions(+)
create mode 100644 arch/arm/mach-bcm/brcmstb.c
create mode 100644 arch/arm/mach-bcm/brcmstb.h
create mode 100644 arch/arm/mach-bcm/headsmp-brcmstb.S
create mode 100644 arch/arm/mach-bcm/platsmp-brcmstb.c

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 534836497998..bf0df396a3cf 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -19,6 +19,7 @@ CONFIG_MACH_DOVE=y
CONFIG_ARCH_BCM=y
CONFIG_ARCH_BCM_MOBILE=y
CONFIG_ARCH_BCM_5301X=y
+CONFIG_ARCH_BRCMSTB=y
CONFIG_ARCH_BERLIN=y
CONFIG_MACH_BERLIN_BG2=y
CONFIG_MACH_BERLIN_BG2CD=y
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 41c839167e87..0073633e7699 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -87,4 +87,18 @@ config ARCH_BCM_5301X
different SoC or with the older BCM47XX and BCM53XX based
network SoC using a MIPS CPU, they are supported by arch/mips/bcm47xx

+config ARCH_BRCMSTB
+ bool "Broadcom BCM7XXX based boards" if ARCH_MULTI_V7
+ depends on MMU
+ select ARM_GIC
+ select MIGHT_HAVE_PCI
+ select HAVE_SMP
+ select HAVE_ARM_ARCH_TIMER
+ help
+ Say Y if you intend to run the kernel on a Broadcom ARM-based STB
+ chipset.
+
+ This enables support for Broadcom ARM-based set-top box chipsets,
+ including the 7445 family of chips.
+
endif
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 731292114975..f3665121729b 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -30,3 +30,8 @@ obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o

# BCM5301X
obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o
+
+ifeq ($(CONFIG_ARCH_BRCMSTB),y)
+obj-y += brcmstb.o
+obj-$(CONFIG_SMP) += headsmp-brcmstb.o platsmp-brcmstb.o
+endif
diff --git a/arch/arm/mach-bcm/brcmstb.c b/arch/arm/mach-bcm/brcmstb.c
new file mode 100644
index 000000000000..60a5afa06ed7
--- /dev/null
+++ b/arch/arm/mach-bcm/brcmstb.c
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2013-2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of_platform.h>
+
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+
+static const char *brcmstb_match[] __initconst = {
+ "brcm,bcm7445",
+ "brcm,brcmstb",
+ NULL
+};
+
+DT_MACHINE_START(BRCMSTB, "Broadcom STB (Flattened Device Tree)")
+ .dt_compat = brcmstb_match,
+MACHINE_END
diff --git a/arch/arm/mach-bcm/brcmstb.h b/arch/arm/mach-bcm/brcmstb.h
new file mode 100644
index 000000000000..ec0c3d112b36
--- /dev/null
+++ b/arch/arm/mach-bcm/brcmstb.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2013-2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __BRCMSTB_H__
+#define __BRCMSTB_H__
+
+void brcmstb_secondary_startup(void);
+
+#endif /* __BRCMSTB_H__ */
diff --git a/arch/arm/mach-bcm/headsmp-brcmstb.S b/arch/arm/mach-bcm/headsmp-brcmstb.S
new file mode 100644
index 000000000000..199c1ea58248
--- /dev/null
+++ b/arch/arm/mach-bcm/headsmp-brcmstb.S
@@ -0,0 +1,33 @@
+/*
+ * SMP boot code for secondary CPUs
+ * Based on arch/arm/mach-tegra/headsmp.S
+ *
+ * Copyright (C) 2010 NVIDIA, Inc.
+ * Copyright (C) 2013-2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/assembler.h>
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+ .section ".text.head", "ax"
+
+ENTRY(brcmstb_secondary_startup)
+ /*
+ * Ensure CPU is in a sane state by disabling all IRQs and switching
+ * into SVC mode.
+ */
+ setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r0
+
+ bl v7_invalidate_l1
+ b secondary_startup
+ENDPROC(brcmstb_secondary_startup)
diff --git a/arch/arm/mach-bcm/platsmp-brcmstb.c b/arch/arm/mach-bcm/platsmp-brcmstb.c
new file mode 100644
index 000000000000..af780e9c23a6
--- /dev/null
+++ b/arch/arm/mach-bcm/platsmp-brcmstb.c
@@ -0,0 +1,363 @@
+/*
+ * Broadcom STB CPU SMP and hotplug support for ARM
+ *
+ * Copyright (C) 2013-2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/printk.h>
+#include <linux/regmap.h>
+#include <linux/smp.h>
+#include <linux/mfd/syscon.h>
+#include <linux/spinlock.h>
+
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
+#include <asm/mach-types.h>
+#include <asm/smp_plat.h>
+
+#include "brcmstb.h"
+
+enum {
+ ZONE_MAN_CLKEN_MASK = BIT(0),
+ ZONE_MAN_RESET_CNTL_MASK = BIT(1),
+ ZONE_MAN_MEM_PWR_MASK = BIT(4),
+ ZONE_RESERVED_1_MASK = BIT(5),
+ ZONE_MAN_ISO_CNTL_MASK = BIT(6),
+ ZONE_MANUAL_CONTROL_MASK = BIT(7),
+ ZONE_PWR_DN_REQ_MASK = BIT(9),
+ ZONE_PWR_UP_REQ_MASK = BIT(10),
+ ZONE_BLK_RST_ASSERT_MASK = BIT(12),
+ ZONE_PWR_OFF_STATE_MASK = BIT(25),
+ ZONE_PWR_ON_STATE_MASK = BIT(26),
+ ZONE_DPG_PWR_STATE_MASK = BIT(28),
+ ZONE_MEM_PWR_STATE_MASK = BIT(29),
+ ZONE_RESET_STATE_MASK = BIT(31),
+ CPU0_PWR_ZONE_CTRL_REG = 1,
+ CPU_RESET_CONFIG_REG = 2,
+};
+
+static void __iomem *cpubiuctrl_block;
+static void __iomem *hif_cont_block;
+static u32 cpu0_pwr_zone_ctrl_reg;
+static u32 cpu_rst_cfg_reg;
+static u32 hif_cont_reg;
+
+#ifdef CONFIG_HOTPLUG_CPU
+static DEFINE_PER_CPU_ALIGNED(int, per_cpu_sw_state);
+
+static int per_cpu_sw_state_rd(u32 cpu)
+{
+ sync_cache_r(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
+ return per_cpu(per_cpu_sw_state, cpu);
+}
+
+static void per_cpu_sw_state_wr(u32 cpu, int val)
+{
+ per_cpu(per_cpu_sw_state, cpu) = val;
+ dmb();
+ sync_cache_w(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
+ dsb_sev();
+}
+#else
+static inline void per_cpu_sw_state_wr(u32 cpu, int val) { }
+#endif
+
+static void __iomem *pwr_ctrl_get_base(u32 cpu)
+{
+ void __iomem *base = cpubiuctrl_block + cpu0_pwr_zone_ctrl_reg;
+ base += (cpu_logical_map(cpu) * 4);
+ return base;
+}
+
+static u32 pwr_ctrl_rd(u32 cpu)
+{
+ void __iomem *base = pwr_ctrl_get_base(cpu);
+ return readl_relaxed(base);
+}
+
+static void pwr_ctrl_wr(u32 cpu, u32 val)
+{
+ void __iomem *base = pwr_ctrl_get_base(cpu);
+ writel(val, base);
+}
+
+static void cpu_rst_cfg_set(u32 cpu, int set)
+{
+ u32 val;
+ val = readl_relaxed(cpubiuctrl_block + cpu_rst_cfg_reg);
+ if (set)
+ val |= BIT(cpu_logical_map(cpu));
+ else
+ val &= ~BIT(cpu_logical_map(cpu));
+ writel_relaxed(val, cpubiuctrl_block + cpu_rst_cfg_reg);
+}
+
+static void cpu_set_boot_addr(u32 cpu, unsigned long boot_addr)
+{
+ const int reg_ofs = cpu_logical_map(cpu) * 8;
+ writel_relaxed(0, hif_cont_block + hif_cont_reg + reg_ofs);
+ writel_relaxed(boot_addr, hif_cont_block + hif_cont_reg + 4 + reg_ofs);
+}
+
+static void brcmstb_cpu_boot(u32 cpu)
+{
+ pr_info("SMP: Booting CPU%d...\n", cpu);
+
+ /*
+ * set the reset vector to point to the secondary_startup
+ * routine
+ */
+ cpu_set_boot_addr(cpu, virt_to_phys(brcmstb_secondary_startup));
+
+ /* unhalt the cpu */
+ cpu_rst_cfg_set(cpu, 0);
+}
+
+static void brcmstb_cpu_power_on(u32 cpu)
+{
+ /*
+ * The secondary cores power was cut, so we must go through
+ * power-on initialization.
+ */
+ u32 tmp;
+
+ pr_info("SMP: Powering up CPU%d...\n", cpu);
+
+ /* Request zone power up */
+ pwr_ctrl_wr(cpu, ZONE_PWR_UP_REQ_MASK);
+
+ /* Wait for the power up FSM to complete */
+ do {
+ tmp = pwr_ctrl_rd(cpu);
+ } while (!(tmp & ZONE_PWR_ON_STATE_MASK));
+
+ per_cpu_sw_state_wr(cpu, 1);
+}
+
+static int brcmstb_cpu_get_power_state(u32 cpu)
+{
+ int tmp = pwr_ctrl_rd(cpu);
+ return (tmp & ZONE_RESET_STATE_MASK) ? 0 : 1;
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+
+static void brcmstb_cpu_die(u32 cpu)
+{
+ v7_exit_coherency_flush(all);
+
+ /* Prevent all interrupts from reaching this CPU. */
+ arch_local_irq_disable();
+
+ /*
+ * Final full barrier to ensure everything before this instruction has
+ * quiesced.
+ */
+ isb();
+ dsb();
+
+ per_cpu_sw_state_wr(cpu, 0);
+
+ /* Sit and wait to die */
+ wfi();
+
+ /* We should never get here... */
+ panic("Spurious interrupt on CPU %d received!\n", cpu);
+}
+
+static int brcmstb_cpu_kill(u32 cpu)
+{
+ u32 tmp;
+
+ pr_info("SMP: Powering down CPU%d...\n", cpu);
+
+ while (per_cpu_sw_state_rd(cpu))
+ ;
+
+ /* Program zone reset */
+ pwr_ctrl_wr(cpu, ZONE_RESET_STATE_MASK | ZONE_BLK_RST_ASSERT_MASK |
+ ZONE_PWR_DN_REQ_MASK);
+
+ /* Verify zone reset */
+ tmp = pwr_ctrl_rd(cpu);
+ if (!(tmp & ZONE_RESET_STATE_MASK))
+ pr_err("%s: Zone reset bit for CPU %d not asserted!\n",
+ __func__, cpu);
+
+ /* Wait for power down */
+ do {
+ tmp = pwr_ctrl_rd(cpu);
+ } while (!(tmp & ZONE_PWR_OFF_STATE_MASK));
+
+ /* Settle-time from Broadcom-internal DVT reference code */
+ udelay(7);
+
+ /* Assert reset on the CPU */
+ cpu_rst_cfg_set(cpu, 1);
+
+ return 1;
+}
+
+#endif /* CONFIG_HOTPLUG_CPU */
+
+static int __init setup_hifcpubiuctrl_regs(struct device_node *np)
+{
+ int rc = 0;
+ char *name;
+ struct device_node *syscon_np = NULL;
+
+ name = "syscon-cpu";
+
+ syscon_np = of_parse_phandle(np, name, 0);
+ if (!syscon_np) {
+ pr_err("can't find phandle %s\n", name);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ cpubiuctrl_block = of_iomap(syscon_np, 0);
+ if (!cpubiuctrl_block) {
+ pr_err("iomap failed for cpubiuctrl_block\n");
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ rc = of_property_read_u32_index(np, name, CPU0_PWR_ZONE_CTRL_REG,
+ &cpu0_pwr_zone_ctrl_reg);
+ if (rc) {
+ pr_err("failed to read 1st entry from %s property (%d)\n", name,
+ rc);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ rc = of_property_read_u32_index(np, name, CPU_RESET_CONFIG_REG,
+ &cpu_rst_cfg_reg);
+ if (rc) {
+ pr_err("failed to read 2nd entry from %s property (%d)\n", name,
+ rc);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+cleanup:
+ if (syscon_np)
+ of_node_put(syscon_np);
+
+ return rc;
+}
+
+static int __init setup_hifcont_regs(struct device_node *np)
+{
+ int rc = 0;
+ char *name;
+ struct device_node *syscon_np = NULL;
+
+ name = "syscon-cont";
+
+ syscon_np = of_parse_phandle(np, name, 0);
+ if (!syscon_np) {
+ pr_err("can't find phandle %s\n", name);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ hif_cont_block = of_iomap(syscon_np, 0);
+ if (!hif_cont_block) {
+ pr_err("iomap failed for hif_cont_block\n");
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ /* offset is at top of hif_cont_block */
+ hif_cont_reg = 0;
+
+cleanup:
+ if (syscon_np)
+ of_node_put(syscon_np);
+
+ return rc;
+}
+
+static void __init brcmstb_cpu_ctrl_setup(unsigned int max_cpus)
+{
+ int rc;
+ struct device_node *np;
+ char *name;
+
+ name = "brcm,brcmstb-smpboot";
+ np = of_find_compatible_node(NULL, NULL, name);
+ if (!np) {
+ pr_err("can't find compatible node %s\n", name);
+ return;
+ }
+
+ rc = setup_hifcpubiuctrl_regs(np);
+ if (rc)
+ return;
+
+ rc = setup_hifcont_regs(np);
+ if (rc)
+ return;
+}
+
+static DEFINE_SPINLOCK(boot_lock);
+
+static void brcmstb_secondary_init(unsigned int cpu)
+{
+ /*
+ * Synchronise with the boot thread.
+ */
+ spin_lock(&boot_lock);
+ spin_unlock(&boot_lock);
+}
+
+static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ /*
+ * set synchronisation state between this boot processor
+ * and the secondary one
+ */
+ spin_lock(&boot_lock);
+
+ /* Bring up power to the core if necessary */
+ if (brcmstb_cpu_get_power_state(cpu) == 0)
+ brcmstb_cpu_power_on(cpu);
+
+ brcmstb_cpu_boot(cpu);
+
+ /*
+ * now the secondary core is starting up let it run its
+ * calibrations, then wait for it to finish
+ */
+ spin_unlock(&boot_lock);
+
+ return 0;
+}
+
+static struct smp_operations brcmstb_smp_ops __initdata = {
+ .smp_prepare_cpus = brcmstb_cpu_ctrl_setup,
+ .smp_secondary_init = brcmstb_secondary_init,
+ .smp_boot_secondary = brcmstb_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_kill = brcmstb_cpu_kill,
+ .cpu_die = brcmstb_cpu_die,
+#endif
+};
+
+CPU_METHOD_OF_DECLARE(brcmstb_smp, "brcm,brahma-b15", &brcmstb_smp_ops);
--
1.9.1

2014-07-22 07:29:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 02/11] power: reset: Add reboot driver for brcmstb

On Monday 21 July 2014 14:07:57 Brian Norris wrote:
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 0073633e7699..9782e8d80647 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -94,6 +94,7 @@ config ARCH_BRCMSTB
> select MIGHT_HAVE_PCI
> select HAVE_SMP
> select HAVE_ARM_ARCH_TIMER
> + select POWER_RESET_BRCMSTB
> help
> Say Y if you intend to run the kernel on a Broadcom ARM-based STB
> chipset.
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index bdcf5173e377..fcb9825debe5 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -20,6 +20,16 @@ config POWER_RESET_AXXIA
>
> Say Y if you have an Axxia family SoC.
>
> +config POWER_RESET_BRCMSTB
> + bool "Broadcom STB reset driver"
> + depends on POWER_RESET && ARCH_BRCMSTB
> + help
> + This driver provides restart support for ARM-based Broadcom STB
> + boards.
> +
> + Say Y here if you have an ARM-based Broadcom STB board and you wish
> + to have restart support.
> +
> config POWER_RESET_GPIO
> bool "GPIO power-off driver"
> depends on OF_GPIO && POWER_RESET
>

(nitpicking)

You shouldn't have both a user-selectable option and 'select' it from
the platform, because it makes it inherently not selectable, in particular
in the combination with 'depends on ARCH_BRCMSTB'.

One way to solve this would be to change the dependency to

config POWER_RESET_BRCMSTB
bool "Broadcom STB reset driver"
depends on POWER_RESET && ARM
depends on ARCH_BRCMSTB || COMPILE_TEST

which in effect would allow building it on any ARM machine as long as
COMPILE_TEST is set (which normally is not).

The same could be expressed using

config POWER_RESET_BRCMSTB
bool "Broadcom STB reset driver" if COMPILE_TEST
depends on POWER_RESET && ARM

My preference in this case however would be to just drop the 'select'
statement and add the driver to the defconfig file.

Arnd

2014-07-22 07:36:56

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

On Monday 21 July 2014 14:07:55 Brian Norris wrote:
> I'm taking over the latest resubmission of this patch series.
> There are a few moderate changes for v8 (noted below), but we
> are waiting mostly for an Ack for the reboot driver.
>
> This patchset contains the board support package for the
> Broadcom BCM7445 ARM-based SoC [1]. These changes contain a
> minimal set of code needed for a BCM7445-based board to boot
> the Linux kernel.
>
> These changes heavily leverage the OF/devicetree framework. The
> machine is also built into the multi-platform ARMv7 image.
>
> Changes are also available here:
>
> https://github.com/brcm/linux/tree/brcmstb-v8
> git://github.com/brcm/linux.git +brcmstb-v8

Whole series

Acked-by: Arnd Bergmann <[email protected]>

I think we should try to get this merged into 3.17, it's already
taken too long and the patches look good.

Please add the core architecture patches for arch/arm into Russell's
patch tracker http://www.arm.linux.org.uk/developer/patches/.

For the platform changes in the first patch, I would prefer to have
Matt pick up the first patch, but we can also apply it directly into
arm-soc if he prefers that.

The reset driver can ideally go through the drivers/power/ maintainers,
but if they are not interested in merging it, we can also take that
through arm-soc. See also my one comment on that driver.

Arnd

2014-07-22 20:02:19

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 02/11] power: reset: Add reboot driver for brcmstb

On Tue, Jul 22, 2014 at 09:28:39AM +0200, Arnd Bergmann wrote:
> On Monday 21 July 2014 14:07:57 Brian Norris wrote:
> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> > index 0073633e7699..9782e8d80647 100644
> > --- a/arch/arm/mach-bcm/Kconfig
> > +++ b/arch/arm/mach-bcm/Kconfig
> > @@ -94,6 +94,7 @@ config ARCH_BRCMSTB
> > select MIGHT_HAVE_PCI
> > select HAVE_SMP
> > select HAVE_ARM_ARCH_TIMER
> > + select POWER_RESET_BRCMSTB
> > help
> > Say Y if you intend to run the kernel on a Broadcom ARM-based STB
> > chipset.
> > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> > index bdcf5173e377..fcb9825debe5 100644
> > --- a/drivers/power/reset/Kconfig
> > +++ b/drivers/power/reset/Kconfig
> > @@ -20,6 +20,16 @@ config POWER_RESET_AXXIA
> >
> > Say Y if you have an Axxia family SoC.
> >
> > +config POWER_RESET_BRCMSTB
> > + bool "Broadcom STB reset driver"
> > + depends on POWER_RESET && ARCH_BRCMSTB
> > + help
> > + This driver provides restart support for ARM-based Broadcom STB
> > + boards.
> > +
> > + Say Y here if you have an ARM-based Broadcom STB board and you wish
> > + to have restart support.
> > +
> > config POWER_RESET_GPIO
> > bool "GPIO power-off driver"
> > depends on OF_GPIO && POWER_RESET
> >
>
> (nitpicking)
>
> You shouldn't have both a user-selectable option and 'select' it from
> the platform, because it makes it inherently not selectable, in particular
> in the combination with 'depends on ARCH_BRCMSTB'.

OK, thanks for the comment. Note that there is some precedent for this
bad Kconfig behavior in the same subsystem (POWER_RESET_GPIO and
POWER_RESET_VEXPRESS are both 'select'ed in arch/arm/mach-*/Kconfig).

[...]
> My preference in this case however would be to just drop the 'select'
> statement and add the driver to the defconfig file.

How about a third option, where we drop the 'select' statement and
set POWER_RESET_BRCMSTB to be 'default y'? Then we don't have to modify
the defconfig, and it gives the added bonus of choosing a sane default
even if you're not based on the multi_v7_defconfig. i.e.:

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 58c01aed9752..634de7b7fd28 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -94,7 +94,6 @@ config ARCH_BRCMSTB
select MIGHT_HAVE_PCI
select HAVE_SMP
select HAVE_ARM_ARCH_TIMER
- select POWER_RESET_BRCMSTB
select BRCMSTB_GISB_ARB
select BRCMSTB_L2_IRQ
help
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index fcb9825debe5..ab5d57e2766d 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -23,6 +23,7 @@ config POWER_RESET_AXXIA
config POWER_RESET_BRCMSTB
bool "Broadcom STB reset driver"
depends on POWER_RESET && ARCH_BRCMSTB
+ default y
help
This driver provides restart support for ARM-based Broadcom STB
boards.

Brian

2014-07-22 20:44:38

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

+ Sebastian

On Tue, Jul 22, 2014 at 09:35:45AM +0200, Arnd Bergmann wrote:
> On Monday 21 July 2014 14:07:55 Brian Norris wrote:
> > I'm taking over the latest resubmission of this patch series.
> > There are a few moderate changes for v8 (noted below), but we
> > are waiting mostly for an Ack for the reboot driver.
> >
> > This patchset contains the board support package for the
> > Broadcom BCM7445 ARM-based SoC [1]. These changes contain a
> > minimal set of code needed for a BCM7445-based board to boot
> > the Linux kernel.
> >
> > These changes heavily leverage the OF/devicetree framework. The
> > machine is also built into the multi-platform ARMv7 image.
> >
> > Changes are also available here:
> >
> > https://github.com/brcm/linux/tree/brcmstb-v8
> > git://github.com/brcm/linux.git +brcmstb-v8
>
> Whole series
>
> Acked-by: Arnd Bergmann <[email protected]>

Thanks!

> I think we should try to get this merged into 3.17, it's already
> taken too long and the patches look good.

Yes, it's about time.

> Please add the core architecture patches for arch/arm into Russell's
> patch tracker http://www.arm.linux.org.uk/developer/patches/.

OK, that's unfamiliar to me, but I'll try that out.

> For the platform changes in the first patch, I would prefer to have
> Matt pick up the first patch, but we can also apply it directly into
> arm-soc if he prefers that.

That brings up a question related to PATCH 11 in the series (MAINTAINERS
update); who will be maintaining arch/arm/mach-bcm/*brcmstb*, and how
will code go upstream? It seems like Matt and Christian are officially
mach-bcm maintainers, although I don't know if Christian is still
involved.

Also, BCM7xxx shares little in common with the rest of mach-bcm, except
a company name, so we'd really like at least the 'Maintainer' entries
for the CC. I was planning on a separate git tree too, although it could
have conflicts if we touch arch/arm/mach-bcm/{Makefile,Kconfig}.

So would we send a separate arm-soc pull request for the arm-soc
targeted changes (and all future development)?

> The reset driver can ideally go through the drivers/power/ maintainers,
> but if they are not interested in merging it, we can also take that
> through arm-soc.

I don't see much activity from the drivers/power/ maintainers on the
mailing lists, and I know David Woodhouse at least has been severely
distracted (I took over for him on driver/mtd/ recently), so I don't
expect much. (EDIT: I see [1]. Nice!)

> See also my one comment on that driver.

I proposed an additional diff on that patch in response.

I can strip the drivers/power/reboot/ driver out and send it separately
to our new drivers/power/ maintainer (thanks Sebastian!), and hope that
it gets through for 3.17.

For the reset of mach-bcm stuff, I'll just send an arm-soc pull request
soon enough, unless Matt/Arnd/Olof object.

Thanks,
Brian

[1] https://lkml.org/lkml/2014/7/16/539

2014-07-22 20:58:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

On Tuesday 22 July 2014 13:44:31 Brian Norris wrote:
> > For the platform changes in the first patch, I would prefer to have
> > Matt pick up the first patch, but we can also apply it directly into
> > arm-soc if he prefers that.
>
> That brings up a question related to PATCH 11 in the series (MAINTAINERS
> update); who will be maintaining arch/arm/mach-bcm/*brcmstb*, and how
> will code go upstream? It seems like Matt and Christian are officially
> mach-bcm maintainers, although I don't know if Christian is still
> involved.

You have to solve that question together with Matt. From my perspective
it would be easier if I only have to deal with one person for mach-bcm,
but it's really up to you.

> Also, BCM7xxx shares little in common with the rest of mach-bcm, except
> a company name, so we'd really like at least the 'Maintainer' entries
> for the CC. I was planning on a separate git tree too, although it could
> have conflicts if we touch arch/arm/mach-bcm/{Makefile,Kconfig}.
>
> So would we send a separate arm-soc pull request for the arm-soc
> targeted changes (and all future development)?

You can definitely have the separate MAINTAINERS entry without necessarily
becoming a maintainer at the same level. I know Matt is very responsive
and can forward your patches to arm-soc if that works for you.

> For the reset of mach-bcm stuff, I'll just send an arm-soc pull request
> soon enough, unless Matt/Arnd/Olof object.

I'll wait for Matt to comment before pulling it, otherwise that sounds
fine.

Arnd

2014-07-22 21:02:42

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 02/11] power: reset: Add reboot driver for brcmstb

On Tuesday 22 July 2014 13:02:13 Brian Norris wrote:
> How about a third option, where we drop the 'select' statement and
> set POWER_RESET_BRCMSTB to be 'default y'? Then we don't have to modify
> the defconfig, and it gives the added bonus of choosing a sane default
> even if you're not based on the multi_v7_defconfig. i.e.:
>
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 58c01aed9752..634de7b7fd28 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -94,7 +94,6 @@ config ARCH_BRCMSTB
> select MIGHT_HAVE_PCI
> select HAVE_SMP
> select HAVE_ARM_ARCH_TIMER
> - select POWER_RESET_BRCMSTB
> select BRCMSTB_GISB_ARB
> select BRCMSTB_L2_IRQ
> help
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index fcb9825debe5..ab5d57e2766d 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -23,6 +23,7 @@ config POWER_RESET_AXXIA
> config POWER_RESET_BRCMSTB
> bool "Broadcom STB reset driver"
> depends on POWER_RESET && ARCH_BRCMSTB
> + default y
> help
> This driver provides restart support for ARM-based Broadcom STB
> boards.

I don't like this too much. Why do you want to allow disabling the
driver if you make it 'default y' in the first place?

We try to avoid 'default y' for user-selectable drivers in general.
I noticed that in my example, I was missing the default. It should
have been

config POWER_RESET_BRCMSTB
bool "Broadcom STB reset driver" if COMPILE_TEST
depends on POWER_RESET && ARM
default ARCH_BRCMSTB

This way, it always gets selected when ARCH_BRCMSTB is on and
COMPILE_TEST is off. With COMPILE_TEST enabled, it defaults to
ARCH_BRCMSTB but can be enabled or disabled for the purpose
of compile testing.

Arnd

2014-07-22 21:33:58

by Matt Porter

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

On Tue, Jul 22, 2014 at 10:57:17PM +0200, Arnd Bergmann wrote:
> On Tuesday 22 July 2014 13:44:31 Brian Norris wrote:
> > > For the platform changes in the first patch, I would prefer to have
> > > Matt pick up the first patch, but we can also apply it directly into
> > > arm-soc if he prefers that.
> >
> > That brings up a question related to PATCH 11 in the series (MAINTAINERS
> > update); who will be maintaining arch/arm/mach-bcm/*brcmstb*, and how
> > will code go upstream? It seems like Matt and Christian are officially
> > mach-bcm maintainers, although I don't know if Christian is still
> > involved.
>
> You have to solve that question together with Matt. From my perspective
> it would be easier if I only have to deal with one person for mach-bcm,
> but it's really up to you.
>
>
> > Also, BCM7xxx shares little in common with the rest of mach-bcm, except
> > a company name, so we'd really like at least the 'Maintainer' entries
> > for the CC. I was planning on a separate git tree too, although it could
> > have conflicts if we touch arch/arm/mach-bcm/{Makefile,Kconfig}.
> >
> > So would we send a separate arm-soc pull request for the arm-soc
> > targeted changes (and all future development)?
>
> You can definitely have the separate MAINTAINERS entry without necessarily
> becoming a maintainer at the same level. I know Matt is very responsive
> and can forward your patches to arm-soc if that works for you.

We had this discussion wrt Hauke's BCM5301x support. The situation is
basically the same. To date, every instance of a BCM ARM SoC shares
nothing more than the company name and in some rare cases a piece of
licensed IP like the Arasan sdhci or dwc2. I can't recall who asked
for it, but I think Olof asked to have one pull request for all of
mach-bcm to keep things simple. So we ended up with:

BROADCOM BCM5301X ARM ARCHICTURE
M: Hauke Mehrtens <[email protected]>
L: [email protected]
S: Maintained
F: arch/arm/mach-bcm/bcm_5301x.c
F: arch/arm/boot/dts/bcm5301x.dtsi
F: arch/arm/boot/dts/bcm470*

which could be a model for BCM7xxx maintainership and Christian and I
can aggregate this stuff in the mach-bcm tree for pull requests to the
arm-soc team as we are doing now. bcm2835 is an exception case since
that was living elsewhere before the new platforms colocated in
mach-bcm.

> > For the reset of mach-bcm stuff, I'll just send an arm-soc pull request
> > soon enough, unless Matt/Arnd/Olof object.
>
> I'll wait for Matt to comment before pulling it, otherwise that sounds
> fine.

I'm fine with it, but we were asked to have one request for bcm5301x to
make life easy for the arm-soc team. If we want each platform to do pull
requests directly with arm-soc we should advise Hauke to do the same as
well with bcm5301x. I think the chances of Kconfig/Makefile conflicts are
relatively low as there's a dwindling amount of code of interest in new
platform mach directories like ours.

I would like to see a consistent path for all BCM platform maintainers.

-Matt

2014-07-22 22:25:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

On Tuesday 22 July 2014 17:33:50 Matt Porter wrote:
>
> > > For the reset of mach-bcm stuff, I'll just send an arm-soc pull request
> > > soon enough, unless Matt/Arnd/Olof object.
> >
> > I'll wait for Matt to comment before pulling it, otherwise that sounds
> > fine.
>
> I'm fine with it, but we were asked to have one request for bcm5301x to
> make life easy for the arm-soc team. If we want each platform to do pull
> requests directly with arm-soc we should advise Hauke to do the same as
> well with bcm5301x. I think the chances of Kconfig/Makefile conflicts are
> relatively low as there's a dwindling amount of code of interest in new
> platform mach directories like ours.
>
> I would like to see a consistent path for all BCM platform maintainers.
>

Ok. Brian, please send the pull request or patches to Matt then.

Arnd

2014-07-22 22:30:59

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 00/11] ARM: brcmstb: Add Broadcom STB SoC support

On Wed, Jul 23, 2014 at 12:24:23AM +0200, Arnd Bergmann wrote:
> On Tuesday 22 July 2014 17:33:50 Matt Porter wrote:
> > > > For the reset of mach-bcm stuff, I'll just send an arm-soc pull request
> > > > soon enough, unless Matt/Arnd/Olof object.
> > >
> > > I'll wait for Matt to comment before pulling it, otherwise that sounds
> > > fine.
> >
> > I'm fine with it, but we were asked to have one request for bcm5301x to
> > make life easy for the arm-soc team.

That's all news to me; thanks for the explanation.

> > If we want each platform to do pull
> > requests directly with arm-soc we should advise Hauke to do the same as
> > well with bcm5301x. I think the chances of Kconfig/Makefile conflicts are
> > relatively low as there's a dwindling amount of code of interest in new
> > platform mach directories like ours.
> >
> > I would like to see a consistent path for all BCM platform maintainers.
>
> Ok. Brian, please send the pull request or patches to Matt then.

Sure thing. I'll drop the 'select POWER_RESET_BRCMSTB' and also drop our
git tree from MAINTAINERS. I'll send v9 of the reboot driver separately,
targeted at Sebastian (or else to also defer to Matt's tree).

Thanks,
Brian

2014-07-22 22:51:33

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 02/11] power: reset: Add reboot driver for brcmstb

+ Sebastian

On Tue, Jul 22, 2014 at 11:02:07PM +0200, Arnd Bergmann wrote:
> On Tuesday 22 July 2014 13:02:13 Brian Norris wrote:
> > How about a third option, where we drop the 'select' statement and
> > set POWER_RESET_BRCMSTB to be 'default y'? Then we don't have to modify
> > the defconfig, and it gives the added bonus of choosing a sane default
> > even if you're not based on the multi_v7_defconfig. i.e.:
> >
> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> > index 58c01aed9752..634de7b7fd28 100644
> > --- a/arch/arm/mach-bcm/Kconfig
> > +++ b/arch/arm/mach-bcm/Kconfig
> > @@ -94,7 +94,6 @@ config ARCH_BRCMSTB
> > select MIGHT_HAVE_PCI
> > select HAVE_SMP
> > select HAVE_ARM_ARCH_TIMER
> > - select POWER_RESET_BRCMSTB
> > select BRCMSTB_GISB_ARB
> > select BRCMSTB_L2_IRQ
> > help
> > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> > index fcb9825debe5..ab5d57e2766d 100644
> > --- a/drivers/power/reset/Kconfig
> > +++ b/drivers/power/reset/Kconfig
> > @@ -23,6 +23,7 @@ config POWER_RESET_AXXIA
> > config POWER_RESET_BRCMSTB
> > bool "Broadcom STB reset driver"
> > depends on POWER_RESET && ARCH_BRCMSTB
> > + default y
> > help
> > This driver provides restart support for ARM-based Broadcom STB
> > boards.
>
> I don't like this too much. Why do you want to allow disabling the
> driver if you make it 'default y' in the first place?

Not much of a reason, but I thought configurability is a positive.
There's really not a good reason to not have this driver though; I can't
imagine someone wanting to build our platform but not be able to reboot
it.

> We try to avoid 'default y' for user-selectable drivers in general.

I didn't know that. A default is technically different than "should a
user be able to disable this," but I suppose the policy makes sense.

> I noticed that in my example, I was missing the default.

I see. That's why I immediately ruled it out.

> It should have been
>
> config POWER_RESET_BRCMSTB
> bool "Broadcom STB reset driver" if COMPILE_TEST
> depends on POWER_RESET && ARM
> default ARCH_BRCMSTB

OK, that's OK with me if it's OK with Sebastian, et al. Respin coming
soon, separate from the mach-bcm and ARM stuff...

(BTW, we might consider rewriting the other config entries to be
similar, if we really want to make this things consistently
nitpick-proof.)

> This way, it always gets selected when ARCH_BRCMSTB is on and
> COMPILE_TEST is off. With COMPILE_TEST enabled, it defaults to
> ARCH_BRCMSTB but can be enabled or disabled for the purpose
> of compile testing.

Thanks,
Brian

2014-07-30 09:26:48

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

On Mon, Jul 21, 2014 at 02:07:56PM -0700, Brian Norris wrote:
> +static DEFINE_SPINLOCK(boot_lock);
> +
> +static void brcmstb_secondary_init(unsigned int cpu)
> +{
> + /*
> + * Synchronise with the boot thread.
> + */
> + spin_lock(&boot_lock);
> + spin_unlock(&boot_lock);
> +}
> +
> +static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + /*
> + * set synchronisation state between this boot processor
> + * and the secondary one
> + */
> + spin_lock(&boot_lock);
> +
> + /* Bring up power to the core if necessary */
> + if (brcmstb_cpu_get_power_state(cpu) == 0)
> + brcmstb_cpu_power_on(cpu);
> +
> + brcmstb_cpu_boot(cpu);
> +
> + /*
> + * now the secondary core is starting up let it run its
> + * calibrations, then wait for it to finish
> + */
> + spin_unlock(&boot_lock);

I've just read through this code (because it caused my allmodconfig to
break) and spotted this.

What function does boot_lock perform here? Please, don't quote the
comments (I know where the comments came from) but what I want to hear
is your comments about why you decided to retain this.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-07-30 17:10:12

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

On Mon, Jul 21, 2014 at 4:07 PM, Brian Norris
<[email protected]> wrote:
> From: Marc Carino <[email protected]>
>
> The BCM7xxx series of Broadcom SoCs are used primarily in set-top boxes.
>
> This patch adds machine support for the ARM-based Broadcom SoCs.
>
> Signed-off-by: Marc Carino <[email protected]>
> Signed-off-by: Brian Norris <[email protected]>
> ---
> arch/arm/configs/multi_v7_defconfig | 1 +
> arch/arm/mach-bcm/Kconfig | 14 ++
> arch/arm/mach-bcm/Makefile | 5 +
> arch/arm/mach-bcm/brcmstb.c | 28 +++
> arch/arm/mach-bcm/brcmstb.h | 19 ++
> arch/arm/mach-bcm/headsmp-brcmstb.S | 33 ++++
> arch/arm/mach-bcm/platsmp-brcmstb.c | 363 ++++++++++++++++++++++++++++++++++++
> 7 files changed, 463 insertions(+)
> create mode 100644 arch/arm/mach-bcm/brcmstb.c
> create mode 100644 arch/arm/mach-bcm/brcmstb.h
> create mode 100644 arch/arm/mach-bcm/headsmp-brcmstb.S
> create mode 100644 arch/arm/mach-bcm/platsmp-brcmstb.c
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 534836497998..bf0df396a3cf 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -19,6 +19,7 @@ CONFIG_MACH_DOVE=y
> CONFIG_ARCH_BCM=y
> CONFIG_ARCH_BCM_MOBILE=y
> CONFIG_ARCH_BCM_5301X=y
> +CONFIG_ARCH_BRCMSTB=y
> CONFIG_ARCH_BERLIN=y
> CONFIG_MACH_BERLIN_BG2=y
> CONFIG_MACH_BERLIN_BG2CD=y
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 41c839167e87..0073633e7699 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -87,4 +87,18 @@ config ARCH_BCM_5301X
> different SoC or with the older BCM47XX and BCM53XX based
> network SoC using a MIPS CPU, they are supported by arch/mips/bcm47xx
>
> +config ARCH_BRCMSTB
> + bool "Broadcom BCM7XXX based boards" if ARCH_MULTI_V7
> + depends on MMU

This was implied, but there are some patches from Arnd in this area.
Does it really not build with !MMU?

> + select ARM_GIC

> + select MIGHT_HAVE_PCI
> + select HAVE_SMP

At least HAVE_SMP is for sure, but I think both of these are selected already .

> + select HAVE_ARM_ARCH_TIMER
> + help
> + Say Y if you intend to run the kernel on a Broadcom ARM-based STB
> + chipset.
> +
> + This enables support for Broadcom ARM-based set-top box chipsets,
> + including the 7445 family of chips.
> +
> endif
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 731292114975..f3665121729b 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -30,3 +30,8 @@ obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
>
> # BCM5301X
> obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o
> +
> +ifeq ($(CONFIG_ARCH_BRCMSTB),y)
> +obj-y += brcmstb.o
> +obj-$(CONFIG_SMP) += headsmp-brcmstb.o platsmp-brcmstb.o
> +endif
> diff --git a/arch/arm/mach-bcm/brcmstb.c b/arch/arm/mach-bcm/brcmstb.c
> new file mode 100644
> index 000000000000..60a5afa06ed7
> --- /dev/null
> +++ b/arch/arm/mach-bcm/brcmstb.c
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright (C) 2013-2014 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +
> +static const char *brcmstb_match[] __initconst = {
> + "brcm,bcm7445",
> + "brcm,brcmstb",
> + NULL
> +};
> +
> +DT_MACHINE_START(BRCMSTB, "Broadcom STB (Flattened Device Tree)")
> + .dt_compat = brcmstb_match,
> +MACHINE_END

Do you plan to add more here? Otherwise you don't need this file.

> diff --git a/arch/arm/mach-bcm/brcmstb.h b/arch/arm/mach-bcm/brcmstb.h
> new file mode 100644
> index 000000000000..ec0c3d112b36
> --- /dev/null
> +++ b/arch/arm/mach-bcm/brcmstb.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2013-2014 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __BRCMSTB_H__
> +#define __BRCMSTB_H__
> +
> +void brcmstb_secondary_startup(void);
> +
> +#endif /* __BRCMSTB_H__ */
> diff --git a/arch/arm/mach-bcm/headsmp-brcmstb.S b/arch/arm/mach-bcm/headsmp-brcmstb.S
> new file mode 100644
> index 000000000000..199c1ea58248
> --- /dev/null
> +++ b/arch/arm/mach-bcm/headsmp-brcmstb.S
> @@ -0,0 +1,33 @@
> +/*
> + * SMP boot code for secondary CPUs
> + * Based on arch/arm/mach-tegra/headsmp.S
> + *
> + * Copyright (C) 2010 NVIDIA, Inc.
> + * Copyright (C) 2013-2014 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <asm/assembler.h>
> +#include <linux/linkage.h>
> +#include <linux/init.h>
> +
> + .section ".text.head", "ax"
> +
> +ENTRY(brcmstb_secondary_startup)
> + /*
> + * Ensure CPU is in a sane state by disabling all IRQs and switching
> + * into SVC mode.
> + */
> + setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r0
> +
> + bl v7_invalidate_l1

This should be in your boot code. That is part of the documented entry
requirements. Also, if you are coming straight from reset, there is
other setup probably missing.

Yes, there are others with this, but that doesn't make it right.

> + b secondary_startup
> +ENDPROC(brcmstb_secondary_startup)
> diff --git a/arch/arm/mach-bcm/platsmp-brcmstb.c b/arch/arm/mach-bcm/platsmp-brcmstb.c
> new file mode 100644
> index 000000000000..af780e9c23a6
> --- /dev/null
> +++ b/arch/arm/mach-bcm/platsmp-brcmstb.c
> @@ -0,0 +1,363 @@
> +/*
> + * Broadcom STB CPU SMP and hotplug support for ARM
> + *
> + * Copyright (C) 2013-2014 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/printk.h>
> +#include <linux/regmap.h>
> +#include <linux/smp.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/spinlock.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/cp15.h>
> +#include <asm/mach-types.h>
> +#include <asm/smp_plat.h>
> +
> +#include "brcmstb.h"
> +
> +enum {
> + ZONE_MAN_CLKEN_MASK = BIT(0),
> + ZONE_MAN_RESET_CNTL_MASK = BIT(1),
> + ZONE_MAN_MEM_PWR_MASK = BIT(4),
> + ZONE_RESERVED_1_MASK = BIT(5),
> + ZONE_MAN_ISO_CNTL_MASK = BIT(6),
> + ZONE_MANUAL_CONTROL_MASK = BIT(7),
> + ZONE_PWR_DN_REQ_MASK = BIT(9),
> + ZONE_PWR_UP_REQ_MASK = BIT(10),
> + ZONE_BLK_RST_ASSERT_MASK = BIT(12),
> + ZONE_PWR_OFF_STATE_MASK = BIT(25),
> + ZONE_PWR_ON_STATE_MASK = BIT(26),
> + ZONE_DPG_PWR_STATE_MASK = BIT(28),
> + ZONE_MEM_PWR_STATE_MASK = BIT(29),
> + ZONE_RESET_STATE_MASK = BIT(31),
> + CPU0_PWR_ZONE_CTRL_REG = 1,
> + CPU_RESET_CONFIG_REG = 2,
> +};
> +
> +static void __iomem *cpubiuctrl_block;
> +static void __iomem *hif_cont_block;
> +static u32 cpu0_pwr_zone_ctrl_reg;
> +static u32 cpu_rst_cfg_reg;
> +static u32 hif_cont_reg;
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> +static DEFINE_PER_CPU_ALIGNED(int, per_cpu_sw_state);
> +
> +static int per_cpu_sw_state_rd(u32 cpu)
> +{
> + sync_cache_r(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
> + return per_cpu(per_cpu_sw_state, cpu);
> +}
> +
> +static void per_cpu_sw_state_wr(u32 cpu, int val)
> +{
> + per_cpu(per_cpu_sw_state, cpu) = val;
> + dmb();
> + sync_cache_w(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
> + dsb_sev();

Barriers need to be documented why they are needed.

> +}
> +#else
> +static inline void per_cpu_sw_state_wr(u32 cpu, int val) { }
> +#endif
> +
> +static void __iomem *pwr_ctrl_get_base(u32 cpu)
> +{
> + void __iomem *base = cpubiuctrl_block + cpu0_pwr_zone_ctrl_reg;
> + base += (cpu_logical_map(cpu) * 4);
> + return base;
> +}
> +
> +static u32 pwr_ctrl_rd(u32 cpu)
> +{
> + void __iomem *base = pwr_ctrl_get_base(cpu);
> + return readl_relaxed(base);
> +}
> +
> +static void pwr_ctrl_wr(u32 cpu, u32 val)
> +{
> + void __iomem *base = pwr_ctrl_get_base(cpu);
> + writel(val, base);
> +}
> +
> +static void cpu_rst_cfg_set(u32 cpu, int set)
> +{
> + u32 val;
> + val = readl_relaxed(cpubiuctrl_block + cpu_rst_cfg_reg);
> + if (set)
> + val |= BIT(cpu_logical_map(cpu));
> + else
> + val &= ~BIT(cpu_logical_map(cpu));
> + writel_relaxed(val, cpubiuctrl_block + cpu_rst_cfg_reg);
> +}
> +
> +static void cpu_set_boot_addr(u32 cpu, unsigned long boot_addr)
> +{
> + const int reg_ofs = cpu_logical_map(cpu) * 8;
> + writel_relaxed(0, hif_cont_block + hif_cont_reg + reg_ofs);
> + writel_relaxed(boot_addr, hif_cont_block + hif_cont_reg + 4 + reg_ofs);
> +}
> +
> +static void brcmstb_cpu_boot(u32 cpu)
> +{
> + pr_info("SMP: Booting CPU%d...\n", cpu);

The core already prints messages.

> +
> + /*
> + * set the reset vector to point to the secondary_startup
> + * routine
> + */
> + cpu_set_boot_addr(cpu, virt_to_phys(brcmstb_secondary_startup));
> +
> + /* unhalt the cpu */
> + cpu_rst_cfg_set(cpu, 0);
> +}
> +
> +static void brcmstb_cpu_power_on(u32 cpu)
> +{
> + /*
> + * The secondary cores power was cut, so we must go through
> + * power-on initialization.
> + */
> + u32 tmp;
> +
> + pr_info("SMP: Powering up CPU%d...\n", cpu);

ditto.

> +
> + /* Request zone power up */
> + pwr_ctrl_wr(cpu, ZONE_PWR_UP_REQ_MASK);
> +
> + /* Wait for the power up FSM to complete */
> + do {
> + tmp = pwr_ctrl_rd(cpu);
> + } while (!(tmp & ZONE_PWR_ON_STATE_MASK));

Perhaps a timeout? Or perhaps you don't need to wait here. The core
code will wait for the core to come online.

> +
> + per_cpu_sw_state_wr(cpu, 1);

The kernel already tracks the state.

> +}
> +
> +static int brcmstb_cpu_get_power_state(u32 cpu)
> +{
> + int tmp = pwr_ctrl_rd(cpu);
> + return (tmp & ZONE_RESET_STATE_MASK) ? 0 : 1;
> +}
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> +
> +static void brcmstb_cpu_die(u32 cpu)
> +{
> + v7_exit_coherency_flush(all);
> +
> + /* Prevent all interrupts from reaching this CPU. */
> + arch_local_irq_disable();

Surely that is already done? And disabling IRQ has no effect on
wake-up from WFI.

> +
> + /*
> + * Final full barrier to ensure everything before this instruction has
> + * quiesced.
> + */
> + isb();
> + dsb();
> +
> + per_cpu_sw_state_wr(cpu, 0);
> +
> + /* Sit and wait to die */
> + wfi();
> +
> + /* We should never get here... */
> + panic("Spurious interrupt on CPU %d received!\n", cpu);
> +}
> +
> +static int brcmstb_cpu_kill(u32 cpu)
> +{
> + u32 tmp;
> +
> + pr_info("SMP: Powering down CPU%d...\n", cpu);
> +
> + while (per_cpu_sw_state_rd(cpu))
> + ;

I don't think this is necessary. You don't need to synchronize die and
kill. Look at other implementations that actually power down the core
(vs. just wfi).

> +
> + /* Program zone reset */
> + pwr_ctrl_wr(cpu, ZONE_RESET_STATE_MASK | ZONE_BLK_RST_ASSERT_MASK |
> + ZONE_PWR_DN_REQ_MASK);
> +
> + /* Verify zone reset */
> + tmp = pwr_ctrl_rd(cpu);
> + if (!(tmp & ZONE_RESET_STATE_MASK))
> + pr_err("%s: Zone reset bit for CPU %d not asserted!\n",
> + __func__, cpu);
> +
> + /* Wait for power down */
> + do {
> + tmp = pwr_ctrl_rd(cpu);
> + } while (!(tmp & ZONE_PWR_OFF_STATE_MASK));
> +
> + /* Settle-time from Broadcom-internal DVT reference code */
> + udelay(7);
> +
> + /* Assert reset on the CPU */
> + cpu_rst_cfg_set(cpu, 1);
> +
> + return 1;
> +}
> +
> +#endif /* CONFIG_HOTPLUG_CPU */
> +
> +static int __init setup_hifcpubiuctrl_regs(struct device_node *np)
> +{
> + int rc = 0;
> + char *name;
> + struct device_node *syscon_np = NULL;
> +
> + name = "syscon-cpu";
> +
> + syscon_np = of_parse_phandle(np, name, 0);
> + if (!syscon_np) {
> + pr_err("can't find phandle %s\n", name);
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + cpubiuctrl_block = of_iomap(syscon_np, 0);
> + if (!cpubiuctrl_block) {
> + pr_err("iomap failed for cpubiuctrl_block\n");
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + rc = of_property_read_u32_index(np, name, CPU0_PWR_ZONE_CTRL_REG,
> + &cpu0_pwr_zone_ctrl_reg);
> + if (rc) {
> + pr_err("failed to read 1st entry from %s property (%d)\n", name,
> + rc);
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + rc = of_property_read_u32_index(np, name, CPU_RESET_CONFIG_REG,
> + &cpu_rst_cfg_reg);
> + if (rc) {
> + pr_err("failed to read 2nd entry from %s property (%d)\n", name,
> + rc);
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> +cleanup:
> + if (syscon_np)
> + of_node_put(syscon_np);
> +
> + return rc;
> +}
> +
> +static int __init setup_hifcont_regs(struct device_node *np)
> +{
> + int rc = 0;
> + char *name;
> + struct device_node *syscon_np = NULL;
> +
> + name = "syscon-cont";
> +
> + syscon_np = of_parse_phandle(np, name, 0);
> + if (!syscon_np) {
> + pr_err("can't find phandle %s\n", name);
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + hif_cont_block = of_iomap(syscon_np, 0);
> + if (!hif_cont_block) {
> + pr_err("iomap failed for hif_cont_block\n");
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + /* offset is at top of hif_cont_block */
> + hif_cont_reg = 0;
> +
> +cleanup:
> + if (syscon_np)
> + of_node_put(syscon_np);
> +
> + return rc;
> +}
> +
> +static void __init brcmstb_cpu_ctrl_setup(unsigned int max_cpus)
> +{
> + int rc;
> + struct device_node *np;
> + char *name;
> +
> + name = "brcm,brcmstb-smpboot";
> + np = of_find_compatible_node(NULL, NULL, name);
> + if (!np) {
> + pr_err("can't find compatible node %s\n", name);
> + return;
> + }
> +
> + rc = setup_hifcpubiuctrl_regs(np);
> + if (rc)
> + return;
> +
> + rc = setup_hifcont_regs(np);
> + if (rc)
> + return;
> +}
> +
> +static DEFINE_SPINLOCK(boot_lock);
> +
> +static void brcmstb_secondary_init(unsigned int cpu)
> +{
> + /*
> + * Synchronise with the boot thread.
> + */
> + spin_lock(&boot_lock);
> + spin_unlock(&boot_lock);

I suggest you read previous discussions on attempts to make this
common before replying to Russell.

> +}
> +
> +static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + /*
> + * set synchronisation state between this boot processor
> + * and the secondary one
> + */
> + spin_lock(&boot_lock);
> +
> + /* Bring up power to the core if necessary */
> + if (brcmstb_cpu_get_power_state(cpu) == 0)
> + brcmstb_cpu_power_on(cpu);
> +
> + brcmstb_cpu_boot(cpu);
> +
> + /*
> + * now the secondary core is starting up let it run its
> + * calibrations, then wait for it to finish
> + */
> + spin_unlock(&boot_lock);
> +
> + return 0;
> +}
> +
> +static struct smp_operations brcmstb_smp_ops __initdata = {
> + .smp_prepare_cpus = brcmstb_cpu_ctrl_setup,
> + .smp_secondary_init = brcmstb_secondary_init,
> + .smp_boot_secondary = brcmstb_boot_secondary,
> +#ifdef CONFIG_HOTPLUG_CPU
> + .cpu_kill = brcmstb_cpu_kill,
> + .cpu_die = brcmstb_cpu_die,
> +#endif
> +};
> +
> +CPU_METHOD_OF_DECLARE(brcmstb_smp, "brcm,brahma-b15", &brcmstb_smp_ops);
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2014-07-31 02:23:26

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

Hi Rob,

I appreciate your comments, but where were many of these 5 months ago on
the first 7 revisions? :)

On a practical note: v9 is already queued for 3.17. Should I send
patches for the 3.17 cycle (or later) to fixup some of these issues? Or
would you recommend pulling the patches out of Matt Porter's tree now,
and reintroducing for 3.18? (I would be much happier with the first.)

I do note that we at least need to fix allmodconfig ASAP; I'll reply to
Russell on that one.

On Wed, Jul 30, 2014 at 12:09:48PM -0500, Rob Herring wrote:
> On Mon, Jul 21, 2014 at 4:07 PM, Brian Norris <[email protected]> wrote:
> > From: Marc Carino <[email protected]>
> >
> > The BCM7xxx series of Broadcom SoCs are used primarily in set-top boxes.
> >
> > This patch adds machine support for the ARM-based Broadcom SoCs.
> >
> > Signed-off-by: Marc Carino <[email protected]>
> > Signed-off-by: Brian Norris <[email protected]>
> > ---
> > arch/arm/configs/multi_v7_defconfig | 1 +
> > arch/arm/mach-bcm/Kconfig | 14 ++
> > arch/arm/mach-bcm/Makefile | 5 +
> > arch/arm/mach-bcm/brcmstb.c | 28 +++
> > arch/arm/mach-bcm/brcmstb.h | 19 ++
> > arch/arm/mach-bcm/headsmp-brcmstb.S | 33 ++++
> > arch/arm/mach-bcm/platsmp-brcmstb.c | 363 ++++++++++++++++++++++++++++++++++++
> > 7 files changed, 463 insertions(+)
> > create mode 100644 arch/arm/mach-bcm/brcmstb.c
> > create mode 100644 arch/arm/mach-bcm/brcmstb.h
> > create mode 100644 arch/arm/mach-bcm/headsmp-brcmstb.S
> > create mode 100644 arch/arm/mach-bcm/platsmp-brcmstb.c
> >
> > diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> > index 534836497998..bf0df396a3cf 100644
> > --- a/arch/arm/configs/multi_v7_defconfig
> > +++ b/arch/arm/configs/multi_v7_defconfig
> > @@ -19,6 +19,7 @@ CONFIG_MACH_DOVE=y
> > CONFIG_ARCH_BCM=y
> > CONFIG_ARCH_BCM_MOBILE=y
> > CONFIG_ARCH_BCM_5301X=y
> > +CONFIG_ARCH_BRCMSTB=y
> > CONFIG_ARCH_BERLIN=y
> > CONFIG_MACH_BERLIN_BG2=y
> > CONFIG_MACH_BERLIN_BG2CD=y
> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> > index 41c839167e87..0073633e7699 100644
> > --- a/arch/arm/mach-bcm/Kconfig
> > +++ b/arch/arm/mach-bcm/Kconfig
> > @@ -87,4 +87,18 @@ config ARCH_BCM_5301X
> > different SoC or with the older BCM47XX and BCM53XX based
> > network SoC using a MIPS CPU, they are supported by arch/mips/bcm47xx
> >
> > +config ARCH_BRCMSTB
> > + bool "Broadcom BCM7XXX based boards" if ARCH_MULTI_V7
> > + depends on MMU
>
> This was implied, but there are some patches from Arnd in this area.
> Does it really not build with !MMU?

I suppose it probably builds, it likely won't run. I can drop this line
and then reassess if ARCH_MULTIPLATFORM becomes buildable with !MMU.

> > + select ARM_GIC
>
> > + select MIGHT_HAVE_PCI
> > + select HAVE_SMP
>
> At least HAVE_SMP is for sure, but I think both of these are selected already .

You're correct. ARCH_MULTIPLATFORM selects MIGHT_HAVE_PCI and
ARCH_MULTI_V7 selects HAVE_SMP. I can look at dropping these.

> > + select HAVE_ARM_ARCH_TIMER
> > + help
> > + Say Y if you intend to run the kernel on a Broadcom ARM-based STB
> > + chipset.
> > +
> > + This enables support for Broadcom ARM-based set-top box chipsets,
> > + including the 7445 family of chips.
> > +
> > endif
> > diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> > index 731292114975..f3665121729b 100644
> > --- a/arch/arm/mach-bcm/Makefile
> > +++ b/arch/arm/mach-bcm/Makefile
> > @@ -30,3 +30,8 @@ obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
> >
> > # BCM5301X
> > obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o
> > +
> > +ifeq ($(CONFIG_ARCH_BRCMSTB),y)
> > +obj-y += brcmstb.o
> > +obj-$(CONFIG_SMP) += headsmp-brcmstb.o platsmp-brcmstb.o
> > +endif
> > diff --git a/arch/arm/mach-bcm/brcmstb.c b/arch/arm/mach-bcm/brcmstb.c
> > new file mode 100644
> > index 000000000000..60a5afa06ed7
> > --- /dev/null
> > +++ b/arch/arm/mach-bcm/brcmstb.c
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Copyright (C) 2013-2014 Broadcom Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/of_platform.h>
> > +
> > +#include <asm/mach-types.h>
> > +#include <asm/mach/arch.h>
> > +
> > +static const char *brcmstb_match[] __initconst = {
> > + "brcm,bcm7445",
> > + "brcm,brcmstb",
> > + NULL
> > +};
> > +
> > +DT_MACHINE_START(BRCMSTB, "Broadcom STB (Flattened Device Tree)")
> > + .dt_compat = brcmstb_match,
> > +MACHINE_END
>
> Do you plan to add more here? Otherwise you don't need this file.

Probably eventually, but not yet (there's out-of-tree stuff that hasn't
been integrated); should we drop the file until it's needed?

(Over 9 revisions, this file has evolved. And the CPU_METHOD_OF_DECLARE
stuff appearing in the meantime, which cut this file down to a no-op.)

> > diff --git a/arch/arm/mach-bcm/brcmstb.h b/arch/arm/mach-bcm/brcmstb.h
> > new file mode 100644
> > index 000000000000..ec0c3d112b36
> > --- /dev/null
> > +++ b/arch/arm/mach-bcm/brcmstb.h
> > @@ -0,0 +1,19 @@
> > +/*
> > + * Copyright (C) 2013-2014 Broadcom Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef __BRCMSTB_H__
> > +#define __BRCMSTB_H__
> > +
> > +void brcmstb_secondary_startup(void);
> > +
> > +#endif /* __BRCMSTB_H__ */
> > diff --git a/arch/arm/mach-bcm/headsmp-brcmstb.S b/arch/arm/mach-bcm/headsmp-brcmstb.S
> > new file mode 100644
> > index 000000000000..199c1ea58248
> > --- /dev/null
> > +++ b/arch/arm/mach-bcm/headsmp-brcmstb.S
> > @@ -0,0 +1,33 @@
> > +/*
> > + * SMP boot code for secondary CPUs
> > + * Based on arch/arm/mach-tegra/headsmp.S
> > + *
> > + * Copyright (C) 2010 NVIDIA, Inc.
> > + * Copyright (C) 2013-2014 Broadcom Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <asm/assembler.h>
> > +#include <linux/linkage.h>
> > +#include <linux/init.h>
> > +
> > + .section ".text.head", "ax"
> > +
> > +ENTRY(brcmstb_secondary_startup)
> > + /*
> > + * Ensure CPU is in a sane state by disabling all IRQs and switching
> > + * into SVC mode.
> > + */
> > + setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r0
> > +
> > + bl v7_invalidate_l1
>
> This should be in your boot code. That is part of the documented entry
> requirements.

By "documented" are you referring to the ARM TRM? Wouldn't any "boot
code" requirements only apply to CPU0? Linux prepares the non-boot CPUs
for SMP, as you see here.

> Also, if you are coming straight from reset, there is
> other setup probably missing.

Like what?

> Yes, there are others with this, but that doesn't make it right.
>
> > + b secondary_startup
> > +ENDPROC(brcmstb_secondary_startup)
> > diff --git a/arch/arm/mach-bcm/platsmp-brcmstb.c b/arch/arm/mach-bcm/platsmp-brcmstb.c
> > new file mode 100644
> > index 000000000000..af780e9c23a6
> > --- /dev/null
> > +++ b/arch/arm/mach-bcm/platsmp-brcmstb.c
> > @@ -0,0 +1,363 @@
> > +/*
> > + * Broadcom STB CPU SMP and hotplug support for ARM
> > + *
> > + * Copyright (C) 2013-2014 Broadcom Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/printk.h>
> > +#include <linux/regmap.h>
> > +#include <linux/smp.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include <asm/cacheflush.h>
> > +#include <asm/cp15.h>
> > +#include <asm/mach-types.h>
> > +#include <asm/smp_plat.h>
> > +
> > +#include "brcmstb.h"
> > +
> > +enum {
> > + ZONE_MAN_CLKEN_MASK = BIT(0),
> > + ZONE_MAN_RESET_CNTL_MASK = BIT(1),
> > + ZONE_MAN_MEM_PWR_MASK = BIT(4),
> > + ZONE_RESERVED_1_MASK = BIT(5),
> > + ZONE_MAN_ISO_CNTL_MASK = BIT(6),
> > + ZONE_MANUAL_CONTROL_MASK = BIT(7),
> > + ZONE_PWR_DN_REQ_MASK = BIT(9),
> > + ZONE_PWR_UP_REQ_MASK = BIT(10),
> > + ZONE_BLK_RST_ASSERT_MASK = BIT(12),
> > + ZONE_PWR_OFF_STATE_MASK = BIT(25),
> > + ZONE_PWR_ON_STATE_MASK = BIT(26),
> > + ZONE_DPG_PWR_STATE_MASK = BIT(28),
> > + ZONE_MEM_PWR_STATE_MASK = BIT(29),
> > + ZONE_RESET_STATE_MASK = BIT(31),
> > + CPU0_PWR_ZONE_CTRL_REG = 1,
> > + CPU_RESET_CONFIG_REG = 2,
> > +};
> > +
> > +static void __iomem *cpubiuctrl_block;
> > +static void __iomem *hif_cont_block;
> > +static u32 cpu0_pwr_zone_ctrl_reg;
> > +static u32 cpu_rst_cfg_reg;
> > +static u32 hif_cont_reg;
> > +
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +static DEFINE_PER_CPU_ALIGNED(int, per_cpu_sw_state);
> > +
> > +static int per_cpu_sw_state_rd(u32 cpu)
> > +{
> > + sync_cache_r(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
> > + return per_cpu(per_cpu_sw_state, cpu);
> > +}
> > +
> > +static void per_cpu_sw_state_wr(u32 cpu, int val)
> > +{
> > + per_cpu(per_cpu_sw_state, cpu) = val;
> > + dmb();
> > + sync_cache_w(SHIFT_PERCPU_PTR(&per_cpu_sw_state, per_cpu_offset(cpu)));
> > + dsb_sev();
>
> Barriers need to be documented why they are needed.

(NB: I see approximately 3 total existing cases where there is a comment
near a dsb()/dsb_sev().)

This was actually adapted from the mcpm code, and

(1) I think the dmb() is in the wrong place (it should be the first
thing in this function);
(2) the SEV is unnecessary (we don't use WFE; just a cache-invalidated
polling loop); and
(3) the DSB is also unnecessary, since sync_cache_w() handles its own
barriers.

Plus, we should probably add some comments to describe what's going on
here. (Follow-up patch?)

> > +}
> > +#else
> > +static inline void per_cpu_sw_state_wr(u32 cpu, int val) { }
> > +#endif
> > +
> > +static void __iomem *pwr_ctrl_get_base(u32 cpu)
> > +{
> > + void __iomem *base = cpubiuctrl_block + cpu0_pwr_zone_ctrl_reg;
> > + base += (cpu_logical_map(cpu) * 4);
> > + return base;
> > +}
> > +
> > +static u32 pwr_ctrl_rd(u32 cpu)
> > +{
> > + void __iomem *base = pwr_ctrl_get_base(cpu);
> > + return readl_relaxed(base);
> > +}
> > +
> > +static void pwr_ctrl_wr(u32 cpu, u32 val)
> > +{
> > + void __iomem *base = pwr_ctrl_get_base(cpu);
> > + writel(val, base);
> > +}
> > +
> > +static void cpu_rst_cfg_set(u32 cpu, int set)
> > +{
> > + u32 val;
> > + val = readl_relaxed(cpubiuctrl_block + cpu_rst_cfg_reg);
> > + if (set)
> > + val |= BIT(cpu_logical_map(cpu));
> > + else
> > + val &= ~BIT(cpu_logical_map(cpu));
> > + writel_relaxed(val, cpubiuctrl_block + cpu_rst_cfg_reg);
> > +}
> > +
> > +static void cpu_set_boot_addr(u32 cpu, unsigned long boot_addr)
> > +{
> > + const int reg_ofs = cpu_logical_map(cpu) * 8;
> > + writel_relaxed(0, hif_cont_block + hif_cont_reg + reg_ofs);
> > + writel_relaxed(boot_addr, hif_cont_block + hif_cont_reg + 4 + reg_ofs);
> > +}
> > +
> > +static void brcmstb_cpu_boot(u32 cpu)
> > +{
> > + pr_info("SMP: Booting CPU%d...\n", cpu);
>
> The core already prints messages.

Ok, we can drop.

> > +
> > + /*
> > + * set the reset vector to point to the secondary_startup
> > + * routine
> > + */
> > + cpu_set_boot_addr(cpu, virt_to_phys(brcmstb_secondary_startup));
> > +
> > + /* unhalt the cpu */
> > + cpu_rst_cfg_set(cpu, 0);
> > +}
> > +
> > +static void brcmstb_cpu_power_on(u32 cpu)
> > +{
> > + /*
> > + * The secondary cores power was cut, so we must go through
> > + * power-on initialization.
> > + */
> > + u32 tmp;
> > +
> > + pr_info("SMP: Powering up CPU%d...\n", cpu);
>
> ditto.
>
> > +
> > + /* Request zone power up */
> > + pwr_ctrl_wr(cpu, ZONE_PWR_UP_REQ_MASK);
> > +
> > + /* Wait for the power up FSM to complete */
> > + do {
> > + tmp = pwr_ctrl_rd(cpu);
> > + } while (!(tmp & ZONE_PWR_ON_STATE_MASK));
>
> Perhaps a timeout? Or perhaps you don't need to wait here. The core
> code will wait for the core to come online.

I believe we do have to wait for power before we can proceed to poke the
core. This is a quirk of our chip, explained below. But sure, I could
try to add a timeout.

> > +
> > + per_cpu_sw_state_wr(cpu, 1);
>
> The kernel already tracks the state.

Yes, but it doesn't synchronize cpu_die() and cpu_kill(). But I see you
objected below.

> > +}
> > +
> > +static int brcmstb_cpu_get_power_state(u32 cpu)
> > +{
> > + int tmp = pwr_ctrl_rd(cpu);
> > + return (tmp & ZONE_RESET_STATE_MASK) ? 0 : 1;
> > +}
> > +
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +
> > +static void brcmstb_cpu_die(u32 cpu)
> > +{
> > + v7_exit_coherency_flush(all);
> > +
> > + /* Prevent all interrupts from reaching this CPU. */
> > + arch_local_irq_disable();
>
> Surely that is already done?

True. Third line in arch/arm/kernel/smp.c cpu_die(). I believe we can
drop this.

> And disabling IRQ has no effect on
> wake-up from WFI.
>
> > +
> > + /*
> > + * Final full barrier to ensure everything before this instruction has
> > + * quiesced.
> > + */
> > + isb();
> > + dsb();
> > +
> > + per_cpu_sw_state_wr(cpu, 0);
> > +
> > + /* Sit and wait to die */
> > + wfi();
> > +
> > + /* We should never get here... */
> > + panic("Spurious interrupt on CPU %d received!\n", cpu);
> > +}
> > +
> > +static int brcmstb_cpu_kill(u32 cpu)
> > +{
> > + u32 tmp;
> > +
> > + pr_info("SMP: Powering down CPU%d...\n", cpu);
> > +
> > + while (per_cpu_sw_state_rd(cpu))
> > + ;
>
> I don't think this is necessary. You don't need to synchronize die and
> kill. Look at other implementations that actually power down the core
> (vs. just wfi).

Hmm, I'm pretty sure the synchronization is required for our B15 core.
If we yank the power before the core has properly quiesced, the whole
CPU complex will lock up. (Similar story for the power-up while loop you
complained about above.)

> > +
> > + /* Program zone reset */
> > + pwr_ctrl_wr(cpu, ZONE_RESET_STATE_MASK | ZONE_BLK_RST_ASSERT_MASK |
> > + ZONE_PWR_DN_REQ_MASK);
> > +
> > + /* Verify zone reset */
> > + tmp = pwr_ctrl_rd(cpu);
> > + if (!(tmp & ZONE_RESET_STATE_MASK))
> > + pr_err("%s: Zone reset bit for CPU %d not asserted!\n",
> > + __func__, cpu);
> > +
> > + /* Wait for power down */
> > + do {
> > + tmp = pwr_ctrl_rd(cpu);
> > + } while (!(tmp & ZONE_PWR_OFF_STATE_MASK));
> > +
> > + /* Settle-time from Broadcom-internal DVT reference code */
> > + udelay(7);
> > +
> > + /* Assert reset on the CPU */
> > + cpu_rst_cfg_set(cpu, 1);
> > +
> > + return 1;
> > +}
> > +
> > +#endif /* CONFIG_HOTPLUG_CPU */
> > +
> > +static int __init setup_hifcpubiuctrl_regs(struct device_node *np)
> > +{
> > + int rc = 0;
> > + char *name;
> > + struct device_node *syscon_np = NULL;
> > +
> > + name = "syscon-cpu";
> > +
> > + syscon_np = of_parse_phandle(np, name, 0);
> > + if (!syscon_np) {
> > + pr_err("can't find phandle %s\n", name);
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + cpubiuctrl_block = of_iomap(syscon_np, 0);
> > + if (!cpubiuctrl_block) {
> > + pr_err("iomap failed for cpubiuctrl_block\n");
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + rc = of_property_read_u32_index(np, name, CPU0_PWR_ZONE_CTRL_REG,
> > + &cpu0_pwr_zone_ctrl_reg);
> > + if (rc) {
> > + pr_err("failed to read 1st entry from %s property (%d)\n", name,
> > + rc);
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + rc = of_property_read_u32_index(np, name, CPU_RESET_CONFIG_REG,
> > + &cpu_rst_cfg_reg);
> > + if (rc) {
> > + pr_err("failed to read 2nd entry from %s property (%d)\n", name,
> > + rc);
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > +cleanup:
> > + if (syscon_np)
> > + of_node_put(syscon_np);
> > +
> > + return rc;
> > +}
> > +
> > +static int __init setup_hifcont_regs(struct device_node *np)
> > +{
> > + int rc = 0;
> > + char *name;
> > + struct device_node *syscon_np = NULL;
> > +
> > + name = "syscon-cont";
> > +
> > + syscon_np = of_parse_phandle(np, name, 0);
> > + if (!syscon_np) {
> > + pr_err("can't find phandle %s\n", name);
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + hif_cont_block = of_iomap(syscon_np, 0);
> > + if (!hif_cont_block) {
> > + pr_err("iomap failed for hif_cont_block\n");
> > + rc = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + /* offset is at top of hif_cont_block */
> > + hif_cont_reg = 0;
> > +
> > +cleanup:
> > + if (syscon_np)
> > + of_node_put(syscon_np);
> > +
> > + return rc;
> > +}
> > +
> > +static void __init brcmstb_cpu_ctrl_setup(unsigned int max_cpus)
> > +{
> > + int rc;
> > + struct device_node *np;
> > + char *name;
> > +
> > + name = "brcm,brcmstb-smpboot";
> > + np = of_find_compatible_node(NULL, NULL, name);
> > + if (!np) {
> > + pr_err("can't find compatible node %s\n", name);
> > + return;
> > + }
> > +
> > + rc = setup_hifcpubiuctrl_regs(np);
> > + if (rc)
> > + return;
> > +
> > + rc = setup_hifcont_regs(np);
> > + if (rc)
> > + return;
> > +}
> > +
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > +static void brcmstb_secondary_init(unsigned int cpu)
> > +{
> > + /*
> > + * Synchronise with the boot thread.
> > + */
> > + spin_lock(&boot_lock);
> > + spin_unlock(&boot_lock);
>
> I suggest you read previous discussions on attempts to make this
> common before replying to Russell.

Can you point me to this discussion? linux-arm-kernel is a big and noisy
world...

BTW, I'm not sure the boot_lock is actually necessary at all for us; it
was just borrowed from one of the other mach-*'s. The synchronization we
need is done in the while loops above.

> > +}
> > +
> > +static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > + /*
> > + * set synchronisation state between this boot processor
> > + * and the secondary one
> > + */
> > + spin_lock(&boot_lock);
> > +
> > + /* Bring up power to the core if necessary */
> > + if (brcmstb_cpu_get_power_state(cpu) == 0)
> > + brcmstb_cpu_power_on(cpu);
> > +
> > + brcmstb_cpu_boot(cpu);
> > +
> > + /*
> > + * now the secondary core is starting up let it run its
> > + * calibrations, then wait for it to finish
> > + */
> > + spin_unlock(&boot_lock);
> > +
> > + return 0;
> > +}
> > +
> > +static struct smp_operations brcmstb_smp_ops __initdata = {
> > + .smp_prepare_cpus = brcmstb_cpu_ctrl_setup,
> > + .smp_secondary_init = brcmstb_secondary_init,
> > + .smp_boot_secondary = brcmstb_boot_secondary,
> > +#ifdef CONFIG_HOTPLUG_CPU
> > + .cpu_kill = brcmstb_cpu_kill,
> > + .cpu_die = brcmstb_cpu_die,
> > +#endif
> > +};
> > +
> > +CPU_METHOD_OF_DECLARE(brcmstb_smp, "brcm,brahma-b15", &brcmstb_smp_ops);

Brian

2014-07-31 02:36:41

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

Hi Russell,

On Wed, Jul 30, 2014 at 10:26:35AM +0100, Russell King wrote:
> On Mon, Jul 21, 2014 at 02:07:56PM -0700, Brian Norris wrote:
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > +static void brcmstb_secondary_init(unsigned int cpu)
> > +{
> > + /*
> > + * Synchronise with the boot thread.
> > + */
> > + spin_lock(&boot_lock);
> > + spin_unlock(&boot_lock);
> > +}
> > +
> > +static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > + /*
> > + * set synchronisation state between this boot processor
> > + * and the secondary one
> > + */
> > + spin_lock(&boot_lock);
> > +
> > + /* Bring up power to the core if necessary */
> > + if (brcmstb_cpu_get_power_state(cpu) == 0)
> > + brcmstb_cpu_power_on(cpu);
> > +
> > + brcmstb_cpu_boot(cpu);
> > +
> > + /*
> > + * now the secondary core is starting up let it run its
> > + * calibrations, then wait for it to finish
> > + */
> > + spin_unlock(&boot_lock);
>
> I've just read through this code (because it caused my allmodconfig to
> break) and spotted this.

Sorry about the allmodconfig problems. I never compile-tested with ARMv6
enabled. This look OK?

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index f3665121729b..5ce82b4ba931 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o

ifeq ($(CONFIG_ARCH_BRCMSTB),y)
+CFLAGS_platsmp-brcmstb.o += -march=armv7-a
obj-y += brcmstb.o
obj-$(CONFIG_SMP) += headsmp-brcmstb.o platsmp-brcmstb.o
endif

> What function does boot_lock perform here? Please, don't quote the
> comments (I know where the comments came from) but what I want to hear
> is your comments about why you decided to retain this.

You might glean a little more from my response to Rob, but I'm not sure
there was a good reason for retaining this. We do need to be sure the
CPU is fully powered online before bringing it out of reset, but the
spinlock seems overkill AFAICT.

Brian

2014-07-31 08:43:30

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

On Wed, Jul 30, 2014 at 07:23:20PM -0700, Brian Norris wrote:
> Hi Rob,
>
> I appreciate your comments, but where were many of these 5 months ago on
> the first 7 revisions? :)
>
> On a practical note: v9 is already queued for 3.17. Should I send
> patches for the 3.17 cycle (or later) to fixup some of these issues? Or
> would you recommend pulling the patches out of Matt Porter's tree now,
> and reintroducing for 3.18? (I would be much happier with the first.)
>
> I do note that we at least need to fix allmodconfig ASAP; I'll reply to
> Russell on that one.
>
> On Wed, Jul 30, 2014 at 12:09:48PM -0500, Rob Herring wrote:
> > On Mon, Jul 21, 2014 at 4:07 PM, Brian Norris <[email protected]> wrote:
> > > +
> > > + per_cpu_sw_state_wr(cpu, 1);
> >
> > The kernel already tracks the state.
>
> Yes, but it doesn't synchronize cpu_die() and cpu_kill(). But I see you
> objected below.

Err.

static DECLARE_COMPLETION(cpu_died);

/*
* called on the thread which is asking for a CPU to be shutdown -
* waits until shutdown has completed, or it is timed out.
*/
void __cpu_die(unsigned int cpu)
{
if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(5000))) {
pr_err("CPU%u: cpu didn't die\n", cpu);
return;
}
printk(KERN_NOTICE "CPU%u: shutdown\n", cpu);
...
if (!platform_cpu_kill(cpu))
printk("CPU%u: unable to kill\n", cpu);
}

void __ref cpu_die(void)
{
...
/*
* Tell __cpu_die() that this CPU is now safe to dispose of. Once
* this returns, power and/or clocks can be removed at any point
* from this CPU and its cache by platform_cpu_kill().
*/
complete(&cpu_died);

There _is_ synchronisation between these two. Your cpu_kill() function
will not be called until we have flushed all data from the dying CPU's
cache. That's the best we can do, because if we cause the dying CPU to
exit the coherency domain, any kind of locking or completion will no
longer work (neither will your state tracking solution because the L1
caches will no longer snoop.)

> Hmm, I'm pretty sure the synchronization is required for our B15 core.
> If we yank the power before the core has properly quiesced, the whole
> CPU complex will lock up. (Similar story for the power-up while loop you
> complained about above.)

If you need to ensure that the power isn't turned off too soon, how about
using a delayed work queue to do the power-off of the CPU (remembering to
cancel the delayed work queue when powering the CPU back up.)

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-07-31 22:06:48

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

Hi Russell,

On Thu, Jul 31, 2014 at 09:43:15AM +0100, Russell King wrote:
> On Wed, Jul 30, 2014 at 07:23:20PM -0700, Brian Norris wrote:
> > I appreciate your comments, but where were many of these 5 months ago on
> > the first 7 revisions? :)
> >
> > On a practical note: v9 is already queued for 3.17. Should I send
> > patches for the 3.17 cycle (or later) to fixup some of these issues? Or
> > would you recommend pulling the patches out of Matt Porter's tree now,
> > and reintroducing for 3.18? (I would be much happier with the first.)
> >
> > I do note that we at least need to fix allmodconfig ASAP; I'll reply to
> > Russell on that one.
> >
> > On Wed, Jul 30, 2014 at 12:09:48PM -0500, Rob Herring wrote:
> > > On Mon, Jul 21, 2014 at 4:07 PM, Brian Norris <[email protected]> wrote:
> > > > +
> > > > + per_cpu_sw_state_wr(cpu, 1);
> > >
> > > The kernel already tracks the state.
> >
> > Yes, but it doesn't synchronize cpu_die() and cpu_kill(). But I see you
> > objected below.
>
> Err.
>
> static DECLARE_COMPLETION(cpu_died);

Yes, I noticed this. What I meant is that smp_ops.cpu_die() and
smp_ops.cpu_kill() are not synchronized.

> /*
> * called on the thread which is asking for a CPU to be shutdown -
> * waits until shutdown has completed, or it is timed out.
> */
> void __cpu_die(unsigned int cpu)
> {
> if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(5000))) {
> pr_err("CPU%u: cpu didn't die\n", cpu);
> return;
> }
> printk(KERN_NOTICE "CPU%u: shutdown\n", cpu);
> ...
> if (!platform_cpu_kill(cpu))
> printk("CPU%u: unable to kill\n", cpu);
> }
>
> void __ref cpu_die(void)
> {
> ...
> /*
> * Tell __cpu_die() that this CPU is now safe to dispose of. Once
> * this returns, power and/or clocks can be removed at any point
> * from this CPU and its cache by platform_cpu_kill().
> */
> complete(&cpu_died);
>
> There _is_ synchronisation between these two. Your cpu_kill() function
> will not be called until we have flushed all data from the dying CPU's
> cache. That's the best we can do, because if we cause the dying CPU to
> exit the coherency domain, any kind of locking or completion will no
> longer work (neither will your state tracking solution because the L1
> caches will no longer snoop.)

We're not relying on the L1 cache, though. Don't sync_cache_{r,w}()
ensure all reads/writes reach at least the L2?

Besides, we modeled this after the MCPM code (e.g., __mcpm_cpu_down()).
Are both implementations incorrect?

> > Hmm, I'm pretty sure the synchronization is required for our B15 core.
> > If we yank the power before the core has properly quiesced, the whole
> > CPU complex will lock up. (Similar story for the power-up while loop you
> > complained about above.)
>
> If you need to ensure that the power isn't turned off too soon, how about
> using a delayed work queue to do the power-off of the CPU (remembering to
> cancel the delayed work queue when powering the CPU back up.)

How does that ensure that the CPU is down by the time the work is
scheduled? It seems like this would just defer the work long enough that
it *most likely* has quiesced, but I don't see how this gives us a
better guarantee. Or maybe I'm missing something. (If so, please do
enlighten!)

Brian