Hi Linus,
This patch series updates the bcm2835 pinctrl driver to support
the BCM7211 SoC which is quite similar to 2711 (Raspberry Pi 4)
except that it also supports wake-up interrupts.
Thanks!
Changes in v2:
- fixed patch #3 to reference the correct data structure (Stefan)
- fixed patch #4 to use conditional initialization and fetching of
interrupt resources to limit the memory overhead for non-7211 chips
Florian Fainelli (4):
dt-bindings: pinctrl: Document 7211 compatible for
brcm,bcm2835-gpio.txt
dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
pinctrl: bcm2835: Match BCM7211 compatible string
pinctrl: bcm2835: Add support for wake-up interrupts
.../bindings/pinctrl/brcm,bcm2835-gpio.txt | 5 +-
drivers/pinctrl/bcm/pinctrl-bcm2835.c | 80 ++++++++++++++++++-
2 files changed, 83 insertions(+), 2 deletions(-)
--
2.17.1
The BCM7211 SoC uses the same pinconf_ops as the ones defined for the
BCM2711 SoC, match the compatible string and use the correct set of
options.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/pinctrl/bcm/pinctrl-bcm2835.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 06bd2b70af3c..1b00d93aa66e 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1137,6 +1137,10 @@ static const struct of_device_id bcm2835_pinctrl_match[] = {
.compatible = "brcm,bcm2711-gpio",
.data = &bcm2711_plat_data,
},
+ {
+ .compatible = "brcm,bcm7211-gpio",
+ .data = &bcm2711_plat_data,
+ },
{}
};
--
2.17.1
BCM7211 supports wake-up interrupts in the form of optional interrupt
lines, one per bank, plus the "all banks" interrupt line.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index dfc67b90591c..5682b2010e50 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -16,7 +16,9 @@ Required properties:
second cell is used to specify optional parameters:
- bit 0 specifies polarity (0 for normal, 1 for inverted)
- interrupts : The interrupt outputs from the controller. One interrupt per
- individual bank followed by the "all banks" interrupt.
+ individual bank followed by the "all banks" interrupt. For BCM7211, an
+ additional set of per-bank interrupt line and an "all banks" wake-up
+ interrupt may be specified.
- interrupt-controller: Marks the device node as an interrupt controller.
- #interrupt-cells : Should be 2.
The first cell is the GPIO number.
--
2.17.1
Document the brcm,bcm7211-gpio compatible string in the
brcm,bcm2835-gpio.txt document.
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index 3cab7336a326..dfc67b90591c 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -9,6 +9,7 @@ Required properties:
"brcm,bcm2835-gpio" - BCM2835 compatible pinctrl
"brcm,bcm7211-gpio" - BCM7211 compatible pinctrl
"brcm,bcm2711-gpio" - BCM2711 compatible pinctrl
+ "brcm,bcm7211-gpio" - BCM7211 compatible pinctrl
- reg: Should contain the physical address of the GPIO module's registers.
- gpio-controller: Marks the device node as a GPIO controller.
- #gpio-cells : Should be two. The first cell is the pin number and the
--
2.17.1
Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
specifically treat the GPIO interrupts during suspend and resume, and
simply implement an irq_set_wake() callback that is responsible for
enabling the parent wake-up interrupt as a wake-up interrupt.
To avoid allocating unnecessary resources for other chips, the wake-up
interrupts are only initialized if we have a brcm,bcm7211-gpio
compatibility string.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
1 file changed, 75 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 1b00d93aa66e..1fbf067a3eed 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -19,6 +19,7 @@
#include <linux/irq.h>
#include <linux/irqdesc.h>
#include <linux/init.h>
+#include <linux/interrupt.h>
#include <linux/of_address.h>
#include <linux/of.h>
#include <linux/of_irq.h>
@@ -76,6 +77,7 @@
struct bcm2835_pinctrl {
struct device *dev;
void __iomem *base;
+ int *wake_irq;
/* note: locking assumes each bank will have its own unsigned long */
unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
@@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
chained_irq_exit(host_chip, desc);
}
+static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
+{
+ return IRQ_HANDLED;
+}
+
static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
unsigned reg, unsigned offset, bool enable)
{
@@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
}
+static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+ struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
+ unsigned gpio = irqd_to_hwirq(data);
+ unsigned int irqgroup;
+ int ret = -EINVAL;
+
+ if (!pc->wake_irq)
+ return ret;
+
+ if (gpio <= 27)
+ irqgroup = 0;
+ else if (gpio >= 28 && gpio <= 45)
+ irqgroup = 1;
+ else if (gpio >= 46 && gpio <= 53)
+ irqgroup = 2;
+ else
+ return ret;
+
+ if (on)
+ ret = enable_irq_wake(pc->wake_irq[irqgroup]);
+ else
+ ret = disable_irq_wake(pc->wake_irq[irqgroup]);
+
+ return ret;
+}
+
static struct irq_chip bcm2835_gpio_irq_chip = {
.name = MODULE_NAME,
.irq_enable = bcm2835_gpio_irq_enable,
@@ -642,6 +677,8 @@ static struct irq_chip bcm2835_gpio_irq_chip = {
.irq_ack = bcm2835_gpio_irq_ack,
.irq_mask = bcm2835_gpio_irq_disable,
.irq_unmask = bcm2835_gpio_irq_enable,
+ .irq_set_wake = bcm2835_gpio_irq_set_wake,
+ .flags = IRQCHIP_MASK_ON_SUSPEND,
};
static int bcm2835_pctl_get_groups_count(struct pinctrl_dev *pctldev)
@@ -1154,6 +1191,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
struct resource iomem;
int err, i;
const struct of_device_id *match;
+ int is_7211 = 0;
BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2711_NUM_GPIOS);
BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_groups) != BCM2711_NUM_GPIOS);
@@ -1180,6 +1218,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
return -EINVAL;
pdata = match->data;
+ is_7211 = of_device_is_compatible(np, "brcm,bcm7211-gpio");
pc->gpio_chip = *pdata->gpio_chip;
pc->gpio_chip.parent = dev;
@@ -1214,6 +1253,15 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!girq->parents)
return -ENOMEM;
+
+ if (is_7211) {
+ pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
+ sizeof(*pc->wake_irq),
+ GFP_KERNEL);
+ if (!pc->wake_irq)
+ return -ENOMEM;
+ }
+
/*
* Use the same handler for all groups: this is necessary
* since we use one gpiochip to cover all lines - the
@@ -1221,8 +1269,34 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
* bank that was firing the IRQ and look up the per-group
* and bank data.
*/
- for (i = 0; i < BCM2835_NUM_IRQS; i++)
+ for (i = 0; i < BCM2835_NUM_IRQS; i++) {
+ int len;
+ char *name;
+
girq->parents[i] = irq_of_parse_and_map(np, i);
+ if (!is_7211)
+ continue;
+
+ /* Skip over the all banks interrupts */
+ pc->wake_irq[i] = irq_of_parse_and_map(np, i +
+ BCM2835_NUM_IRQS + 1);
+
+ len = strlen(dev_name(pc->dev)) + 16;
+ name = devm_kzalloc(pc->dev, len, GFP_KERNEL);
+ if (!name)
+ return -ENOMEM;
+
+ snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);
+
+ /* These are optional interrupts */
+ err = devm_request_irq(dev, pc->wake_irq[i],
+ bcm2835_gpio_wake_irq_handler,
+ IRQF_SHARED, name, pc);
+ if (err)
+ dev_warn(dev, "unable to request wake IRQ %d\n",
+ pc->wake_irq[i]);
+ }
+
girq->default_type = IRQ_TYPE_NONE;
girq->handler = handle_level_irq;
--
2.17.1
On Fri, 29 May 2020 12:15:19 -0700, Florian Fainelli wrote:
> Document the brcm,bcm7211-gpio compatible string in the
> brcm,bcm2835-gpio.txt document.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <[email protected]>
On Fri, 29 May 2020 12:15:20 -0700, Florian Fainelli wrote:
> BCM7211 supports wake-up interrupts in the form of optional interrupt
> lines, one per bank, plus the "all banks" interrupt line.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
Acked-by: Rob Herring <[email protected]>
Hi Florian,
Am 29.05.20 um 21:15 schrieb Florian Fainelli:
> Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
> specifically treat the GPIO interrupts during suspend and resume, and
> simply implement an irq_set_wake() callback that is responsible for
> enabling the parent wake-up interrupt as a wake-up interrupt.
>
> To avoid allocating unnecessary resources for other chips, the wake-up
> interrupts are only initialized if we have a brcm,bcm7211-gpio
> compatibility string.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
> 1 file changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 1b00d93aa66e..1fbf067a3eed 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -19,6 +19,7 @@
> #include <linux/irq.h>
> #include <linux/irqdesc.h>
> #include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/of_address.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> @@ -76,6 +77,7 @@
> struct bcm2835_pinctrl {
> struct device *dev;
> void __iomem *base;
> + int *wake_irq;
>
> /* note: locking assumes each bank will have its own unsigned long */
> unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
> @@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
> chained_irq_exit(host_chip, desc);
> }
>
> +static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
> +{
> + return IRQ_HANDLED;
> +}
> +
> static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
> unsigned reg, unsigned offset, bool enable)
> {
> @@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
> bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
> }
>
> +static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
> + unsigned gpio = irqd_to_hwirq(data);
> + unsigned int irqgroup;
> + int ret = -EINVAL;
> +
> + if (!pc->wake_irq)
> + return ret;
> +
> + if (gpio <= 27)
> + irqgroup = 0;
> + else if (gpio >= 28 && gpio <= 45)
> + irqgroup = 1;
> + else if (gpio >= 46 && gpio <= 53)
> + irqgroup = 2;
in case the BCM7211 has 58 GPIOs, but the wake up interrupts are only
available for the first 54 this should deserve a comment.
> + else
> + return ret;
> +
> + if (on)
> + ret = enable_irq_wake(pc->wake_irq[irqgroup]);
> + else
> + ret = disable_irq_wake(pc->wake_irq[irqgroup]);
> +
> + return ret;
> +}
> +
> static struct irq_chip bcm2835_gpio_irq_chip = {
> .name = MODULE_NAME,
> .irq_enable = bcm2835_gpio_irq_enable,
> @@ -642,6 +677,8 @@ static struct irq_chip bcm2835_gpio_irq_chip = {
> .irq_ack = bcm2835_gpio_irq_ack,
> .irq_mask = bcm2835_gpio_irq_disable,
> .irq_unmask = bcm2835_gpio_irq_enable,
> + .irq_set_wake = bcm2835_gpio_irq_set_wake,
> + .flags = IRQCHIP_MASK_ON_SUSPEND,
> };
>
> static int bcm2835_pctl_get_groups_count(struct pinctrl_dev *pctldev)
> @@ -1154,6 +1191,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
> struct resource iomem;
> int err, i;
> const struct of_device_id *match;
> + int is_7211 = 0;
>
> BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2711_NUM_GPIOS);
> BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_groups) != BCM2711_NUM_GPIOS);
> @@ -1180,6 +1218,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
> return -EINVAL;
>
> pdata = match->data;
> + is_7211 = of_device_is_compatible(np, "brcm,bcm7211-gpio");
>
> pc->gpio_chip = *pdata->gpio_chip;
> pc->gpio_chip.parent = dev;
> @@ -1214,6 +1253,15 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
> GFP_KERNEL);
> if (!girq->parents)
> return -ENOMEM;
> +
> + if (is_7211) {
> + pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
> + sizeof(*pc->wake_irq),
> + GFP_KERNEL);
> + if (!pc->wake_irq)
> + return -ENOMEM;
> + }
> +
> /*
> * Use the same handler for all groups: this is necessary
> * since we use one gpiochip to cover all lines - the
> @@ -1221,8 +1269,34 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
> * bank that was firing the IRQ and look up the per-group
> * and bank data.
> */
> - for (i = 0; i < BCM2835_NUM_IRQS; i++)
> + for (i = 0; i < BCM2835_NUM_IRQS; i++) {
> + int len;
> + char *name;
> +
> girq->parents[i] = irq_of_parse_and_map(np, i);
> + if (!is_7211)
> + continue;
> +
> + /* Skip over the all banks interrupts */
> + pc->wake_irq[i] = irq_of_parse_and_map(np, i +
> + BCM2835_NUM_IRQS + 1);
> +
> + len = strlen(dev_name(pc->dev)) + 16;
> + name = devm_kzalloc(pc->dev, len, GFP_KERNEL);
> + if (!name)
> + return -ENOMEM;
> +
> + snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);
> +
> + /* These are optional interrupts */
> + err = devm_request_irq(dev, pc->wake_irq[i],
> + bcm2835_gpio_wake_irq_handler,
> + IRQF_SHARED, name, pc);
> + if (err)
> + dev_warn(dev, "unable to request wake IRQ %d\n",
> + pc->wake_irq[i]);
> + }
> +
> girq->default_type = IRQ_TYPE_NONE;
> girq->handler = handle_level_irq;
>
On 5/30/2020 12:49 AM, Stefan Wahren wrote:
> Hi Florian,
>
> Am 29.05.20 um 21:15 schrieb Florian Fainelli:
>> Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
>> specifically treat the GPIO interrupts during suspend and resume, and
>> simply implement an irq_set_wake() callback that is responsible for
>> enabling the parent wake-up interrupt as a wake-up interrupt.
>>
>> To avoid allocating unnecessary resources for other chips, the wake-up
>> interrupts are only initialized if we have a brcm,bcm7211-gpio
>> compatibility string.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
>> 1 file changed, 75 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> index 1b00d93aa66e..1fbf067a3eed 100644
>> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
>> @@ -19,6 +19,7 @@
>> #include <linux/irq.h>
>> #include <linux/irqdesc.h>
>> #include <linux/init.h>
>> +#include <linux/interrupt.h>
>> #include <linux/of_address.h>
>> #include <linux/of.h>
>> #include <linux/of_irq.h>
>> @@ -76,6 +77,7 @@
>> struct bcm2835_pinctrl {
>> struct device *dev;
>> void __iomem *base;
>> + int *wake_irq;
>>
>> /* note: locking assumes each bank will have its own unsigned long */
>> unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
>> @@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
>> chained_irq_exit(host_chip, desc);
>> }
>>
>> +static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
>> +{
>> + return IRQ_HANDLED;
>> +}
>> +
>> static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
>> unsigned reg, unsigned offset, bool enable)
>> {
>> @@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
>> bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
>> }
>>
>> +static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
>> +{
>> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>> + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
>> + unsigned gpio = irqd_to_hwirq(data);
>> + unsigned int irqgroup;
>> + int ret = -EINVAL;
>> +
>> + if (!pc->wake_irq)
>> + return ret;
>> +
>> + if (gpio <= 27)
>> + irqgroup = 0;
>> + else if (gpio >= 28 && gpio <= 45)
>> + irqgroup = 1;
>> + else if (gpio >= 46 && gpio <= 53)
>> + irqgroup = 2;
> in case the BCM7211 has 58 GPIOs, but the wake up interrupts are only
> available for the first 54 this should deserve a comment.
irqgroup 2 covers GPIOs 46 through 57, thanks for noticing. Do you have
more comments before I spin a v3? Thank you for reviewing.
--
Florian
Hi Florian,
Am 30.05.20 um 23:19 schrieb Florian Fainelli:
>
> On 5/30/2020 12:49 AM, Stefan Wahren wrote:
>> Hi Florian,
>>
>> Am 29.05.20 um 21:15 schrieb Florian Fainelli:
>>> }
>>>
>>> +static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
>>> +{
>>> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>>> + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
>>> + unsigned gpio = irqd_to_hwirq(data);
>>> + unsigned int irqgroup;
>>> + int ret = -EINVAL;
>>> +
>>> + if (!pc->wake_irq)
>>> + return ret;
>>> +
>>> + if (gpio <= 27)
>>> + irqgroup = 0;
>>> + else if (gpio >= 28 && gpio <= 45)
>>> + irqgroup = 1;
>>> + else if (gpio >= 46 && gpio <= 53)
>>> + irqgroup = 2;
>> in case the BCM7211 has 58 GPIOs, but the wake up interrupts are only
>> available for the first 54 this should deserve a comment.
> irqgroup 2 covers GPIOs 46 through 57, thanks for noticing. Do you have
> more comments before I spin a v3? Thank you for reviewing.
no, i don't.
Regards
Stefan