Re: [PATCH v2 2/3] soc: brcmstb: Add Bus Interface Unit control setup
On Thu, Sep 17, 2015 at 10:42 AM, Florian Fainelli wrote:
> On 16/09/15 23:08, Gregory Fong wrote:
>>> [...]
>>> diff --git a/drivers/soc/brcmstb/biuctrl.c b/drivers/soc/brcmstb/biuctrl.c
>>> new file mode 100644
>>> index ..1d4deada1c4d
>>> --- /dev/null
>>> +++ b/drivers/soc/brcmstb/biuctrl.c
>>> @@ -0,0 +1,119 @@
>>> [...]
>>> +int __init brcmstb_biuctrl_init(void)
>>> +{
>>> + int ret = 0;
>>> +
>>> + ret = setup_hifcpubiuctrl_regs();
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = mcp_write_pairing_set();
>>> + if (ret) {
>>> + pr_err("MCP: Unable to disable write pairing!\n");
>>> + return ret;
>>
>> The return value isn't used in patch 3. Is there a point to returning
>> an error from this function in either of the above two locations,
>> considering that?
>>
>> Looks good otherwise.
>>
>> Acked-by: Gregory Fong
>
> Not really, how about this:
>
> void __init brcmstb_biuctrl_init(void)
> {
> int ret;
>
> setup_hifcpubiuctrl_regs();
>
> ret = mcp_write_pairing_set();
> if (ret) {
> pr_err("MCP: Unable to disable write pairing!\n");
> return;
> }
>
> #ifdef CONFIG_PM_SLEEP
> register_syscore_ops(&brcmstb_cpu_credit_syscore_ops);
> #endif
> }
>
> and updating the function prototype accordingly in the header file?
Sure, that works.
Thanks,
Gregory
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ARM: brcmstb: Setup BIU control registers during boot
On Tue, Sep 15, 2015 at 11:15 AM, Florian Fainelli wrote: > Call brcmstb_biuctrl_init() in brcmstb's init_irq machine descriptor > callback since we need to setup the Bus Interface Unit before SMP in > particular, but we also need to be able to remap registers. > > Signed-off-by: Florian Fainelli Acked-by: Gregory Fong -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] Documentation: bindings: brcmstb: Document write-pairing
On Tue, Sep 15, 2015 at 11:14 AM, Florian Fainelli wrote: > Document the hif-cpubiuctrl node a bit more, and add a documentation > entry for the optional "brcm,write-pairing" property. > > Signed-off-by: Florian Fainelli Acked-by: Gregory Fong -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] soc: brcmstb: Add Bus Interface Unit control setup
On Tue, Sep 15, 2015 at 11:14 AM, Florian Fainelli wrote:
> Broadcom STB SoCs (brcmstb) require an early setup of their Bus
> Interface Unit control register, this needs to happen before SMP is
> brought up because it affects how the CPU complex will be interfaced to
> the memory controller.
>
> Add support code which properly initializes the BIU registers based on
> whether "brcm,write-pairing" is present in Device Tree, and take care of
> saving and restoring credit register settings during system-wide
> suspend/resume operations.
>
> Signed-off-by: Florian Fainelli
> ---
> Changes in v2:
>
> - add a pr_fmt prefix which is more descriptive
>
> drivers/soc/brcmstb/Makefile| 2 +-
> drivers/soc/brcmstb/biuctrl.c | 119
>
> include/linux/soc/brcmstb/brcmstb.h | 10 +++
> 3 files changed, 130 insertions(+), 1 deletion(-)
> create mode 100644 drivers/soc/brcmstb/biuctrl.c
> create mode 100644 include/linux/soc/brcmstb/brcmstb.h
>
> [...]
> diff --git a/drivers/soc/brcmstb/biuctrl.c b/drivers/soc/brcmstb/biuctrl.c
> new file mode 100644
> index ..1d4deada1c4d
> --- /dev/null
> +++ b/drivers/soc/brcmstb/biuctrl.c
> @@ -0,0 +1,119 @@
> [...]
> +int __init brcmstb_biuctrl_init(void)
> +{
> + int ret = 0;
> +
> + ret = setup_hifcpubiuctrl_regs();
> + if (ret)
> + return ret;
> +
> + ret = mcp_write_pairing_set();
> + if (ret) {
> + pr_err("MCP: Unable to disable write pairing!\n");
> + return ret;
The return value isn't used in patch 3. Is there a point to returning
an error from this function in either of the above two locations,
considering that?
Looks good otherwise.
Acked-by: Gregory Fong
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] watchdog: Watchdog driver for Broadcom Set-Top Box
Gah, sorry, my email client did something really weird. Sorry for the duplication, please don't reply to those. -- Gregory-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] watchdog: bcm7038: add device tree binding documentation
Add device tree binding documentation for the watchdog hardware block
on bcm7038 and newer SoCs.
Signed-off-by: Justin Chen
---
.../devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt | 19 +++
1 file changed, 19 insertions(+)
create mode 100644
Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
b/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
new file mode 100644
index 000..39e5cf5
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
@@ -0,0 +1,19 @@
+BCM7038 Watchdog timer
+
+Required properties:
+
+- compatible : should be "brcm,bcm7038-wdt"
+- reg : Specifies base physical address and size of the registers.
+
+Optional properties:
+
+- clocks: The clock running the watchdog. If no clock is found the
+ driver will default to 2700 HZ.
+
+Example:
+
+watchdog {
+ compatible = "brcm,bcm7038-wdt";
+ clocks = <&upg_fixed>;
+ reg = <0xf040a7e8 0x16>;
+};
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] watchdog: Watchdog driver for Broadcom Set-Top Box
Watchdog driver for Broadcom 7038 and newer chips.
Signed-off-by: Justin Chen
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/bcm7038_wdt.c | 235 +
3 files changed, 244 insertions(+)
create mode 100644 drivers/watchdog/bcm7038_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 241fafd..4fbe8ab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1291,6 +1291,14 @@ config BCM_KONA_WDT_DEBUG
If in doubt, say 'N'.
+config BCM7038_WDT
+ tristate "BCM7038 Watchdog"
+ select WATCHDOG_CORE
+ help
+Watchdog driver for the built-in hardware in Broadcom 7038 SoCs.
+
+Say 'Y or 'M' here to enable the driver.
+
config IMGPDC_WDT
tristate "Imagination Technologies PDC Watchdog Timer"
depends on HAS_IOMEM
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 59ea9a1..65d4169 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o
obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
+obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
# AVR32 Architecture
obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm7038_wdt.c b/drivers/watchdog/bcm7038_wdt.c
new file mode 100644
index 000..5e54c1b
--- /dev/null
+++ b/drivers/watchdog/bcm7038_wdt.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright (C) 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
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define WDT_START_10xff00
+#define WDT_START_20x00ff
+#define WDT_STOP_1 0xee00
+#define WDT_STOP_2 0x00ee
+
+#define WDT_TIMEOUT_REG0x0
+#define WDT_CMD_REG0x4
+
+#define WDT_MIN_TIMEOUT1 /* seconds */
+#define WDT_DEFAULT_TIMEOUT30 /* seconds */
+#define WDT_DEFAULT_RATE 2700
+
+struct bcm7038_watchdog {
+ void __iomem*base;
+ struct watchdog_device wdd;
+ u32 rate;
+ struct clk *clk;
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static void bcm7038_wdt_set_timeout_reg(struct watchdog_device *wdog)
+{
+ struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
+ u32 timeout;
+
+ timeout = wdt->rate * wdog->timeout;
+
+ writel(timeout, wdt->base + WDT_TIMEOUT_REG);
+}
+
+static int bcm7038_wdt_ping(struct watchdog_device *wdog)
+{
+ struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
+
+ writel(WDT_START_1, wdt->base + WDT_CMD_REG);
+ writel(WDT_START_2, wdt->base + WDT_CMD_REG);
+
+ return 0;
+}
+
+static int bcm7038_wdt_start(struct watchdog_device *wdog)
+{
+ bcm7038_wdt_set_timeout_reg(wdog);
+ bcm7038_wdt_ping(wdog);
+
+ return 0;
+}
+
+static int bcm7038_wdt_stop(struct watchdog_device *wdog)
+{
+ struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
+
+ writel(WDT_STOP_1, wdt->base + WDT_CMD_REG);
+ writel(WDT_STOP_2, wdt->base + WDT_CMD_REG);
+
+ return 0;
+}
+
+static int bcm7038_wdt_set_timeout(struct watchdog_device *wdog,
+ unsigned int t)
+{
+ /* Can't modify timeout value if watchdog timer is running */
+ bcm7038_wdt_stop(wdog);
+ wdog->timeout = t;
+ bcm7038_wdt_start(wdog);
+
+ return 0;
+}
+
+static unsigned int bcm7038_wdt_get_timeleft(struct watchdog_device *wdog)
+{
+ struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
+ u32 time_left;
+
+ time_left = readl(wdt->base + WDT_CMD_REG);
+
+ return time_left / wdt->rate;
+}
+
+static struct watchdog_info bcm7038_wdt_info = {
+ .identity = "Broadcom BCM7038 Watchdog Timer",
+ .options= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+ WDIOF_MAGICCLOSE
+};
+
+static struct watchdog_ops bcm7038_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = bcm7038_wdt_start,
+ .stop = bcm7038_wdt_stop,
+ .set_timeout= bcm7038_wdt_set_timeout,
+ .get_timeleft = bcm7038_wdt_get_timeleft,
+};
+
+static int bcm7038_wdt_probe(struct platform_device *pdev)
+{
+ struct devic
[PATCH v2 0/2] watchdog: driver for BCM7038 and newer chips.
This driver is for a watchdog block contained in all Broadcom Set-top Box chips since BCM7038. BCM7038 was made public during the 2004 CES, and since then, many chips use this watchdog block including some cable modem chips. Changes since v1: Removed clock-frequency because it brought unnecessary complexity to the driver. Renamed a few variables. Patch 1: watchdog device tree binding documentation Patch 2: watchdog driver Justin Chen (2): watchdog: bcm7038: add device tree binding documentation watchdog: Watchdog driver for Broadcom Set-Top Box .../bindings/watchdog/brcm,bcm7038-wdt.txt | 19 ++ drivers/watchdog/Kconfig | 8 + drivers/watchdog/Makefile | 1 + drivers/watchdog/bcm7038_wdt.c | 235 + 4 files changed, 263 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt create mode 100644 drivers/watchdog/bcm7038_wdt.c -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support
On Thu, Aug 13, 2015 at 4:11 AM, Linus Walleij wrote:
> On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong wrote:
>
>> Uses the gpiolib irqchip helpers. For this to work, the irq setup
>> function is called once per bank instead of once per device. Note
>> that all known uses of this block have a BCM7120 L2 interrupt
>> controller as a parent. Supports interrupts for all GPIOs.
>>
>> In the IRQ handler, we check for raised IRQs for invalid GPIOs and
>> warn (ratelimited) if they're encountered.
>>
>> Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
>> configured as wakeup sources, and this GPIO controller supports that
>> through a separate interrupt path.
>>
>> The de-facto standard DT property "wakeup-source" is checked, since
>> that indicates whether the GPIO controller hardware can wake. Uses
>> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
>> any of its own wakeup source configuration.
>>
>> Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
>> that you can have multiple chained irqchips and associated IRQ domains
>> for a single parent IRQ, and as long as the xlate function is written
>> correctly, a GPIO IRQ request end up checking the correct domain and
>> will get associated with the correct IRQ. What helps make this clear
>> is to read
>> drivers/gpio/gpiolib-of.c:
>>- of_gpiochip_find_and_xlate()
>>- of_get_named_gpiod_flags()
>> drivers/gpio/gpiolib.c:
>>- gpiochip_find()
>>
>> Signed-off-by: Gregory Fong
>> ---
>> v4:
>> - when checking parent_irq, use <= 0 or > 0 since 0 is NO_IRQ.
>
> Patch applied, but:
>
>
>> + if (of_property_read_bool(np, "interrupt-controller")) {
>> + priv->parent_irq = platform_get_irq(pdev, 0);
>> + if (priv->parent_irq <= 0) {
>> + dev_err(dev, "Couldn't get IRQ");
>> + return -ENOENT;
>> + }
>> + } else {
>> + priv->parent_irq = -ENOENT;
>> + }
>> +
>> if (brcmstb_gpio_sanity_check_banks(dev, np, res))
>> return -EINVAL;
>
> This patch does not apply cleanly because the version in my
> tree has INIT_LIST_HEAD(&priv->bank_list);
> before the sanity check.
>
> I applied it manually, but check that things are working right.
Your tree seems to be missing
commit 2252607d327d5219a6331b50e6ec266d56402be0
Author: Gregory Fong
Date: Wed Jun 17 18:00:40 2015 -0700
gpio: brcmstb: fix null ptr dereference in driver remove
which you had pulled in and sent for 4.2-rc1. It should apply cleanly
on top of that.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] dt-bindings: brcmstb-gpio: document properties for wakeup
Some brcmstb GPIO controllers can be used to wake from suspend, so use
the de facto standard property 'wakeup-source' to mark the nodes of
controllers with that capability.
Also document interrupts-extended, which will be used for wakeup
handling because the interrupt parent for the wake IRQ is different
from the regular IRQ.
While we're at it, a few more fixes: We don't actually use the
"interrupt-names" property, so remove it from the listed optional
properties and from the examples. And since we're modifying the
examples, also follow Brian's suggestions to:
- change #gpio-cells, #interrupt-cells, and brcm,gpio-bank-widths from
hex to dec
- use phandles
Reviewed-by: Brian Norris
Acked-by: Florian Fainelli
Signed-off-by: Gregory Fong
---
v4: no changes from v3
.../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 35 +-
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
index 435f1bc..b405b44 100644
--- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
@@ -33,6 +33,13 @@ Optional properties:
- interrupt-parent:
phandle of the parent interrupt controller
+- interrupts-extended:
+Alternate form of specifying interrupts and parents that allows for
+multiple parents. This takes precedence over 'interrupts' and
+'interrupt-parent'. Wakeup-capable GPIO controllers often route their
+wakeup interrupt lines through a different interrupt controller than the
+primary interrupt line, making this property necessary.
+
- #interrupt-cells:
Should be <2>. The first cell is the GPIO number, the second should
specify
flags. The following subset of flags is supported:
@@ -47,19 +54,33 @@ Optional properties:
- interrupt-controller:
Marks the device node as an interrupt controller
-- interrupt-names:
-The name of the IRQ resource used by this controller
+- wakeup-source:
+GPIOs for this controller can be used as a wakeup source
Example:
upg_gio: gpio@f040a700 {
- #gpio-cells = <0x2>;
- #interrupt-cells = <0x2>;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
gpio-controller;
interrupt-controller;
reg = <0xf040a700 0x80>;
- interrupt-parent = <0xf>;
+ interrupt-parent = <&irq0_intc>;
+ interrupts = <0x6>;
+ brcm,gpio-bank-widths = <32 32 32 24>;
+ };
+
+ upg_gio_aon: gpio@f04172c0 {
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+ gpio-controller;
+ interrupt-controller;
+ reg = <0xf04172c0 0x40>;
+ interrupt-parent = <&irq0_aon_intc>;
interrupts = <0x6>;
- interrupt-names = "upg_gio";
- brcm,gpio-bank-widths = <0x20 0x20 0x20 0x18>;
+ interrupts-extended = <&irq0_aon_intc 0x6>,
+ <&aon_pm_l2_intc 0x5>;
+ wakeup-source;
+ brcm,gpio-bank-widths = <18 4>;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] ARM: dts: brcmstb: add BCM7445 GPIO nodes
Need the aon_pm_l2_intc and irq0_aon_intc descriptions, so included
those as well.
Signed-off-by: Gregory Fong
---
New in v4.
arch/arm/boot/dts/bcm7445.dtsi | 50 ++
1 file changed, 50 insertions(+)
diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
index 58dcd66..3b6b175 100644
--- a/arch/arm/boot/dts/bcm7445.dtsi
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -109,6 +109,20 @@
brcm,int-fwd-mask = <0x7>;
};
+ irq0_aon_intc: interrupt-controller@417280 {
+ compatible = "brcm,bcm7120-l2-intc";
+ reg = <0x417280 0x8>;
+ interrupt-parent = <&gic>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ interrupts = ,
+,
+;
+ brcm,int-map-mask = <0x1e3 0x1800 0x10>;
+ brcm,int-fwd-mask = <0x0>;
+ brcm,irq-can-wake;
+ };
+
hif_intr2_intc: interrupt-controller@3e1000 {
compatible = "brcm,l2-intc";
reg = <0x3e1000 0x30>;
@@ -119,6 +133,16 @@
interrupt-names = "hif";
};
+aon_pm_l2_intc: interrupt-controller@410640 {
+ compatible = "brcm,l2-intc";
+ reg = <0x410640 0x30>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ interrupts = ;
+ interrupt-parent = <&gic>;
+ brcm,irq-can-wake;
+ };
+
nand: nand@3e2800 {
status = "disabled";
#address-cells = <1>;
@@ -167,6 +191,32 @@
#phy-cells = <0>;
};
};
+
+ upg_gio: gpio@40a700 {
+ compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+ reg = <0x40a700 0x80>;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ interrupt-parent = <&irq0_intc>;
+ interrupts = <6>;
+ brcm,gpio-bank-widths = <32 32 32 24>;
+ };
+
+ upg_gio_aon: gpio@4172c0 {
+ compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+ reg = <0x4172c0 0x40>;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ interrupts-extended = <&irq0_aon_intc 0x6>,
+ <&aon_pm_l2_intc 0x5>;
+ wakeup-source;
+ brcm,gpio-bank-widths = <18 4>;
+ };
+
};
smpboot {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] gpio: brcmstb: support wakeup from S5 cold boot
For wake from S5, we need to:
- register a reboot handler
- set wakeup capability before requesting IRQ so wakeup count is
incremented
- mask all GPIO IRQs and clear any pending interrupts during driver
probe to since no driver will yet be registered to handle any IRQs
carried over from boot at that time, and it's possible that the
booted kernel does not request the same IRQ anyway.
This means that /sys/.../power/wakeup_count is valid at boot time, and
we can properly account for S5 wakeup stats. e.g.:
### After waking from S5 from a GPIO key
# cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup
enabled
# cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup_count
1
Signed-off-by: Gregory Fong
---
v4: rename __brcmstb_gpio_irq_set_wake() to brcmstb_gpio_priv_set_wake().
drivers/gpio/gpio-brcmstb.c | 56 -
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 46cc4e9..9ea86d2 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -48,6 +49,7 @@ struct brcmstb_gpio_priv {
int gpio_base;
bool can_wake;
int parent_wake_irq;
+ struct notifier_block reboot_notifier;
};
#define MAX_GPIO_PER_BANK 32
@@ -167,10 +169,9 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d,
unsigned int type)
return 0;
}
-static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+static int brcmstb_gpio_priv_set_wake(struct brcmstb_gpio_priv *priv,
+ unsigned int enable)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
int ret = 0;
/*
@@ -188,6 +189,14 @@ static int brcmstb_gpio_irq_set_wake(struct irq_data *d,
unsigned int enable)
return ret;
}
+static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+
+ return brcmstb_gpio_priv_set_wake(priv, enable);
+}
+
static irqreturn_t brcmstb_gpio_wake_irq_handler(int irq, void *data)
{
struct brcmstb_gpio_priv *priv = data;
@@ -246,6 +255,19 @@ static void brcmstb_gpio_irq_handler(unsigned int irq,
struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+static int brcmstb_gpio_reboot(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct brcmstb_gpio_priv *priv =
+ container_of(nb, struct brcmstb_gpio_priv, reboot_notifier);
+
+ /* Enable GPIO for S5 cold boot */
+ if (action == SYS_POWER_OFF)
+ brcmstb_gpio_priv_set_wake(priv, 1);
+
+ return NOTIFY_DONE;
+}
+
/* Make sure that the number of banks matches up between properties */
static int brcmstb_gpio_sanity_check_banks(struct device *dev,
struct device_node *np, struct resource *res)
@@ -285,6 +307,12 @@ static int brcmstb_gpio_remove(struct platform_device
*pdev)
if (ret)
dev_err(&pdev->dev, "gpiochip_remove fail in
cleanup\n");
}
+ if (priv->reboot_notifier.notifier_call) {
+ ret = unregister_reboot_notifier(&priv->reboot_notifier);
+ if (ret)
+ dev_err(&pdev->dev,
+ "failed to unregister reboot notifier\n");
+ }
return ret;
}
@@ -342,7 +370,16 @@ static int brcmstb_gpio_irq_setup(struct platform_device
*pdev,
dev_warn(dev,
"Couldn't get wake IRQ - GPIOs will not be able
to wake from sleep");
} else {
- int err = devm_request_irq(dev, priv->parent_wake_irq,
+ int err;
+
+ /*
+* Set wakeup capability before requesting wakeup
+* interrupt, so we can process boot-time "wakeups"
+* (e.g., from S5 cold boot)
+*/
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+ err = devm_request_irq(dev, priv->parent_wake_irq,
brcmstb_gpio_wake_irq_handler, 0,
"brcmstb-gpio-wake", priv);
@@ -351,8 +388,9 @@ static int brcmstb_gpio_irq_setup(struct platform_device
*pdev,
return err;
}
-
[PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support
Uses the gpiolib irqchip helpers. For this to work, the irq setup
function is called once per bank instead of once per device. Note
that all known uses of this block have a BCM7120 L2 interrupt
controller as a parent. Supports interrupts for all GPIOs.
In the IRQ handler, we check for raised IRQs for invalid GPIOs and
warn (ratelimited) if they're encountered.
Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
configured as wakeup sources, and this GPIO controller supports that
through a separate interrupt path.
The de-facto standard DT property "wakeup-source" is checked, since
that indicates whether the GPIO controller hardware can wake. Uses
the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
any of its own wakeup source configuration.
Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
that you can have multiple chained irqchips and associated IRQ domains
for a single parent IRQ, and as long as the xlate function is written
correctly, a GPIO IRQ request end up checking the correct domain and
will get associated with the correct IRQ. What helps make this clear
is to read
drivers/gpio/gpiolib-of.c:
- of_gpiochip_find_and_xlate()
- of_get_named_gpiod_flags()
drivers/gpio/gpiolib.c:
- gpiochip_find()
Signed-off-by: Gregory Fong
---
v4:
- when checking parent_irq, use <= 0 or > 0 since 0 is NO_IRQ.
drivers/gpio/Kconfig| 1 +
drivers/gpio/gpio-brcmstb.c | 262 +++-
2 files changed, 257 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8f1fe73..0b77175 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -131,6 +131,7 @@ config GPIO_BRCMSTB
default y if ARCH_BRCMSTB
depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
help
Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 4630a81..46cc4e9 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -17,6 +17,9 @@
#include
#include
#include
+#include
+#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -34,14 +37,17 @@ struct brcmstb_gpio_bank {
struct bgpio_chip bgc;
struct brcmstb_gpio_priv *parent_priv;
u32 width;
+ struct irq_chip irq_chip;
};
struct brcmstb_gpio_priv {
struct list_head bank_list;
void __iomem *reg_base;
- int num_banks;
struct platform_device *pdev;
+ int parent_irq;
int gpio_base;
+ bool can_wake;
+ int parent_wake_irq;
};
#define MAX_GPIO_PER_BANK 32
@@ -63,6 +69,183 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
return bank->parent_priv;
}
+static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
+ unsigned int offset, bool enable)
+{
+ struct bgpio_chip *bgc = &bank->bgc;
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = bgc->pin2mask(bgc, offset);
+ u32 imask;
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ imask = bgc->read_reg(priv->reg_base + GIO_MASK(bank->id));
+ if (enable)
+ imask |= mask;
+ else
+ imask &= ~mask;
+ bgc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+/* IRQ chip functions */
+
+static void brcmstb_gpio_irq_mask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+ brcmstb_gpio_set_imask(bank, d->hwirq, false);
+}
+
+static void brcmstb_gpio_irq_unmask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+ brcmstb_gpio_set_imask(bank, d->hwirq, true);
+}
+
+static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = BIT(d->hwirq);
+ u32 edge_insensitive, iedge_insensitive;
+ u32 edge_config, iedge_config;
+ u32 level, ilevel;
+ unsigned long flags;
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_LOW:
+ level = 0;
+ edge_config = 0;
+ edge_insensitive = 0;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ level = mask;
+ edge_conf
[PATCH v4 0/4] GPIO support for BRCMSTB
Adds interrupt support for the GPIO controller (UPG GIO) used on Broadcom's various BRCMSTB SoCs (BCM7XXX and others). For all existing hardware, this block hooks up to the BCM7120 L2 IRQ controller and so will require CONFIG_BCM7120_L2_IRQ=y. New in v4: - add nodes for the BRCMSTB GPIO controller to the BCM7445 dts file - a few improvements suggested by Linus Walleij on v3 - remove unused 'irq' argument to brcmstb_gpio_irq_bank_handler() The following are not included in this patchset: - Initial device tree bindings (merged from v1 to GPIO tree) - Initial GPIO support w/o interrupts (merged from v2 to GPIO tree) - ARM Kconfig changes (merged from v2 to arm-soc tree) - fix for null ptr deref in driver remove (merged from v3) Previous versions: v1: https://lkml.org/lkml/2015/5/6/199 v2: https://lkml.org/lkml/2015/5/28/853 v3: https://lkml.org/lkml/2015/6/17/960 Gregory Fong (4): dt-bindings: brcmstb-gpio: document properties for wakeup gpio: brcmstb: Add interrupt and wakeup source support gpio: brcmstb: support wakeup from S5 cold boot ARM: dts: brcmstb: add BCM7445 GPIO nodes .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 35 ++- arch/arm/boot/dts/bcm7445.dtsi | 50 drivers/gpio/Kconfig | 1 + drivers/gpio/gpio-brcmstb.c| 306 - 4 files changed, 379 insertions(+), 13 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] gpio: brcmstb: support wakeup from S5 cold boot
On Mon, Jul 13, 2015 at 6:03 AM, Linus Walleij wrote:
> On Thu, Jun 18, 2015 at 3:00 AM, Gregory Fong wrote:
>
>> For wake from S5, we need to:
>> - register a reboot handler
>> - set wakeup capability before requesting IRQ so wakeup count is
>> incremented
>> - mask all GPIO IRQs and clear any pending interrupts during driver
>> probe to since no driver will yet be registered to handle any IRQs
>> carried over from boot at that time, and it's possible that the
>> booted kernel does not request the same IRQ anyway.
>>
>> This means that /sys/.../power/wakeup_count is valid at boot time, and
>> we can properly account for S5 wakeup stats. e.g.:
>>
>> ### After waking from S5 from a GPIO key
>> # cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup
>> enabled
>> # cat
>> /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup_count
>> 1
>>
>> Signed-off-by: Gregory Fong
>
> (...)
>> -static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int
>> enable)
>> +static int __brcmstb_gpio_irq_set_wake(struct brcmstb_gpio_priv *priv,
>> + unsigned int enable)
>> {
>> - struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>> - struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
>> int ret = 0;
>
> I don't usually like to refactor code with __foo wrapper functions with
> underscores or double underscores in front of them.
>
> Is it possible to give this a more unique name?
Sure, just have to come up with a good one :-).
>
>> + /*
>> +* Mask all interrupts by default, since wakeup interrupts
>> may
>> +* be retained from S5 cold boot
>> +*/
>> + bank->bgc.write_reg(reg_base + GIO_MASK(bank->id), 0);
>
> Aha I see, that's some clever code, nice.
>
> Yours,
> Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] gpio: brcmstb: Add interrupt and wakeup source support
On Mon, Jul 13, 2015 at 5:58 AM, Linus Walleij wrote:
> On Thu, Jun 18, 2015 at 3:00 AM, Gregory Fong wrote:
>
>> Uses the gpiolib irqchip helpers. For this to work, the irq setup
>> function is called once per bank instead of once per device. Note
>> that all known uses of this block have a BCM7120 L2 interrupt
>> controller as a parent. Supports interrupts for all GPIOs.
>>
>> In the IRQ handler, we check for raised IRQs for invalid GPIOs and
>> warn (ratelimited) if they're encountered.
>>
>> Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
>> configured as wakeup sources, and this GPIO controller supports that
>> through a separate interrupt path.
>>
>> The de-facto standard DT property "wakeup-source" is checked, since
>> that indicates whether the GPIO controller hardware can wake. Uses
>> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
>> any of its own wakeup source configuration.
>>
>> Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
>> that you can have multiple chained irqchips and associated IRQ domains
>> for a single parent IRQ, and as long as the xlate function is written
>> correctly, a GPIO IRQ request end up checking the correct domain and
>> will get associated with the correct IRQ. What helps make this clear
>> is to read
>> drivers/gpio/gpiolib-of.c:
>>- of_gpiochip_find_and_xlate()
>>- of_get_named_gpiod_flags()
>> drivers/gpio/gpiolib.c:
>>- gpiochip_find()
>
> Sorry for the unclarities, this is a bit hairy. Suggestions as to
> how we can make it easier and/or bring code for this into gpiolib
> are welcome. Right now I have a hard time seeing any way to
> make this more generic and helpful :/
I'll see about putting together an update to the documentation
discussing more about the case where you have one IRQ shared by
multiple GPIO banks.
>
> Overall this patch looks real nice. Some nitpicks below.
>
>> @@ -164,6 +398,16 @@ static int brcmstb_gpio_probe(struct platform_device
>> *pdev)
>> priv->reg_base = reg_base;
>> priv->pdev = pdev;
>>
>> + if (of_property_read_bool(np, "interrupt-controller")) {
>> + priv->parent_irq = platform_get_irq(pdev, 0);
>> + if (priv->parent_irq < 0) {
>
> This should be <= 0 since 0 is NO_IRQ
Will fix.
>
>> + dev_err(dev, "Couldn't get IRQ");
>> + return -ENOENT;
>> + }
>> + } else {
>> + priv->parent_irq = -ENOENT;
>> + }
>
> Why should this code only execute if the node is marked
> "interrupt-controller"? It makes it seem like IRQs cannot arrive
> to it unless it is intended to serve other consumers.
>
> Maybe in practice this is true, but...
If the node does not contain the "interrupt-controller" property, the
hardware does not support GPIO interrupts, and I designed the driver
specifically to allow that to work.
If the node does contain that property, then being unable to complete
IRQ setup is a fatal error, because something is badly misconfigured.
>
>> + if (priv->parent_irq >= 0) {
>> + err = brcmstb_gpio_irq_setup(pdev, bank);
>> + if (err)
>> + goto fail;
>> + }
>
> Again 0 is NO_IRQ so this should be > 0 not >= 0.
OK, will change.
Thanks,
Gregory
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] soc: brcmstb: add system suspend support for STB SoCs
On Thu, Jun 18, 2015 at 5:11 PM, 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 > > 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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] soc: brcmstb: add wake-timer driver
On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris
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
> ---
> .../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 ..e563f8b8d969
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-wktmr-brcmstb
> @@ -0,0 +1,12 @@
> +What:
> /sys/bus/platform/drivers/brcm-waketimer/.waketimer/timeout
> +Date: November 21, 2014
> +KernelVersion: 3.14
> +Contact: Brian Norris
Shouldn't this be "Brian Norris "?
> +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 ..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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] Documentation: dt: brcmstb: add waketimer documentation
On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris wrote: > Signed-off-by: Brian Norris Acked-by: Gregory Fong -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: brcmstb: mask GIC IRQs on suspend
On Thu, Jun 18, 2015 at 5:11 PM, Brian Norris 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 Acked-by: Gregory Fong -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] gpio: brcmstb: support wakeup from S5 cold boot
For wake from S5, we need to:
- register a reboot handler
- set wakeup capability before requesting IRQ so wakeup count is
incremented
- mask all GPIO IRQs and clear any pending interrupts during driver
probe to since no driver will yet be registered to handle any IRQs
carried over from boot at that time, and it's possible that the
booted kernel does not request the same IRQ anyway.
This means that /sys/.../power/wakeup_count is valid at boot time, and
we can properly account for S5 wakeup stats. e.g.:
### After waking from S5 from a GPIO key
# cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup
enabled
# cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup_count
1
Signed-off-by: Gregory Fong
---
New in v3.
drivers/gpio/gpio-brcmstb.c | 56 -
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 141509b..dedb35c 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -48,6 +49,7 @@ struct brcmstb_gpio_priv {
int gpio_base;
bool can_wake;
int parent_wake_irq;
+ struct notifier_block reboot_notifier;
};
#define MAX_GPIO_PER_BANK 32
@@ -167,10 +169,9 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d,
unsigned int type)
return 0;
}
-static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+static int __brcmstb_gpio_irq_set_wake(struct brcmstb_gpio_priv *priv,
+ unsigned int enable)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
int ret = 0;
/*
@@ -188,6 +189,14 @@ static int brcmstb_gpio_irq_set_wake(struct irq_data *d,
unsigned int enable)
return ret;
}
+static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+
+ return __brcmstb_gpio_irq_set_wake(priv, enable);
+}
+
static irqreturn_t brcmstb_gpio_wake_irq_handler(int irq, void *data)
{
struct brcmstb_gpio_priv *priv = data;
@@ -247,6 +256,19 @@ static void brcmstb_gpio_irq_handler(unsigned int irq,
struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+static int brcmstb_gpio_reboot(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct brcmstb_gpio_priv *priv =
+ container_of(nb, struct brcmstb_gpio_priv, reboot_notifier);
+
+ /* Enable GPIO for S5 cold boot */
+ if (action == SYS_POWER_OFF)
+ __brcmstb_gpio_irq_set_wake(priv, 1);
+
+ return NOTIFY_DONE;
+}
+
/* Make sure that the number of banks matches up between properties */
static int brcmstb_gpio_sanity_check_banks(struct device *dev,
struct device_node *np, struct resource *res)
@@ -286,6 +308,12 @@ static int brcmstb_gpio_remove(struct platform_device
*pdev)
if (ret)
dev_err(&pdev->dev, "gpiochip_remove fail in
cleanup\n");
}
+ if (priv->reboot_notifier.notifier_call) {
+ ret = unregister_reboot_notifier(&priv->reboot_notifier);
+ if (ret)
+ dev_err(&pdev->dev,
+ "failed to unregister reboot notifier\n");
+ }
return ret;
}
@@ -343,7 +371,16 @@ static int brcmstb_gpio_irq_setup(struct platform_device
*pdev,
dev_warn(dev,
"Couldn't get wake IRQ - GPIOs will not be able
to wake from sleep");
} else {
- int err = devm_request_irq(dev, priv->parent_wake_irq,
+ int err;
+
+ /*
+* Set wakeup capability before requesting wakeup
+* interrupt, so we can process boot-time "wakeups"
+* (e.g., from S5 cold boot)
+*/
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+ err = devm_request_irq(dev, priv->parent_wake_irq,
brcmstb_gpio_wake_irq_handler, 0,
"brcmstb-gpio-wake", priv);
@@ -352,8 +389,9 @@ static int brcmstb_gpio_irq_setup(struct platform_device
*pdev,
return err;
}
- device_set_wakeup_capable(dev, true);
-
[PATCH v3 3/4] gpio: brcmstb: Add interrupt and wakeup source support
Uses the gpiolib irqchip helpers. For this to work, the irq setup
function is called once per bank instead of once per device. Note
that all known uses of this block have a BCM7120 L2 interrupt
controller as a parent. Supports interrupts for all GPIOs.
In the IRQ handler, we check for raised IRQs for invalid GPIOs and
warn (ratelimited) if they're encountered.
Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
configured as wakeup sources, and this GPIO controller supports that
through a separate interrupt path.
The de-facto standard DT property "wakeup-source" is checked, since
that indicates whether the GPIO controller hardware can wake. Uses
the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
any of its own wakeup source configuration.
Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
that you can have multiple chained irqchips and associated IRQ domains
for a single parent IRQ, and as long as the xlate function is written
correctly, a GPIO IRQ request end up checking the correct domain and
will get associated with the correct IRQ. What helps make this clear
is to read
drivers/gpio/gpiolib-of.c:
- of_gpiochip_find_and_xlate()
- of_get_named_gpiod_flags()
drivers/gpio/gpiolib.c:
- gpiochip_find()
Signed-off-by: Gregory Fong
---
v3:
- combine commits to add interrupt support and allow GPIOs to be wakeup sources
- change to use the gpiolib irqchip helpers, reducing unnecessary code
duplication.
drivers/gpio/Kconfig| 1 +
drivers/gpio/gpio-brcmstb.c | 263 +++-
2 files changed, 258 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5f79b7f..f723c7e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -131,6 +131,7 @@ config GPIO_BRCMSTB
default y if ARCH_BRCMSTB
depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
help
Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 4630a81..141509b 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -17,6 +17,9 @@
#include
#include
#include
+#include
+#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -34,14 +37,17 @@ struct brcmstb_gpio_bank {
struct bgpio_chip bgc;
struct brcmstb_gpio_priv *parent_priv;
u32 width;
+ struct irq_chip irq_chip;
};
struct brcmstb_gpio_priv {
struct list_head bank_list;
void __iomem *reg_base;
- int num_banks;
struct platform_device *pdev;
+ int parent_irq;
int gpio_base;
+ bool can_wake;
+ int parent_wake_irq;
};
#define MAX_GPIO_PER_BANK 32
@@ -63,6 +69,184 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
return bank->parent_priv;
}
+static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
+ unsigned int offset, bool enable)
+{
+ struct bgpio_chip *bgc = &bank->bgc;
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = bgc->pin2mask(bgc, offset);
+ u32 imask;
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ imask = bgc->read_reg(priv->reg_base + GIO_MASK(bank->id));
+ if (enable)
+ imask |= mask;
+ else
+ imask &= ~mask;
+ bgc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+/* IRQ chip functions */
+
+static void brcmstb_gpio_irq_mask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+ brcmstb_gpio_set_imask(bank, d->hwirq, false);
+}
+
+static void brcmstb_gpio_irq_unmask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+ brcmstb_gpio_set_imask(bank, d->hwirq, true);
+}
+
+static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = BIT(d->hwirq);
+ u32 edge_insensitive, iedge_insensitive;
+ u32 edge_config, iedge_config;
+ u32 level, ilevel;
+ unsigned long flags;
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_LOW:
+ level = 0;
+ edge_config = 0;
+ edge_insensitive = 0;
+ break
[PATCH v3 2/4] dt-bindings: brcmstb-gpio: document properties for wakeup
Some brcmstb GPIO controllers can be used to wake from suspend, so use
the de facto standard property 'wakeup-source' to mark the nodes of
controllers with that capability.
Also document interrupts-extended, which will be used for wakeup
handling because the interrupt parent for the wake IRQ is different
from the regular IRQ.
While we're at it, a few more fixes: We don't actually use the
"interrupt-names" property, so remove it from the listed optional
properties and from the examples. And since we're modifying the
examples, also follow Brian's suggestions to:
- change #gpio-cells, #interrupt-cells, and brcm,gpio-bank-widths from
hex to dec
- use phandles
Reviewed-by: Brian Norris
Signed-off-by: Gregory Fong
---
v3: Update per Brian's suggestions described in above message.
.../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 35 +-
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
index 435f1bc..b405b44 100644
--- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
@@ -33,6 +33,13 @@ Optional properties:
- interrupt-parent:
phandle of the parent interrupt controller
+- interrupts-extended:
+Alternate form of specifying interrupts and parents that allows for
+multiple parents. This takes precedence over 'interrupts' and
+'interrupt-parent'. Wakeup-capable GPIO controllers often route their
+wakeup interrupt lines through a different interrupt controller than the
+primary interrupt line, making this property necessary.
+
- #interrupt-cells:
Should be <2>. The first cell is the GPIO number, the second should
specify
flags. The following subset of flags is supported:
@@ -47,19 +54,33 @@ Optional properties:
- interrupt-controller:
Marks the device node as an interrupt controller
-- interrupt-names:
-The name of the IRQ resource used by this controller
+- wakeup-source:
+GPIOs for this controller can be used as a wakeup source
Example:
upg_gio: gpio@f040a700 {
- #gpio-cells = <0x2>;
- #interrupt-cells = <0x2>;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
gpio-controller;
interrupt-controller;
reg = <0xf040a700 0x80>;
- interrupt-parent = <0xf>;
+ interrupt-parent = <&irq0_intc>;
+ interrupts = <0x6>;
+ brcm,gpio-bank-widths = <32 32 32 24>;
+ };
+
+ upg_gio_aon: gpio@f04172c0 {
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+ gpio-controller;
+ interrupt-controller;
+ reg = <0xf04172c0 0x40>;
+ interrupt-parent = <&irq0_aon_intc>;
interrupts = <0x6>;
- interrupt-names = "upg_gio";
- brcm,gpio-bank-widths = <0x20 0x20 0x20 0x18>;
+ interrupts-extended = <&irq0_aon_intc 0x6>,
+ <&aon_pm_l2_intc 0x5>;
+ wakeup-source;
+ brcm,gpio-bank-widths = <18 4>;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] gpio: brcmstb: fix null ptr dereference in driver remove
If a failure occurs during probe, brcmstb_gpio_remove() is called. In
remove, we call platform_get_drvdata(), but at the time of failure in
the probe the driver data hadn't yet been set which leads to a NULL
ptr dereference in the remove's list_for_each. Call
platform_set_drvdata() and set up list head right after allocating the
priv struct to both avoid the null pointer dereference that could
occur today. To guard against potential future changes, check for
null pointer in remove.
Reported-by: Tim Ross
Signed-off-by: Gregory Fong
---
New in v3.
drivers/gpio/gpio-brcmstb.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 7a3cb1f..4630a81 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -87,6 +87,15 @@ static int brcmstb_gpio_remove(struct platform_device *pdev)
struct brcmstb_gpio_bank *bank;
int ret = 0;
+ if (!priv) {
+ dev_err(&pdev->dev, "called %s without drvdata!\n", __func__);
+ return -EFAULT;
+ }
+
+ /*
+* You can lose return values below, but we report all errors, and it's
+* more important to actually perform all of the steps.
+*/
list_for_each(pos, &priv->bank_list) {
bank = list_entry(pos, struct brcmstb_gpio_bank, node);
ret = bgpio_remove(&bank->bgc);
@@ -143,6 +152,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ platform_set_drvdata(pdev, priv);
+ INIT_LIST_HEAD(&priv->bank_list);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
reg_base = devm_ioremap_resource(dev, res);
@@ -153,7 +164,6 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
priv->reg_base = reg_base;
priv->pdev = pdev;
- INIT_LIST_HEAD(&priv->bank_list);
if (brcmstb_gpio_sanity_check_banks(dev, np, res))
return -EINVAL;
@@ -221,8 +231,6 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
dev_info(dev, "Registered %d banks (GPIO(s): %d-%d)\n",
priv->num_banks, priv->gpio_base, gpio_base - 1);
- platform_set_drvdata(pdev, priv);
-
return 0;
fail:
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] GPIO support for BRCMSTB
Adds interrupt support for the GPIO controller (UPG GIO) used on Broadcom's various BRCMSTB SoCs (BCM7XXX and others). For all existing hardware, this block hooks up to the BCM7120 L2 IRQ controller and so will require CONFIG_BCM7120_L2_IRQ=y. New in v3: - fix a null pointer dereference noticed by Tim Ross - use the gpiolib irqchip helpers as recommended by Linus Walleij - update device tree bindings as suggested by Brian Norris: https://lkml.org/lkml/2015/5/29/802 - add S5 (cold boot) support The following are not included in this patchset: - Initial device tree bindings (merged from v1 to GPIO tree) - Initial GPIO support w/o interrupts (merged from v2 to GPIO tree) - ARM Kconfig changes (merged from v2 to arm-soc tree) Previous revisions: v1: https://lkml.org/lkml/2015/5/6/199 v2: https://lkml.org/lkml/2015/5/28/853 Gregory Fong (4): gpio: brcmstb: fix null ptr dereference in driver remove dt-bindings: brcmstb-gpio: document properties for wakeup gpio: brcmstb: Add interrupt and wakeup source support gpio: brcmstb: support wakeup from S5 cold boot .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 35 ++- drivers/gpio/Kconfig | 1 + drivers/gpio/gpio-brcmstb.c| 321 - 3 files changed, 341 insertions(+), 16 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] GPIO support for BRCMSTB
On Tue, Jun 2, 2015 at 2:05 AM, Linus Walleij wrote: > On Wed, May 27, 2015 at 5:26 AM, Gregory Fong wrote: >> I've now actually attempted to use the gpiolib irqchip code. This >> driver can't directly use gpiochip_irqchip_add() because of the >> multiple gpiochip : one irqchip map. At first, I thought it might be >> possible to simply add a new argument (or break things into a helper >> function) to allow setting the associated IRQ domain, but then I can't >> use the generic map and unmap functions which expect the irq_domain >> host_data member to be struct gpiochip *, which makes no sense in this >> case. That puts me right back to implementing a special version of >> the map and unmap function. > > I see. > I think I was wrong, it isn't actually necessary to have a single irq_chip as long as all of the IRQs use the same handler. I have a preliminary implementation doing that instead that seems to work fine. Once I've had a chance to verify that it works in all cases, will post that as v3. Cheers, Gregory -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] gpio: brcmstb: Allow GPIOs to be wakeup sources
On Tue, Jun 2, 2015 at 6:45 AM, Linus Walleij wrote: > On Fri, May 29, 2015 at 4:14 AM, Gregory Fong wrote: > >> Several drivers (e.g. gpio-keys) allow for GPIOs to be configured as >> wakeup sources, and this GPIO controller supports that through a >> separate interrupt path. >> >> The de-facto standard DT property "wakeup-source" is checked, since >> that indicates whether the GPIO controller hardware can wake. Uses >> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have >> any of its own wakeup source configuration. >> >> Signed-off-by: Gregory Fong > > (...) >> + if (enable) >> + enable_irq_wake(priv->parent_wake_irq); >> + else >> + disable_irq_wake(priv->parent_wake_irq); >> + return 0; > > No error handling? If the code assumes these calls will > always succeed, atleast write that in a comment. Will add error handling. Thanks, Gregory -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] gpio: brcmstb: Allow GPIOs to be wakeup sources
On Fri, May 29, 2015 at 5:43 PM, Brian Norris
wrote:
> On Thu, May 28, 2015 at 07:14:08PM -0700, Gregory Fong wrote:
>> Several drivers (e.g. gpio-keys) allow for GPIOs to be configured as
>> wakeup sources, and this GPIO controller supports that through a
>> separate interrupt path.
>>
>> The de-facto standard DT property "wakeup-source" is checked, since
>> that indicates whether the GPIO controller hardware can wake. Uses
>> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
>> any of its own wakeup source configuration.
>>
>> Signed-off-by: Gregory Fong
>> ---
>> New in v2.
>>
>> drivers/gpio/gpio-brcmstb.c | 53
>> +
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
>> index b9962ff..2598c1e 100644
>> --- a/drivers/gpio/gpio-brcmstb.c
>> +++ b/drivers/gpio/gpio-brcmstb.c
>> [...]
>> @@ -369,6 +396,32 @@ static int brcmstb_gpio_irq_setup(struct
>> platform_device *pdev,
>> priv->irq_chip.irq_mask = brcmstb_gpio_irq_mask;
>> priv->irq_chip.irq_unmask = brcmstb_gpio_irq_unmask;
>> priv->irq_chip.irq_set_type = brcmstb_gpio_irq_set_type;
>> +
>> + /* Ensures that all non-wakeup IRQs are disabled at suspend */
>> + priv->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND;
>> +
>> + if (IS_ENABLED(CONFIG_PM_SLEEP) &&
>> + of_get_property(np, "wakeup-source", NULL)) {
>
> of_property_read_bool()?
Will change.
>
>> + priv->parent_wake_irq = platform_get_irq(pdev, 1);
>> + if (priv->parent_wake_irq < 0) {
>> + dev_warn(dev,
>> + "Couldn't get wake IRQ - GPIOs will not be
>> able to wake from sleep");
>> + } else {
>> + int err = devm_request_irq(dev, priv->parent_wake_irq,
>> + brcmstb_gpio_wake_irq_handler, 0,
>> + "brcmstb-gpio-wake", priv);
>> +
>> + if (err < 0) {
>> + dev_err(dev, "Couldn't request wake IRQ");
>> + return err;
>> + }
>> +
>> + device_set_wakeup_capable(dev, true);
>> + device_wakeup_enable(dev);
>
> Might want to move these two lines above the devm_request_irq(), so that
> you're ready to record PM events immediately at probe time. This is
> important when waking from S5 states, where we sometimes want to be able
> to check the /sys/devices/.../wakeup_count stats to see what woke us up.
Makes sense. We'll also need a reboot notifier for S5 to work.
>
>> + priv->irq_chip.irq_set_wake =
>> brcmstb_gpio_irq_set_wake;
>> + }
>> + }
>> +
>> priv->irq_domain =
>> irq_domain_add_linear(np, priv->num_gpios,
>> &brcmstb_gpio_irq_domain_ops,
>
> With that:
>
> Reviewed-by: Brian Norris
Thanks,
Gregory
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] dt-bindings: brcmstb-gpio: document properties for wakeup
On Fri, May 29, 2015 at 6:37 PM, Brian Norris wrote: > On Fri, May 29, 2015 at 05:57:50PM -0700, Gregory Fong wrote: >> On Fri, May 29, 2015 at 5:36 PM, Brian Norris >> wrote: >> > On Thu, May 28, 2015 at 07:14:07PM -0700, Gregory Fong wrote: >> >> --- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt >> >> +++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt >> >> @@ -33,6 +33,12 @@ Optional properties: > ... >> >> - #interrupt-cells: >> >> Should be <2>. The first cell is the GPIO number, the second should >> >> specify >> >> flags. The following subset of flags is supported: >> >> @@ -48,7 +54,10 @@ Optional properties: >> >> Marks the device node as an interrupt controller >> >> >> >> - interrupt-names: >> >> -The name of the IRQ resource used by this controller >> >> +The names of the IRQ resources used by this controller >> > >> > If you're specifying names, you should list them here. >> >> I was wondering about that. Some bindings have them listed, some >> don't. In this case I know what names currently exist but there could >> certainly be different ones in the future. How does that work? Or am >> I misunderstanding what this field is used for? Where are the >> documented rules for this? > > The only documentation I see is: > Documentation/devicetree/bindings/resource-names.txt > > That documents the basics of the *-names properties, not their expected > usage. > > In practice, they're only useful if you have enough optional resources > that fixed indexing isn't sufficient, and you need to use > platform_get_resource_byname(). > > So IMO, their purposes seems to be one of these: > (1) functional (e.g., for get_resource_byname(), when you have more than > one optional resource) > (2) self-documentation (which might run counter to #1, as you begin > generating too many unique names) > (3) no purpose > > So IMO, if you ever want (1), they shouldn't have instance-specific > naming, but should use something generic to the device class. Otherwise, > they are just self-documentation, and aren't functionally useful. So > IMO, these sorts of names: > > interrupt-names = "upg_gio_aon", "upg_gio_aon_wakeup"; > > work better as functional descriptions: > > interrupt-names = "gio", "wakeup"; > > But in the end, I wouldn't foresee you needing to do (1), so you're left > with (2) or (3), at which point I'm not sure if you should even mention > the property. I'm fine with leaving out the interrupt-names property, since we're not using it here anyway. Unless there are serious objections, I'll plan on remove it from the bindings doc next round. Thanks, Gregory -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/6] gpio: brcmstb: Add interrupt support
On Fri, May 29, 2015 at 5:10 PM, Brian Norris
wrote:
> A few small comments:
>
> On Thu, May 28, 2015 at 07:14:06PM -0700, Gregory Fong wrote:
>> v2:
>> - since imask member of bank struct was removed, just read and write from
>> mask
>> reg and don't maintain a shadow
>
> ^^ this comment may be addressing what I'm going to ask about below? Not
> sure why this was changed, actually.
Yes, see below...
>
>> - warn on invalid IRQs
>> - move some irq setup to a separate function since probe is getting unwieldy
>>
>> drivers/gpio/Kconfig| 1 +
>> drivers/gpio/gpio-brcmstb.c | 276
>>
>> 2 files changed, 277 insertions(+)
>>
> ...
>> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
>> index 7a3cb1f..b9962ff 100644
>> --- a/drivers/gpio/gpio-brcmstb.c
>> +++ b/drivers/gpio/gpio-brcmstb.c
> ...
>> @@ -63,6 +69,231 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
> ...
>> +static void brcmstb_gpio_irq_bank_handler(int irq,
>> + struct brcmstb_gpio_bank *bank)
>> +{
>> + struct brcmstb_gpio_priv *priv = bank->parent_priv;
>> + void __iomem *reg_base = priv->reg_base;
>> + unsigned long status;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&bank->bgc.lock, flags);
>> + while ((status = bank->bgc.read_reg(reg_base + GIO_STAT(bank->id)) &
>> + bank->bgc.read_reg(reg_base + GIO_MASK(bank->id {
>
> In case you do run this loop multiple times (multiple interrupts in
> progress?), wouldn't it make sense to stash the mask exactly once,
> outside the loop? It's probably not a real big deal in practice, I
> guess.
I made this change after Linus's remark at
https://lkml.org/lkml/2015/5/12/303 on v1, which I agree with mostly
since it's a premature optimization---I haven't determined whether
keeping a shadow mask actually helps performance at all in practice,
and better to keep it simpler without actual data.
>
>> + int bit;
>> + for_each_set_bit(bit, &status, 32) {
>> + int hwirq = bank->bgc.gc.base -
>> + priv->gpio_base + bit;
>> + int child_irq =
>> + irq_find_mapping(priv->irq_domain,
>> + hwirq);
>> + u32 stat = bank->bgc.read_reg(reg_base +
>> + GIO_STAT(bank->id));
>> + if (bit >= bank->width)
>> + dev_warn(&priv->pdev->dev,
>> + "IRQ for invalid GPIO (bank=%d,
>> offset=%d)\n",
>> + bank->id, bit);
>> + bank->bgc.write_reg(reg_base + GIO_STAT(bank->id),
>> + stat | BIT(bit));
>> + generic_handle_irq(child_irq);
>> + }
>> + }
>> + spin_unlock_irqrestore(&bank->bgc.lock, flags);
>> +}
> ...
>> @@ -153,6 +410,16 @@ static int brcmstb_gpio_probe(struct platform_device
>> *pdev)
>> priv->reg_base = reg_base;
>> priv->pdev = pdev;
>>
>> + if (of_find_property(np, "interrupt-controller", NULL)) {
>
> of_property_read_bool()?
OK.
>
>> + priv->parent_irq = platform_get_irq(pdev, 0);
>> + if (priv->parent_irq < 0) {
>> + dev_err(dev, "Couldn't get IRQ");
>> + return -ENOENT;
>> + }
>> + } else {
>> + priv->parent_irq = -ENOENT;
>> + }
>> +
>> INIT_LIST_HEAD(&priv->bank_list);
>> if (brcmstb_gpio_sanity_check_banks(dev, np, res))
>> return -EINVAL;
>
> Otherwise, looks OK to my inexpert eyes.
>
> Reviewed-by: Brian Norris
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] dt-bindings: brcmstb-gpio: document properties for wakeup
On Fri, May 29, 2015 at 5:36 PM, Brian Norris
wrote:
> On Thu, May 28, 2015 at 07:14:07PM -0700, Gregory Fong wrote:
>> Some brcmstb GPIO controllers can be used to wake from suspend, so use the
>> de facto standard property 'wakeup-source' to mark the nodes of controllers
>> with that capability.
>>
>> Also document interrupts-extended, which will be used for wakeup handling
>> because the interrupt parent for the wake IRQ is different from the regular
>> IRQ.
>>
>> Signed-off-by: Gregory Fong
>> ---
>> New in v2.
>>
>> .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 26
>> +-
>> 1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
>> b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
>> index 435f1bc..568814f 100644
>> --- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
>> +++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
>> @@ -33,6 +33,12 @@ Optional properties:
>> - interrupt-parent:
>> phandle of the parent interrupt controller
>>
>> +- interrupts-extended:
>> +Alternate form of specifying interrupts and parents that allows for
>> +multiple parents. This takes precedence over 'interrupts' and
>> +'interrupt-parent'. This probably must be used if the wakeup-source
>> +property is provided because that may have a different interrupt parent.
>> +
>
> "This probably must be used" seems a little awkward, especially when
> you're just explaining an implementation detail of our SoCs, rather than
> something unique about this binding. Maybe:
>
> "Wakeup-capable GPIO controllers often route their wakeup interrupt
> lines through a different interrupt controller than the primary
> interrupt line, making this property necessary."
That wording does seem better, will change.
>
>> - #interrupt-cells:
>> Should be <2>. The first cell is the GPIO number, the second should
>> specify
>> flags. The following subset of flags is supported:
>> @@ -48,7 +54,10 @@ Optional properties:
>> Marks the device node as an interrupt controller
>>
>> - interrupt-names:
>> -The name of the IRQ resource used by this controller
>> +The names of the IRQ resources used by this controller
>
> If you're specifying names, you should list them here.
I was wondering about that. Some bindings have them listed, some
don't. In this case I know what names currently exist but there could
certainly be different ones in the future. How does that work? Or am
I misunderstanding what this field is used for? Where are the
documented rules for this?
>
>> +
>> +- wakeup-source:
>> +GPIOs for this controller can be used as a wakeup source
>>
>> Example:
>> upg_gio: gpio@f040a700 {
>> @@ -63,3 +72,18 @@ Example:
>> interrupt-names = "upg_gio";
>> brcm,gpio-bank-widths = <0x20 0x20 0x20 0x18>;
>> };
>> +
>> + upg_gio_aon: gpio@f04172c0 {
>> + #gpio-cells = <0x2>;
>> + #interrupt-cells = <0x2>;
>
> Might use decimal instead of hex for the above 2 lines?
Sure.
>
>> + compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
>> + gpio-controller;
>> + interrupt-controller;
>> + reg = <0xf04172c0 0x40>;
>> + interrupt-parent = <0xc>;
>
> That should be a phandle, not an int (I realize phandles resolve down to
> an integer, but we're speaking DTS, not DTB).
OK.
>
>> + interrupts = <0x6>;
>> + interrupts-extended = <0xc 0x6 0xa 0x5>;
>
> Same here (phandles).
>
> Also, even though the interrupt binding semantics specify precedence
> between interrupts and interrupts-extended, I'd think an example should
> stick to one or the other, no?
This is the output that we actually get from the bootloader. But
regardless, IMO the example should have both cases: precedence is
well-defined, both sets of information are valid, and the driver can
handle the case where interrupts-extended is not an understood
property.
>
>> + interrupt-names = "upg_gio_aon", "upg_gio_aon_wakeup";
>> + wakeup-source;
>> + brcm,gpio-bank-widths = <0x12 0x4>;
>> + };
>
> Reviewed-by: Brian Norris
Thanks for the review,
Gregory
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/6] gpio: Add GPIO support for Broadcom STB SoCs
This adds support for the GPIO IP "UPG GIO" used on Broadcom STB SoCs
(BCM7XXX and some others). Uses basic_mmio_gpio to instantiate a
gpio_chip for each bank. The driver assumes that it handles the base
set of GPIOs on the system and that it can start its numbering sequence
from 0, so any GPIO expanders used with it must dynamically assign GPIO
numbers after this driver has finished registering its GPIOs.
Does not implement the interrupt-controller portion yet, will be done in a
future commit.
List-usage-fixed-by: Brian Norris
Signed-off-by: Gregory Fong
---
v2:
- change include to use instead of
- get rid of unnecessary imask member in struct bank
- rename GPIO_PER_BANK -> MAX_GPIO_PER_BANK
- always have 32 GPIOs per bank and add 'width' member in struct bank to hold
actual number of GPIOs in use
- mark of_match table as const
MAINTAINERS | 7 ++
drivers/gpio/Kconfig| 8 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-brcmstb.c | 252
4 files changed, 268 insertions(+)
create mode 100644 drivers/gpio/gpio-brcmstb.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 198a429..1d1b6f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2290,6 +2290,13 @@ N: bcm9583*
N: bcm583*
N: bcm113*
+BROADCOM BRCMSTB GPIO DRIVER
+M: Gregory Fong
+L: bcm-kernel-feedback-l...@broadcom.com>
+S: Supported
+F: drivers/gpio/gpio-brcmstb.c
+F: Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
+
BROADCOM KONA GPIO DRIVER
M: Ray Jui
L: bcm-kernel-feedback-l...@broadcom.com
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5faaf5f..d86de6a 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -126,6 +126,14 @@ config GPIO_BCM_KONA
help
Turn on GPIO support for Broadcom "Kona" chips.
+config GPIO_BRCMSTB
+ tristate "BRCMSTB GPIO support"
+ default y if ARCH_BRCMSTB
+ depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
+ select GPIO_GENERIC
+ help
+ Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
+
config GPIO_CLPS711X
tristate "CLPS711X GPIO support"
depends on ARCH_CLPS711X || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 1b55fda..893bbff 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_GPIO_ALTERA) += gpio-altera.o
obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o
obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o
obj-$(CONFIG_GPIO_BCM_KONA)+= gpio-bcm-kona.o
+obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o
obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
new file mode 100644
index 000..7a3cb1f
--- /dev/null
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -0,0 +1,252 @@
+/*
+ * Copyright (C) 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 as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define GIO_BANK_SIZE 0x20
+#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
+#define GIO_DATA(bank) (((bank) * GIO_BANK_SIZE) + 0x04)
+#define GIO_IODIR(bank) (((bank) * GIO_BANK_SIZE) + 0x08)
+#define GIO_EC(bank)(((bank) * GIO_BANK_SIZE) + 0x0c)
+#define GIO_EI(bank)(((bank) * GIO_BANK_SIZE) + 0x10)
+#define GIO_MASK(bank) (((bank) * GIO_BANK_SIZE) + 0x14)
+#define GIO_LEVEL(bank) (((bank) * GIO_BANK_SIZE) + 0x18)
+#define GIO_STAT(bank) (((bank) * GIO_BANK_SIZE) + 0x1c)
+
+struct brcmstb_gpio_bank {
+ struct list_head node;
+ int id;
+ struct bgpio_chip bgc;
+ struct brcmstb_gpio_priv *parent_priv;
+ u32 width;
+};
+
+struct brcmstb_gpio_priv {
+ struct list_head bank_list;
+ void __iomem *reg_base;
+ int num_banks;
+ struct platform_device *pdev;
+ int gpio_base;
+};
+
+#define MAX_GPIO_PER_BANK 32
+#define GPIO_BANK(gpio) ((gpio) >> 5)
+/* assumes MAX_GPIO_PER_BANK is a multiple of 2 */
+#define GPIO_BIT(gpio) ((gpio) & (MAX_GPIO_PER_BANK - 1))
+
+static inline struct brcmstb_gpio_bank *
+brcmstb_gpio_gc_to_bank(struct gpio_chip *gc)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ return container_
[PATCH v2 2/6] gpio: brcmstb: Add interrupt support
Create an irq_chip for each GIO block. Uses chained IRQ handling since
known uses of this block have a BCM7120 L2 interrupt controller as a
parent. Supports interrupts for all GPIOs.
In the IRQ handler, we check for raised IRQs for invalid GPIOs and warn
(ratelimited) if they're encountered.
Signed-off-by: Gregory Fong
---
v2:
- since imask member of bank struct was removed, just read and write from mask
reg and don't maintain a shadow
- warn on invalid IRQs
- move some irq setup to a separate function since probe is getting unwieldy
drivers/gpio/Kconfig| 1 +
drivers/gpio/gpio-brcmstb.c | 276
2 files changed, 277 insertions(+)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d86de6a..7249dba 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -131,6 +131,7 @@ config GPIO_BRCMSTB
default y if ARCH_BRCMSTB
depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
select GPIO_GENERIC
+ select IRQ_DOMAIN
help
Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 7a3cb1f..b9962ff 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -17,6 +17,8 @@
#include
#include
#include
+#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -40,7 +42,11 @@ struct brcmstb_gpio_priv {
struct list_head bank_list;
void __iomem *reg_base;
int num_banks;
+ int num_gpios;
struct platform_device *pdev;
+ struct irq_chip irq_chip;
+ struct irq_domain *irq_domain;
+ int parent_irq;
int gpio_base;
};
@@ -63,6 +69,231 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
return bank->parent_priv;
}
+static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
+ unsigned int offset, bool enable)
+{
+ struct bgpio_chip *bgc = &bank->bgc;
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = bgc->pin2mask(bgc, offset);
+ u32 imask;
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ imask = bgc->read_reg(priv->reg_base + GIO_MASK(bank->id));
+ if (enable)
+ imask |= mask;
+ else
+ imask &= ~mask;
+ bgc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int brcmstb_gpio_to_irq(struct gpio_chip *gc, unsigned gc_offset)
+{
+ struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+ /* gc_offset is relative to this gpio_chip; want real offset */
+ int offset = gc_offset + (gc->base - priv->gpio_base);
+
+ if (offset >= priv->num_gpios)
+ return -ENXIO;
+ return irq_create_mapping(priv->irq_domain, offset);
+}
+
+/* IRQ chip functions */
+
+static int brcmstb_gpio_hwirq_to_offset(irq_hw_number_t hwirq,
+ struct brcmstb_gpio_bank *bank)
+{
+ return hwirq - (bank->bgc.gc.base - bank->parent_priv->gpio_base);
+}
+
+static void brcmstb_gpio_irq_mask(struct irq_data *d)
+{
+ struct brcmstb_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ int offset = brcmstb_gpio_hwirq_to_offset(d->hwirq, bank);
+
+ brcmstb_gpio_set_imask(bank, offset, false);
+}
+
+static void brcmstb_gpio_irq_unmask(struct irq_data *d)
+{
+ struct brcmstb_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ int offset = brcmstb_gpio_hwirq_to_offset(d->hwirq, bank);
+
+ brcmstb_gpio_set_imask(bank, offset, true);
+}
+
+static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct brcmstb_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+ u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(d->hwirq, bank));
+ u32 edge_insensitive, iedge_insensitive;
+ u32 edge_config, iedge_config;
+ u32 level, ilevel;
+ unsigned long flags;
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_LOW:
+ level = 0;
+ edge_config = 0;
+ edge_insensitive = 0;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ level = mask;
+ edge_config = 0;
+ edge_insensitive = 0;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ level = 0;
+ edge_config = 0;
+ edge_insensitive = 0;
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ level = 0;
+ edge_config = mask;
+ edge_insensitive = 0;
+ break;
+ case IRQ_TYPE_E
[PATCH v2 4/6] gpio: brcmstb: Allow GPIOs to be wakeup sources
Several drivers (e.g. gpio-keys) allow for GPIOs to be configured as
wakeup sources, and this GPIO controller supports that through a
separate interrupt path.
The de-facto standard DT property "wakeup-source" is checked, since
that indicates whether the GPIO controller hardware can wake. Uses
the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
any of its own wakeup source configuration.
Signed-off-by: Gregory Fong
---
New in v2.
drivers/gpio/gpio-brcmstb.c | 53 +
1 file changed, 53 insertions(+)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index b9962ff..2598c1e 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#define GIO_BANK_SIZE 0x20
#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -48,6 +49,7 @@ struct brcmstb_gpio_priv {
struct irq_domain *irq_domain;
int parent_irq;
int gpio_base;
+ int parent_wake_irq;
};
#define MAX_GPIO_PER_BANK 32
@@ -183,6 +185,31 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d,
unsigned int type)
return 0;
}
+static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+{
+ struct brcmstb_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct brcmstb_gpio_priv *priv = bank->parent_priv;
+
+ /*
+* Only enable wake IRQ once for however many hwirqs can wake
+* since they all use the same wake IRQ. Mask will be set
+* up appropriately thanks to IRQCHIP_MASK_ON_SUSPEND flag.
+*/
+ if (enable)
+ enable_irq_wake(priv->parent_wake_irq);
+ else
+ disable_irq_wake(priv->parent_wake_irq);
+ return 0;
+}
+
+static irqreturn_t brcmstb_gpio_wake_irq_handler(int irq, void *data)
+{
+ struct brcmstb_gpio_priv *priv = data;
+
+ pm_wakeup_event(&priv->pdev->dev, 0);
+ return IRQ_HANDLED;
+}
+
static void brcmstb_gpio_irq_bank_handler(int irq,
struct brcmstb_gpio_bank *bank)
{
@@ -369,6 +396,32 @@ static int brcmstb_gpio_irq_setup(struct platform_device
*pdev,
priv->irq_chip.irq_mask = brcmstb_gpio_irq_mask;
priv->irq_chip.irq_unmask = brcmstb_gpio_irq_unmask;
priv->irq_chip.irq_set_type = brcmstb_gpio_irq_set_type;
+
+ /* Ensures that all non-wakeup IRQs are disabled at suspend */
+ priv->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND;
+
+ if (IS_ENABLED(CONFIG_PM_SLEEP) &&
+ of_get_property(np, "wakeup-source", NULL)) {
+ priv->parent_wake_irq = platform_get_irq(pdev, 1);
+ if (priv->parent_wake_irq < 0) {
+ dev_warn(dev,
+ "Couldn't get wake IRQ - GPIOs will not be able
to wake from sleep");
+ } else {
+ int err = devm_request_irq(dev, priv->parent_wake_irq,
+ brcmstb_gpio_wake_irq_handler, 0,
+ "brcmstb-gpio-wake", priv);
+
+ if (err < 0) {
+ dev_err(dev, "Couldn't request wake IRQ");
+ return err;
+ }
+
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+ priv->irq_chip.irq_set_wake = brcmstb_gpio_irq_set_wake;
+ }
+ }
+
priv->irq_domain =
irq_domain_add_linear(np, priv->num_gpios,
&brcmstb_gpio_irq_domain_ops,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/6] ARM: brcmstb: Add default gpio number
Out of the brcmstb SoCs that I know, BCM3390 has the largest numbers of GPIOs, with its - 320 "peripheral" GPIOs - 5*32 = 160 UPG GPIOs (counting unused lines, which do get counted) - 2*32 = 64 UPG AON GPIOs (counting unused lines) Total: 544 I suspect that the upper limit will only need to be higher in the future, so set it to 1024. Signed-off-by: Gregory Fong --- New in v2. arch/arm/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e717642..bbe6bf7 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1509,7 +1509,8 @@ config ARM_PSCI # selected platforms. config ARCH_NR_GPIO int - default 1024 if ARCH_SHMOBILE || ARCH_TEGRA || ARCH_ZYNQ + default 1024 if ARCH_BRCMSTB || ARCH_SHMOBILE || ARCH_TEGRA || \ + ARCH_ZYNQ default 512 if ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \ SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 default 416 if ARCH_SUNXI -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/6] dt-bindings: brcmstb-gpio: document properties for wakeup
Some brcmstb GPIO controllers can be used to wake from suspend, so use the
de facto standard property 'wakeup-source' to mark the nodes of controllers
with that capability.
Also document interrupts-extended, which will be used for wakeup handling
because the interrupt parent for the wake IRQ is different from the regular
IRQ.
Signed-off-by: Gregory Fong
---
New in v2.
.../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
index 435f1bc..568814f 100644
--- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
@@ -33,6 +33,12 @@ Optional properties:
- interrupt-parent:
phandle of the parent interrupt controller
+- interrupts-extended:
+Alternate form of specifying interrupts and parents that allows for
+multiple parents. This takes precedence over 'interrupts' and
+'interrupt-parent'. This probably must be used if the wakeup-source
+property is provided because that may have a different interrupt parent.
+
- #interrupt-cells:
Should be <2>. The first cell is the GPIO number, the second should
specify
flags. The following subset of flags is supported:
@@ -48,7 +54,10 @@ Optional properties:
Marks the device node as an interrupt controller
- interrupt-names:
-The name of the IRQ resource used by this controller
+The names of the IRQ resources used by this controller
+
+- wakeup-source:
+GPIOs for this controller can be used as a wakeup source
Example:
upg_gio: gpio@f040a700 {
@@ -63,3 +72,18 @@ Example:
interrupt-names = "upg_gio";
brcm,gpio-bank-widths = <0x20 0x20 0x20 0x18>;
};
+
+ upg_gio_aon: gpio@f04172c0 {
+ #gpio-cells = <0x2>;
+ #interrupt-cells = <0x2>;
+ compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+ gpio-controller;
+ interrupt-controller;
+ reg = <0xf04172c0 0x40>;
+ interrupt-parent = <0xc>;
+ interrupts = <0x6>;
+ interrupts-extended = <0xc 0x6 0xa 0x5>;
+ interrupt-names = "upg_gio_aon", "upg_gio_aon_wakeup";
+ wakeup-source;
+ brcm,gpio-bank-widths = <0x12 0x4>;
+ };
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/6] ARM: brcmstb: Select ARCH_WANT_OPTIONAL_GPIOLIB
Select ARCH_WANT_OPTIONAL_GPIOLIB from BRCMSTB to allow GPIOLIB and GPIO_BRCMSTB to be enabled. Signed-off-by: Gregory Fong --- v2: this was split out so that it can go through the ARM SoC tree. arch/arm/mach-bcm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig index 8b11f44..0ac9e4b3 100644 --- a/arch/arm/mach-bcm/Kconfig +++ b/arch/arm/mach-bcm/Kconfig @@ -144,6 +144,7 @@ config ARCH_BRCMSTB select BRCMSTB_GISB_ARB select BRCMSTB_L2_IRQ select BCM7120_L2_IRQ + select ARCH_WANT_OPTIONAL_GPIOLIB help Say Y if you intend to run the kernel on a Broadcom ARM-based STB chipset. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] GPIO support for BRCMSTB
This patchset adds support for the GPIO controller (UPG GIO) used on Broadcom's various BRCMSTB SoCs (BCM7XXX and others). It uses the "basic-mmio-gpio" interface to try to reduce duplication of the base logic. For all existing hardware, this block hooked up to the BCM7120 L2 IRQ controller and so will require CONFIG_BCM7120_L2_IRQ=y. New in v2: - fix license mismatch as pointed out by Paul Bolle - move select ARCH_WANT_OPTIONAL_GPIOLIB to separate patch - change to have 32 lines per bank per Linus Walleij's comments - allow this controller to be used as a wakeup source - add default GPIO number for BRCMSTB The device tree bindings from v1 were merged to the GPIO tree, so this patchset only contains an addition to allow GPIOs to be used as a wakeup source (patch 3). The initial bindings from v1 can be found at https://lkml.org/lkml/2015/5/6/200 . Gregory Fong (6): gpio: Add GPIO support for Broadcom STB SoCs gpio: brcmstb: Add interrupt support dt-bindings: brcmstb-gpio: document properties for wakeup gpio: brcmstb: Allow GPIOs to be wakeup sources ARM: brcmstb: Select ARCH_WANT_OPTIONAL_GPIOLIB ARM: brcmstb: Add default gpio number .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 26 +- MAINTAINERS| 7 + arch/arm/Kconfig | 3 +- arch/arm/mach-bcm/Kconfig | 1 + drivers/gpio/Kconfig | 9 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-brcmstb.c| 581 + 7 files changed, 626 insertions(+), 2 deletions(-) create mode 100644 drivers/gpio/gpio-brcmstb.c -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] GPIO support for BRCMSTB
Hi Linus, On Wed, May 13, 2015 at 1:59 AM, Linus Walleij wrote: > On Tue, May 12, 2015 at 9:38 PM, Gregory Fong wrote: >> On Tue, May 12, 2015 at 3:59 AM, Linus Walleij >> wrote: >>> On Wed, May 6, 2015 at 10:37 AM, Gregory Fong >>> wrote: >>> >>>> There is only one IRQ for each GIO IP block (i.e. several register banks >>>> share >>>> an IRQ). After briefly looking into the generic IRQ chip implementation, >>>> it >>>> seemed like in this case that using it would result in the driver being >>>> more >>>> complex than necessary because AFAICT it expects a 1:1 mapping of >>>> irq_chip_generic to gpio_chip. It seemed like less of a pain to have a >>>> single >>>> irq_chip since we have a single IRQ for all register banks (multiple >>>> gpio_chips). I might be missing something, maybe using a shared IRQ across >>>> multiple irq_chips is easier than I think? Suggestions welcome. >>> >>> What is needed is a 1:1 mapping between GPIO offsets and IRQ >>> offsets. >>> >>> If you just number your GPIOs 0...n and your IRQs 0...n >>> it should work just fine with one irqchip for all banks. >>> >>> What screws things up is likely that the hardware supports >>> 32 lines per bank and not all are used. >>> >>> I suggest you enable 32 line and 32 IRQs per bank, >>> so that hwirq maps nicely 1:1 on the GPIO offsets, >>> then just use the width thing to NACK operations on >>> GPIO lines you are not using. This way you can also >>> decode and warn on spurious IRQs on the unused lines. >> >> For having 32 lines per bank, the big problem here is the upper limit >> of 256 GPIOs. > > Which arch is this? > Usually this limit comes from > arch/*/include/asm/gpio.h > > For ARM that was bumped to 512 a while back. It is also possible > to define a custom value for your system by defining > ARCH_NR_GPIOS > >> Anyway, I don't think I understand IRQ domains and irq_chip_generic >> very well. One possibility _might_ be to use multiple irq_chips. > > That is probably not possible if there is just one IRQ for all > banks. > > The task of the irqdomain is a 1-to-1 translation from one > hardware numberspace to the Linux IRQ number space. > > In your case the hardware IRQ (hwirq) numberspace > should be: > > bank0: 0..31 > bank1: 32..63 > > bankn: 32*n..32*n+31 > > I think the gpiolib irqchip code can translate that properly > as it is just a simple 0...x mapping, the irq handler need > some magic to loop over all banks from 0..n though. I've now actually attempted to use the gpiolib irqchip code. This driver can't directly use gpiochip_irqchip_add() because of the multiple gpiochip : one irqchip map. At first, I thought it might be possible to simply add a new argument (or break things into a helper function) to allow setting the associated IRQ domain, but then I can't use the generic map and unmap functions which expect the irq_domain host_data member to be struct gpiochip *, which makes no sense in this case. That puts me right back to implementing a special version of the map and unmap function. Since there doesn't appear to be any benefit to using the gpiolib irqchip code for this case, I'm going to stick with my implementation from patch 3 of this patchset. I've also added to it to allow for using the GPIOs as a wakeup source, and will submit that as well with V2. Thanks, Gregory -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] gpio: Add GPIO support for Broadcom STB SoCs
On Thu, May 07, 2015 at 10:18:49AM +0200, Paul Bolle wrote:
> Just a nit: a license mismatch.
>
> On Wed, 2015-05-06 at 01:37 -0700, Gregory Fong wrote:
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-brcmstb.c
>
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
>
> This states the license is GPL v2.
>
> > +MODULE_LICENSE("GPL");
>
> And, according to include/linux/module.h, this states the license is GPL
> v2 or later. So I think either the comment at the top of this file or
> the ident used in the MODULE_LICENSE() macro needs to change.
Will fix that, thanks.
Gregory
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: brcmstb: un-hexify clock frequency
On Wed, Mar 18, 2015 at 5:31 PM, Brian Norris wrote: > This value makes much more sense in decimal. > > Signed-off-by: Brian Norris Acked-by: Gregory Fong -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 5/9] ARM: Enable erratum 798181 for Broadcom Brahma-B15
Hi Will, On Wed, Feb 26, 2014 at 2:47 AM, Will Deacon wrote: > On Wed, Feb 26, 2014 at 10:29:08AM +, Marc Carino wrote: >> From: Gregory Fong >> >> Broadcom Brahma-B15 (r0p0..r0p2) is also affected by Cortex-A15 >> erratum 798181, so enable the workaround for Brahma-B15. > > Really... *exactly* the same erratum? That sounds pretty unlikely, so I'd > really like to be sure that the workaround we have indeed solves your > problem (issuing a dummy TLBI to 0x0 + dsb, then followed by a dmb + clrex > on each core). The point is that the workaround doesn't simply perform > non-shareable invalidation on each core using IPIs. Yes, it should be the same. I've sent you more detail off-list. Regards, Gregory -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
