2015-06-19 00:12:16

by Brian Norris

[permalink] [raw]
Subject: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

Hi,

This patch set introduces system suspend/resume support for Broadcom STB SoCs.
There are two suspend modes (S2 and S3) as well as a related low-power shutdown
mode (S5).

Along with the core PM support, include a driver for the wakeup-timer, which
allows for simple testing of suspend/resume wakeup cycles.

Brian

Brian Norris (7):
Documentation: dt: brcmstb: add system PM bindings
Documentation: dt: brcmstb: add waketimer documentation
soc: add stubs for brcmstb SoC's
soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
soc: brcmstb: add wake-timer driver
ARM: brcmstb: mask GIC IRQs on suspend
ARM: dts: brcmstb: add BCM7445 system PM DT nodes

.../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
.../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 142 +++++-
.../soc/brcmstb/brcm,brcmstb-waketimer.txt | 20 +
arch/arm/boot/dts/bcm7445.dtsi | 102 ++++
arch/arm/mach-bcm/Kconfig | 1 +
arch/arm/mach-bcm/brcmstb.c | 10 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/brcmstb/Kconfig | 22 +
drivers/soc/brcmstb/Makefile | 4 +
drivers/soc/brcmstb/common.c | 33 ++
drivers/soc/brcmstb/pm/Makefile | 1 +
drivers/soc/brcmstb/pm/aon_defs.h | 85 ++++
drivers/soc/brcmstb/pm/pm.c | 512 +++++++++++++++++++++
drivers/soc/brcmstb/pm/pm.h | 40 ++
drivers/soc/brcmstb/pm/s2.S | 73 +++
drivers/soc/brcmstb/wktmr.c | 242 ++++++++++
include/soc/brcmstb/common.h | 15 +
18 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
create mode 100644 Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt
create mode 100644 drivers/soc/brcmstb/Kconfig
create mode 100644 drivers/soc/brcmstb/Makefile
create mode 100644 drivers/soc/brcmstb/common.c
create mode 100644 drivers/soc/brcmstb/pm/Makefile
create mode 100644 drivers/soc/brcmstb/pm/aon_defs.h
create mode 100644 drivers/soc/brcmstb/pm/pm.c
create mode 100644 drivers/soc/brcmstb/pm/pm.h
create mode 100644 drivers/soc/brcmstb/pm/s2.S
create mode 100644 drivers/soc/brcmstb/wktmr.c
create mode 100644 include/soc/brcmstb/common.h

--
1.9.1


2015-06-19 00:12:50

by Brian Norris

[permalink] [raw]
Subject: [PATCH 1/7] Documentation: dt: brcmstb: add system PM bindings

Signed-off-by: Brian Norris <[email protected]>
---
.../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 142 ++++++++++++++++++++-
1 file changed, 140 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
index 430608ec09f0..94429649687e 100644
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
+++ b/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
@@ -43,8 +43,7 @@ example:
};
};

-Lastly, nodes that allow for support of SMP initialization and reboot are
-required:
+Nodes that allow for support of SMP initialization and reboot are required:

smpboot
-------
@@ -95,3 +94,142 @@ example:
compatible = "brcm,brcmstb-reboot";
syscon = <&sun_top_ctrl 0x304 0x308>;
};
+
+
+
+Power management
+----------------
+
+For power management (particularly, S2/S3/S5 system suspend), the following SoC
+components are needed:
+
+= Always-On control block (AON CTRL)
+
+This hardware provides control registers for the "always-on" (even in low-power
+modes) hardware, such as the Power Management State Machine (PMSM).
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-aon-ctrl"
+- reg : the register start and length for the AON CTRL block
+
+Example:
+
+aon-ctrl@410000 {
+ compatible = "brcm,brcmstb-aon-ctrl";
+ reg = <0x410000 0x400>;
+};
+
+= Memory controllers
+
+A Broadcom STB SoC typically has a number of independent memory controllers,
+each of which may have several associated hardware blocks, which are versioned
+independently (control registers, DDR PHYs, etc.). One might consider
+describing these controllers as a parent "memory controllers" block, which
+contains N sub-nodes (one for each controller in the system), each of which is
+associated with a number of hardware register resources (e.g., its PHY). See
+the example device tree snippet below.
+
+== MEMC (MEMory Controller)
+
+Represents a single memory controller instance.
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-memc" and "simple-bus"
+
+Should contain subnodes for any of the following relevant hardware resources:
+
+== DDR PHY control
+
+Control registers for this memory controller's DDR PHY.
+
+Required properties:
+- compatible : should contain one of these
+ "brcm,brcmstb-ddr-phy-v225.1"
+ "brcm,brcmstb-ddr-phy-v240.1"
+ "brcm,brcmstb-ddr-phy-v240.2"
+
+- reg : the DDR PHY register range
+
+== DDR SHIMPHY
+
+Control registers for this memory controller's DDR SHIMPHY.
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-ddr-shimphy-v1.0"
+- reg : the DDR SHIMPHY register range
+
+== MEMC DDR control
+
+Sequencer DRAM parameters and control registers. Used for Self-Refresh
+Power-Down (SRPD), among other things.
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-memc-ddr"
+- reg : the MEMC DDR register range
+
+Example:
+
+memory_controllers {
+ ranges;
+ compatible = "simple-bus";
+
+ memc@0 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ ranges;
+
+ ddr-phy@f1106000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0xf1106000 0x21c>;
+ };
+
+ shimphy@f1108000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0xf1108000 0xe4>;
+ };
+
+ memc-ddr@f1102000 {
+ reg = <0xf1102000 0x800>;
+ compatible = "brcm,brcmstb-memc-ddr";
+ };
+ };
+
+ memc@1 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ ranges;
+
+ ddr-phy@f1186000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0xf1186000 0x21c>;
+ };
+
+ shimphy@f1188000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0xf1188000 0xe4>;
+ };
+
+ memc-ddr@f1182000 {
+ reg = <0xf1182000 0x800>;
+ compatible = "brcm,brcmstb-memc-ddr";
+ };
+ };
+
+ memc@2 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ ranges;
+
+ ddr-phy@f1206000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0xf1206000 0x21c>;
+ };
+
+ shimphy@f1208000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0xf1208000 0xe4>;
+ };
+
+ memc-ddr@f1202000 {
+ reg = <0xf1202000 0x800>;
+ compatible = "brcm,brcmstb-memc-ddr";
+ };
+ };
+};
--
1.9.1

2015-06-19 00:12:29

by Brian Norris

[permalink] [raw]
Subject: [PATCH 2/7] Documentation: dt: brcmstb: add waketimer documentation

Signed-off-by: Brian Norris <[email protected]>
---
.../bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt

diff --git a/Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt b/Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt
new file mode 100644
index 000000000000..68c4329b6af0
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt
@@ -0,0 +1,20 @@
+* Broadcom STB Wake-up Timer
+
+The Broadcom STB wake-up timer provides a high-resolution timer, with the
+ability to wake up the system from low-power suspend/standby modes.
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-waketimer"
+- reg : the register start and length for the WKTMR block
+- interrupts : The TIMER interrupt
+- interrupt-parent: The phandle to the Always-On (AON) Power Management (PM) L2
+ interrupt controller node
+
+Example:
+
+waketimer@f0411580 {
+ compatible = "brcm,brcmstb-waketimer";
+ reg = <0xf0411580 0x14>;
+ interrupts = <0x3>;
+ interrupt-parent = <&aon_pm_l2_intc>;
+};
--
1.9.1

2015-06-19 00:12:40

by Brian Norris

[permalink] [raw]
Subject: [PATCH 3/7] soc: add stubs for brcmstb SoC's

Used on BCM7xxx Set-Top Box chips (e.g., BCM7445).

Signed-off-by: Brian Norris <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 1 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/brcmstb/Kconfig | 9 +++++++++
drivers/soc/brcmstb/Makefile | 1 +
drivers/soc/brcmstb/common.c | 33 +++++++++++++++++++++++++++++++++
include/soc/brcmstb/common.h | 15 +++++++++++++++
7 files changed, 61 insertions(+)
create mode 100644 drivers/soc/brcmstb/Kconfig
create mode 100644 drivers/soc/brcmstb/Makefile
create mode 100644 drivers/soc/brcmstb/common.c
create mode 100644 include/soc/brcmstb/common.h

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index e9184feffc4e..11cf1c97093f 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -146,6 +146,7 @@ config ARCH_BRCMSTB
select BRCMSTB_L2_IRQ
select BCM7120_L2_IRQ
select ARCH_WANT_OPTIONAL_GPIOLIB
+ select SOC_BRCMSTB
help
Say Y if you intend to run the kernel on a Broadcom ARM-based STB
chipset.
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 96ddecb92254..c9c0fcce98a7 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -1,5 +1,6 @@
menu "SOC (System On Chip) specific Drivers"

+source "drivers/soc/brcmstb/Kconfig"
source "drivers/soc/mediatek/Kconfig"
source "drivers/soc/qcom/Kconfig"
source "drivers/soc/sunxi/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 7dc7c0d8a2c1..2706b1159e32 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -2,6 +2,7 @@
# Makefile for the Linux Kernel SOC specific device drivers.
#

+obj-$(CONFIG_SOC_BRCMSTB) += brcmstb/
obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
obj-$(CONFIG_ARCH_QCOM) += qcom/
obj-$(CONFIG_ARCH_SUNXI) += sunxi/
diff --git a/drivers/soc/brcmstb/Kconfig b/drivers/soc/brcmstb/Kconfig
new file mode 100644
index 000000000000..39cab3bd544d
--- /dev/null
+++ b/drivers/soc/brcmstb/Kconfig
@@ -0,0 +1,9 @@
+menuconfig SOC_BRCMSTB
+ bool "Broadcom STB SoC drivers"
+ depends on ARM
+ help
+ Enables drivers for the Broadcom Set-Top Box (STB) series of chips.
+ This option alone enables only some support code, while the drivers
+ can be enabled individually within this menu.
+
+ If unsure, say N.
diff --git a/drivers/soc/brcmstb/Makefile b/drivers/soc/brcmstb/Makefile
new file mode 100644
index 000000000000..183280e39f80
--- /dev/null
+++ b/drivers/soc/brcmstb/Makefile
@@ -0,0 +1 @@
+obj-y += common.o
diff --git a/drivers/soc/brcmstb/common.c b/drivers/soc/brcmstb/common.c
new file mode 100644
index 000000000000..c262c029b1b8
--- /dev/null
+++ b/drivers/soc/brcmstb/common.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright © 2014 NVIDIA Corporation
+ * Copyright © 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of.h>
+
+#include <soc/brcmstb/common.h>
+
+static const struct of_device_id brcmstb_machine_match[] = {
+ { .compatible = "brcm,brcmstb", },
+ { }
+};
+
+bool soc_is_brcmstb(void)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path("/");
+ if (!root)
+ return false;
+
+ return of_match_node(brcmstb_machine_match, root) != NULL;
+}
diff --git a/include/soc/brcmstb/common.h b/include/soc/brcmstb/common.h
new file mode 100644
index 000000000000..cfb5335f2a15
--- /dev/null
+++ b/include/soc/brcmstb/common.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright © 2014 NVIDIA Corporation
+ * Copyright © 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __SOC_BRCMSTB_COMMON_H__
+#define __SOC_BRCMSTB_COMMON_H__
+
+bool soc_is_brcmstb(void);
+
+#endif /* __SOC_BRCMSTB_COMMON_H__ */
--
1.9.1

2015-06-19 00:13:33

by Brian Norris

[permalink] [raw]
Subject: [PATCH 4/7] soc: brcmstb: add PM suspend/resume support (S2/S3/S5)

Support S2, S3, and S5 through /sys/power/state ("standby" and "mem")
and the poweroff code paths. See comments in pm.c for a few more details
about these system suspend modes.

Signed-off-by: Brian Norris <[email protected]>
---
drivers/soc/brcmstb/Kconfig | 10 +
drivers/soc/brcmstb/Makefile | 2 +
drivers/soc/brcmstb/pm/Makefile | 1 +
drivers/soc/brcmstb/pm/aon_defs.h | 85 +++++++
drivers/soc/brcmstb/pm/pm.c | 512 ++++++++++++++++++++++++++++++++++++++
drivers/soc/brcmstb/pm/pm.h | 40 +++
drivers/soc/brcmstb/pm/s2.S | 73 ++++++
7 files changed, 723 insertions(+)
create mode 100644 drivers/soc/brcmstb/pm/Makefile
create mode 100644 drivers/soc/brcmstb/pm/aon_defs.h
create mode 100644 drivers/soc/brcmstb/pm/pm.c
create mode 100644 drivers/soc/brcmstb/pm/pm.h
create mode 100644 drivers/soc/brcmstb/pm/s2.S

diff --git a/drivers/soc/brcmstb/Kconfig b/drivers/soc/brcmstb/Kconfig
index 39cab3bd544d..5025dacce6f0 100644
--- a/drivers/soc/brcmstb/Kconfig
+++ b/drivers/soc/brcmstb/Kconfig
@@ -7,3 +7,13 @@ menuconfig SOC_BRCMSTB
can be enabled individually within this menu.

If unsure, say N.
+
+if SOC_BRCMSTB
+
+config BRCMSTB_PM
+ bool "Support suspend/resume for STB platforms"
+ default y
+ depends on PM
+ depends on ARM && ARCH_BRCMSTB
+
+endif # SOC_BRCMSTB
diff --git a/drivers/soc/brcmstb/Makefile b/drivers/soc/brcmstb/Makefile
index 183280e39f80..677e3fa0d042 100644
--- a/drivers/soc/brcmstb/Makefile
+++ b/drivers/soc/brcmstb/Makefile
@@ -1 +1,3 @@
+obj-$(CONFIG_BRCMSTB_PM) += pm/
+
obj-y += common.o
diff --git a/drivers/soc/brcmstb/pm/Makefile b/drivers/soc/brcmstb/pm/Makefile
new file mode 100644
index 000000000000..71a1ff4c368b
--- /dev/null
+++ b/drivers/soc/brcmstb/pm/Makefile
@@ -0,0 +1 @@
+obj-y += pm.o s2.o
diff --git a/drivers/soc/brcmstb/pm/aon_defs.h b/drivers/soc/brcmstb/pm/aon_defs.h
new file mode 100644
index 000000000000..2b660fa788e4
--- /dev/null
+++ b/drivers/soc/brcmstb/pm/aon_defs.h
@@ -0,0 +1,85 @@
+/*
+ * Always ON (AON) register interface between bootloader and Linux
+ *
+ * Copyright © 2014-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __BRCMSTB_AON_DEFS_H__
+#define __BRCMSTB_AON_DEFS_H__
+
+#include <linux/compiler.h>
+
+/* Magic number in upper 16-bits */
+#define BRCMSTB_S3_MAGIC_MASK 0xffff0000
+#define BRCMSTB_S3_MAGIC_SHORT 0x5AFE0000
+
+enum {
+ /* Restore random key for AES memory verification (off = fixed key) */
+ S3_FLAG_LOAD_RANDKEY = (1 << 0),
+
+ /* Scratch buffer page table is present */
+ S3_FLAG_SCRATCH_BUFFER_TABLE = (1 << 1),
+
+ /* Skip all memory verification */
+ S3_FLAG_NO_MEM_VERIFY = (1 << 2),
+};
+
+#define BRCMSTB_HASH_LEN (128 / 8) /* 128-bit hash */
+
+#define AON_REG_MAGIC_FLAGS 0x00
+#define AON_REG_CONTROL_LOW 0x04
+#define AON_REG_CONTROL_HIGH 0x08
+#define AON_REG_S3_HASH 0x0c /* hash of S3 params */
+#define AON_REG_CONTROL_HASH_LEN 0x1c
+
+#define BRCMSTB_S3_MAGIC 0x5AFEB007
+#define BOOTLOADER_SCRATCH_SIZE 64
+#define IMAGE_DESCRIPTORS_BUFSIZE (2 * 1024)
+
+/*
+ * Bootloader utilizes a custom parameter block left in DRAM for handling S3
+ * warm resume
+ */
+struct brcmstb_s3_params {
+ /* scratch memory for bootloader */
+ uint8_t scratch[BOOTLOADER_SCRATCH_SIZE];
+
+ uint32_t magic; /* BRCMSTB_S3_MAGIC */
+ uint64_t reentry; /* PA */
+
+ /* descriptors */
+ uint32_t hash[BRCMSTB_HASH_LEN / 4];
+
+ /*
+ * If 0, then ignore this parameter (there is only one set of
+ * descriptors)
+ *
+ * If non-0, then a second set of descriptors is stored at:
+ *
+ * descriptors + desc_offset_2
+ *
+ * The MAC result of both descriptors is XOR'd and stored in @hash
+ */
+ uint32_t desc_offset_2;
+
+ /*
+ * (Physical) address of a brcmstb_bootloader_scratch_table, for
+ * providing a large DRAM buffer to the bootloader
+ */
+ uint64_t buffer_table;
+
+ uint32_t spare[70];
+
+ uint8_t descriptors[IMAGE_DESCRIPTORS_BUFSIZE];
+} __packed;
+
+#endif /* __BRCMSTB_AON_DEFS_H__ */
diff --git a/drivers/soc/brcmstb/pm/pm.c b/drivers/soc/brcmstb/pm/pm.c
new file mode 100644
index 000000000000..04f0528d5322
--- /dev/null
+++ b/drivers/soc/brcmstb/pm/pm.c
@@ -0,0 +1,512 @@
+/*
+ * ARM-specific support for Broadcom STB S2/S3/S5 power management
+ *
+ * S2: clock gate CPUs and as many peripherals as possible
+ * S3: power off all of the chip except the Always ON (AON) island; keep DDR is
+ * self-refresh
+ * S5: (a.k.a. S3 cold boot) much like S3, except DDR is powered down, so we
+ * treat this mode like a soft power-off, with wakeup allowed from AON
+ *
+ * Copyright © 2014-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "brcmstb-pm: " fmt
+
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/suspend.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/compiler.h>
+#include <linux/pm.h>
+#include <linux/bitops.h>
+#include <linux/dma-mapping.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/kconfig.h>
+#include <asm/fncpy.h>
+#include <asm/suspend.h>
+#include <asm/setup.h>
+
+#include <soc/brcmstb/common.h>
+
+#include "pm.h"
+#include "aon_defs.h"
+
+#define BRCMSTB_DDR_PHY_PLL_STATUS 0x04
+
+#define SHIMPHY_DDR_PAD_CNTRL 0x8c
+#define SHIMPHY_PAD_PLL_SEQUENCE BIT(8)
+#define SHIMPHY_PAD_GATE_PLL_S3 BIT(9)
+
+#define MAX_NUM_MEMC 3
+
+/* Capped for performance reasons */
+#define MAX_HASH_SIZE SZ_256M
+/* Max per bank, to keep some fairness */
+#define MAX_HASH_SIZE_BANK SZ_64M
+
+struct brcmstb_memc {
+ void __iomem *ddr_phy_base;
+ void __iomem *ddr_shimphy_base;
+};
+
+struct brcmstb_pm_control {
+ void __iomem *aon_ctrl_base;
+ void __iomem *aon_sram;
+ struct brcmstb_memc memcs[MAX_NUM_MEMC];
+
+ void __iomem *boot_sram;
+ size_t boot_sram_len;
+
+ bool support_warm_boot;
+ int num_memc;
+
+ struct brcmstb_s3_params *s3_params;
+ dma_addr_t s3_params_pa;
+};
+
+enum bsp_initiate_command {
+ BSP_CLOCK_STOP = 0x00,
+ BSP_GEN_RANDOM_KEY = 0x4A,
+ BSP_RESTORE_RANDOM_KEY = 0x55,
+ BSP_GEN_FIXED_KEY = 0x63,
+};
+
+#define PM_INITIATE 0x01
+#define PM_INITIATE_SUCCESS 0x00
+#define PM_INITIATE_FAIL 0xfe
+
+static struct brcmstb_pm_control ctrl;
+
+extern const unsigned long brcmstb_pm_do_s2_sz;
+extern asmlinkage int brcmstb_pm_do_s2(void __iomem *aon_ctrl_base,
+ void __iomem *ddr_phy_pll_status);
+
+static int (*brcmstb_pm_do_s2_sram)(void __iomem *aon_ctrl_base,
+ void __iomem *ddr_phy_pll_status);
+
+static int brcmstb_init_sram(struct device_node *dn)
+{
+ void __iomem *sram;
+ struct resource res;
+ int ret;
+
+ ret = of_address_to_resource(dn, 0, &res);
+ if (ret)
+ return ret;
+
+ /* Cached, executable remapping of SRAM */
+ sram = __arm_ioremap_exec(res.start, resource_size(&res), true);
+ if (!sram)
+ return -ENOMEM;
+
+ ctrl.boot_sram = sram;
+ ctrl.boot_sram_len = resource_size(&res);
+
+ return 0;
+}
+
+/*
+ * Latch into the BRCM SRAM compatible property here to be more specific than
+ * the standard "mmio-sram". Could be supported with genalloc too, but that
+ * would be overkill for its current single use-case.
+ */
+static const struct of_device_id sram_dt_ids[] = {
+ { .compatible = "brcm,boot-sram" },
+ {}
+};
+
+static int do_bsp_initiate_command(enum bsp_initiate_command cmd)
+{
+ void __iomem *base = ctrl.aon_ctrl_base;
+ int ret;
+ int timeo = 1000 * 1000; /* 1 second */
+
+ __raw_writel(0, base + AON_CTRL_PM_INITIATE);
+ (void)__raw_readl(base + AON_CTRL_PM_INITIATE);
+
+ /* Go! */
+ __raw_writel((cmd << 1) | PM_INITIATE, base + AON_CTRL_PM_INITIATE);
+
+ for (;;) {
+ ret = __raw_readl(base + AON_CTRL_PM_INITIATE);
+ if (!(ret & PM_INITIATE))
+ break;
+ if (timeo <= 0) {
+ pr_err("error: timeout waiting for BSP (%x)\n", ret);
+ break;
+ }
+ timeo -= 50;
+ udelay(50);
+ }
+
+ return (ret & 0xff) != PM_INITIATE_SUCCESS;
+}
+
+static int brcmstb_pm_handshake(void)
+{
+ void __iomem *base = ctrl.aon_ctrl_base;
+ u32 tmp;
+ int ret;
+
+ /* BSP power handshake, v1 */
+ tmp = __raw_readl(base + AON_CTRL_HOST_MISC_CMDS);
+ tmp &= ~1UL;
+ __raw_writel(tmp, base + AON_CTRL_HOST_MISC_CMDS);
+ (void)__raw_readl(base + AON_CTRL_HOST_MISC_CMDS);
+
+ ret = do_bsp_initiate_command(BSP_CLOCK_STOP);
+ if (ret)
+ pr_err("BSP handshake failed\n");
+
+ /*
+ * HACK: BSP may have internal race on the CLOCK_STOP command.
+ * Avoid touching the BSP for a few milliseconds.
+ */
+ mdelay(3);
+
+ return ret;
+}
+
+/*
+ * Run a Power Management State Machine (PMSM) shutdown command and put the CPU
+ * into a low-power mode
+ */
+static void brcmstb_do_pmsm_power_down(unsigned long base_cmd)
+{
+ void __iomem *base = ctrl.aon_ctrl_base;
+
+ /* pm_start_pwrdn transition 0->1 */
+ __raw_writel(base_cmd, base + AON_CTRL_PM_CTRL);
+ (void)__raw_readl(base + AON_CTRL_PM_CTRL);
+
+ __raw_writel(base_cmd | PM_PWR_DOWN, base + AON_CTRL_PM_CTRL);
+ (void)__raw_readl(base + AON_CTRL_PM_CTRL);
+
+ wfi();
+}
+
+/* Support S5 cold boot out of "poweroff" */
+static void brcmstb_pm_poweroff(void)
+{
+ brcmstb_pm_handshake();
+
+ /* Clear magic S3 warm-boot value */
+ __raw_writel(0, ctrl.aon_sram + AON_REG_MAGIC_FLAGS);
+ (void)__raw_readl(ctrl.aon_sram + AON_REG_MAGIC_FLAGS);
+
+ /* Skip wait-for-interrupt signal; just use a countdown */
+ __raw_writel(0x10, ctrl.aon_ctrl_base + AON_CTRL_PM_CPU_WAIT_COUNT);
+ (void)__raw_readl(ctrl.aon_ctrl_base + AON_CTRL_PM_CPU_WAIT_COUNT);
+
+ brcmstb_do_pmsm_power_down(PM_COLD_CONFIG);
+}
+
+static void *brcmstb_pm_copy_to_sram(void *fn, size_t len)
+{
+ unsigned int size = ALIGN(len, FNCPY_ALIGN);
+
+ if (ctrl.boot_sram_len < size) {
+ pr_err("standby code will not fit in SRAM\n");
+ return NULL;
+ }
+
+ return fncpy(ctrl.boot_sram, fn, size);
+}
+
+/*
+ * S2 suspend/resume picks up where we left off, so we must execute carefully
+ * from SRAM, in order to allow DDR to come back up safely before we continue.
+ */
+static int brcmstb_pm_s2(void)
+{
+ brcmstb_pm_do_s2_sram = brcmstb_pm_copy_to_sram(&brcmstb_pm_do_s2,
+ brcmstb_pm_do_s2_sz);
+ if (!brcmstb_pm_do_s2_sram)
+ return -EINVAL;
+
+ return brcmstb_pm_do_s2_sram(ctrl.aon_ctrl_base,
+ ctrl.memcs[0].ddr_phy_base +
+ BRCMSTB_DDR_PHY_PLL_STATUS);
+}
+
+static int brcmstb_pm_s3_finish(void)
+{
+ struct brcmstb_s3_params *params = ctrl.s3_params;
+ phys_addr_t params_pa = ctrl.s3_params_pa;
+ phys_addr_t reentry = virt_to_phys(&cpu_resume);
+ u32 flags = S3_FLAG_NO_MEM_VERIFY;
+ int i;
+
+ /* Clear parameter structure */
+ memset(params, 0, sizeof(*params));
+
+ params->magic = BRCMSTB_S3_MAGIC;
+ params->reentry = reentry;
+
+ flush_cache_all();
+
+ flags |= BRCMSTB_S3_MAGIC_SHORT;
+
+ __raw_writel(flags, ctrl.aon_sram + AON_REG_MAGIC_FLAGS);
+ __raw_writel(lower_32_bits(params_pa),
+ ctrl.aon_sram + AON_REG_CONTROL_LOW);
+ __raw_writel(upper_32_bits(params_pa),
+ ctrl.aon_sram + AON_REG_CONTROL_HIGH);
+
+ /* gate PLL | S3 */
+ for (i = 0; i < ctrl.num_memc; i++) {
+ u32 tmp;
+ tmp = __raw_readl(ctrl.memcs[i].ddr_shimphy_base +
+ SHIMPHY_DDR_PAD_CNTRL);
+ tmp |= SHIMPHY_PAD_GATE_PLL_S3 | SHIMPHY_PAD_PLL_SEQUENCE;
+ __raw_writel(tmp, ctrl.memcs[i].ddr_shimphy_base +
+ SHIMPHY_DDR_PAD_CNTRL);
+ }
+
+ brcmstb_do_pmsm_power_down(PM_WARM_CONFIG);
+
+ /* Must have been interrupted from wfi()? */
+ return -EINTR;
+}
+
+/*
+ * S3 mode resume to the bootloader before jumping back to Linux, so we can be
+ * a little less careful about running from DRAM.
+ */
+static int brcmstb_pm_do_s3(unsigned long sp)
+{
+ int ret;
+
+ /* should not return */
+ ret = brcmstb_pm_s3_finish();
+
+ pr_err("Could not enter S3\n");
+
+ return ret;
+}
+
+static int brcmstb_pm_s3(void)
+{
+ void __iomem *sp = ctrl.boot_sram + ctrl.boot_sram_len - 8;
+
+ return cpu_suspend((unsigned long)sp, brcmstb_pm_do_s3);
+}
+
+static int brcmstb_pm_standby(bool deep_standby)
+{
+ int ret;
+
+ if (brcmstb_pm_handshake())
+ return -EIO;
+
+ if (deep_standby)
+ ret = brcmstb_pm_s3();
+ else
+ ret = brcmstb_pm_s2();
+ if (ret)
+ pr_err("%s: standby failed\n", __func__);
+
+ return ret;
+}
+
+static int brcmstb_pm_enter(suspend_state_t state)
+{
+ int ret = -EINVAL;
+
+ switch (state) {
+ case PM_SUSPEND_STANDBY:
+ ret = brcmstb_pm_standby(false);
+ break;
+ case PM_SUSPEND_MEM:
+ ret = brcmstb_pm_standby(true);
+ break;
+ }
+
+ return ret;
+}
+
+static int brcmstb_pm_valid(suspend_state_t state)
+{
+ switch (state) {
+ case PM_SUSPEND_STANDBY:
+ return true;
+ case PM_SUSPEND_MEM:
+ return ctrl.support_warm_boot;
+ default:
+ return false;
+ }
+}
+
+static const struct platform_suspend_ops brcmstb_pm_ops = {
+ .enter = brcmstb_pm_enter,
+ .valid = brcmstb_pm_valid,
+};
+
+static const struct of_device_id aon_ctrl_dt_ids[] = {
+ { .compatible = "brcm,brcmstb-aon-ctrl" },
+ {}
+};
+
+struct ddr_phy_ofdata {
+ bool supports_warm_boot;
+};
+
+static struct ddr_phy_ofdata ddr_phy_225_1 = { .supports_warm_boot = false, };
+static struct ddr_phy_ofdata ddr_phy_240_1 = { .supports_warm_boot = true, };
+
+static const struct of_device_id ddr_phy_dt_ids[] = {
+ {
+ .compatible = "brcm,brcmstb-ddr-phy-v225.1",
+ .data = &ddr_phy_225_1,
+ },
+ {
+ .compatible = "brcm,brcmstb-ddr-phy-v240.1",
+ .data = &ddr_phy_240_1,
+ },
+ {
+ /* Same as v240.1, for the registers we care about */
+ .compatible = "brcm,brcmstb-ddr-phy-v240.2",
+ .data = &ddr_phy_240_1,
+ },
+ {}
+};
+
+static const struct of_device_id ddr_shimphy_dt_ids[] = {
+ { .compatible = "brcm,brcmstb-ddr-shimphy-v1.0" },
+ {}
+};
+
+static inline void __iomem *brcmstb_ioremap_node(struct device_node *dn,
+ int index)
+{
+ return of_io_request_and_map(dn, index, dn->full_name);
+}
+
+static void __iomem *brcmstb_ioremap_match(const struct of_device_id *matches,
+ int index, const void **ofdata)
+{
+ struct device_node *dn;
+ const struct of_device_id *match;
+
+ dn = of_find_matching_node_and_match(NULL, matches, &match);
+ if (!dn)
+ return ERR_PTR(-EINVAL);
+
+ if (ofdata)
+ *ofdata = match->data;
+
+ return brcmstb_ioremap_node(dn, index);
+}
+
+static int brcmstb_pm_init(void)
+{
+ struct device_node *dn;
+ void __iomem *base;
+ int ret, i;
+ const struct ddr_phy_ofdata *ddr_phy_data;
+
+ if (!soc_is_brcmstb())
+ return 0;
+
+ /* AON ctrl registers */
+ base = brcmstb_ioremap_match(aon_ctrl_dt_ids, 0, NULL);
+ if (IS_ERR(base)) {
+ pr_err("error mapping AON_CTRL\n");
+ return PTR_ERR(base);
+ }
+ ctrl.aon_ctrl_base = base;
+
+ /* AON SRAM registers */
+ base = brcmstb_ioremap_match(aon_ctrl_dt_ids, 1, NULL);
+ if (IS_ERR(base)) {
+ /* Assume standard offset */
+ ctrl.aon_sram = ctrl.aon_ctrl_base +
+ AON_CTRL_SYSTEM_DATA_RAM_OFS;
+ } else {
+ ctrl.aon_sram = base;
+ }
+
+ /* DDR PHY registers */
+ base = brcmstb_ioremap_match(ddr_phy_dt_ids, 0,
+ (const void **)&ddr_phy_data);
+ if (IS_ERR(base)) {
+ pr_err("error mapping DDR PHY\n");
+ return PTR_ERR(base);
+ }
+ ctrl.support_warm_boot = ddr_phy_data->supports_warm_boot;
+ /* Only need DDR PHY 0 for now? */
+ ctrl.memcs[0].ddr_phy_base = base;
+
+ /* DDR SHIM-PHY registers */
+ for_each_matching_node(dn, ddr_shimphy_dt_ids) {
+ i = ctrl.num_memc;
+ if (i >= MAX_NUM_MEMC) {
+ pr_warn("too many MEMCs (max %d)\n", MAX_NUM_MEMC);
+ break;
+ }
+ base = brcmstb_ioremap_node(dn, 0);
+ if (IS_ERR(base)) {
+ if (!ctrl.support_warm_boot)
+ break;
+
+ pr_err("error mapping DDR SHIMPHY %d\n", i);
+ return PTR_ERR(base);
+ }
+ ctrl.memcs[i].ddr_shimphy_base = base;
+ ctrl.num_memc++;
+ }
+
+ dn = of_find_matching_node(NULL, sram_dt_ids);
+ if (!dn) {
+ pr_err("SRAM not found\n");
+ return -EINVAL;
+ }
+
+ ret = brcmstb_init_sram(dn);
+ if (ret) {
+ pr_err("error setting up SRAM for PM\n");
+ return ret;
+ }
+
+ ctrl.s3_params = kmalloc(sizeof(*ctrl.s3_params), GFP_KERNEL);
+ if (!ctrl.s3_params)
+ return -ENOMEM;
+ ctrl.s3_params_pa = dma_map_single(NULL, ctrl.s3_params,
+ sizeof(*ctrl.s3_params),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(NULL, ctrl.s3_params_pa)) {
+ pr_err("error mapping DMA memory\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pm_power_off = brcmstb_pm_poweroff;
+ suspend_set_ops(&brcmstb_pm_ops);
+ return 0;
+
+out:
+ kfree(ctrl.s3_params);
+
+ pr_warn("PM: initialization failed with code %d\n", ret);
+
+ return ret;
+}
+
+arch_initcall(brcmstb_pm_init);
diff --git a/drivers/soc/brcmstb/pm/pm.h b/drivers/soc/brcmstb/pm/pm.h
new file mode 100644
index 000000000000..fadab6dc424a
--- /dev/null
+++ b/drivers/soc/brcmstb/pm/pm.h
@@ -0,0 +1,40 @@
+/*
+ * Definitions for Broadcom STB power management / Always ON (AON) block
+ *
+ * Copyright © 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 version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __BRCMSTB_PM_H__
+#define __BRCMSTB_PM_H__
+
+#define AON_CTRL_RESET_CTRL 0x00
+#define AON_CTRL_PM_CTRL 0x04
+#define AON_CTRL_PM_STATUS 0x08
+#define AON_CTRL_PM_CPU_WAIT_COUNT 0x10
+#define AON_CTRL_PM_INITIATE 0x88
+#define AON_CTRL_HOST_MISC_CMDS 0x8c
+#define AON_CTRL_SYSTEM_DATA_RAM_OFS 0x200
+
+/* PM_CTRL bitfield */
+#define PM_FAST_PWRDOWN (1 << 6)
+#define PM_WARM_BOOT (1 << 5)
+#define PM_DEEP_STANDBY (1 << 4)
+#define PM_CPU_PWR (1 << 3)
+#define PM_USE_CPU_RDY (1 << 2)
+#define PM_PLL_PWRDOWN (1 << 1)
+#define PM_PWR_DOWN (1 << 0)
+
+#define PM_S2_COMMAND (PM_PLL_PWRDOWN | PM_USE_CPU_RDY | PM_PWR_DOWN)
+#define PM_COLD_CONFIG (PM_PLL_PWRDOWN | PM_DEEP_STANDBY)
+#define PM_WARM_CONFIG (PM_COLD_CONFIG | PM_USE_CPU_RDY | PM_WARM_BOOT)
+
+#endif /* __BRCMSTB_PM_H__ */
diff --git a/drivers/soc/brcmstb/pm/s2.S b/drivers/soc/brcmstb/pm/s2.S
new file mode 100644
index 000000000000..704c4ecb6f6f
--- /dev/null
+++ b/drivers/soc/brcmstb/pm/s2.S
@@ -0,0 +1,73 @@
+/*
+ * Copyright © 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 version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+#include "pm.h"
+
+ .text
+ .align 3
+
+#define AON_CTRL_REG r10
+#define DDR_PHY_STATUS_REG r11
+
+/*
+ * r0: AON_CTRL base address
+ * r1: DDRY PHY PLL status register address
+ */
+ENTRY(brcmstb_pm_do_s2)
+ stmfd sp!, {r4-r11, lr}
+ mov AON_CTRL_REG, r0
+ mov DDR_PHY_STATUS_REG, r1
+
+ /* Flush memory transactions */
+ dsb
+
+ /* power down request */
+ ldr r0, =PM_S2_COMMAND
+ ldr r1, =0
+ str r1, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+ ldr r1, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+ str r0, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+ ldr r0, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+
+ /* Wait for interrupt */
+ wfi
+ nop
+
+ /* Bring MEMC back up */
+1: ldr r0, [DDR_PHY_STATUS_REG]
+ ands r0, #1
+ beq 1b
+
+ /* Power-up handshake */
+ ldr r0, =1
+ str r0, [AON_CTRL_REG, #AON_CTRL_HOST_MISC_CMDS]
+ ldr r0, [AON_CTRL_REG, #AON_CTRL_HOST_MISC_CMDS]
+
+ ldr r0, =0
+ str r0, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+ ldr r0, [AON_CTRL_REG, #AON_CTRL_PM_CTRL]
+
+ /* Return to caller */
+ ldr r0, =0
+ ldmfd sp!, {r4-r11, pc}
+
+ ENDPROC(brcmstb_pm_do_s2)
+
+ /* Place literal pool here */
+ .ltorg
+
+ENTRY(brcmstb_pm_do_s2_sz)
+ .word . - brcmstb_pm_do_s2
--
1.9.1

2015-06-19 00:13:27

by Brian Norris

[permalink] [raw]
Subject: [PATCH 5/7] soc: brcmstb: add wake-timer driver

Useful for waking the system from suspend after a specified period of
time.

This IP could potentially be supported as an RTC driver (for use with
the 'rtcwake' utility), but it is not battery backed, so that's not a
great fit. Implement a custom sysfs interface instead.

Signed-off-by: Brian Norris <[email protected]>
---
.../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
drivers/soc/brcmstb/Kconfig | 3 +
drivers/soc/brcmstb/Makefile | 1 +
drivers/soc/brcmstb/wktmr.c | 242 +++++++++++++++++++++
4 files changed, 258 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
create mode 100644 drivers/soc/brcmstb/wktmr.c

diff --git a/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
new file mode 100644
index 000000000000..e563f8b8d969
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
@@ -0,0 +1,12 @@
+What: /sys/bus/platform/drivers/brcm-waketimer/<MMIO-address>.waketimer/timeout
+Date: November 21, 2014
+KernelVersion: 3.14
+Contact: Brian Norris <[email protected]>
+Description: The control file for the wakeup timer. This integer value
+ represents the number of seconds between a suspend operation
+ (e.g., S3 suspend-to-RAM) and the time at which the wakeup
+ timer should fire.
+
+ Values are -1 (default) or any non-negative integer. Units are
+ in seconds. The special value of -1 means the timer should not
+ wake up the system.
diff --git a/drivers/soc/brcmstb/Kconfig b/drivers/soc/brcmstb/Kconfig
index 5025dacce6f0..e818eca7e847 100644
--- a/drivers/soc/brcmstb/Kconfig
+++ b/drivers/soc/brcmstb/Kconfig
@@ -16,4 +16,7 @@ config BRCMSTB_PM
depends on PM
depends on ARM && ARCH_BRCMSTB

+config BRCMSTB_WKTMR
+ tristate "Support wake-up timer"
+
endif # SOC_BRCMSTB
diff --git a/drivers/soc/brcmstb/Makefile b/drivers/soc/brcmstb/Makefile
index 677e3fa0d042..6ecba0644229 100644
--- a/drivers/soc/brcmstb/Makefile
+++ b/drivers/soc/brcmstb/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_BRCMSTB_PM) += pm/

obj-y += common.o
+obj-$(CONFIG_BRCMSTB_WKTMR) += wktmr.o
diff --git a/drivers/soc/brcmstb/wktmr.c b/drivers/soc/brcmstb/wktmr.c
new file mode 100644
index 000000000000..89f989724d3c
--- /dev/null
+++ b/drivers/soc/brcmstb/wktmr.c
@@ -0,0 +1,242 @@
+/*
+ * Copyright © 2014-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/stat.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/suspend.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/pm_wakeup.h>
+#include <linux/reboot.h>
+
+#define DRV_NAME "brcm-waketimer"
+
+struct brcmstb_waketmr {
+ struct device *dev;
+ void __iomem *base;
+ unsigned int irq;
+
+ int wake_timeout;
+ struct notifier_block reboot_notifier;
+};
+
+/* No timeout */
+#define BRCMSTB_WKTMR_DEFAULT_TIMEOUT (-1)
+
+#define BRCMSTB_WKTMR_EVENT 0x00
+#define BRCMSTB_WKTMR_COUNTER 0x04
+#define BRCMSTB_WKTMR_ALARM 0x08
+#define BRCMSTB_WKTMR_PRESCALER 0x0C
+#define BRCMSTB_WKTMR_PRESACALER_VAL 0x10
+
+static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
+{
+ writel_relaxed(1, timer->base + BRCMSTB_WKTMR_EVENT);
+ (void)readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+}
+
+static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
+ unsigned int secs)
+{
+ unsigned int t;
+
+ brcmstb_waketmr_clear_alarm(timer);
+
+ t = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+ writel_relaxed(t + secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
+}
+
+static irqreturn_t brcmstb_waketmr_irq(int irq, void *data)
+{
+ struct brcmstb_waketmr *timer = data;
+ pm_wakeup_event(timer->dev, 0);
+ return IRQ_HANDLED;
+}
+
+static ssize_t brcmstb_waketmr_timeout_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", timer->wake_timeout);
+}
+
+static ssize_t brcmstb_waketmr_timeout_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ int timeout;
+ int ret;
+
+ ret = kstrtoint(buf, 0, &timeout);
+ if (ret < 0)
+ return ret;
+
+ /* Allow -1 as "no timeout" */
+ if (timeout < -1)
+ return -EINVAL;
+
+ timer->wake_timeout = timeout;
+
+ return count;
+}
+
+static const DEVICE_ATTR(timeout, S_IRUGO | S_IWUSR,
+ brcmstb_waketmr_timeout_show,
+ brcmstb_waketmr_timeout_store);
+
+static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
+{
+ struct device *dev = timer->dev;
+ int ret;
+
+ if (device_may_wakeup(dev) && timer->wake_timeout >= 0) {
+ ret = enable_irq_wake(timer->irq);
+ if (ret) {
+ dev_err(dev, "failed to enable wake-up interrupt\n");
+ return ret;
+ }
+
+ brcmstb_waketmr_set_alarm(timer, timer->wake_timeout);
+ }
+ return 0;
+}
+
+/* If enabled as a wakeup-source, arm the timer when powering off */
+static int brcmstb_waketmr_reboot(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct brcmstb_waketmr *timer;
+ timer = container_of(nb, struct brcmstb_waketmr, reboot_notifier);
+
+ /* Set timer for cold boot */
+ if (action == SYS_POWER_OFF)
+ brcmstb_waketmr_prepare_suspend(timer);
+
+ return NOTIFY_DONE;
+}
+
+static int brcmstb_waketmr_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct brcmstb_waketmr *timer;
+ struct resource *res;
+ int ret;
+
+ timer = devm_kzalloc(dev, sizeof(*timer), GFP_KERNEL);
+ if (!timer)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, timer);
+
+ timer->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ timer->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(timer->base))
+ return PTR_ERR(timer->base);
+
+ /*
+ * Set wakeup capability before requesting wakeup interrupt, so we can
+ * process boot-time "wakeups" (e.g., from S5 soft-off)
+ */
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+
+ timer->irq = platform_get_irq(pdev, 0);
+ if ((int)timer->irq < 0)
+ return -ENODEV;
+
+ ret = devm_request_irq(dev, timer->irq, brcmstb_waketmr_irq, 0,
+ DRV_NAME, timer);
+ if (ret < 0)
+ return ret;
+
+ timer->reboot_notifier.notifier_call = brcmstb_waketmr_reboot;
+ register_reboot_notifier(&timer->reboot_notifier);
+
+ timer->wake_timeout = BRCMSTB_WKTMR_DEFAULT_TIMEOUT;
+
+ ret = device_create_file(dev, &dev_attr_timeout);
+ if (ret)
+ unregister_reboot_notifier(&timer->reboot_notifier);
+ else
+ dev_info(dev, "registered, with irq %d\n", timer->irq);
+ return ret;
+}
+
+static int brcmstb_waketmr_remove(struct platform_device *pdev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(&pdev->dev);
+
+ device_remove_file(&pdev->dev, &dev_attr_timeout);
+ unregister_reboot_notifier(&timer->reboot_notifier);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int brcmstb_waketmr_suspend(struct device *dev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+
+ return brcmstb_waketmr_prepare_suspend(timer);
+}
+
+static int brcmstb_waketmr_resume(struct device *dev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ int ret;
+
+ if (!device_may_wakeup(dev) || timer->wake_timeout < 0)
+ return 0;
+
+ ret = disable_irq_wake(timer->irq);
+
+ brcmstb_waketmr_clear_alarm(timer);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(brcmstb_waketmr_pm_ops, brcmstb_waketmr_suspend,
+ brcmstb_waketmr_resume);
+
+static const struct of_device_id brcmstb_waketmr_of_match[] = {
+ { .compatible = "brcm,brcmstb-waketimer" },
+ {},
+};
+
+static struct platform_driver brcmstb_waketmr_driver = {
+ .probe = brcmstb_waketmr_probe,
+ .remove = brcmstb_waketmr_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &brcmstb_waketmr_pm_ops,
+ .of_match_table = of_match_ptr(brcmstb_waketmr_of_match),
+ }
+};
+module_platform_driver(brcmstb_waketmr_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("Wake-up timer driver for STB chips");
--
1.9.1

2015-06-19 00:13:20

by Brian Norris

[permalink] [raw]
Subject: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend

Lazily-masked IRQs can cause system suspend problems (e.g., spurious
wakeups from WFI), so we need to be sure non-wakeup GIC interrupts get
masked, not just disabled, during system suspend.

Signed-off-by: Brian Norris <[email protected]>
---
arch/arm/mach-bcm/brcmstb.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-bcm/brcmstb.c b/arch/arm/mach-bcm/brcmstb.c
index 3a60f7ee3f0c..8d9ec9d01306 100644
--- a/arch/arm/mach-bcm/brcmstb.c
+++ b/arch/arm/mach-bcm/brcmstb.c
@@ -12,11 +12,20 @@
*/

#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/arm-gic.h>
#include <linux/of_platform.h>

#include <asm/mach-types.h>
#include <asm/mach/arch.h>

+static void __init brcmstb_init_irq(void)
+{
+ gic_set_irqchip_flags(IRQCHIP_MASK_ON_SUSPEND);
+ irqchip_init();
+}
+
static const char *const brcmstb_match[] __initconst = {
"brcm,bcm7445",
"brcm,brcmstb",
@@ -25,4 +34,5 @@ static const char *const brcmstb_match[] __initconst = {

DT_MACHINE_START(BRCMSTB, "Broadcom STB (Flattened Device Tree)")
.dt_compat = brcmstb_match,
+ .init_irq = brcmstb_init_irq,
MACHINE_END
--
1.9.1

2015-06-19 00:13:15

by Brian Norris

[permalink] [raw]
Subject: [PATCH 7/7] ARM: dts: brcmstb: add BCM7445 system PM DT nodes

Need the aon_pm_l2_intc, aon-ctrl, waketimer, boot SRAM, and
memory_controller descriptions.

Signed-off-by: Brian Norris <[email protected]>
---
arch/arm/boot/dts/bcm7445.dtsi | 102 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)

diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
index 58dcd666257c..5b1aeaf5ac40 100644
--- a/arch/arm/boot/dts/bcm7445.dtsi
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -119,6 +119,30 @@
interrupt-names = "hif";
};

+ aon_pm_l2_intc: interrupt-controller@410640 {
+ compatible = "brcm,l2-intc";
+ reg = <0x410640 0x30>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ interrupts = <GIC_SPI 0x40 0x0>;
+ interrupt-parent = <&gic>;
+ brcm,irq-can-wake;
+ };
+
+ aon-ctrl@410000 {
+ compatible = "brcm,brcmstb-aon-ctrl";
+ reg = <0x410000 0x200>, <0x410200 0x400>;
+ reg-names = "aon-ctrl", "aon-sram";
+ };
+
+ waketimer@f0417580 {
+ compatible = "brcm,brcmstb-waketimer";
+ reg = <0x417580 0x14>;
+ interrupt-parent = <&aon_pm_l2_intc>;
+ interrupts = <3>;
+ interrupt-names = "timer";
+ };
+
nand: nand@3e2800 {
status = "disabled";
#address-cells = <1>;
@@ -169,6 +193,84 @@
};
};

+ memory_controllers {
+ compatible = "simple-bus";
+ ranges = <0x0 0x0 0xf1100000 0x200000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ memc@0 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x0 0x80000>;
+
+ memc-ddr@2000 {
+ compatible = "brcm,brcmstb-memc-ddr";
+ reg = <0x2000 0x800>;
+ };
+
+ ddr-phy@6000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0x6000 0x21c>;
+ };
+
+ shimphy@8000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0x8000 0xe4>;
+ };
+ };
+
+ memc@1 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x80000 0x80000>;
+
+ memc-ddr@2000 {
+ compatible = "brcm,brcmstb-memc-ddr";
+ reg = <0x2000 0x800>;
+ };
+
+ ddr-phy@6000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0x6000 0x21c>;
+ };
+
+ shimphy@8000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0x8000 0xe4>;
+ };
+ };
+
+ memc@2 {
+ compatible = "brcm,brcmstb-memc", "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x100000 0x80000>;
+
+ memc-ddr@2000 {
+ compatible = "brcm,brcmstb-memc-ddr";
+ reg = <0x2000 0x800>;
+ };
+
+ ddr-phy@6000 {
+ compatible = "brcm,brcmstb-ddr-phy-v240.1";
+ reg = <0x6000 0x21c>;
+ };
+
+ shimphy@8000 {
+ compatible = "brcm,brcmstb-ddr-shimphy-v1.0";
+ reg = <0x8000 0xe4>;
+ };
+ };
+ };
+
+ sram@ffe00000 {
+ compatible = "brcm,boot-sram", "mmio-sram";
+ reg = <0x0 0xffe00000 0x0 0x10000>;
+ };
+
smpboot {
compatible = "brcm,brcmstb-smpboot";
syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
--
1.9.1

2015-06-19 01:49:12

by Gregory Fong

[permalink] [raw]
Subject: Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend

On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
<[email protected]> wrote:
> Lazily-masked IRQs can cause system suspend problems (e.g., spurious
> wakeups from WFI), so we need to be sure non-wakeup GIC interrupts get
> masked, not just disabled, during system suspend.
>
> Signed-off-by: Brian Norris <[email protected]>

Acked-by: Gregory Fong <[email protected]>

2015-06-19 02:09:51

by Gregory Fong

[permalink] [raw]
Subject: Re: [PATCH 2/7] Documentation: dt: brcmstb: add waketimer documentation

On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
<[email protected]> wrote:
> Signed-off-by: Brian Norris <[email protected]>

Acked-by: Gregory Fong <[email protected]>

2015-06-19 02:21:07

by Gregory Fong

[permalink] [raw]
Subject: Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver

On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
<[email protected]> wrote:
> Useful for waking the system from suspend after a specified period of
> time.
>
> This IP could potentially be supported as an RTC driver (for use with
> the 'rtcwake' utility), but it is not battery backed, so that's not a
> great fit. Implement a custom sysfs interface instead.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
> .../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
> drivers/soc/brcmstb/Kconfig | 3 +
> drivers/soc/brcmstb/Makefile | 1 +
> drivers/soc/brcmstb/wktmr.c | 242 +++++++++++++++++++++
> 4 files changed, 258 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> create mode 100644 drivers/soc/brcmstb/wktmr.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> new file mode 100644
> index 000000000000..e563f8b8d969
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> @@ -0,0 +1,12 @@
> +What: /sys/bus/platform/drivers/brcm-waketimer/<MMIO-address>.waketimer/timeout
> +Date: November 21, 2014
> +KernelVersion: 3.14
> +Contact: Brian Norris <[email protected]>

Shouldn't this be "Brian Norris <[email protected]>"?

> +Description: The control file for the wakeup timer. This integer value
> + represents the number of seconds between a suspend operation
> + (e.g., S3 suspend-to-RAM) and the time at which the wakeup
> + timer should fire.
> [...]
> diff --git a/drivers/soc/brcmstb/wktmr.c b/drivers/soc/brcmstb/wktmr.c
> new file mode 100644
> index 000000000000..89f989724d3c
> --- /dev/null
> +++ b/drivers/soc/brcmstb/wktmr.c
> @@ -0,0 +1,242 @@
> [...]
> +
> +static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
> + unsigned int secs)
> +{
> + unsigned int t;

Use u32.

> +
> + brcmstb_waketmr_clear_alarm(timer);
> +
> + t = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
> + writel_relaxed(t + secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
> +}
> +
> [...]
> +static int brcmstb_waketmr_probe(struct platform_device *pdev)
> +{
> [...]
> + /*
> + * Set wakeup capability before requesting wakeup interrupt, so we can
> + * process boot-time "wakeups" (e.g., from S5 soft-off)
> + */
> + device_set_wakeup_capable(dev, true);
> + device_wakeup_enable(dev);
> +
> + timer->irq = platform_get_irq(pdev, 0);
> + if ((int)timer->irq < 0)

Probably better to just have timer->irq be a signed int so you don't
need this cast.

> [snip]

Acked-by: Gregory Fong <[email protected]>

2015-06-19 03:21:35

by Gregory Fong

[permalink] [raw]
Subject: Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
<[email protected]> wrote:
> Hi,
>
> This patch set introduces system suspend/resume support for Broadcom STB SoCs.
> There are two suspend modes (S2 and S3) as well as a related low-power shutdown
> mode (S5).
>
> Along with the core PM support, include a driver for the wakeup-timer, which
> allows for simple testing of suspend/resume wakeup cycles.
>
> Brian
>
> Brian Norris (7):
> Documentation: dt: brcmstb: add system PM bindings
> Documentation: dt: brcmstb: add waketimer documentation
> soc: add stubs for brcmstb SoC's
> soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
> soc: brcmstb: add wake-timer driver
> ARM: brcmstb: mask GIC IRQs on suspend
> ARM: dts: brcmstb: add BCM7445 system PM DT nodes
>

I tested this series two ways: with the device tree built into the
bootloader (BOLT) on BCM7445 and by using
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb

With the device tree from BOLT, everything works fine. Tested the
waketimer for S2, S3, and S5.

With the device tree in arch/arm/boot/dts/bcm7445-bcm97445svmb.dts, S2
works, but S3 and S5 do not. It comes back up but doesn't reach the
prompt:

[ 6.050808] PM: suspend of devices complete after 1.425 msecs
[ 6.051760] PM: late suspend of devices complete after 0.947 msecs
[ 6.052535] PM: noirq suspend of devices complete after 0.770 msecs
[ 6.052537] Disabling non-boot CPUs ...
[ 6.053005] CPU1: shutdown
[ 6.065914] CPU2: shutdown
[ 6.080756] CPU3: shutdown
[ 6.095214] Enabling non-boot CPUs ...
[ 6.111496] CPU1 is up
[ 6.126934] CPU2 is up
[ 6.142511] CPU3 is up
[ 6.145308] PM: noirq resume of devices complete after 2.772 msecs
[ 6.148022] PM: early resume of devices complete after 2.626 msecs
[ 6.151017] PM: resume of devices complete after 2.976 msecs
[ 6.212771] Restarting tasks ... done.
[[[ output stops here ]]]

I suspect there might be an issue somewhere in
[PATCH 7/7] ARM: dts: brcmstb: add BCM7445 system PM DT nodes.

2015-06-19 17:36:26

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver

On Thu, Jun 18, 2015 at 07:20:26PM -0700, Gregory Fong wrote:
> On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
> <[email protected]> wrote:
> > Useful for waking the system from suspend after a specified period of
> > time.
> >
> > This IP could potentially be supported as an RTC driver (for use with
> > the 'rtcwake' utility), but it is not battery backed, so that's not a
> > great fit. Implement a custom sysfs interface instead.
> >
> > Signed-off-by: Brian Norris <[email protected]>
> > ---
> > .../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
> > drivers/soc/brcmstb/Kconfig | 3 +
> > drivers/soc/brcmstb/Makefile | 1 +
> > drivers/soc/brcmstb/wktmr.c | 242 +++++++++++++++++++++
> > 4 files changed, 258 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> > create mode 100644 drivers/soc/brcmstb/wktmr.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> > new file mode 100644
> > index 000000000000..e563f8b8d969
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> > @@ -0,0 +1,12 @@
> > +What: /sys/bus/platform/drivers/brcm-waketimer/<MMIO-address>.waketimer/timeout
> > +Date: November 21, 2014
> > +KernelVersion: 3.14
> > +Contact: Brian Norris <[email protected]>
>
> Shouldn't this be "Brian Norris <[email protected]>"?

Indeed, thanks. Wouldn't want things bouncing...

> > +Description: The control file for the wakeup timer. This integer value
> > + represents the number of seconds between a suspend operation
> > + (e.g., S3 suspend-to-RAM) and the time at which the wakeup
> > + timer should fire.
> > [...]
> > diff --git a/drivers/soc/brcmstb/wktmr.c b/drivers/soc/brcmstb/wktmr.c
> > new file mode 100644
> > index 000000000000..89f989724d3c
> > --- /dev/null
> > +++ b/drivers/soc/brcmstb/wktmr.c
> > @@ -0,0 +1,242 @@
> > [...]
> > +
> > +static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
> > + unsigned int secs)
> > +{
> > + unsigned int t;
>
> Use u32.

Sure.

> > +
> > + brcmstb_waketmr_clear_alarm(timer);
> > +
> > + t = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
> > + writel_relaxed(t + secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
> > +}
> > +
> > [...]
> > +static int brcmstb_waketmr_probe(struct platform_device *pdev)
> > +{
> > [...]
> > + /*
> > + * Set wakeup capability before requesting wakeup interrupt, so we can
> > + * process boot-time "wakeups" (e.g., from S5 soft-off)
> > + */
> > + device_set_wakeup_capable(dev, true);
> > + device_wakeup_enable(dev);
> > +
> > + timer->irq = platform_get_irq(pdev, 0);
> > + if ((int)timer->irq < 0)
>
> Probably better to just have timer->irq be a signed int so you don't
> need this cast.

That's the only place where it needs to be signed. All other APIs are
unsigned, I think. But sure, I can change that.

> > [snip]
>
> Acked-by: Gregory Fong <[email protected]>

Thanks for the review!

Brian

2015-06-19 22:41:40

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

+ tglx, Kevin

On Thu, Jun 18, 2015 at 08:20:52PM -0700, Gregory Fong wrote:
> On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris <[email protected]> wrote:
> > This patch set introduces system suspend/resume support for Broadcom STB SoCs.
> > There are two suspend modes (S2 and S3) as well as a related low-power shutdown
> > mode (S5).
> >
> > Along with the core PM support, include a driver for the wakeup-timer, which
> > allows for simple testing of suspend/resume wakeup cycles.
> >
> > Brian
> >
> > Brian Norris (7):
> > Documentation: dt: brcmstb: add system PM bindings
> > Documentation: dt: brcmstb: add waketimer documentation
> > soc: add stubs for brcmstb SoC's
> > soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
> > soc: brcmstb: add wake-timer driver
> > ARM: brcmstb: mask GIC IRQs on suspend
> > ARM: dts: brcmstb: add BCM7445 system PM DT nodes
> >
>
> I tested this series two ways: with the device tree built into the
> bootloader (BOLT) on BCM7445 and by using
> arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
>
> With the device tree from BOLT, everything works fine. Tested the
> waketimer for S2, S3, and S5.

Thanks for the testing!

> With the device tree in arch/arm/boot/dts/bcm7445-bcm97445svmb.dts, S2
> works, but S3 and S5 do not. It comes back up but doesn't reach the
> prompt:
>
> [ 6.050808] PM: suspend of devices complete after 1.425 msecs
> [ 6.051760] PM: late suspend of devices complete after 0.947 msecs
> [ 6.052535] PM: noirq suspend of devices complete after 0.770 msecs
> [ 6.052537] Disabling non-boot CPUs ...
> [ 6.053005] CPU1: shutdown
> [ 6.065914] CPU2: shutdown
> [ 6.080756] CPU3: shutdown
> [ 6.095214] Enabling non-boot CPUs ...
> [ 6.111496] CPU1 is up
> [ 6.126934] CPU2 is up
> [ 6.142511] CPU3 is up
> [ 6.145308] PM: noirq resume of devices complete after 2.772 msecs
> [ 6.148022] PM: early resume of devices complete after 2.626 msecs
> [ 6.151017] PM: resume of devices complete after 2.976 msecs
> [ 6.212771] Restarting tasks ... done.
> [[[ output stops here ]]]

Right, I noticed this problem, but found that it was independent of the
core PM code. UART interrupts are dead, but nothing else is.

> I suspect there might be an issue somewhere in
> [PATCH 7/7] ARM: dts: brcmstb: add BCM7445 system PM DT nodes.

It's not a problem with patch 7, exactly, it's a problem with the
irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
The problem is that with a trimmed down device tree (such as the one
found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
interrupts of the 'irq0_intc' node are described -- we don't have device
tree nodes for them yet -- but we still require saving and restoring the
forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
interrupts to continue operating.

This is all a problem because when irq_chip_generic::installed == 0, the
irq_{suspend,resume} functions never get called in irq-bcm7120-l2.c.

Anyway, I think this suggests that irq-bcm7120-l2.c needs a slightly
different semantics to its PM callbacks; they must be called regardless
of whether there are any child interrupts. I've cooked up some patches
for this and will send them shortly in reply to this thread.

Thanks,
Brian

2015-06-19 22:55:33

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

On Fri, Jun 19, 2015 at 3:41 PM, Brian Norris
<[email protected]> wrote:
> + tglx, Kevin

^^ for real this time

> On Thu, Jun 18, 2015 at 08:20:52PM -0700, Gregory Fong wrote:
>> On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris <[email protected]> wrote:
>> > This patch set introduces system suspend/resume support for Broadcom STB SoCs.
>> > There are two suspend modes (S2 and S3) as well as a related low-power shutdown
>> > mode (S5).
>> >
>> > Along with the core PM support, include a driver for the wakeup-timer, which
>> > allows for simple testing of suspend/resume wakeup cycles.
>> >
>> > Brian
>> >
>> > Brian Norris (7):
>> > Documentation: dt: brcmstb: add system PM bindings
>> > Documentation: dt: brcmstb: add waketimer documentation
>> > soc: add stubs for brcmstb SoC's
>> > soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
>> > soc: brcmstb: add wake-timer driver
>> > ARM: brcmstb: mask GIC IRQs on suspend
>> > ARM: dts: brcmstb: add BCM7445 system PM DT nodes
>> >
>>
>> I tested this series two ways: with the device tree built into the
>> bootloader (BOLT) on BCM7445 and by using
>> arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
>>
>> With the device tree from BOLT, everything works fine. Tested the
>> waketimer for S2, S3, and S5.
>
> Thanks for the testing!
>
>> With the device tree in arch/arm/boot/dts/bcm7445-bcm97445svmb.dts, S2
>> works, but S3 and S5 do not. It comes back up but doesn't reach the
>> prompt:
>>
>> [ 6.050808] PM: suspend of devices complete after 1.425 msecs
>> [ 6.051760] PM: late suspend of devices complete after 0.947 msecs
>> [ 6.052535] PM: noirq suspend of devices complete after 0.770 msecs
>> [ 6.052537] Disabling non-boot CPUs ...
>> [ 6.053005] CPU1: shutdown
>> [ 6.065914] CPU2: shutdown
>> [ 6.080756] CPU3: shutdown
>> [ 6.095214] Enabling non-boot CPUs ...
>> [ 6.111496] CPU1 is up
>> [ 6.126934] CPU2 is up
>> [ 6.142511] CPU3 is up
>> [ 6.145308] PM: noirq resume of devices complete after 2.772 msecs
>> [ 6.148022] PM: early resume of devices complete after 2.626 msecs
>> [ 6.151017] PM: resume of devices complete after 2.976 msecs
>> [ 6.212771] Restarting tasks ... done.
>> [[[ output stops here ]]]
>
> Right, I noticed this problem, but found that it was independent of the
> core PM code. UART interrupts are dead, but nothing else is.
>
>> I suspect there might be an issue somewhere in
>> [PATCH 7/7] ARM: dts: brcmstb: add BCM7445 system PM DT nodes.
>
> It's not a problem with patch 7, exactly, it's a problem with the
> irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
> The problem is that with a trimmed down device tree (such as the one
> found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
> interrupts of the 'irq0_intc' node are described -- we don't have device
> tree nodes for them yet -- but we still require saving and restoring the
> forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
> interrupts to continue operating.
>
> This is all a problem because when irq_chip_generic::installed == 0, the
> irq_{suspend,resume} functions never get called in irq-bcm7120-l2.c.
>
> Anyway, I think this suggests that irq-bcm7120-l2.c needs a slightly
> different semantics to its PM callbacks; they must be called regardless
> of whether there are any child interrupts. I've cooked up some patches
> for this and will send them shortly in reply to this thread.
>
> Thanks,
> Brian

2015-06-19 23:27:09

by Brian Norris

[permalink] [raw]
Subject: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

Some (admittedly odd) irqchips perform functions that are not directly
related to any of their child IRQ lines, and therefore need to perform
some tasks during suspend/resume regardless of whether there are
any "installed" interrupts for the irqchip. However, the current
generic-chip framework does not call the chip's irq_{suspend,resume}
when there are no interrupts installed (this makes sense, because there
are no irq_data objects for such a call to be made).

More specifically, irq-bcm7120-l2 configures both a forwarding mask
(which affects other top-level GIC IRQs) and a second-level interrupt
mask (for managing its own child interrupts). The former must be
saved/restored on suspend/resume, even when there's nothing to do for
the latter.

This patch adds a second set of suspend/resume hooks to irq_chip, this
time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
These callbacks will always be called for an irqchip and are based on
the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
struct.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
---
include/linux/irq.h | 10 ++++++++++
kernel/irq/generic-chip.c | 6 ++++++
2 files changed, 16 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index de3213d271ff..2a672d2434a5 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -293,6 +293,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
return d->hwirq;
}

+struct irq_chip_generic;
+
/**
* struct irq_chip - hardware interrupt chip descriptor
*
@@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
* @irq_suspend: function called from core code on suspend once per chip
* @irq_resume: function called from core code on resume once per chip
* @irq_pm_shutdown: function called from core code on shutdown once per chip
+ * @chip_suspend: function called from core code on suspend once per
+ * chip; for handling chip details even when no interrupts
+ * are in use
+ * @chip_resume: function called from core code on resume once per chip;
+ * for handling chip details even when no interrupts are
+ * in use
* @irq_calc_mask: Optional function to set irq_data.mask for special cases
* @irq_print_chip: optional to print special chip info in show_interrupts
* @irq_request_resources: optional to request resources before calling
@@ -357,6 +365,8 @@ struct irq_chip {
void (*irq_suspend)(struct irq_data *data);
void (*irq_resume)(struct irq_data *data);
void (*irq_pm_shutdown)(struct irq_data *data);
+ void (*chip_suspend)(struct irq_chip_generic *gc);
+ void (*chip_resume)(struct irq_chip_generic *gc);

void (*irq_calc_mask)(struct irq_data *data);

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 15b370daf234..fe25e72d5d13 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -553,6 +553,9 @@ static int irq_gc_suspend(void)
if (data)
ct->chip.irq_suspend(data);
}
+
+ if (ct->chip.chip_suspend)
+ ct->chip.chip_suspend(gc);
}
return 0;
}
@@ -564,6 +567,9 @@ static void irq_gc_resume(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;

+ if (ct->chip.chip_resume)
+ ct->chip.chip_resume(gc);
+
if (ct->chip.irq_resume) {
struct irq_data *data = irq_gc_get_irq_data(gc);

--
1.9.1

2015-06-19 23:27:20

by Brian Norris

[permalink] [raw]
Subject: [PATCH 2/2] IRQCHIP: bcm7120-l2: perform suspend/resume even without installed child IRQs

Make use of the new irq_chip chip_{suspend,resume} callbacks.

This is required because if there are no installed child IRQs for
this chip, the irq_{suspend,resume} functions will not be called.
However, we still need to save/restore the forwarding mask, to enable
the top-level GIC interrupt; otherwise, we lose UART output after S3
resume.

In addition to refactoring the callbacks, we have to self-initialize the
mask cache, since the genirq core also doesn't initialize this until the
first child IRQ is installed.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 299d4de2deb5..98f0129fe843 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -81,10 +81,9 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

-static void bcm7120_l2_intc_suspend(struct irq_data *d)
+static void bcm7120_l2_intc_suspend(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct irq_chip_type *ct = gc->chip_types;
struct bcm7120_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
@@ -94,10 +93,9 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
irq_gc_unlock(gc);
}

-static void bcm7120_l2_intc_resume(struct irq_data *d)
+static void bcm7120_l2_intc_resume(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct irq_chip_type *ct = gc->chip_types;

/* Restore the saved mask */
irq_gc_lock(gc);
@@ -281,8 +279,15 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
ct->chip.irq_mask = irq_gc_mask_clr_bit;
ct->chip.irq_unmask = irq_gc_mask_set_bit;
ct->chip.irq_ack = irq_gc_noop;
- ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
- ct->chip.irq_resume = bcm7120_l2_intc_resume;
+ ct->chip.chip_suspend = bcm7120_l2_intc_suspend;
+ ct->chip.chip_resume = bcm7120_l2_intc_resume;
+
+ /*
+ * Initialize mask-cache, in case we need it for
+ * saving/restoring fwd mask even w/o any child interrupts
+ * installed
+ */
+ gc->mask_cache = irq_reg_readl(gc, ct->regs.mask);

if (data->can_wake) {
/* This IRQ chip can wake the system, set all
--
1.9.1

2015-06-19 23:39:28

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On 19/06/15 16:26, Brian Norris wrote:
> Some (admittedly odd) irqchips perform functions that are not directly
> related to any of their child IRQ lines, and therefore need to perform
> some tasks during suspend/resume regardless of whether there are
> any "installed" interrupts for the irqchip. However, the current
> generic-chip framework does not call the chip's irq_{suspend,resume}
> when there are no interrupts installed (this makes sense, because there
> are no irq_data objects for such a call to be made).
>
> More specifically, irq-bcm7120-l2 configures both a forwarding mask
> (which affects other top-level GIC IRQs) and a second-level interrupt
> mask (for managing its own child interrupts). The former must be
> saved/restored on suspend/resume, even when there's nothing to do for
> the latter.
>
> This patch adds a second set of suspend/resume hooks to irq_chip, this
> time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
> These callbacks will always be called for an irqchip and are based on
> the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
> struct.
>
> The original problem report is described in extra detail here:
> http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074
>
> Signed-off-by: Brian Norris <[email protected]>

Acked-by: Florian Fainelli <[email protected]>

> ---
> include/linux/irq.h | 10 ++++++++++
> kernel/irq/generic-chip.c | 6 ++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index de3213d271ff..2a672d2434a5 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -293,6 +293,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
> return d->hwirq;
> }
>
> +struct irq_chip_generic;
> +
> /**
> * struct irq_chip - hardware interrupt chip descriptor
> *
> @@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
> * @irq_suspend: function called from core code on suspend once per chip
> * @irq_resume: function called from core code on resume once per chip
> * @irq_pm_shutdown: function called from core code on shutdown once per chip
> + * @chip_suspend: function called from core code on suspend once per
> + * chip; for handling chip details even when no interrupts
> + * are in use
> + * @chip_resume: function called from core code on resume once per chip;
> + * for handling chip details even when no interrupts are
> + * in use
> * @irq_calc_mask: Optional function to set irq_data.mask for special cases
> * @irq_print_chip: optional to print special chip info in show_interrupts
> * @irq_request_resources: optional to request resources before calling
> @@ -357,6 +365,8 @@ struct irq_chip {
> void (*irq_suspend)(struct irq_data *data);
> void (*irq_resume)(struct irq_data *data);
> void (*irq_pm_shutdown)(struct irq_data *data);
> + void (*chip_suspend)(struct irq_chip_generic *gc);
> + void (*chip_resume)(struct irq_chip_generic *gc);
>
> void (*irq_calc_mask)(struct irq_data *data);
>
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index 15b370daf234..fe25e72d5d13 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -553,6 +553,9 @@ static int irq_gc_suspend(void)
> if (data)
> ct->chip.irq_suspend(data);
> }
> +
> + if (ct->chip.chip_suspend)
> + ct->chip.chip_suspend(gc);
> }
> return 0;
> }
> @@ -564,6 +567,9 @@ static void irq_gc_resume(void)
> list_for_each_entry(gc, &gc_list, list) {
> struct irq_chip_type *ct = gc->chip_types;
>
> + if (ct->chip.chip_resume)
> + ct->chip.chip_resume(gc);
> +
> if (ct->chip.irq_resume) {
> struct irq_data *data = irq_gc_get_irq_data(gc);
>
>


--
Florian

2015-06-19 23:40:40

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 2/2] IRQCHIP: bcm7120-l2: perform suspend/resume even without installed child IRQs

On 19/06/15 16:26, Brian Norris wrote:
> Make use of the new irq_chip chip_{suspend,resume} callbacks.
>
> This is required because if there are no installed child IRQs for
> this chip, the irq_{suspend,resume} functions will not be called.
> However, we still need to save/restore the forwarding mask, to enable
> the top-level GIC interrupt; otherwise, we lose UART output after S3
> resume.
>
> In addition to refactoring the callbacks, we have to self-initialize the
> mask cache, since the genirq core also doesn't initialize this until the
> first child IRQ is installed.
>
> The original problem report is described in extra detail here:
> http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074
>
> Signed-off-by: Brian Norris <[email protected]>

Acked-by: Florian Fainelli <[email protected]>

> ---
> drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
> index 299d4de2deb5..98f0129fe843 100644
> --- a/drivers/irqchip/irq-bcm7120-l2.c
> +++ b/drivers/irqchip/irq-bcm7120-l2.c
> @@ -81,10 +81,9 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> -static void bcm7120_l2_intc_suspend(struct irq_data *d)
> +static void bcm7120_l2_intc_suspend(struct irq_chip_generic *gc)
> {
> - struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> - struct irq_chip_type *ct = irq_data_get_chip_type(d);
> + struct irq_chip_type *ct = gc->chip_types;
> struct bcm7120_l2_intc_data *b = gc->private;
>
> irq_gc_lock(gc);
> @@ -94,10 +93,9 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
> irq_gc_unlock(gc);
> }
>
> -static void bcm7120_l2_intc_resume(struct irq_data *d)
> +static void bcm7120_l2_intc_resume(struct irq_chip_generic *gc)
> {
> - struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> - struct irq_chip_type *ct = irq_data_get_chip_type(d);
> + struct irq_chip_type *ct = gc->chip_types;
>
> /* Restore the saved mask */
> irq_gc_lock(gc);
> @@ -281,8 +279,15 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
> ct->chip.irq_mask = irq_gc_mask_clr_bit;
> ct->chip.irq_unmask = irq_gc_mask_set_bit;
> ct->chip.irq_ack = irq_gc_noop;
> - ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
> - ct->chip.irq_resume = bcm7120_l2_intc_resume;
> + ct->chip.chip_suspend = bcm7120_l2_intc_suspend;
> + ct->chip.chip_resume = bcm7120_l2_intc_resume;
> +
> + /*
> + * Initialize mask-cache, in case we need it for
> + * saving/restoring fwd mask even w/o any child interrupts
> + * installed
> + */

Your commit message is a tad clearer than this comment, but this is
still good enough to understand what is being done here.

> + gc->mask_cache = irq_reg_readl(gc, ct->regs.mask);
>
> if (data->can_wake) {
> /* This IRQ chip can wake the system, set all
>


--
Florian

2015-06-20 14:11:52

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On Fri, 19 Jun 2015, Brian Norris wrote:
> This patch adds a second set of suspend/resume hooks to irq_chip, this
> time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
> These callbacks will always be called for an irqchip and are based on
> the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
> struct.

There is no per-chip irq_chip_generic struct. It's only there if the
irq chip has been instantiated as a generic chip.

> /**
> * struct irq_chip - hardware interrupt chip descriptor
> *
> @@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
> * @irq_suspend: function called from core code on suspend once per chip
> * @irq_resume: function called from core code on resume once per chip
> * @irq_pm_shutdown: function called from core code on shutdown once per chip
> + * @chip_suspend: function called from core code on suspend once per
> + * chip; for handling chip details even when no interrupts
> + * are in use
> + * @chip_resume: function called from core code on resume once per chip;
> + * for handling chip details even when no interrupts are
> + * in use
> * @irq_calc_mask: Optional function to set irq_data.mask for special cases
> * @irq_print_chip: optional to print special chip info in show_interrupts
> * @irq_request_resources: optional to request resources before calling
> @@ -357,6 +365,8 @@ struct irq_chip {
> void (*irq_suspend)(struct irq_data *data);
> void (*irq_resume)(struct irq_data *data);
> void (*irq_pm_shutdown)(struct irq_data *data);
> + void (*chip_suspend)(struct irq_chip_generic *gc);
> + void (*chip_resume)(struct irq_chip_generic *gc);

I really don't want to set a precedent for random (*foo)(*bar)
callbacks.

> +
> + if (ct->chip.chip_suspend)
> + ct->chip.chip_suspend(gc);

So wouldn't it be the more intuitive solution to make this a callback
in the struct gc itself?

Thanks,

tglx

2015-06-22 19:47:23

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

+ others

On Thu, Jun 18, 2015 at 05:11:29PM -0700, Brian Norris wrote:
> Hi,
>
> This patch set introduces system suspend/resume support for Broadcom STB SoCs.
> There are two suspend modes (S2 and S3) as well as a related low-power shutdown
> mode (S5).
>
> Along with the core PM support, include a driver for the wakeup-timer, which
> allows for simple testing of suspend/resume wakeup cycles.
>
> Brian

Somehow I completely missed out on sending this to a few of the right
places, like linux-pm and the PM maintainers. I guess I just trusted
get_maintainer.pl (which doesn't know what to do with drivers/soc/) too
much, and forgot to turn my brain on...

Anyway, if y'all can track down the patches via archives, feel free. Or
I will resend this shortly as a v2, with a few suggested fixes and with
a more complete CC list.

Regards,
Brian

> Brian Norris (7):
> Documentation: dt: brcmstb: add system PM bindings
> Documentation: dt: brcmstb: add waketimer documentation
> soc: add stubs for brcmstb SoC's
> soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
> soc: brcmstb: add wake-timer driver
> ARM: brcmstb: mask GIC IRQs on suspend
> ARM: dts: brcmstb: add BCM7445 system PM DT nodes
>
> .../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
> .../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 142 +++++-
> .../soc/brcmstb/brcm,brcmstb-waketimer.txt | 20 +
> arch/arm/boot/dts/bcm7445.dtsi | 102 ++++
> arch/arm/mach-bcm/Kconfig | 1 +
> arch/arm/mach-bcm/brcmstb.c | 10 +
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/brcmstb/Kconfig | 22 +
> drivers/soc/brcmstb/Makefile | 4 +
> drivers/soc/brcmstb/common.c | 33 ++
> drivers/soc/brcmstb/pm/Makefile | 1 +
> drivers/soc/brcmstb/pm/aon_defs.h | 85 ++++
> drivers/soc/brcmstb/pm/pm.c | 512 +++++++++++++++++++++
> drivers/soc/brcmstb/pm/pm.h | 40 ++
> drivers/soc/brcmstb/pm/s2.S | 73 +++
> drivers/soc/brcmstb/wktmr.c | 242 ++++++++++
> include/soc/brcmstb/common.h | 15 +
> 18 files changed, 1314 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> create mode 100644 Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt
> create mode 100644 drivers/soc/brcmstb/Kconfig
> create mode 100644 drivers/soc/brcmstb/Makefile
> create mode 100644 drivers/soc/brcmstb/common.c
> create mode 100644 drivers/soc/brcmstb/pm/Makefile
> create mode 100644 drivers/soc/brcmstb/pm/aon_defs.h
> create mode 100644 drivers/soc/brcmstb/pm/pm.c
> create mode 100644 drivers/soc/brcmstb/pm/pm.h
> create mode 100644 drivers/soc/brcmstb/pm/s2.S
> create mode 100644 drivers/soc/brcmstb/wktmr.c
> create mode 100644 include/soc/brcmstb/common.h
>
> --
> 1.9.1
>

2015-06-24 04:47:30

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs

Le 06/22/15 12:47, Brian Norris a écrit :
> + others
>
> On Thu, Jun 18, 2015 at 05:11:29PM -0700, Brian Norris wrote:
>> Hi,
>>
>> This patch set introduces system suspend/resume support for Broadcom STB SoCs.
>> There are two suspend modes (S2 and S3) as well as a related low-power shutdown
>> mode (S5).
>>
>> Along with the core PM support, include a driver for the wakeup-timer, which
>> allows for simple testing of suspend/resume wakeup cycles.
>>
>> Brian
>
> Somehow I completely missed out on sending this to a few of the right
> places, like linux-pm and the PM maintainers. I guess I just trusted
> get_maintainer.pl (which doesn't know what to do with drivers/soc/) too
> much, and forgot to turn my brain on...
>
> Anyway, if y'all can track down the patches via archives, feel free. Or
> I will resend this shortly as a v2, with a few suggested fixes and with
> a more complete CC list.

Works for me, thanks for getting this out:

Reviewed-by: Florian Fainelli <[email protected]>

I would like some comments on the Device Tree binding portion so I can
take these patches and submit them for our next Broadcom arm-soc pull
requests, thanks!

>
> Regards,
> Brian
>
>> Brian Norris (7):
>> Documentation: dt: brcmstb: add system PM bindings
>> Documentation: dt: brcmstb: add waketimer documentation
>> soc: add stubs for brcmstb SoC's
>> soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
>> soc: brcmstb: add wake-timer driver
>> ARM: brcmstb: mask GIC IRQs on suspend
>> ARM: dts: brcmstb: add BCM7445 system PM DT nodes
>>
>> .../ABI/testing/sysfs-driver-wktmr-brcmstb | 12 +
>> .../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 142 +++++-
>> .../soc/brcmstb/brcm,brcmstb-waketimer.txt | 20 +
>> arch/arm/boot/dts/bcm7445.dtsi | 102 ++++
>> arch/arm/mach-bcm/Kconfig | 1 +
>> arch/arm/mach-bcm/brcmstb.c | 10 +
>> drivers/soc/Kconfig | 1 +
>> drivers/soc/Makefile | 1 +
>> drivers/soc/brcmstb/Kconfig | 22 +
>> drivers/soc/brcmstb/Makefile | 4 +
>> drivers/soc/brcmstb/common.c | 33 ++
>> drivers/soc/brcmstb/pm/Makefile | 1 +
>> drivers/soc/brcmstb/pm/aon_defs.h | 85 ++++
>> drivers/soc/brcmstb/pm/pm.c | 512 +++++++++++++++++++++
>> drivers/soc/brcmstb/pm/pm.h | 40 ++
>> drivers/soc/brcmstb/pm/s2.S | 73 +++
>> drivers/soc/brcmstb/wktmr.c | 242 ++++++++++
>> include/soc/brcmstb/common.h | 15 +
>> 18 files changed, 1314 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
>> create mode 100644 Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt
>> create mode 100644 drivers/soc/brcmstb/Kconfig
>> create mode 100644 drivers/soc/brcmstb/Makefile
>> create mode 100644 drivers/soc/brcmstb/common.c
>> create mode 100644 drivers/soc/brcmstb/pm/Makefile
>> create mode 100644 drivers/soc/brcmstb/pm/aon_defs.h
>> create mode 100644 drivers/soc/brcmstb/pm/pm.c
>> create mode 100644 drivers/soc/brcmstb/pm/pm.h
>> create mode 100644 drivers/soc/brcmstb/pm/s2.S
>> create mode 100644 drivers/soc/brcmstb/wktmr.c
>> create mode 100644 include/soc/brcmstb/common.h
>>
>> --
>> 1.9.1
>>


--
Florian

2015-07-21 18:26:26

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On 20/06/15 07:11, Thomas Gleixner wrote:
> On Fri, 19 Jun 2015, Brian Norris wrote:
>> This patch adds a second set of suspend/resume hooks to irq_chip, this
>> time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
>> These callbacks will always be called for an irqchip and are based on
>> the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
>> struct.
>
> There is no per-chip irq_chip_generic struct. It's only there if the
> irq chip has been instantiated as a generic chip.
>
>> /**
>> * struct irq_chip - hardware interrupt chip descriptor
>> *
>> @@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>> * @irq_suspend: function called from core code on suspend once per chip
>> * @irq_resume: function called from core code on resume once per chip
>> * @irq_pm_shutdown: function called from core code on shutdown once per chip
>> + * @chip_suspend: function called from core code on suspend once per
>> + * chip; for handling chip details even when no interrupts
>> + * are in use
>> + * @chip_resume: function called from core code on resume once per chip;
>> + * for handling chip details even when no interrupts are
>> + * in use
>> * @irq_calc_mask: Optional function to set irq_data.mask for special cases
>> * @irq_print_chip: optional to print special chip info in show_interrupts
>> * @irq_request_resources: optional to request resources before calling
>> @@ -357,6 +365,8 @@ struct irq_chip {
>> void (*irq_suspend)(struct irq_data *data);
>> void (*irq_resume)(struct irq_data *data);
>> void (*irq_pm_shutdown)(struct irq_data *data);
>> + void (*chip_suspend)(struct irq_chip_generic *gc);
>> + void (*chip_resume)(struct irq_chip_generic *gc);
>
> I really don't want to set a precedent for random (*foo)(*bar)
> callbacks.
>
>> +
>> + if (ct->chip.chip_suspend)
>> + ct->chip.chip_suspend(gc);
>
> So wouldn't it be the more intuitive solution to make this a callback
> in the struct gc itself?

Brian can correct me, but his approach is more generic, if there is
another irqchip driver needing a similar infrastructure, this would be
already there, and directly usable. Maybe all we need to is to change
the chip_suspend/resume arguments to pass a reference to irq_chip instead?

I can go ahead and rewrite that part of the patch to make this is
exclusively located to the irq_chip_generic structure instead.
--
Florian

2015-07-21 21:23:50

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On Tue, 21 Jul 2015, Florian Fainelli wrote:
> On 20/06/15 07:11, Thomas Gleixner wrote:
> > On Fri, 19 Jun 2015, Brian Norris wrote:
> >> This patch adds a second set of suspend/resume hooks to irq_chip, this
> >> time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
> >> These callbacks will always be called for an irqchip and are based on
> >> the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
> >> struct.
> >
> > There is no per-chip irq_chip_generic struct. It's only there if the
> > irq chip has been instantiated as a generic chip.
> >
> >> /**
> >> * struct irq_chip - hardware interrupt chip descriptor
> >> *
> >> @@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
> >> * @irq_suspend: function called from core code on suspend once per chip
> >> * @irq_resume: function called from core code on resume once per chip
> >> * @irq_pm_shutdown: function called from core code on shutdown once per chip
> >> + * @chip_suspend: function called from core code on suspend once per
> >> + * chip; for handling chip details even when no interrupts
> >> + * are in use
> >> + * @chip_resume: function called from core code on resume once per chip;
> >> + * for handling chip details even when no interrupts are
> >> + * in use
> >> * @irq_calc_mask: Optional function to set irq_data.mask for special cases
> >> * @irq_print_chip: optional to print special chip info in show_interrupts
> >> * @irq_request_resources: optional to request resources before calling
> >> @@ -357,6 +365,8 @@ struct irq_chip {
> >> void (*irq_suspend)(struct irq_data *data);
> >> void (*irq_resume)(struct irq_data *data);
> >> void (*irq_pm_shutdown)(struct irq_data *data);
> >> + void (*chip_suspend)(struct irq_chip_generic *gc);
> >> + void (*chip_resume)(struct irq_chip_generic *gc);
> >
> > I really don't want to set a precedent for random (*foo)(*bar)
> > callbacks.
> >
> >> +
> >> + if (ct->chip.chip_suspend)
> >> + ct->chip.chip_suspend(gc);
> >
> > So wouldn't it be the more intuitive solution to make this a callback
> > in the struct gc itself?
>
> Brian can correct me, but his approach is more generic, if there is
> another irqchip driver needing a similar infrastructure, this would be
> already there, and directly usable.

No it's not directly usable. It's only usable with irq_chip_generic
incarnations.

> Maybe all we need to is to change the chip_suspend/resume arguments
> to pass a reference to irq_chip instead?

I just read back on the problem report which was mentioned in the
changelog:

"It's not a problem with patch 7, exactly, it's a problem with the
irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
The problem is that with a trimmed down device tree (such as the one
found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
interrupts of the 'irq0_intc' node are described -- we don't have device
tree nodes for them yet -- but we still require saving and restoring the
forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
interrupts to continue operating."

So you are trying to work around a flaw in the device tree by adding
random callbacks to the core kernel?

Thanks,

tglx

2015-07-21 21:28:49

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On 21/07/15 14:23, Thomas Gleixner wrote:
> On Tue, 21 Jul 2015, Florian Fainelli wrote:
>> On 20/06/15 07:11, Thomas Gleixner wrote:
>>> On Fri, 19 Jun 2015, Brian Norris wrote:
>>>> This patch adds a second set of suspend/resume hooks to irq_chip, this
>>>> time to represent *chip* suspend/resume, rather than IRQ suspend/resume.
>>>> These callbacks will always be called for an irqchip and are based on
>>>> the per-chip irq_chip_generic struct, rather than the per-IRQ irq_data
>>>> struct.
>>>
>>> There is no per-chip irq_chip_generic struct. It's only there if the
>>> irq chip has been instantiated as a generic chip.
>>>
>>>> /**
>>>> * struct irq_chip - hardware interrupt chip descriptor
>>>> *
>>>> @@ -317,6 +319,12 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>> * @irq_suspend: function called from core code on suspend once per chip
>>>> * @irq_resume: function called from core code on resume once per chip
>>>> * @irq_pm_shutdown: function called from core code on shutdown once per chip
>>>> + * @chip_suspend: function called from core code on suspend once per
>>>> + * chip; for handling chip details even when no interrupts
>>>> + * are in use
>>>> + * @chip_resume: function called from core code on resume once per chip;
>>>> + * for handling chip details even when no interrupts are
>>>> + * in use
>>>> * @irq_calc_mask: Optional function to set irq_data.mask for special cases
>>>> * @irq_print_chip: optional to print special chip info in show_interrupts
>>>> * @irq_request_resources: optional to request resources before calling
>>>> @@ -357,6 +365,8 @@ struct irq_chip {
>>>> void (*irq_suspend)(struct irq_data *data);
>>>> void (*irq_resume)(struct irq_data *data);
>>>> void (*irq_pm_shutdown)(struct irq_data *data);
>>>> + void (*chip_suspend)(struct irq_chip_generic *gc);
>>>> + void (*chip_resume)(struct irq_chip_generic *gc);
>>>
>>> I really don't want to set a precedent for random (*foo)(*bar)
>>> callbacks.
>>>
>>>> +
>>>> + if (ct->chip.chip_suspend)
>>>> + ct->chip.chip_suspend(gc);
>>>
>>> So wouldn't it be the more intuitive solution to make this a callback
>>> in the struct gc itself?
>>
>> Brian can correct me, but his approach is more generic, if there is
>> another irqchip driver needing a similar infrastructure, this would be
>> already there, and directly usable.
>
> No it's not directly usable. It's only usable with irq_chip_generic
> incarnations.
>
>> Maybe all we need to is to change the chip_suspend/resume arguments
>> to pass a reference to irq_chip instead?
>
> I just read back on the problem report which was mentioned in the
> changelog:
>
> "It's not a problem with patch 7, exactly, it's a problem with the
> irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
> The problem is that with a trimmed down device tree (such as the one
> found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
> interrupts of the 'irq0_intc' node are described -- we don't have device
> tree nodes for them yet -- but we still require saving and restoring the
> forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
> interrupts to continue operating."
>
> So you are trying to work around a flaw in the device tree by adding
> random callbacks to the core kernel?

Not quite, you could have your interrupt controller node declared in
Device Tree, but have no "interrupts" property referencing it because:

- the hardware is just not there, but you inherit a common Device Tree
skleten (*.dtsi)
- you could have Device Tree overlays which may or may not be loaded as
a result of finding expansion boards etc...

It just appeared that Brian was specifically testing with something that
exposed the problem.
--
Florian

2015-07-21 21:36:30

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

Hi Florian, Thomas,

On Tue, Jul 21, 2015 at 11:24:29AM -0700, Florian Fainelli wrote:
> On 20/06/15 07:11, Thomas Gleixner wrote:
> > On Fri, 19 Jun 2015, Brian Norris wrote:
...
> > I really don't want to set a precedent for random (*foo)(*bar)
> > callbacks.
> >
> >> +
> >> + if (ct->chip.chip_suspend)
> >> + ct->chip.chip_suspend(gc);
> >
> > So wouldn't it be the more intuitive solution to make this a callback
> > in the struct gc itself?
>
> Brian can correct me, but his approach is more generic, if there is
> another irqchip driver needing a similar infrastructure, this would be
> already there, and directly usable. Maybe all we need to is to change
> the chip_suspend/resume arguments to pass a reference to irq_chip instead?

I believe Thomas is right. We could just make these into
irq_chip_generic callbacks, which would probably be the right
abstraction level. Wouldn't be much code change from this submission,
AFAICT.

(Sorry for dropping the ball on this one.)

Brian

2015-07-21 21:58:18

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On Tue, 21 Jul 2015, Florian Fainelli wrote:
> On 21/07/15 14:23, Thomas Gleixner wrote:
> > I just read back on the problem report which was mentioned in the
> > changelog:
> >
> > "It's not a problem with patch 7, exactly, it's a problem with the
> > irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
> > The problem is that with a trimmed down device tree (such as the one
> > found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
> > interrupts of the 'irq0_intc' node are described -- we don't have device
> > tree nodes for them yet -- but we still require saving and restoring the
> > forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
> > interrupts to continue operating."
> >
> > So you are trying to work around a flaw in the device tree by adding
> > random callbacks to the core kernel?
>
> Not quite, you could have your interrupt controller node declared in
> Device Tree, but have no "interrupts" property referencing it because:
>
> - the hardware is just not there, but you inherit a common Device Tree
> skleten (*.dtsi)
> - you could have Device Tree overlays which may or may not be loaded as
> a result of finding expansion boards etc...

So if no hardware is there which uses any of those interrupts, then
WHY is it a problem at all?

If it's a requirement that these registers must be restored (once, not
per irq), then I can see that it'd be nice to do that from the core.

Though that core suspend/resume function is generic chip specific. So
it does not make any sense to force it into struct irq_chip because we
have no core infrastructure to deal with it.

Thanks,

tglx

2015-07-22 23:22:19

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

Some (admittedly odd) irqchips perform functions that are not directly
related to any of their child IRQ lines, and therefore need to perform
some tasks during suspend/resume regardless of whether there are
any "installed" interrupts for the irqchip. However, the current
generic-chip framework does not call the chip's irq_{suspend,resume}
when there are no interrupts installed (this makes sense, because there
are no irq_data objects for such a call to be made).

More specifically, irq-bcm7120-l2 configures both a forwarding mask
(which affects other top-level GIC IRQs) and a second-level interrupt
mask (for managing its own child interrupts). The former must be
saved/restored on suspend/resume, even when there's nothing to do for
the latter.

This patch adds a new set of suspend/resume hooks to irq_chip_generic,
to help represent *chip* suspend/resume, rather than IRQ suspend/resume.
These callbacks will always be called for an IRQ chip (regardless of the
installed interrupts) and are based on the per-chip irq_chip_generic
struct, rather than the per-IRQ irq_data struct.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
---
v1: https://lkml.org/lkml/2015/6/19/760

v1 -> v2:
* clarify the comments on irq_chip::irq_{suspend,resume}
* add new suspend/resume hooks to the irq_chip_generic instead of irq_chip,
since that is the right level of abstraction (see v1 discussion)

include/linux/irq.h | 14 ++++++++++++--
kernel/irq/generic-chip.c | 6 ++++++
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 92188b0225bb..9fd346e605ff 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -324,8 +324,10 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
* @irq_bus_sync_unlock:function to sync and unlock slow bus (i2c) chips
* @irq_cpu_online: configure an interrupt source for a secondary CPU
* @irq_cpu_offline: un-configure an interrupt source for a secondary CPU
- * @irq_suspend: function called from core code on suspend once per chip
- * @irq_resume: function called from core code on resume once per chip
+ * @irq_suspend: function called from core code on suspend once per
+ * chip, when one or more interrupts are installed
+ * @irq_resume: function called from core code on resume once per chip,
+ * when one ore more interrupts are installed
* @irq_pm_shutdown: function called from core code on shutdown once per chip
* @irq_calc_mask: Optional function to set irq_data.mask for special cases
* @irq_print_chip: optional to print special chip info in show_interrupts
@@ -761,6 +763,12 @@ struct irq_chip_type {
* @reg_base: Register base address (virtual)
* @reg_readl: Alternate I/O accessor (defaults to readl if NULL)
* @reg_writel: Alternate I/O accessor (defaults to writel if NULL)
+ * @suspend: Function called from core code on suspend once per
+ * chip; can be useful instead of irq_chip::suspend to
+ * handle chip details even when no interrupts are in use
+ * @resume: Function called from core code on resume once per chip;
+ * can be useful instead of irq_chip::suspend to handle
+ * chip details even when no interrupts are in use
* @irq_base: Interrupt base nr for this chip
* @irq_cnt: Number of interrupts handled by this chip
* @mask_cache: Cached mask register shared between all chip types
@@ -787,6 +795,8 @@ struct irq_chip_generic {
void __iomem *reg_base;
u32 (*reg_readl)(void __iomem *addr);
void (*reg_writel)(u32 val, void __iomem *addr);
+ void (*suspend)(struct irq_chip_generic *gc);
+ void (*resume)(struct irq_chip_generic *gc);
unsigned int irq_base;
unsigned int irq_cnt;
u32 mask_cache;
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 15b370daf234..abd286afbd27 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -553,6 +553,9 @@ static int irq_gc_suspend(void)
if (data)
ct->chip.irq_suspend(data);
}
+
+ if (gc->suspend)
+ gc->suspend(gc);
}
return 0;
}
@@ -564,6 +567,9 @@ static void irq_gc_resume(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;

+ if (gc->resume)
+ gc->resume(gc);
+
if (ct->chip.irq_resume) {
struct irq_data *data = irq_gc_get_irq_data(gc);

--
2.4.3.573.g4eafbef

2015-07-22 23:22:35

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 2/2] IRQCHIP: bcm7120-l2: perform suspend/resume even without installed child IRQs

Make use of the new irq_chip_generic suspend/resume callbacks.

This is required because if there are no installed child IRQs for this
chip, the irq_chip::irq_{suspend,resume} functions will not be called.
However, we still need to save/restore the forwarding mask, to enable
the top-level GIC interrupt; otherwise, we lose UART output after S3
resume.

In addition to refactoring the callbacks, we have to self-initialize the
mask cache, since the genirq core also doesn't initialize this until the
first child IRQ is installed.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
---
v1: https://lkml.org/lkml/2015/6/19/761

v1 -> v2: adapt to changes in patch 1

drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 3ba5cc780fcb..e5e51bd9fa48 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -81,10 +81,9 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

-static void bcm7120_l2_intc_suspend(struct irq_data *d)
+static void bcm7120_l2_intc_suspend(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct irq_chip_type *ct = gc->chip_types;
struct bcm7120_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
@@ -94,10 +93,9 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
irq_gc_unlock(gc);
}

-static void bcm7120_l2_intc_resume(struct irq_data *d)
+static void bcm7120_l2_intc_resume(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct irq_chip_type *ct = gc->chip_types;

/* Restore the saved mask */
irq_gc_lock(gc);
@@ -280,8 +278,15 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
ct->chip.irq_mask = irq_gc_mask_clr_bit;
ct->chip.irq_unmask = irq_gc_mask_set_bit;
ct->chip.irq_ack = irq_gc_noop;
- ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
- ct->chip.irq_resume = bcm7120_l2_intc_resume;
+ gc->suspend = bcm7120_l2_intc_suspend;
+ gc->resume = bcm7120_l2_intc_resume;
+
+ /*
+ * Initialize mask-cache, in case we need it for
+ * saving/restoring fwd mask even w/o any child interrupts
+ * installed
+ */
+ gc->mask_cache = irq_reg_readl(gc, ct->regs.mask);

if (data->can_wake) {
/* This IRQ chip can wake the system, set all
--
2.4.3.573.g4eafbef

2015-07-22 23:28:36

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip

On Tue, Jul 21, 2015 at 11:58:01PM +0200, Thomas Gleixner wrote:
> On Tue, 21 Jul 2015, Florian Fainelli wrote:
> > On 21/07/15 14:23, Thomas Gleixner wrote:
> > > I just read back on the problem report which was mentioned in the
> > > changelog:
> > >
> > > "It's not a problem with patch 7, exactly, it's a problem with the
> > > irqchip driver which handles the UART interrupt mask (irq-bcm7120-l2.c).
> > > The problem is that with a trimmed down device tree (such as the one
> > > found at arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb), none of the child
> > > interrupts of the 'irq0_intc' node are described -- we don't have device
> > > tree nodes for them yet -- but we still require saving and restoring the
> > > forwarding mask (see 'brcm,int-fwd-mask') in order for the UART
> > > interrupts to continue operating."
> > >
> > > So you are trying to work around a flaw in the device tree by adding
> > > random callbacks to the core kernel?
> >
> > Not quite, you could have your interrupt controller node declared in
> > Device Tree, but have no "interrupts" property referencing it because:
> >
> > - the hardware is just not there, but you inherit a common Device Tree
> > skleten (*.dtsi)
> > - you could have Device Tree overlays which may or may not be loaded as
> > a result of finding expansion boards etc...
>
> So if no hardware is there which uses any of those interrupts, then
> WHY is it a problem at all?

This particular badly-designed L2 interrupt controller not only
configures its own constituent interrupts, but it controls whether some
interrupts are seen at level 1 (e.g., GIC), rather than L2. So some
interrupts are affected, but not owned, by this hardware (and driver).

> If it's a requirement that these registers must be restored (once, not
> per irq), then I can see that it'd be nice to do that from the core.

Right, they must be restored for the whole chip.

> Though that core suspend/resume function is generic chip specific. So
> it does not make any sense to force it into struct irq_chip because we
> have no core infrastructure to deal with it.

Right, and that's what v2 does.

Thanks for the comments.

Brian

Subject: [tip:irq/core] genirq: Add chip_[suspend|resume] PM support to irq_chip

Commit-ID: be9b22b6a7e6725162c64155a08b71f0654b675c
Gitweb: http://git.kernel.org/tip/be9b22b6a7e6725162c64155a08b71f0654b675c
Author: Brian Norris <[email protected]>
AuthorDate: Wed, 22 Jul 2015 16:21:39 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Mon, 27 Jul 2015 08:09:38 +0200

genirq: Add chip_[suspend|resume] PM support to irq_chip

Some (admittedly odd) irqchips perform functions that are not directly
related to any of their child IRQ lines, and therefore need to perform
some tasks during suspend/resume regardless of whether there are
any "installed" interrupts for the irqchip. However, the current
generic-chip framework does not call the chip's irq_{suspend,resume}
when there are no interrupts installed (this makes sense, because there
are no irq_data objects for such a call to be made).

More specifically, irq-bcm7120-l2 configures both a forwarding mask
(which affects other top-level GIC IRQs) and a second-level interrupt
mask (for managing its own child interrupts). The former must be
saved/restored on suspend/resume, even when there's nothing to do for
the latter.

This patch adds a new set of suspend/resume hooks to irq_chip_generic,
to help represent *chip* suspend/resume, rather than IRQ suspend/resume.
These callbacks will always be called for an IRQ chip (regardless of the
installed interrupts) and are based on the per-chip irq_chip_generic
struct, rather than the per-IRQ irq_data struct.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
Cc: Gregory Fong <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Kevin Cernekee <[email protected]>
Cc: Jason Cooper <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/irq.h | 14 ++++++++++++--
kernel/irq/generic-chip.c | 6 ++++++
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5284cb1..2c8730a 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -324,8 +324,10 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
* @irq_bus_sync_unlock:function to sync and unlock slow bus (i2c) chips
* @irq_cpu_online: configure an interrupt source for a secondary CPU
* @irq_cpu_offline: un-configure an interrupt source for a secondary CPU
- * @irq_suspend: function called from core code on suspend once per chip
- * @irq_resume: function called from core code on resume once per chip
+ * @irq_suspend: function called from core code on suspend once per
+ * chip, when one or more interrupts are installed
+ * @irq_resume: function called from core code on resume once per chip,
+ * when one ore more interrupts are installed
* @irq_pm_shutdown: function called from core code on shutdown once per chip
* @irq_calc_mask: Optional function to set irq_data.mask for special cases
* @irq_print_chip: optional to print special chip info in show_interrupts
@@ -760,6 +762,12 @@ struct irq_chip_type {
* @reg_base: Register base address (virtual)
* @reg_readl: Alternate I/O accessor (defaults to readl if NULL)
* @reg_writel: Alternate I/O accessor (defaults to writel if NULL)
+ * @suspend: Function called from core code on suspend once per
+ * chip; can be useful instead of irq_chip::suspend to
+ * handle chip details even when no interrupts are in use
+ * @resume: Function called from core code on resume once per chip;
+ * can be useful instead of irq_chip::suspend to handle
+ * chip details even when no interrupts are in use
* @irq_base: Interrupt base nr for this chip
* @irq_cnt: Number of interrupts handled by this chip
* @mask_cache: Cached mask register shared between all chip types
@@ -786,6 +794,8 @@ struct irq_chip_generic {
void __iomem *reg_base;
u32 (*reg_readl)(void __iomem *addr);
void (*reg_writel)(u32 val, void __iomem *addr);
+ void (*suspend)(struct irq_chip_generic *gc);
+ void (*resume)(struct irq_chip_generic *gc);
unsigned int irq_base;
unsigned int irq_cnt;
u32 mask_cache;
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 15b370d..abd286a 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -553,6 +553,9 @@ static int irq_gc_suspend(void)
if (data)
ct->chip.irq_suspend(data);
}
+
+ if (gc->suspend)
+ gc->suspend(gc);
}
return 0;
}
@@ -564,6 +567,9 @@ static void irq_gc_resume(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;

+ if (gc->resume)
+ gc->resume(gc);
+
if (ct->chip.irq_resume) {
struct irq_data *data = irq_gc_get_irq_data(gc);

Subject: [tip:irq/core] irqchip/bcm7120-l2: Perform suspend/ resume even without installed child IRQs

Commit-ID: fd537766715e9b6bf7ff07abb22f4817201433db
Gitweb: http://git.kernel.org/tip/fd537766715e9b6bf7ff07abb22f4817201433db
Author: Brian Norris <[email protected]>
AuthorDate: Wed, 22 Jul 2015 16:21:40 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Mon, 27 Jul 2015 08:09:38 +0200

irqchip/bcm7120-l2: Perform suspend/resume even without installed child IRQs

Make use of the new irq_chip_generic suspend/resume callbacks.

This is required because if there are no installed child IRQs for this
chip, the irq_chip::irq_{suspend,resume} functions will not be called.
However, we still need to save/restore the forwarding mask, to enable
the top-level GIC interrupt; otherwise, we lose UART output after S3
resume.

In addition to refactoring the callbacks, we have to self-initialize the
mask cache, since the genirq core also doesn't initialize this until the
first child IRQ is installed.

The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074

Signed-off-by: Brian Norris <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
Cc: Gregory Fong <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Kevin Cernekee <[email protected]>
Cc: Jason Cooper <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 88c9719..c885a5c 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -80,11 +80,10 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

-static void bcm7120_l2_intc_suspend(struct irq_data *d)
+static void bcm7120_l2_intc_suspend(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
struct bcm7120_l2_intc_data *b = gc->private;
+ struct irq_chip_type *ct = gc->chip_types;

irq_gc_lock(gc);
if (b->can_wake)
@@ -93,10 +92,9 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
irq_gc_unlock(gc);
}

-static void bcm7120_l2_intc_resume(struct irq_data *d)
+static void bcm7120_l2_intc_resume(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct irq_chip_type *ct = gc->chip_types;

/* Restore the saved mask */
irq_gc_lock(gc);
@@ -279,8 +277,15 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
ct->chip.irq_mask = irq_gc_mask_clr_bit;
ct->chip.irq_unmask = irq_gc_mask_set_bit;
ct->chip.irq_ack = irq_gc_noop;
- ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
- ct->chip.irq_resume = bcm7120_l2_intc_resume;
+ gc->suspend = bcm7120_l2_intc_suspend;
+ gc->resume = bcm7120_l2_intc_resume;
+
+ /*
+ * Initialize mask-cache, in case we need it for
+ * saving/restoring fwd mask even w/o any child interrupts
+ * installed
+ */
+ gc->mask_cache = irq_reg_readl(gc, ct->regs.mask);

if (data->can_wake) {
/* This IRQ chip can wake the system, set all