qcom: support wakeup capable GPIOs
Subsystems (9)
v2 cover qcom: support wakeup capable GPIOs
Hi all,
This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.
The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.
Changes in v2:
- Fix bug related to unmasking PDC interrupt
- Address review comments from Doug and Stepehn
- Rebase on top of 5.0-rc2
- Fix signed-off-by tags
- Enable QCOM_PDC in defconfig
Do note that this patch uses the register address convention updated by Bjorn per [3].
Thanks,
Lina
[1]. https://lkml.org/lkml/2018/12/19/807
[2]. https://lkml.org/lkml/2018/12/19/813
[3]. https://lkml.org/lkml/2019/1/17/924
Lina Iyer (7):
irqdomain: add bus token DOMAIN_BUS_WAKEUP
drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO
drivers: pinctrl: msm: setup GPIO irqchip hierarchy
arm64: dts: qcom: add PDC interrupt controller for SDM845
arm64: dts: qcom: setup PDC as wakeup parent for GPIOs for SDM845
arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
Thierry Reding (1):
gpio: Add support for hierarchical IRQ domains
.../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 7 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 + arch/arm64/configs/defconfig | 1 + drivers/gpio/gpiolib.c | 15 +- drivers/irqchip/qcom-pdc.c | 204 +++++++++++++++++- drivers/pinctrl/qcom/pinctrl-msm.c | 133 ++++++++++-- include/linux/gpio/driver.h | 6 + include/linux/irqdomain.h | 1 + include/linux/soc/qcom/irq.h | 23 ++ 9 files changed, 371 insertions(+), 29 deletions(-) create mode 100644 include/linux/soc/qcom/irq.h
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/8 gpio: Add support for hierarchical IRQ domains
From: Thierry Reding <[email protected]>
Hierarchical IRQ domains can be used to stack different IRQ controllers on top of each other. One specific use-case where this can be useful is if a power management controller has top-level controls for wakeup interrupts. In such cases, the power management controller can be a parent to other interrupt controllers and program additional registers when an IRQ has its wake capability enabled or disabled.
--- drivers/gpio/gpiolib.c | 15 +++++++++++---- include/linux/gpio/driver.h | 6 ++++++ 2 files changed, 17 insertions(+), 4 deletions(-)
drivers/gpio/gpiolib.c +11 -4
@@ -1888,7 +1888,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip, type =IRQ_TYPE_NONE ; } - gpiochip->to_irq = gpiochip_to_irq; +if (!gpiochip->to_irq) + gpiochip->to_irq = gpiochip_to_irq; + gpiochip->irq.default_type = type; gpiochip->irq.lock_key = lock_key; gpiochip->irq.request_key = request_key;
@@ -1898,9 +1900,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,else ops = &gpiochip_domain_ops; - gpiochip->irq.domain =irq_domain_add_simple (np, gpiochip->ngpio, - gpiochip->irq.first, - ops, gpiochip); +if (gpiochip->irq.parent_domain) + gpiochip->irq.domain =irq_domain_add_hierarchy (gpiochip->irq.parent_domain, +0 , gpiochip->ngpio, + np, ops, gpiochip); +else + gpiochip->irq.domain =irq_domain_add_simple (np, gpiochip->ngpio, + gpiochip->irq.first, + ops, gpiochip);if (!gpiochip->irq.domain)return -EINVAL ;
include/linux/gpio/driver.h +6 -0
@@ -48,6 +48,12 @@ struct gpio_irq_chip {*/ const struct irq_domain_ops *domain_ops; +/** +* @parent_domain: +* +*/ +struct irq_domain *parent_domain; +/** * @handler: *
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 2/8 irqdomain: add bus token DOMAIN_BUS_WAKEUP
In reply to Lina IyerAdd new bus token to describe domains that are wakeup capable.
--- include/linux/irqdomain.h | 1 + 1 file changed, 1 insertion(+)
include/linux/irqdomain.h +1 -0
@@ -82,6 +82,7 @@ enum irq_domain_bus_token {DOMAIN_BUS_NEXUS ,DOMAIN_BUS_IPI ,DOMAIN_BUS_FSL_MC_MSI , + DOMAIN_BUS_WAKEUP, };/**
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP
Quoting Lina Iyer (2019-01-24 12:21:59)
Quoted 13 lines
Add new bus token to describe domains that are wakeup capable.
Suggested-by Stephen Boyd <[email protected]>Signed-off-by Lina Iyer <[email protected]>--- include/linux/irqdomain.h | 1 + 1 file changed, 1 insertion(+)include/linux/irqdomain.h +0
-0
include/linux/irqdomain.h enum irq_domain_bus_token { · line 87
Quoted 6 lines
include/linux/irqdomain.h +1
-0@@ -82,6 +82,7 @@ enum irq_domain_bus_token {DOMAIN_BUS_NEXUS ,DOMAIN_BUS_IPI ,DOMAIN_BUS_FSL_MC_MSI , + DOMAIN_BUS_WAKEUP, };
Marc, can you review this patch? It would be good to know if introducing this token is acceptable, or if we need to come up with some other approach here.
PATCH v2 3/8 drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
In reply to Lina IyerIntroduce a new domain for wakeup capable GPIOs. The domain can be requested using the bus token DOMAIN_BUS_WAKEUP. In the following patches, we will specify PDC as the wakeup-parent for the TLMM GPIO irqchip. Requesting a wakeup GPIO will setup the GPIO and the corresponding PDC interrupt as its parent.
Also, provide the map of the PDC pins for the GPIOs for SDM845.
---
Changes in v2:
- Remove separate file for PDC GPIO map data
- Error checks and return
- Whitespace fixes
--- drivers/irqchip/qcom-pdc.c | 204 +++++++++++++++++++++++++++++++++-- include/linux/soc/qcom/irq.h | 23 ++++ 2 files changed, 216 insertions(+), 11 deletions(-) create mode 100644 include/linux/soc/qcom/irq.h
drivers/irqchip/qcom-pdc.c +193 -11
@@ -13,12 +13,13 @@#include <linux/of.h> #include <linux/of_address.h> #include <linux/of_device.h> +#include <linux/soc/qcom/irq.h> #include <linux/spinlock.h> -#include <linux/platform_device.h> #include <linux/slab.h> #include <linux/types.h> #define PDC_MAX_IRQS 126 +#define PDC_MAX_GPIO_IRQS 256#define CLEAR_INTR (reg, intr) (reg & ~(1 << intr))#define ENABLE_INTR (reg, intr) (reg | (1 << intr))
@@ -32,6 +33,16 @@ struct pdc_pin_region {u32 cnt; }; +struct pdc_gpio_pin_map { +unsigned int gpio; +u32 pdc_pin; +}; + +struct pdc_gpio_pin_data { +size_t size; +const struct pdc_gpio_pin_map *map; +}; +static DEFINE_RAW_SPINLOCK (pdc_lock);static void __iomem *pdc_base;static struct pdc_pin_region *pdc_region;
@@ -47,9 +58,8 @@ static u32 pdc_reg_read(int reg, u32 i)return readl_relaxed (pdc_base + reg + i *sizeof (u32)); } -static void pdc_enable_intr (struct irq_data *d,bool on) +static void pdc_enable_intr (irq_hw_number_t pin_out,bool on) { -int pin_out = d->hwirq;u32 index, mask;u32 enable;
@@ -65,13 +75,23 @@ static void pdc_enable_intr(struct irq_data *d, bool on)static void qcom_pdc_gic_mask (struct irq_data *d) { -pdc_enable_intr (d, false); +irq_hw_number_t hwirq = d->hwirq; + +if (hwirq ==ULONG_MAX ) +return ; + +pdc_enable_intr (hwirq, false);irq_chip_mask_parent (d); }static void qcom_pdc_gic_unmask (struct irq_data *d) { -pdc_enable_intr (d, true); +irq_hw_number_t hwirq = d->hwirq; + +if (hwirq ==ULONG_MAX ) +return ; + +pdc_enable_intr (hwirq, true);irq_chip_unmask_parent (d); }
@@ -111,9 +131,12 @@ enum pdc_irq_config_bits {*/ static int qcom_pdc_gic_set_type (struct irq_data *d,unsigned int type) { -int pin_out = d->hwirq; +irq_hw_number_t pin_out = d->hwirq;enum pdc_irq_config_bits pdc_type; +if (pin_out ==ULONG_MAX ) +return 0 ; +switch (type) {case IRQ_TYPE_EDGE_RISING : pdc_type =PDC_EDGE_RISING ;
@@ -157,7 +180,7 @@ static struct irq_chip qcom_pdc_gic_chip = { .irq_set_affinity = irq_chip_set_affinity_parent, }; -static irq_hw_number_t get_parent_hwirq (int pin) +static irq_hw_number_t get_parent_hwirq (irq_hw_number_t pin) {int i;struct pdc_pin_region *region;
@@ -169,7 +192,6 @@ static irq_hw_number_t get_parent_hwirq(int pin)return (region->parent_base + pin - region->pin_base); } -WARN_ON (1 );return ~0UL ; }
@@ -232,6 +254,64 @@ static const struct irq_domain_ops qcom_pdc_ops = { .free = irq_domain_free_irqs_common, }; +static irq_hw_number_t qcom_gpio_to_pdc_pin (struct irq_domain *domain, +unsigned int gpio) +{ +unsigned int i; +const struct pdc_gpio_pin_data *data = domain->host_data; + +if (data) { +for (i =0 ; i < data->size; i++) +if (gpio == data->map[i].gpio) +return data->map[i].pdc_pin; + } + +return ULONG_MAX ; +} + +static int qcom_pdc_gpio_alloc (struct irq_domain *domain,unsigned int virq, +unsigned int nr_irqs,void *data) +{ +struct qcom_irq_fwspec *qcom_fwspec = data; +struct irq_fwspec *fwspec = &qcom_fwspec->fwspec; +struct irq_fwspec parent_fwspec; +irq_hw_number_t hwirq, parent_hwirq; +unsigned int type; +int ret; + + hwirq =qcom_gpio_to_pdc_pin (domain, fwspec->param[0 ]); +if (hwirq ==ULONG_MAX ) +return 0 ; + + parent_hwirq =get_parent_hwirq (hwirq); + ret =irq_domain_set_hwirq_and_chip (domain, virq, hwirq, + &qcom_pdc_gic_chip,NULL ); +if (ret) +return ret; + + qcom_fwspec->mask = true; + +if (type &IRQ_TYPE_EDGE_BOTH ) + type =IRQ_TYPE_EDGE_RISING ; + +if (type &IRQ_TYPE_LEVEL_MASK ) + type =IRQ_TYPE_LEVEL_HIGH ; + + parent_fwspec.fwnode = domain->parent->fwnode; + parent_fwspec.param_count =3 ; + parent_fwspec.param[0 ] =0 ; + parent_fwspec.param[1 ] = parent_hwirq; + parent_fwspec.param[2 ] = type; + +return irq_domain_alloc_irqs_parent (domain, virq, nr_irqs, + &parent_fwspec); +} + +static const struct irq_domain_ops qcom_pdc_gpio_ops = { + .alloc = qcom_pdc_gpio_alloc, + .free = irq_domain_free_irqs_common, +}; +static int pdc_setup_pin_mapping (struct device_node *np) {int ret, n;
@@ -268,9 +348,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)return 0 ; } -static int qcom_pdc_init (struct device_node *node,struct device_node *parent) +static int qcom_pdc_init (struct device_node *node,struct device_node *parent, +struct pdc_gpio_pin_data *data) { -struct irq_domain *parent_domain, *pdc_domain; +struct irq_domain *parent_domain, *pdc_domain, *pdc_gpio_domain;int ret; pdc_base =of_iomap (node,0 );
@@ -301,6 +382,18 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) goto fail; } + pdc_gpio_domain =irq_domain_create_hierarchy (parent_domain,0 , +PDC_MAX_GPIO_IRQS , +of_fwnode_handle (node), + &qcom_pdc_gpio_ops, data); +if (!pdc_gpio_domain) { +pr_err ("%pOF: GIC domain add failed for GPIO domain\n" , node); + ret = -ENOMEM ; + goto fail; + } + +irq_domain_update_bus_token (pdc_gpio_domain,DOMAIN_BUS_WAKEUP ); +return 0 ; fail:
@@ -309,4 +402,93 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)return ret; } -IRQCHIP_DECLARE (pdc_sdm845,"qcom,sdm845-pdc" , qcom_pdc_init); +static const struct pdc_gpio_pin_map sdm845_gpio_pdc_map[] = { + {1 ,30 }, + {3 ,31 }, + {5 ,32 }, + {10 ,33 }, + {11 ,34 }, + {20 ,35 }, + {22 ,36 }, + {24 ,37 }, + {26 ,38 }, + {30 ,39 }, + {31 ,117 }, + {32 ,41 }, + {34 ,42 }, + {36 ,43 }, + {37 ,44 }, + {38 ,45 }, + {39 ,46 }, + {40 ,47 }, + {41 ,115 }, + {43 ,49 }, + {44 ,50 }, + {46 ,51 }, + {48 ,52 }, + {49 ,118 }, + {52 ,54 }, + {53 ,55 }, + {54 ,56 }, + {56 ,57 }, + {57 ,58 }, + {58 ,59 }, + {59 ,60 }, + {60 ,61 }, + {61 ,62 }, + {62 ,63 }, + {63 ,64 }, + {64 ,65 }, + {66 ,66 }, + {68 ,67 }, + {71 ,68 }, + {73 ,69 }, + {77 ,70 }, + {78 ,71 }, + {79 ,72 }, + {80 ,73 }, + {84 ,74 }, + {85 ,75 }, + {86 ,76 }, + {88 ,77 }, + {89 ,116 }, + {91 ,79 }, + {92 ,80 }, + {95 ,81 }, + {96 ,82 }, + {97 ,83 }, + {101 ,84 }, + {103 ,85 }, + {104 ,86 }, + {115 ,90 }, + {116 ,91 }, + {117 ,92 }, + {118 ,93 }, + {119 ,94 }, + {120 ,95 }, + {121 ,96 }, + {122 ,97 }, + {123 ,98 }, + {124 ,99 }, + {125 ,100 }, + {127 ,102 }, + {128 ,103 }, + {129 ,104 }, + {130 ,105 }, + {132 ,106 }, + {133 ,107 }, + {145 ,108 }, +}; + +static struct pdc_gpio_pin_data sdm845_gpio_data = { + .size =ARRAY_SIZE (sdm845_gpio_pdc_map), + .map = sdm845_gpio_pdc_map, +}; + +static int qcom_sdm845_pdc_init (struct device_node *node, +struct device_node *parent) +{ +return qcom_pdc_init (node, parent, &sdm845_gpio_data); +} + +IRQCHIP_DECLARE (pdc_sdm845,"qcom,sdm845-pdc" , qcom_sdm845_pdc_init);
include/linux/soc/qcom/irq.h new file +23 -0
@@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __QCOM_IRQ_H +#define __QCOM_IRQ_H + +#include <linux/irqdomain.h> + +/** +* struct qcom_irq_fwspec - qcom specific irq fwspec wrapper +* @fwspec: irq fwspec +* @mask: if true, keep the irq masked in the gpio controller +* +* Use this structure to communicate between the parent irq chip, MPM or PDC, +* to the gpio chip, TLMM, about the gpio being allocated in the parent +* and if the gpio chip should keep the line masked because the parent irq +* chip is handling everything about the irq line. +*/ +struct qcom_irq_fwspec { +struct irq_fwspec fwspec; +bool mask; +}; + +#endif
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 4/8 dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO
In reply to Lina IyerSDM845 SoC has an always-on interrupt controller (PDC) with select GPIO routed to the PDC as interrupts that can be used to wake the system up from deep low power modes and suspend.
--- .../devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt +6 -1
@@ -29,6 +29,11 @@ SDM845 platform. Definition: must be 2. Specifying the pin number and flags, as defined in <dt-bindings/interrupt-controller/irq.h> +- wakeup-parent: + Usage: optional + Value type: <phandle> + Definition: A phandle to the wakeup interrupt controller for the SoC. + - gpio-controller: Usage: required Value type: <none>
@@ -53,7 +58,6 @@ pin, a group, or a list of pins or groups. This configuration can include the mux function to select on those pin(s)/group(s), and various pin configuration parameters, such as pull-up, drive strength, etc. - PIN CONFIGURATION NODES: The name of each subnode is not important; all subnodes should be enumerated
@@ -160,6 +164,7 @@ Example: #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + wake-parent = <&pdc_intc>; qup9_active: qup9-active { mux {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 5/8 drivers: pinctrl: msm: setup GPIO irqchip hierarchy
In reply to Lina IyerTo allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.
In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.
---
Changes in v2:
- Fix bug when unmaksing PDC interrupt
--- drivers/pinctrl/qcom/pinctrl-msm.c | 133 ++++++++++++++++++++++++++--- 1 file changed, 120 insertions(+), 13 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +120 -13
@@ -17,6 +17,7 @@#include <linux/io.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_irq.h> #include <linux/platform_device.h> #include <linux/pinctrl/machine.h> #include <linux/pinctrl/pinctrl.h>
@@ -27,6 +28,7 @@#include <linux/gpio/driver.h> #include <linux/interrupt.h> #include <linux/spinlock.h> +#include <linux/soc/qcom/irq.h> #include <linux/reboot.h> #include <linux/pm.h> #include <linux/log2.h>
@@ -69,6 +71,7 @@ struct msm_pinctrl {DECLARE_BITMAP (dual_edge_irqs,MAX_NR_GPIO );DECLARE_BITMAP (enabled_irqs,MAX_NR_GPIO ); +DECLARE_BITMAP (wakeup_masked_irqs,MAX_NR_GPIO );const struct msm_pinctrl_soc_data *soc;void __iomem *regs[MAX_NR_TILES ];
@@ -747,6 +750,13 @@ static void msm_gpio_irq_unmask(struct irq_data *d) g = &pctrl->soc->groups[d->hwirq]; +if (d->parent_data) +irq_chip_unmask_parent (d); + +/* Monitored by parent wakeup controller? Keep masked */ +if (test_bit (d->hwirq, pctrl->wakeup_masked_irqs)) +return ; +raw_spin_lock_irqsave (&pctrl->lock, flags); val =msm_readl_intr_cfg (pctrl, g);
@@ -767,6 +777,10 @@ static void msm_gpio_irq_ack(struct irq_data *d)unsigned long flags;u32 val; +/* Handled by parent wakeup controller? Do nothing */ +if (test_bit (d->hwirq, pctrl->wakeup_masked_irqs)) +return ; + g = &pctrl->soc->groups[d->hwirq];raw_spin_lock_irqsave (&pctrl->lock, flags);
@@ -794,6 +808,13 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type) g = &pctrl->soc->groups[d->hwirq]; +if (d->parent_data) +irq_chip_set_type_parent (d, type); + +/* Monitored by parent wakeup controller? Keep masked */ +if (test_bit (d->hwirq, pctrl->wakeup_masked_irqs)) +return 0 ; +raw_spin_lock_irqsave (&pctrl->lock, flags);/*
@@ -890,6 +911,9 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)raw_spin_unlock_irqrestore (&pctrl->lock, flags); +if (d->parent_data) +irq_chip_set_wake_parent (d, on); +return 0 ; }
@@ -967,11 +991,86 @@ static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl)return device_property_read_u16_array (pctrl->dev,"gpios" ,NULL ,0 ) >0 ; } +static int msm_gpio_domain_translate (struct irq_domain *d, +struct irq_fwspec *fwspec, +unsigned long *hwirq,unsigned int *type) +{ +if (is_of_node (fwspec->fwnode)) { +if (fwspec->param_count <2 ) +return -EINVAL ; + *hwirq = fwspec->param[0 ]; + *type = fwspec->param[1 ] &IRQ_TYPE_SENSE_MASK ; +return 0 ; + } + +return -EINVAL ; +} + +static int msm_gpio_domain_alloc (struct irq_domain *domain,unsigned int virq, +unsigned int nr_irqs,void *arg) +{ +int ret; +irq_hw_number_t hwirq; +struct gpio_chip *gc = domain->host_data; +struct msm_pinctrl *pctrl =gpiochip_get_data (gc); +struct irq_fwspec *fwspec = arg; +struct qcom_irq_fwspec parent = { }; +unsigned int type; + + ret =msm_gpio_domain_translate (domain, fwspec, &hwirq, &type); +if (ret) +return ret; + + ret =irq_domain_set_hwirq_and_chip (domain, virq, hwirq, + &pctrl->irq_chip, gc); +if (ret <0 ) +return ret; + +if (!domain->parent) +return 0 ; + + parent.fwspec.fwnode = domain->parent->fwnode; + parent.fwspec.param_count =2 ; + parent.fwspec.param[0 ] = hwirq; + parent.fwspec.param[1 ] = type; + + ret =irq_domain_alloc_irqs_parent (domain, virq, nr_irqs, &parent); +if (ret) +return ret; + +if (parent.mask) +set_bit (hwirq, pctrl->wakeup_masked_irqs); + +return 0 ; +} + +/* +* TODO: Get rid of this and push it into gpiochip_to_irq() +*/ +static int msm_gpio_to_irq (struct gpio_chip *chip,unsigned int offset) +{ +struct irq_fwspec fwspec; + + fwspec.fwnode =of_node_to_fwnode (chip->of_node); + fwspec.param[0 ] = offset; + fwspec.param[1 ] =IRQ_TYPE_LEVEL_HIGH ; + fwspec.param_count =2 ; + +return irq_create_fwspec_mapping (&fwspec); +} + +static const struct irq_domain_ops msm_gpio_domain_ops = { + .translate = msm_gpio_domain_translate, + .alloc = msm_gpio_domain_alloc, + .free = irq_domain_free_irqs_top, +}; +static int msm_gpio_init (struct msm_pinctrl *pctrl) {struct gpio_chip *chip;int ret;unsigned ngpio = pctrl->soc->ngpios; +struct device_node *dn;if (WARN_ON (ngpio >MAX_NR_GPIO ))return -EINVAL ;
@@ -986,6 +1085,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) chip->need_valid_mask =msm_gpio_needs_valid_mask (pctrl); pctrl->irq_chip.name ="msmgpio" ; + pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent; pctrl->irq_chip.irq_mask = msm_gpio_irq_mask; pctrl->irq_chip.irq_unmask = msm_gpio_irq_unmask; pctrl->irq_chip.irq_ack = msm_gpio_irq_ack;
@@ -994,6 +1094,22 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres; pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres; + chip->irq.chip = &pctrl->irq_chip; + chip->irq.domain_ops = &msm_gpio_domain_ops; + chip->irq.handler = handle_edge_irq; + chip->irq.default_type =IRQ_TYPE_NONE ; + + dn =of_parse_phandle (pctrl->dev->of_node,"wakeup-parent" ,0 ); +if (dn) { + chip->irq.parent_domain =irq_find_matching_host (dn, +DOMAIN_BUS_WAKEUP ); +of_node_put (dn); +if (!chip->irq.parent_domain) +return -EPROBE_DEFER ; + + chip->to_irq = msm_gpio_to_irq; + } + ret =gpiochip_add_data (&pctrl->chip, pctrl);if (ret) {dev_err (pctrl->dev,"Failed register gpiochip\n" );
@@ -1015,26 +1131,17 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) dev_name(pctrl->dev), 0, 0, chip->ngpio);if (ret) {dev_err (pctrl->dev,"Failed to add pin range\n" ); -gpiochip_remove (&pctrl->chip); -return ret; + goto fail; } } - ret =gpiochip_irqchip_add (chip, - &pctrl->irq_chip, -0 , - handle_edge_irq, -IRQ_TYPE_NONE ); -if (ret) { -dev_err (pctrl->dev,"Failed to add irqchip to gpiochip\n" ); -gpiochip_remove (&pctrl->chip); -return -ENOSYS ; - } -gpiochip_set_chained_irqchip (chip, &pctrl->irq_chip, pctrl->irq, msm_gpio_irq_handler);return 0 ; +fail: +gpiochip_remove (&pctrl->chip); +return ret; } static int msm_ps_hold_restart(struct notifier_block *nb, unsigned long action,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
Quoting Lina Iyer (2019-01-24 12:22:02)
Quoted 11 lines
To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.
In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.
Quoted 6 lines
Co-developed-by Stephen Boyd <[email protected]>Signed-off-by Lina Iyer <[email protected]>---
Changes in v2:
- Fix bug when unmaksing PDC interrupt
What was the bug? Is that why the mask callback in this gpio chip no longer calls the parent irq chip? We should keep calling the parent irqchip from what I can tell. Otherwise, we may never mask the irq at the PDC and only mask it at the GPIO level, which may not even care about it if it's being monitored by the PDC.
This causes me to get a bunch of interrupts on my touchscreen when I touch it once vs. only a handful (like 4) when I fix it with the below patch:
Can you fold it in?
drivers/pinctrl/qcom/pinctrl-msm.c +3 -0
@@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d)clear_bit (d->hwirq, pctrl->enabled_irqs);raw_spin_unlock_irqrestore (&pctrl->lock, flags); + +if (d->parent_data) +irq_chip_mask_parent (d); }static void msm_gpio_irq_unmask (struct irq_data *d)
Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
Quoted 12 lines · Stephen Boyd
Quoting Lina Iyer (2019-01-24 12:22:02)
Quoted 11 lines
To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.
In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.
Quoted 8 lines
Quoted 6 lines
Co-developed-by Stephen Boyd <[email protected]>Signed-off-by Lina Iyer <[email protected]>---
Changes in v2:
- Fix bug when unmaksing PDC interruptWhat was the bug?
The problem was an incorrect merge (possibly manual), causing the PDC to be not used at all. This is what I had -
static void msm_gpio_irq_mask(struct irq_data *d) {
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
const struct msm_pingroup *g;
unsigned long flags;
u32 val;
g = &pctrl->soc->groups[d->hwirq];
if (d->parent_data)
irq_chip_unmask_parent(d);
/* Monitored by parent wakeup controller? Keep masked */
if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))
return; The above unmask_parent() call in an irq_mask() callback was the bug.
Quoted 10 lines
Is that why the mask callback in this gpio chip no
longer calls the parent irq chip? We should keep calling the parent irqchip from what I can tell. Otherwise, we may never mask the irq at the PDC and only mask it at the GPIO level, which may not even care about it if it's being monitored by the PDC.This causes me to get a bunch of interrupts on my touchscreen when I touch it once vs. only a handful (like 4) when I fix it with the below patch:
Hmm... I did not see an issue in my local testing :( Thanks for the fix.
Quoted 2 lines
Can you fold it in?
Yes, I will fold it in and send a rev out with the fix.
Thanks,
Lina
Quoted 14 lines
drivers/pinctrl/qcom/pinctrl-msm.c +3
-0@@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d)clear_bit (d->hwirq, pctrl->enabled_irqs);raw_spin_unlock_irqrestore (&pctrl->lock, flags); + +if (d->parent_data) +irq_chip_mask_parent (d); }static void msm_gpio_irq_unmask (struct irq_data *d)
PATCH v2 6/8 arm64: dts: qcom: add PDC interrupt controller for SDM845
In reply to Lina IyerAdd PDC interrupt controller device bindings for SDM845.
---
Changes in v2:
- Use updated address specification in reg
- Rename to pdc_intc
- Sort per address in DT
--- arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +9 -0
@@ -1534,6 +1534,15 @@ #power-domain-cells = <1>; }; + pdc_intc: interrupt-controller@b220000 { + compatible = "qcom,sdm845-pdc"; + reg = <0 0x0b220000 0 0x30000>; + qcom,pdc-ranges = <0 480 94>, <94 609 15>, <115 630 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + tsens0: thermal-sensor@c263000 { compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; reg = <0xc263000 0x1ff>, /* TM */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 7/8 arm64: dts: qcom: setup PDC as wakeup parent for GPIOs for SDM845
In reply to Lina IyerSetup PDC wakeup parent for TLMM for SDM845 SoC.
--- arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
@@ -994,6 +994,7 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + wakeup-parent = <&pdc_intc>; qup_i2c0_default: qup-i2c0-default { pinmux {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 8/8 arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
In reply to Lina IyerEnable PDC interrupt controller for SDM845 devices. The interrupt controller can detect wakeup capable interrupts when the SoC is in a low power state.
--- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+)
arch/arm64/configs/defconfig +1 -0
@@ -658,6 +658,7 @@ CONFIG_QCOM_SMEM=y CONFIG_QCOM_SMD_RPM=y CONFIG_QCOM_SMP2P=y CONFIG_QCOM_SMSM=y +CONFIG_QCOM_PDC=y CONFIG_ROCKCHIP_PM_DOMAINS=y CONFIG_ARCH_TEGRA_132_SOC=y CONFIG_ARCH_TEGRA_210_SOC=y
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 7 lines · Lina Iyer
This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.
The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.
I kind of start to get the hang of this now. This is starting to look finished. Some words on the hierarchical GPIO IRQs:
I have started to look into hierarchical GPIO+irqchip since the usage of such is spreading.
I have been on to Thierry patches trying to make him implement more generic helpers in the gpiolib irqchip library functions.
In short I see the following:
- Hierarchical gpiochips all have .alloc() and .translate() functions that look more or less the same.
- They all fall down to remapping either tuples or entire ranges of IRQs from parent to child with a linear look-up table on allocation.
So my idea would be to add support for this into the gpiolib hierarchical irqchip helper: by supplying a parent irqdomain and a list of translation tuples, we should be able to handle pretty much any hierarchical GPIO irqdomain (famous last words).
Right now I am converting the IXP4xx platform to hierarchical IRQ from the ground up (it is not even using device tree so it is a bit of a challenge) but it seems to be working.
So I will try to post this in some hopefully working form, and on top of those changes or before them introduce some helpers in the core for hierarchical irqs. Or I fail.
Anyways, I do not think my ambitions for refactoring the helpers can stand in the way of support for these use cases and new hardware, so maybe we need to merge a few more hierarchical drivers just bypassing the gpiolib helpers. I don't want to block development, and I suspect Thierry might be getting annoyed at me at this point, so we should maybe just pile up a few more hierarchical chips so I can refactor them later.
What do you think?
Yours,
Linus Walleij
Quoted 16 lines · Linus Walleij
Quoted 7 lines · Lina Iyer
This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.
The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.
I kind of start to get the hang of this now. This is starting to look finished. Some words on the hierarchical GPIO IRQs:
I have started to look into hierarchical GPIO+irqchip since the usage of such is spreading.
Quoted 8 lines
I have been on to Thierry patches trying to make him implement more generic helpers in the gpiolib irqchip library functions.
In short I see the following:
- Hierarchical gpiochips all have .alloc() and .translate() functions that look more or less the same.
Yes.
Quoted 4 lines
- They all fall down to remapping either tuples or entire ranges of IRQs from parent to child with a linear look-up table on allocation.
I would think this would be the generic case, unless its QCOM, where we would want to push the tuples up hierarchy :(
Quoted 6 lines
So my idea would be to add support for this into the gpiolib hierarchical irqchip helper: by supplying a parent irqdomain and a list of translation tuples, we should be able to handle pretty much any hierarchical GPIO irqdomain (famous last words).
We would read the tuples from DT? Also please consider Rob's idea of specifying hierarchy from [2].
Quoted 8 lines
Right now I am converting the IXP4xx platform to hierarchical IRQ from the ground up (it is not even using device tree so it is a bit of a challenge) but it seems to be working.
So I will try to post this in some hopefully working form, and on top of those changes or before them introduce some helpers in the core for hierarchical irqs. Or I fail.
Thanks, please copy me on the thread. I would not want to miss this.
Quoted 11 lines
Anyways, I do not think my ambitions for refactoring the helpers can stand in the way of support for these use cases and new hardware, so maybe we need to merge a few more hierarchical drivers just bypassing the gpiolib helpers. I don't want to block development, and I suspect Thierry might be getting annoyed at me at this point, so we should maybe just pile up a few more hierarchical chips so I can refactor them later.
What do you think?
I attempted refactoring the .alloc and .translate and for a generic case and it seemed like it would fit the bill very well. Will await your patches.
Thanks,
Lina