soc: brcmstb: add system suspend support for STB SoCs
Subsystems (9)
cover 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
Signature · 1 line
1.9.1
PATCH 1/7 Documentation: dt: brcmstb: add system PM bindings
--- .../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 142 ++++++++++++++++++++- 1 file changed, 140 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt +140 -2
@@ -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 at 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 at 0 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + ranges; + + ddr-phy at f1106000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0xf1106000 0x21c>; + }; + + shimphy at f1108000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0xf1108000 0xe4>; + }; + + memc-ddr at f1102000 { + reg = <0xf1102000 0x800>; + compatible = "brcm,brcmstb-memc-ddr"; + }; + }; + + memc at 1 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + ranges; + + ddr-phy at f1186000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0xf1186000 0x21c>; + }; + + shimphy at f1188000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0xf1188000 0xe4>; + }; + + memc-ddr at f1182000 { + reg = <0xf1182000 0x800>; + compatible = "brcm,brcmstb-memc-ddr"; + }; + }; + + memc at 2 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + ranges; + + ddr-phy at f1206000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0xf1206000 0x21c>; + }; + + shimphy at f1208000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0xf1208000 0xe4>; + }; + + memc-ddr at f1202000 { + reg = <0xf1202000 0x800>; + compatible = "brcm,brcmstb-memc-ddr"; + }; + }; +};
Signature · 1 line
1.9.1
Florian Fainelli
AppliedRe: [PATCH 1/7] Documentation: dt: brcmstb: add system PM bindings
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 1 line
Signed-off-by Brian Norris <[email protected]>
Applied to devicetree/next, thanks!
Signature · 1 line
Florian
PATCH 2/7 Documentation: dt: brcmstb: add waketimer documentation
In reply to Brian Norris--- .../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
Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt new file +20 -0
@@ -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 at f0411580 { + compatible = "brcm,brcmstb-waketimer"; + reg = <0xf0411580 0x14>; + interrupts = <0x3>; + interrupt-parent = <&aon_pm_l2_intc>; +};
Signature · 1 line
1.9.1
Re: [PATCH 2/7] Documentation: dt: brcmstb: add waketimer documentation
Quoted 1 line · Brian Norris
Signed-off-by Brian Norris <[email protected]>
Re: [PATCH 2/7] Documentation: dt: brcmstb: add waketimer documentation
In reply to Brian NorrisLe 06/18/15 17:11, Brian Norris a ?crit :
Quoted 1 line
Signed-off-by Brian Norris <[email protected]>
Although the binding looks good to me, I am not applying this one just yet since we would want to merge the wake-timer driver and its binding at the same time, thanks!
Quoted 32 lines
--- .../bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txtDocumentation/devicetree/bindings/soc/brcmstb/brcm,brcmstb-waketimer.txt new file +20
-0@@ -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 at f0411580 { + compatible = "brcm,brcmstb-waketimer"; + reg = <0xf0411580 0x14>; + interrupts = <0x3>; + interrupt-parent = <&aon_pm_l2_intc>; +};
Signature · 1 line
Florian
PATCH 3/7 soc: add stubs for brcmstb SoC's
In reply to Brian NorrisUsed on BCM7xxx Set-Top Box chips (e.g., BCM7445).
--- 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
arch/arm/mach-bcm/Kconfig +1 -0
@@ -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.
drivers/soc/Kconfig +1 -0
@@ -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"
drivers/soc/Makefile +1 -0
@@ -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/
drivers/soc/brcmstb/Kconfig new file +9 -0
@@ -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.
drivers/soc/brcmstb/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-y += common.o
drivers/soc/brcmstb/common.c new file +33 -0
@@ -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 ; +}
include/soc/brcmstb/common.h new file +15 -0
@@ -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__ */
Signature · 1 line
1.9.1
Florian Fainelli
AppliedRe: [PATCH 3/7] soc: add stubs for brcmstb SoC's
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 3 lines
Used on BCM7xxx Set-Top Box chips (e.g., BCM7445).
Signed-off-by Brian Norris <[email protected]>
Applied to soc/next, thanks!
Signature · 1 line
Florian
PATCH 4/7 soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
In reply to Brian NorrisSupport 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.
--- 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
drivers/soc/brcmstb/Kconfig +10 -0
@@ -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
drivers/soc/brcmstb/Makefile +2 -0
@@ -1 +1,3 @@ +obj-$(CONFIG_BRCMSTB_PM) += pm/ + obj-y += common.o
drivers/soc/brcmstb/pm/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-y += pm.o s2.o
drivers/soc/brcmstb/pm/aon_defs.h new file +85 -0
85 lines not on this page · raw message
drivers/soc/brcmstb/pm/pm.c new file +512 -0
512 lines not on this page · raw message
drivers/soc/brcmstb/pm/pm.h new file +40 -0
@@ -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__ */
drivers/soc/brcmstb/pm/s2.S new file +73 -0
73 lines not on this page · raw message
Signature · 1 line
1.9.1
Florian Fainelli
AppliedRe: [PATCH 4/7] soc: brcmstb: add PM suspend/resume support (S2/S3/S5)
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 5 lines
Support S2, S3, and S5 through
/sys/power/state("standby" and "mem") and the poweroff code paths. See comments inpm.cfor a few more details about these system suspend modes.Signed-off-by Brian Norris <[email protected]>
Applied to soc/next with an appropriate AFLAGS to force the assembler in ARMv7-a mode for s2.S, thanks!
Signature · 1 line
Florian
PATCH 5/7 soc: brcmstb: add wake-timer driver
In reply to Brian NorrisUseful 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.
--- .../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
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +12 -0
@@ -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.
drivers/soc/brcmstb/Kconfig +3 -0
@@ -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
drivers/soc/brcmstb/Makefile +1 -0
@@ -1,3 +1,4 @@ obj-$(CONFIG_BRCMSTB_PM) += pm/ obj-y += common.o +obj-$(CONFIG_BRCMSTB_WKTMR) += wktmr.o
drivers/soc/brcmstb/wktmr.c new file +242 -0
242 lines not on this page · raw message
Signature · 1 line
1.9.1
Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver
Quoted 23 lines · Brian Norris
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.cDocumentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +0
-0
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb line 4
Quoted 4 lines
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +4
-0@@ -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]>"?
Quoted 11 lines
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +4
-0+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.drivers/soc/brcmstb/wktmr.c new file +0
-0
drivers/soc/brcmstb/wktmr.c line 61
Quoted 6 lines
drivers/soc/brcmstb/wktmr.c new file +5
-0@@ -0,0 +1,242 @@ + +static void brcmstb_waketmr_set_alarm (struct brcmstb_waketmr *timer, +unsigned int secs) +{ +unsigned int t;
Use u32.
Quoted 10 lines
drivers/soc/brcmstb/wktmr.c new file +9
-0+ +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) +{
drivers/soc/brcmstb/wktmr.c line 166
Quoted 10 lines
drivers/soc/brcmstb/wktmr.c new file +9
-0+/* +* 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.
Quoted 1 line
[snip]
Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver
Quoted 19 lines · Gregory Fong
Quoted 17 lines · Brian Norris
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
Quoted 12 lines
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb line 4
Quoted 10 lines
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +4
-0@@ -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...
Quoted 11 lines
Quoted 11 lines
Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb new file +4
-0+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.drivers/soc/brcmstb/wktmr.c new file +0
-0@@ -0,0 +1,242 @@
Quoted 8 lines
drivers/soc/brcmstb/wktmr.c line 61
Quoted 6 lines
[...]
drivers/soc/brcmstb/wktmr.c new file +5
-0+ +static void brcmstb_waketmr_set_alarm (struct brcmstb_waketmr *timer, +unsigned int secs) +{ +unsigned int t;Use u32.
Sure.
Quoted 15 lines
Quoted 15 lines
drivers/soc/brcmstb/wktmr.c new file +13
-0+ +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) +*/
Quoted 8 lines
drivers/soc/brcmstb/wktmr.c line 166
Quoted 5 lines
drivers/soc/brcmstb/wktmr.c new file +5
-0+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->irqbe 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.
Quoted 3 lines
Quoted 1 line
[snip]
Acked-by Gregory Fong <[email protected]>
Thanks for the review!
Brian
Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver
In reply to Brian NorrisLe 06/18/15 17:11, Brian Norris a ?crit :
Quoted 6 lines
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.
I would very much like to see this driver be implemented as a degenerate RTC driver because this sorts of maps pretty well to the "rtcwake" use mode, and this is a persistent clock source, and even a clock source as well (I would rather avoid going the MFD route to expose these 3 "functions" though).
Thanks!
Signature · 1 line
Florian
PATCH 6/7 ARM: brcmstb: mask GIC IRQs on suspend
In reply to Brian NorrisLazily-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.
--- arch/arm/mach-bcm/brcmstb.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/arm/mach-bcm/brcmstb.c +10 -0
@@ -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
Signature · 1 line
1.9.1
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
Quoted 5 lines · Brian Norris
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]>
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
In reply to Brian NorrisLe 06/18/15 17:11, Brian Norris a ?crit :
Quoted 5 lines
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]>
Dropped this patch, no longer needed as of 4.3-rc0 with aec89ef72ba6c94420f599dcb684ed66937cdacf ("irqchip/gic: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND")
Signature · 1 line
Florian
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
Quoted 8 lines · Florian Fainelli
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 5 lines
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]>Dropped this patch, no longer needed as of 4.3-rc0 with
Ooh, rc0! I didn't know we started doing sane numbering :)
Quoted 2 lines
aec89ef72ba6c94420f599dcb684ed66937cdacf("irqchip/gic: EnableSKIP_SET_WAKEandMASK_ON_SUSPEND")
Nice! I was wanting this done for a while, but I believe this approach was rejected a few times before. Glad Sudeep dug to the bottom of the issue to argue *why* that's the right thing to do.
Brian
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
In reply to Florian FainelliOne more thing:
Quoted 10 lines · Florian Fainelli
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 5 lines
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]>Dropped this patch, no longer needed as of 4.3-rc0 with
aec89ef72ba6c94420f599dcb684ed66937cdacf("irqchip/gic: EnableSKIP_SET_WAKEandMASK_ON_SUSPEND")
With this patch dropped, then we really don't need arch/arm/mach-bcm/brcmstb.c any more, AFAICT. Unless you have more plans for this file, I can send a patch...
Brian
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
Quoted 10 lines · Brian Norris
One more thing:
Quoted 7 lines · Florian Fainelli
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 5 lines
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]>
Quoted 7 lines
Quoted 3 lines
Dropped this patch, no longer needed as of 4.3-rc0 with
aec89ef72ba6c94420f599dcb684ed66937cdacf("irqchip/gic: EnableSKIP_SET_WAKEandMASK_ON_SUSPEND")With this patch dropped, then we really don't need
arch/arm/mach-bcm/brcmstb.cany more, AFAICT. Unless you have more plans for this file, I can send a patch...
I am in the process of cleaning up the BIU control register setup for the memory controller write-pairing support to get that submitted, and that needs to be initialized before SMP, so we will still need an entry point in the machine code to do that.
Thanks!
Signature · 1 line
Florian
PATCH 7/7 ARM: dts: brcmstb: add BCM7445 system PM DT nodes
In reply to Brian NorrisNeed the aon_pm_l2_intc, aon-ctrl, waketimer, boot SRAM, and memory_controller descriptions.
--- arch/arm/boot/dts/bcm7445.dtsi | 102 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
arch/arm/boot/dts/bcm7445.dtsi +102 -0
@@ -119,6 +119,30 @@ interrupt-names = "hif"; }; + aon_pm_l2_intc: interrupt-controller at 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 at 410000 { + compatible = "brcm,brcmstb-aon-ctrl"; + reg = <0x410000 0x200>, <0x410200 0x400>; + reg-names = "aon-ctrl", "aon-sram"; + }; + + waketimer at f0417580 { + compatible = "brcm,brcmstb-waketimer"; + reg = <0x417580 0x14>; + interrupt-parent = <&aon_pm_l2_intc>; + interrupts = <3>; + interrupt-names = "timer"; + }; + nand: nand at 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 at 0 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x80000>; + + memc-ddr at 2000 { + compatible = "brcm,brcmstb-memc-ddr"; + reg = <0x2000 0x800>; + }; + + ddr-phy at 6000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0x6000 0x21c>; + }; + + shimphy at 8000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0x8000 0xe4>; + }; + }; + + memc at 1 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80000 0x80000>; + + memc-ddr at 2000 { + compatible = "brcm,brcmstb-memc-ddr"; + reg = <0x2000 0x800>; + }; + + ddr-phy at 6000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0x6000 0x21c>; + }; + + shimphy at 8000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0x8000 0xe4>; + }; + }; + + memc at 2 { + compatible = "brcm,brcmstb-memc", "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x100000 0x80000>; + + memc-ddr at 2000 { + compatible = "brcm,brcmstb-memc-ddr"; + reg = <0x2000 0x800>; + }; + + ddr-phy at 6000 { + compatible = "brcm,brcmstb-ddr-phy-v240.1"; + reg = <0x6000 0x21c>; + }; + + shimphy at 8000 { + compatible = "brcm,brcmstb-ddr-shimphy-v1.0"; + reg = <0x8000 0xe4>; + }; + }; + }; + + sram at ffe00000 { + compatible = "brcm,boot-sram", "mmio-sram"; + reg = <0x0 0xffe00000 0x0 0x10000>; + }; + smpboot { compatible = "brcm,brcmstb-smpboot"; syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
Signature · 1 line
1.9.1
Florian Fainelli
AppliedRe: [PATCH 7/7] ARM: dts: brcmstb: add BCM7445 system PM DT nodes
Le 06/18/15 17:11, Brian Norris a ?crit :
Quoted 4 lines
Need the
aon_pm_l2_intc, aon-ctrl, waketimer, boot SRAM, andmemory_controllerdescriptions.Signed-off-by Brian Norris <[email protected]>
Applied to devicetree/next sans the wake-timer node and the now duplicate aon_pm_l2_intc entry, thanks!
Signature · 1 line
Florian
Quoted 11 lines · Brian Norris
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
Quoted 9 lines
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.
+ tglx, Kevin
Quoted 20 lines · Gregory Fong
Quoted 9 lines · Brian Norris
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
Quoted 9 lines
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
Quoted 6 lines
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.dtbWith the device tree from BOLT, everything works fine. Tested the waketimer for S2, S3, and S5.
Thanks for the testing!
Quoted 19 lines
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.
Quoted 1 line
[[[ 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.
Quoted 2 lines
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
Quoted 1 line · Brian Norris
+ tglx, Kevin
^^ for real this time
Quoted 76 lines · Gregory Fong
Quoted 20 lines · Brian Norris
Quoted 9 lines
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
Quoted 9 lines
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 nodesQuoted 6 lines
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.dtbWith the device tree from BOLT, everything works fine. Tested the waketimer for S2, S3, and S5.
Thanks for the testing!
Quoted 19 lines
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.Quoted 1 line
[[[ 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.
Quoted 2 lines
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 atarch/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 inirq-bcm7120-l2.c.Anyway, I think this suggests that
irq-bcm7120-l2.cneeds 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
PATCH 1/2 genirq: add chip_{suspend,resume} PM support to irq_chip
In reply to Brian NorrisSome (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
--- include/linux/irq.h | 10 ++++++++++ kernel/irq/generic-chip.c | 6 ++++++ 2 files changed, 16 insertions(+)
include/linux/irq.h +10 -0
@@ -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);
kernel/irq/generic-chip.c +6 -0
@@ -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);
Signature · 1 line
1.9.1
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
--- drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/irqchip/irq-bcm7120-l2.c +13 -8
@@ -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
Signature · 1 line
1.9.1
Re: [PATCH 2/2] IRQCHIP: bcm7120-l2: perform suspend/resume even without installed child IRQs
Quoted 8 lines · Brian Norris
Make use of the new
irq_chipchip_{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.
Quoted 8 lines
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-0074Signed-off-by Brian Norris <[email protected]>
Quoted 38 lines
--- drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)drivers/irqchip/irq-bcm7120-l2.c +4
-6@@ -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;
drivers/irqchip/irq-bcm7120-l2.c int __init bcm7120_l2_intc_probe(struct device_node *dn, · line 289
Quoted 10 lines
drivers/irqchip/irq-bcm7120-l2.c +8
-2- 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.
Quoted 5 lines
drivers/irqchip/irq-bcm7120-l2.c +1
-0+ gc->mask_cache =irq_reg_readl (gc, ct->regs.mask);if (data->can_wake) {/* This IRQ chip can wake the system, set all
Signature · 4 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
In reply to Brian NorrisQuoted 20 lines · Brian Norris
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_dataobjects 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-chipirq_chip_genericstruct, rather than the per-IRQirq_datastruct.
Quoted 4 lines
The original problem report is described in extra detail here:
http://lkml.kernel.org/g/20150619224123.GL4917@ld-irv-0074Signed-off-by Brian Norris <[email protected]>
Quoted 65 lines
--- include/linux/irq.h | 10 ++++++++++ kernel/irq/generic-chip.c | 6 ++++++ 2 files changed, 16 insertions(+)include/linux/irq.h +10
-0@@ -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);kernel/irq/generic-chip.c +6
-0@@ -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);
Signature · 1 line
Florian
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
In reply to Brian NorrisQuoted 5 lines · Brian Norris
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-chipirq_chip_genericstruct, rather than the per-IRQirq_datastruct.
There is no per-chip irq_chip_generic struct. It's only there if the irq chip has been instantiated as a generic chip.
Quoted 17 lines
/**
* structirq_chip- hardware interrupt chip descriptor *include/linux/irq.h +6
-0@@ -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
include/linux/irq.h struct irq_chip { · line 369
Quoted 5 lines
include/linux/irq.h +2
-0@@ -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.
kernel/irq/generic-chip.c static int irq_gc_suspend(void) · line 558
Quoted 3 lines
kernel/irq/generic-chip.c +3
-0+ +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
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
Quoted 36 lines · Thomas Gleixner
Quoted 5 lines · Brian Norris
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-chipirq_chip_genericstruct, rather than the per-IRQirq_datastruct.There is no per-chip
irq_chip_genericstruct. It's only there if the irq chip has been instantiated as a generic chip.Quoted 17 lines
/**
* structirq_chip- hardware interrupt chip descriptor *include/linux/irq.h +6
-0@@ -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 include/linux/irq.h struct irq_chip { · line 369
Quoted 5 lines
include/linux/irq.h +2
-0@@ -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.
Quoted 6 lines
kernel/irq/generic-chip.c static int irq_gc_suspend(void) · line 558
Quoted 3 lines
kernel/irq/generic-chip.c +3
-0+ +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
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
Quoted 44 lines · Florian Fainelli
Quoted 36 lines · Thomas Gleixner
Quoted 5 lines · Brian Norris
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-chipirq_chip_genericstruct, rather than the per-IRQirq_datastruct.There is no per-chip
irq_chip_genericstruct. It's only there if the irq chip has been instantiated as a generic chip.Quoted 17 lines
/**
* structirq_chip- hardware interrupt chip descriptor *include/linux/irq.h +6
-0@@ -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 include/linux/irq.h struct irq_chip { · line 369
Quoted 5 lines
include/linux/irq.h +2
-0@@ -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.
Quoted 6 lines
kernel/irq/generic-chip.c static int irq_gc_suspend(void) · line 558
Quoted 3 lines
kernel/irq/generic-chip.c +3
-0+ +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?
Quoted 3 lines
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.
Quoted 2 lines
Maybe all we need to is to change the chip_suspend/resume arguments to pass a reference to
irq_chipinstead?
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
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
Quoted 67 lines · Thomas Gleixner
Quoted 44 lines · Florian Fainelli
Quoted 36 lines · Thomas Gleixner
Quoted 5 lines · Brian Norris
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-chipirq_chip_genericstruct, rather than the per-IRQirq_datastruct.There is no per-chip
irq_chip_genericstruct. It's only there if the irq chip has been instantiated as a generic chip.Quoted 17 lines
/**
* structirq_chip- hardware interrupt chip descriptor *include/linux/irq.h +6
-0@@ -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 include/linux/irq.h struct irq_chip { · line 369
Quoted 5 lines
include/linux/irq.h +2
-0@@ -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.
Quoted 6 lines
kernel/irq/generic-chip.c static int irq_gc_suspend(void) · line 558
Quoted 3 lines
kernel/irq/generic-chip.c +3
-0+ +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?
Quoted 3 lines
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_genericincarnations.Quoted 2 lines
Maybe all we need to is to change the chip_suspend/resume arguments to pass a reference to
irq_chipinstead?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 atarch/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."
Quoted 2 lines
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.
Signature · 1 line
Florian
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
Quoted 16 lines · Florian Fainelli
Quoted 12 lines · Thomas Gleixner
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 atarch/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."Quoted 2 lines
So you are trying to work around a flaw in the device tree by adding random callbacks to the core kernel?
Quoted 7 lines
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
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
Quoted 20 lines · Thomas Gleixner
Quoted 19 lines · Florian Fainelli
Quoted 12 lines · Thomas Gleixner
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 atarch/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."Quoted 2 lines
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:
Quoted 7 lines
Quoted 4 lines
- 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).
Quoted 2 lines
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.
Quoted 3 lines
Though that core suspend/resume function is generic chip specific. So it does not make any sense to force it into struct
irq_chipbecause we have no core infrastructure to deal with it.
Right, and that's what v2 does.
Thanks for the comments.
Brian
Re: [PATCH 1/2] genirq: add chip_{suspend,resume} PM support to irq_chip
In reply to Florian FainelliHi Florian, Thomas,
Quoted 2 lines · Florian Fainelli
Quoted 1 line · Thomas Gleixner
On Fri, 19 Jun 2015, Brian Norris wrote:
...
Quoted 10 lines
Quoted 9 lines
I really don't want to set a precedent for random (*foo)(*bar) callbacks.
kernel/irq/generic-chip.c static int irq_gc_suspend(void) · line 558
Quoted 3 lines
kernel/irq/generic-chip.c +3
-0+ +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?
Quoted 4 lines
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_chipinstead?
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
PATCH v2 1/2 genirq: add chip_{suspend,resume} PM support to irq_chip
In reply to Brian NorrisSome (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
---
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(-)
include/linux/irq.h +12 -2
@@ -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;
kernel/irq/generic-chip.c +6 -0
@@ -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);
Signature · 1 line
2.4.3.573.g4eafbef
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
---
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(-)
drivers/irqchip/irq-bcm7120-l2.c +13 -8
@@ -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
Signature · 1 line
2.4.3.573.g4eafbef
tip-bot for Brian Norris
merge bot[tip:irq/core] irqchip/bcm7120-l2: Perform suspend/ resume even without installed child IRQs
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
--- drivers/irqchip/irq-bcm7120-l2.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/irqchip/irq-bcm7120-l2.c +13 -8
@@ -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
tip-bot for Brian Norris
merge bot[tip:irq/core] genirq: Add chip_[suspend|resume] PM support to irq_chip
In reply to Brian Norrisgenirq: 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
--- include/linux/irq.h | 14 ++++++++++++-- kernel/irq/generic-chip.c | 6 ++++++ 2 files changed, 18 insertions(+), 2 deletions(-)
include/linux/irq.h +12 -2
@@ -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;
kernel/irq/generic-chip.c +6 -0
@@ -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);
+ others
Quoted 10 lines · Brian Norris
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
Quoted 44 lines
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.hSignature · 1 line
1.9.1
Florian Fainelli
Reviewed-byLe 06/22/15 12:47, Brian Norris a ?crit :
Quoted 14 lines
+ others
Quoted 10 lines · Brian Norris
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
Quoted 8 lines
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:
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!
Quoted 48 lines
Regards,
BrianQuoted 44 lines
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.hSignature · 1 line
1.9.1
Signature · 1 line
Florian