* Re: [PATCH RFC 03/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
From: Lina Iyer @ 2019-08-30 15:58 UTC (permalink / raw)
To: Marc Zyngier
Cc: swboyd, evgreen, linus.walleij, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, rnayak
In-Reply-To: <d2a45d45-3071-ab8d-060b-92a2812a8d42@kernel.org>
On Fri, Aug 30 2019 at 08:50 -0600, Marc Zyngier wrote:
>[Please use my kernel.org address in the future. The days of this
>arm.com address are numbered...]
>
Sure, will update and repost.
>On 29/08/2019 19:11, Lina Iyer wrote:
>> Introduce 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.
>>
>> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ---
>> drivers/irqchip/qcom-pdc.c | 104 ++++++++++++++++++++++++++++++++---
>> include/linux/soc/qcom/irq.h | 34 ++++++++++++
>> 2 files changed, 129 insertions(+), 9 deletions(-)
>> create mode 100644 include/linux/soc/qcom/irq.h
>>
>> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
>> index 338fae604af5..ad1faf634bcf 100644
>> --- a/drivers/irqchip/qcom-pdc.c
>> +++ b/drivers/irqchip/qcom-pdc.c
>> @@ -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))
>> @@ -26,6 +27,8 @@
>> #define IRQ_ENABLE_BANK 0x10
>> #define IRQ_i_CFG 0x110
>>
>> +#define PDC_NO_PARENT_IRQ ~0UL
>> +
>> struct pdc_pin_region {
>> u32 pin_base;
>> u32 parent_base;
>> @@ -65,23 +68,35 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
>>
>> static void qcom_pdc_gic_disable(struct irq_data *d)
>> {
>> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
>> + return;
>> +
>> pdc_enable_intr(d, false);
>> irq_chip_disable_parent(d);
>> }
>>
>> static void qcom_pdc_gic_enable(struct irq_data *d)
>> {
>> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
>> + return;
>> +
>> pdc_enable_intr(d, true);
>> irq_chip_enable_parent(d);
>> }
>>
>> static void qcom_pdc_gic_mask(struct irq_data *d)
>> {
>> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
>> + return;
>> +
>> irq_chip_mask_parent(d);
>> }
>>
>> static void qcom_pdc_gic_unmask(struct irq_data *d)
>> {
>> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
>> + return;
>> +
>> irq_chip_unmask_parent(d);
>> }
>>
>> @@ -124,6 +139,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
>> int pin_out = d->hwirq;
>> enum pdc_irq_config_bits pdc_type;
>>
>> + if (pin_out == GPIO_NO_WAKE_IRQ)
>> + return 0;
>> +
>> switch (type) {
>> case IRQ_TYPE_EDGE_RISING:
>> pdc_type = PDC_EDGE_RISING;
>> @@ -181,8 +199,7 @@ static irq_hw_number_t get_parent_hwirq(int pin)
>> return (region->parent_base + pin - region->pin_base);
>> }
>>
>> - WARN_ON(1);
>> - return ~0UL;
>> + return PDC_NO_PARENT_IRQ;
>> }
>>
>> static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
>> @@ -211,17 +228,17 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
>>
>> ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
>> if (ret)
>> - return -EINVAL;
>> -
>> - parent_hwirq = get_parent_hwirq(hwirq);
>> - if (parent_hwirq == ~0UL)
>> - return -EINVAL;
>> + return ret;
>>
>> ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
>> &qcom_pdc_gic_chip, NULL);
>> if (ret)
>> return ret;
>>
>> + parent_hwirq = get_parent_hwirq(hwirq);
>> + if (parent_hwirq == PDC_NO_PARENT_IRQ)
>> + return 0;
>> +
>> if (type & IRQ_TYPE_EDGE_BOTH)
>> type = IRQ_TYPE_EDGE_RISING;
>>
>> @@ -244,6 +261,60 @@ static const struct irq_domain_ops qcom_pdc_ops = {
>> .free = irq_domain_free_irqs_common,
>> };
>>
>> +static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs, void *data)
>> +{
>> + struct irq_fwspec *fwspec = data;
>> + struct irq_fwspec parent_fwspec;
>> + irq_hw_number_t hwirq, parent_hwirq;
>> + unsigned int type;
>> + int ret;
>> +
>> + ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
>> + if (ret)
>> + return ret;
>> +
>> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
>> + &qcom_pdc_gic_chip, NULL);
>> + if (ret)
>> + return ret;
>> +
>> + if (hwirq == GPIO_NO_WAKE_IRQ)
>> + return 0;
>> +
>> + parent_hwirq = get_parent_hwirq(hwirq);
>> + if (parent_hwirq == PDC_NO_PARENT_IRQ)
>> + return 0;
>> +
>> + 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 int qcom_pdc_gpio_domain_select(struct irq_domain *d,
>> + struct irq_fwspec *fwspec,
>> + enum irq_domain_bus_token bus_token)
>> +{
>> + return (bus_token == DOMAIN_BUS_WAKEUP);
>> +}
>> +
>> +static const struct irq_domain_ops qcom_pdc_gpio_ops = {
>> + .select = qcom_pdc_gpio_domain_select,
>> + .alloc = qcom_pdc_gpio_alloc,
>> + .free = irq_domain_free_irqs_common,
>> +};
>> +
>> static int pdc_setup_pin_mapping(struct device_node *np)
>> {
>> int ret, n;
>> @@ -282,7 +353,7 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>>
>> static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
>> {
>> - 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);
>> @@ -313,8 +384,23 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
>> goto fail;
>> }
>>
>> + pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
>> + IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
>> + PDC_MAX_GPIO_IRQS,
>> + of_fwnode_handle(node),
>> + &qcom_pdc_gpio_ops, NULL);
>> + if (!pdc_gpio_domain) {
>> + pr_err("%pOF: GIC domain add failed for GPIO domain\n", node);
>> + ret = -ENOMEM;
>> + goto remove;
>> + }
>> +
>> + irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
>> +
>> return 0;
>>
>> +remove:
>> + irq_domain_remove(pdc_domain);
>> fail:
>> kfree(pdc_region);
>> iounmap(pdc_base);
>> diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
>> new file mode 100644
>> index 000000000000..73239917dc38
>> --- /dev/null
>> +++ b/include/linux/soc/qcom/irq.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef __QCOM_IRQ_H
>> +#define __QCOM_IRQ_H
>> +
>> +#include <linux/irqdomain.h>
>> +
>> +#define GPIO_NO_WAKE_IRQ ~0U
>> +
>> +/**
>> + * QCOM specific IRQ domain flags that distinguishes the handling of wakeup
>> + * capable interrupts by different interrupt controllers.
>> + *
>> + * IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP: Line must be masked at TLMM and the
>> + * interrupt configuration is done at PDC
>> + * IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP: Interrupt configuration is handled at TLMM
>> + */
>> +#define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP (1 << 17)
>> +#define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP (1 << 18)
>
>Any reason why you're starting at bit 17? The available range in from
>bit 16... But overall, it would be better if you expressed it as:
>
>#define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP (IRQ_DOMAIN_FLAG_NONCORE << 0)
>#define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP (IRQ_DOMAIN_FLAG_NONCORE << 1)
>
Okay.
>> +
>> +/**
>> + * irq_domain_qcom_handle_wakeup: Return if the domain handles interrupt
>> + * configuration
>> + * @parent: irq domain
>> + *
>> + * This QCOM specific irq domain call returns if the interrupt controller
>> + * requires the interrupt be masked at the child interrupt controller.
>> + */
>> +static inline bool irq_domain_qcom_handle_wakeup(struct irq_domain *parent)
>> +{
>> + return (parent->flags & IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP);
>> +}
>> +
>> +#endif
>>
>
>But most of this file isn't used by this patch, so maybe it should be
>moved somewhere else...
>
Apart from creating the domain, this is not used here, but a separate
patch seemed excessive. Let me know if you have any suggestions.
Thanks,
Lina
^ permalink raw reply
* Re: [PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
From: Peter Cai @ 2019-08-30 15:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
Bastien Nocera, Dmitry Torokhov, Rafael J. Wysocki, Len Brown,
linux-gpio, linux-acpi, linux-kernel, linux-input
In-Reply-To: <20190830115505.GX2680@smile.fi.intel.com>
On August 30, 2019 7:55:05 PM GMT+08:00, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>On Fri, Aug 30, 2019 at 08:00:24AM +0800, Peter Cai wrote:
>> The firmware of GPD P2 Max could not handle panel resets although
>code
>> is present in DSDT. The kernel needs to take on this job instead, but
>> the DSDT does not provide _DSD, rendering kernel helpless when trying
>to
>> find the respective GPIO pins.
>>
>> Fortunately, this time GPD has proper DMI vendor / product strings
>that
>> we could match against. We simply apply an acpi_gpio_mapping table
>when
>> GPD P2 Max is matched.
>>
>> Additionally, the DSDT definition of the irq pin specifies a wrong
>> polarity. The new quirk introduced in the previous patch
>> (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.
>
>> +#ifdef CONFIG_ACPI
>
>I guess most of these #ifdef:s makes code less readable for exchange of
>saving
>few bytes in the module footprint.
>
>> + { "irq-gpios", &irq_gpios_default, 1,
>> + ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
>
>One line?
>
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
>
>Comma at the end?
>
>> + },
>> + .driver_data = &gpio_mapping_force_irq_active_high
>
>Ditto.
> I guess most of these #ifdef:s makes code less readable for exchange of saving
few bytes in the module footprint.
Since they can only be used when ACPI is supported (devm_acpi_dev_add_driver_gpios does not exist without ACPI defined, thus the last guard must exist), if they were not guarded then we would be left with a bunch of unused variables warnings when building without ACPI which doesn't seem good.
Should we use __maybe_unused here instead of #ifdef guards?
> Comma at the end?
I was trying to follow the style of this driver but it doesn't seem to be really consistent within itself. Another dmi_system_id definition in the same file mixed both styles so I was kind of confused.
--
Regards,
Xiyu Cai
^ permalink raw reply
* Re: [PATCH RFC 03/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
From: Marc Zyngier @ 2019-08-30 14:50 UTC (permalink / raw)
To: Lina Iyer, swboyd, evgreen, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak
In-Reply-To: <20190829181203.2660-4-ilina@codeaurora.org>
[Please use my kernel.org address in the future. The days of this
arm.com address are numbered...]
On 29/08/2019 19:11, Lina Iyer wrote:
> Introduce 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.
>
> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
> drivers/irqchip/qcom-pdc.c | 104 ++++++++++++++++++++++++++++++++---
> include/linux/soc/qcom/irq.h | 34 ++++++++++++
> 2 files changed, 129 insertions(+), 9 deletions(-)
> create mode 100644 include/linux/soc/qcom/irq.h
>
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 338fae604af5..ad1faf634bcf 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -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))
> @@ -26,6 +27,8 @@
> #define IRQ_ENABLE_BANK 0x10
> #define IRQ_i_CFG 0x110
>
> +#define PDC_NO_PARENT_IRQ ~0UL
> +
> struct pdc_pin_region {
> u32 pin_base;
> u32 parent_base;
> @@ -65,23 +68,35 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
>
> static void qcom_pdc_gic_disable(struct irq_data *d)
> {
> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
> + return;
> +
> pdc_enable_intr(d, false);
> irq_chip_disable_parent(d);
> }
>
> static void qcom_pdc_gic_enable(struct irq_data *d)
> {
> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
> + return;
> +
> pdc_enable_intr(d, true);
> irq_chip_enable_parent(d);
> }
>
> static void qcom_pdc_gic_mask(struct irq_data *d)
> {
> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
> + return;
> +
> irq_chip_mask_parent(d);
> }
>
> static void qcom_pdc_gic_unmask(struct irq_data *d)
> {
> + if (d->hwirq == GPIO_NO_WAKE_IRQ)
> + return;
> +
> irq_chip_unmask_parent(d);
> }
>
> @@ -124,6 +139,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
> int pin_out = d->hwirq;
> enum pdc_irq_config_bits pdc_type;
>
> + if (pin_out == GPIO_NO_WAKE_IRQ)
> + return 0;
> +
> switch (type) {
> case IRQ_TYPE_EDGE_RISING:
> pdc_type = PDC_EDGE_RISING;
> @@ -181,8 +199,7 @@ static irq_hw_number_t get_parent_hwirq(int pin)
> return (region->parent_base + pin - region->pin_base);
> }
>
> - WARN_ON(1);
> - return ~0UL;
> + return PDC_NO_PARENT_IRQ;
> }
>
> static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
> @@ -211,17 +228,17 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
>
> ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
> if (ret)
> - return -EINVAL;
> -
> - parent_hwirq = get_parent_hwirq(hwirq);
> - if (parent_hwirq == ~0UL)
> - return -EINVAL;
> + return ret;
>
> ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> &qcom_pdc_gic_chip, NULL);
> if (ret)
> return ret;
>
> + parent_hwirq = get_parent_hwirq(hwirq);
> + if (parent_hwirq == PDC_NO_PARENT_IRQ)
> + return 0;
> +
> if (type & IRQ_TYPE_EDGE_BOTH)
> type = IRQ_TYPE_EDGE_RISING;
>
> @@ -244,6 +261,60 @@ static const struct irq_domain_ops qcom_pdc_ops = {
> .free = irq_domain_free_irqs_common,
> };
>
> +static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *data)
> +{
> + struct irq_fwspec *fwspec = data;
> + struct irq_fwspec parent_fwspec;
> + irq_hw_number_t hwirq, parent_hwirq;
> + unsigned int type;
> + int ret;
> +
> + ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &qcom_pdc_gic_chip, NULL);
> + if (ret)
> + return ret;
> +
> + if (hwirq == GPIO_NO_WAKE_IRQ)
> + return 0;
> +
> + parent_hwirq = get_parent_hwirq(hwirq);
> + if (parent_hwirq == PDC_NO_PARENT_IRQ)
> + return 0;
> +
> + 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 int qcom_pdc_gpio_domain_select(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + enum irq_domain_bus_token bus_token)
> +{
> + return (bus_token == DOMAIN_BUS_WAKEUP);
> +}
> +
> +static const struct irq_domain_ops qcom_pdc_gpio_ops = {
> + .select = qcom_pdc_gpio_domain_select,
> + .alloc = qcom_pdc_gpio_alloc,
> + .free = irq_domain_free_irqs_common,
> +};
> +
> static int pdc_setup_pin_mapping(struct device_node *np)
> {
> int ret, n;
> @@ -282,7 +353,7 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>
> static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> {
> - 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);
> @@ -313,8 +384,23 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> goto fail;
> }
>
> + pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
> + IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
> + PDC_MAX_GPIO_IRQS,
> + of_fwnode_handle(node),
> + &qcom_pdc_gpio_ops, NULL);
> + if (!pdc_gpio_domain) {
> + pr_err("%pOF: GIC domain add failed for GPIO domain\n", node);
> + ret = -ENOMEM;
> + goto remove;
> + }
> +
> + irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
> +
> return 0;
>
> +remove:
> + irq_domain_remove(pdc_domain);
> fail:
> kfree(pdc_region);
> iounmap(pdc_base);
> diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
> new file mode 100644
> index 000000000000..73239917dc38
> --- /dev/null
> +++ b/include/linux/soc/qcom/irq.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __QCOM_IRQ_H
> +#define __QCOM_IRQ_H
> +
> +#include <linux/irqdomain.h>
> +
> +#define GPIO_NO_WAKE_IRQ ~0U
> +
> +/**
> + * QCOM specific IRQ domain flags that distinguishes the handling of wakeup
> + * capable interrupts by different interrupt controllers.
> + *
> + * IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP: Line must be masked at TLMM and the
> + * interrupt configuration is done at PDC
> + * IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP: Interrupt configuration is handled at TLMM
> + */
> +#define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP (1 << 17)
> +#define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP (1 << 18)
Any reason why you're starting at bit 17? The available range in from
bit 16... But overall, it would be better if you expressed it as:
#define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP (IRQ_DOMAIN_FLAG_NONCORE << 0)
#define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP (IRQ_DOMAIN_FLAG_NONCORE << 1)
> +
> +/**
> + * irq_domain_qcom_handle_wakeup: Return if the domain handles interrupt
> + * configuration
> + * @parent: irq domain
> + *
> + * This QCOM specific irq domain call returns if the interrupt controller
> + * requires the interrupt be masked at the child interrupt controller.
> + */
> +static inline bool irq_domain_qcom_handle_wakeup(struct irq_domain *parent)
> +{
> + return (parent->flags & IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP);
> +}
> +
> +#endif
>
But most of this file isn't used by this patch, so maybe it should be
moved somewhere else...
Thanks,
M.
--
Jazz is not dead, it just smells funny...
^ permalink raw reply
* Re: [GIT PULL] gpio: fixes for v5.3-rc7
From: Linus Walleij @ 2019-08-30 13:28 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
In-Reply-To: <20190830075856.9261-1-brgl@bgdev.pl>
On Fri, Aug 30, 2019 at 9:58 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Hi Linus,
>
> please pull the following fixes for a regression in pca953x.
Pulled to my fixes branch!
Thanks a lot!
Linus Walleij
^ permalink raw reply
* RE: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: David Laight @ 2019-08-30 13:21 UTC (permalink / raw)
To: 'Enrico Weigelt, metux IT consult', Uwe Kleine-König,
Andrew Morton
Cc: Jonathan Corbet, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <a5f535af-09f7-e65b-1527-7d6dd8553c1d@metux.net>
From: Enrico Weigelt, metux IT consult
> Sent: 26 August 2019 14:29
> On 25.08.19 01:37, Uwe Kleine-König wrote:
>
> Hi,
>
> > +static noinline_for_stack > +char *errstr(char *buf, char *end, unsigned long long num,> +
> struct printf_spec spec)> +{
> #1: why not putting that into some separate strerror() lib function ?
> This is something I've been looking for quite some time (actually
> already hacked it up somewhere, sometime, but forgotten ...)
>
> #2: why not just having a big case statement and leave the actual lookup
> logic to the compiler ? IMHO, could be written in a very compact way
> by some macro magic
And generate an enormous amount of code and long chains of mispredicted branches.
Is it also worth looking at how long the strings are.
If they can be truncated to 16 bytes then char[][16] will generate
much better code than the array of pointers.
OTOH I'm not really sure it is all a good idea.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
From: Andy Shevchenko @ 2019-08-30 11:55 UTC (permalink / raw)
To: Peter Cai
Cc: Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
Bastien Nocera, Dmitry Torokhov, Rafael J. Wysocki, Len Brown,
linux-gpio, linux-acpi, linux-kernel, linux-input
In-Reply-To: <20190830000024.20384-2-peter@typeblog.net>
On Fri, Aug 30, 2019 at 08:00:24AM +0800, Peter Cai wrote:
> The firmware of GPD P2 Max could not handle panel resets although code
> is present in DSDT. The kernel needs to take on this job instead, but
> the DSDT does not provide _DSD, rendering kernel helpless when trying to
> find the respective GPIO pins.
>
> Fortunately, this time GPD has proper DMI vendor / product strings that
> we could match against. We simply apply an acpi_gpio_mapping table when
> GPD P2 Max is matched.
>
> Additionally, the DSDT definition of the irq pin specifies a wrong
> polarity. The new quirk introduced in the previous patch
> (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.
> +#ifdef CONFIG_ACPI
I guess most of these #ifdef:s makes code less readable for exchange of saving
few bytes in the module footprint.
> + { "irq-gpios", &irq_gpios_default, 1,
> + ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
One line?
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
Comma at the end?
> + },
> + .driver_data = &gpio_mapping_force_irq_active_high
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/2] gpio: acpi: add quirk to override GpioInt polarity
From: Andy Shevchenko @ 2019-08-30 11:42 UTC (permalink / raw)
To: Peter Cai
Cc: Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
Bastien Nocera, Dmitry Torokhov, Rafael J. Wysocki, Len Brown,
linux-gpio, linux-acpi, linux-kernel, linux-input
In-Reply-To: <20190830000024.20384-1-peter@typeblog.net>
On Fri, Aug 30, 2019 at 08:00:23AM +0800, Peter Cai wrote:
> On GPD P2 Max, the firmware could not reset the touch panel correctly.
> The kernel needs to take on the job instead, but the GpioInt definition
> in DSDT specifies ActiveHigh while the GPIO pin should actually be
> ActiveLow.
>
> We need to override the polarity defined by DSDT. The GPIO driver
> already allows defining polarity in acpi_gpio_params, but the option is
> not applied to GpioInt.
>
> This patch adds a new quirk that enables the polarity specified in
> acpi_gpio_params to also be applied to GpioInt.
In general if it's really the case, I'm not objecting to have another quirk.
So, I would wait for the comments on the second patch to see how it's going.
> include/linux/acpi.h | 6 ++++++
The GPIO part of the header had been moved to the drivers/gpio/gpiolib-acpi.h.
Please, base your series on top of the gpio/for-next.
> lookup->info.flags = GPIOD_IN;
> - lookup->info.polarity = agpio->polarity;
> + if (lookup->info.quirks &
> + ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) {
Disregard checkpatch I would leave this on one line.
> + dev_warn(&lookup->info.adev->dev, FW_BUG "Incorrect polarity specified by GpioInt, overriding.\n");
> + lookup->info.polarity = lookup->active_low;
> + } else {
> + lookup->info.polarity = agpio->polarity;
> + }
> lookup->info.triggering = agpio->triggering;
Since the quirk makes sense only for GpioInt and basically no-op for GpioIo,
I would move the check out of if (gpioint) {} else {} conditional:
if (gpioint) {
...
} else {
...
}
if (quirk) {
dev_warn();
polarity = ...;
}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] microblaze: Switch to standard restart handler
From: Michal Simek @ 2019-08-30 9:13 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Arnd Bergmann
In-Reply-To: <CACRpkdaa_vQ0Pko0Log5uwomFb+gbSPTjpZ6S-3VJgFGMYYpWw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1532 bytes --]
On 23. 08. 19 13:19, Linus Walleij wrote:
> On Fri, Aug 23, 2019 at 12:56 PM Michal Simek <monstr@monstr.eu> wrote:
>
>>> +void machine_restart(char *cmd)
>>> +{
>>> + do_kernel_restart(cmd);
>>> + /* Give the restart hook 1 s to take us down */
>>> + mdelay(1000);
>>> + pr_emerg("Reboot failed -- System halted\n");
>>> + while (1);
>>> +}
>>> +
>>> /*
>>> * MMU_init sets up the basic memory mappings for the kernel,
>>> * including both RAM and possibly some I/O regions,
>>>
>>
>> I will test this for sure. What's the reason to add machine_restart to
>> mm/ folder? You can simply keep it in a location where it was.
>
> It is because the three calls to machine_restart() were in this
> file and therefore I just put it in proximity.
>
> I can move it.
>
>> Do you know why of_find_gpio can failed in connection to gpio-xilinx driver?
>
> The only reason it'd fail would be if the module is not compiled
> in, or deferred probe happens, but then -EPROBE_DEFER
> would be returned I think.
I have retest it. I have done one dt issue that was the reason why it
didn't work.
Please move that machine_restart() back to reset.c and then I will apply
this patch.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* [GIT PULL] gpio: fixes for v5.3-rc7
From: Bartosz Golaszewski @ 2019-08-30 7:58 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Hi Linus,
please pull the following fixes for a regression in pca953x.
The following changes since commit a55aa89aab90fae7c815b0551b07be37db359d76:
Linux 5.3-rc6 (2019-08-25 12:01:23 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-v5.3-rc7-fixes-for-linus
for you to fetch changes up to 438b6c20e6161a1a7542490baa093c86732f77d6:
gpio: pca953x: use pca953x_read_regs instead of regmap_bulk_read (2019-08-28 12:55:24 +0200)
----------------------------------------------------------------
gpio fixes for v5.3-rc7
- two patches fixing a regression in the pca953x driver
----------------------------------------------------------------
David Jander (2):
gpio: pca953x: correct type of reg_direction
gpio: pca953x: use pca953x_read_regs instead of regmap_bulk_read
drivers/gpio/gpio-pca953x.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
^ permalink raw reply
* [PATCH] pinctrl: qcom: sdm845: Fix UFS_RESET pin
From: Stephen Boyd @ 2019-08-30 6:02 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, linux-gpio, Linus Walleij,
Andy Gross
The UFS_RESET pin is the magical pin #150 now, not 153 per the
sdm845_groups array declared in this file. Fix the order of pins so that
UFS_RESET is 150 and the SDC pins follow after.
Fixes: 53a5372ce326 ("pinctrl: qcom: sdm845: Expose ufs_reset as gpio")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/pinctrl/qcom/pinctrl-sdm845.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-sdm845.c b/drivers/pinctrl/qcom/pinctrl-sdm845.c
index 39f498c09906..ce495970459d 100644
--- a/drivers/pinctrl/qcom/pinctrl-sdm845.c
+++ b/drivers/pinctrl/qcom/pinctrl-sdm845.c
@@ -262,10 +262,10 @@ static const struct pinctrl_pin_desc sdm845_pins[] = {
PINCTRL_PIN(147, "GPIO_147"),
PINCTRL_PIN(148, "GPIO_148"),
PINCTRL_PIN(149, "GPIO_149"),
- PINCTRL_PIN(150, "SDC2_CLK"),
- PINCTRL_PIN(151, "SDC2_CMD"),
- PINCTRL_PIN(152, "SDC2_DATA"),
- PINCTRL_PIN(153, "UFS_RESET"),
+ PINCTRL_PIN(150, "UFS_RESET"),
+ PINCTRL_PIN(151, "SDC2_CLK"),
+ PINCTRL_PIN(152, "SDC2_CMD"),
+ PINCTRL_PIN(153, "SDC2_DATA"),
};
#define DECLARE_MSM_GPIO_PINS(pin) \
--
Sent by a computer through tubes
^ permalink raw reply related
* Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
From: Harish Jenny K N @ 2019-08-30 5:21 UTC (permalink / raw)
To: Linus Walleij, Rob Herring
Cc: Bartosz Golaszewski, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM, Balasubramani Vivekanandan
In-Reply-To: <f47588d5-226a-6a7a-6c74-c0caaafaf572@mentor.com>
Hi Rob,
On 27/08/19 1:17 PM, Harish Jenny K N wrote:
> Hi Rob,
>
>
> On 19/08/19 3:06 PM, Harish Jenny K N wrote:
>> Hi Rob,
>>
>>
>> On 10/08/19 2:21 PM, Linus Walleij wrote:
>>> On Fri, Aug 9, 2019 at 4:08 PM Rob Herring <robh+dt@kernel.org> wrote:
>>>> On Mon, Aug 5, 2019 at 5:15 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>> There is some level of ambition here which is inherently a bit fuzzy
>>>>> around the edges. ("How long is the coast of Britain?" comes to mind.)
>>>>>
>>>>> Surely the intention of device tree is not to recreate the schematic
>>>>> in all detail. What we want is a model of the hardware that will
>>>>> suffice for the operating system usecases.
>>>>>
>>>>> But sometimes the DTS files will become confusing: why is this
>>>>> component using GPIO_ACTIVE_LOW when another system
>>>>> doesn't have that flag? If there is an explicit inverter, the
>>>>> DTS gets more readable for a human.
>>>>>
>>>>> But arguable that is case for adding inverters as syntactic
>>>>> sugar in the DTS compiler instead...
>>>> If you really want something more explicit, then add a new GPIO
>>>> 'inverted' flag. Then a device can always have the same HIGH/LOW flag.
>>>> That also solves the abstract it for userspace problem.
>>> I think there are some intricate ontologies at work here.
>>>
>>> Consider this example: a GPIO is controlling a chip select
>>> regulator, say Acme Foo. The chip select
>>> has a pin named CSN. We know from convention that the
>>> "N" at the end of that pin name means "negative" i.e. active
>>> low, and that is how the electronics engineers think about
>>> that chip select line: it activates the IC when
>>> the line goes low.
>>>
>>> The regulator subsystem and I think all subsystems in the
>>> Linux kernel say the consumer pin should be named and
>>> tagged after the datsheet of the regulator.
>>>
>>> So it has for example:
>>>
>>> foo {
>>> compatible = "acme,foo";
>>> cs-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
>>> };
>>>
>>> (It would be inappropriate to name it "csn-gpios" since
>>> we have an established flag for active low. But it is another
>>> of these syntactic choices where people likely do mistakes.)
>>>
>>> I think it would be appropriate for the DT binding to say
>>> that this flag must always be GPIO_ACTIVE_LOW since
>>> the bindings are seen from the component point of view,
>>> and thus this is always active low.
>>>
>>> It would even be reasonable for a yaml schema to enfore
>>> this, if it could. It is defined as active low after all.
>>>
>>> Now if someone adds an inverter on that line between
>>> gpio0 and Acme Foo it looks like this:
>>>
>>> foo {
>>> compatible = "acme,foo";
>>> cs-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
>>> };
>>>
>>> And now we get cognitive dissonance or whatever I should
>>> call it: someone reading this DTS sheet and the data
>>> sheet for the component Acme Foo to troubleshoot
>>> this will be confused: this component has CS active
>>> low and still it is specified as active high? Unless they
>>> also look at the schematic or the board and find the
>>> inverter things are pretty muddy and they will likely curse
>>> and solve the situation with the usual trial-and-error,
>>> inserting some random cursewords as a comment.
>>>
>>> With an intermediate inverter node, the cs-gpios
>>> can go back to GPIO_ACTIVE_LOW and follow
>>> the bindings:
>>>
>>> inv0: inverter {
>>> compatible = "gpio-inverter";
>>> gpio-controller;
>>> #gpio-cells = <1>;
>>> inverted-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
>>> };
>>>
>>> foo {
>>> compatible = "acme,foo";
>>> cs-gpios = <&inv0 0 GPIO_ACTIVE_LOW>;
>>> };
>>>
>>> And now Acme Foo bindings can keep enforcing cs-gpios
>>> to always be tagged GPIO_ACTIVE_LOW.
>> Can you please review/let us know your opinion on this ? I think the idea here is to also isolate the changes to a separate consumer driver and avoid getting inversions inside gpiolib.
>>
>>
>> Thanks.
>>
>>
>> Regards,
>>
>> Harish Jenny K N
>>
> Can you please comment on this ?
>
>
> Thanks,
>
> Harish Jenny K N
>
Friendly Reminder.
can we please finalize this ?
Linus has also mentioned in another patchset "[PATCH v2] Input: tsc2007 - use GPIO descriptor" that
he is in favor of introducing explicit inverters in device tree.
Please consider this and let us know your inputs.
Thanks,
Harish Jenny K N
^ permalink raw reply
* [PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
From: Peter Cai @ 2019-08-30 0:00 UTC (permalink / raw)
Cc: Peter Cai, Mika Westerberg, Andy Shevchenko, Linus Walleij,
Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov,
Rafael J. Wysocki, Len Brown, linux-gpio, linux-acpi,
linux-kernel, linux-input
In-Reply-To: <20190830000024.20384-1-peter@typeblog.net>
The firmware of GPD P2 Max could not handle panel resets although code
is present in DSDT. The kernel needs to take on this job instead, but
the DSDT does not provide _DSD, rendering kernel helpless when trying to
find the respective GPIO pins.
Fortunately, this time GPD has proper DMI vendor / product strings that
we could match against. We simply apply an acpi_gpio_mapping table when
GPD P2 Max is matched.
Additionally, the DSDT definition of the irq pin specifies a wrong
polarity. The new quirk introduced in the previous patch
(ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.
Signed-off-by: Peter Cai <peter@typeblog.net>
---
drivers/input/touchscreen/goodix.c | 37 ++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 5178ea8b5f30..65b8d04b6dcf 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -144,6 +144,34 @@ static const struct dmi_system_id rotated_screen[] = {
{}
};
+#ifdef CONFIG_ACPI
+static const struct acpi_gpio_params irq_gpios_default = { 0, 0, false };
+static const struct acpi_gpio_params reset_gpios_default = { 1, 0, false };
+static const struct acpi_gpio_mapping gpio_mapping_force_irq_active_high[] = {
+ { "irq-gpios", &irq_gpios_default, 1,
+ ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
+ { "reset-gpios", &reset_gpios_default, 1 },
+ {}
+};
+
+/*
+ * Devices that need acpi_gpio_mapping to function correctly
+ */
+static const struct dmi_system_id need_gpio_mapping[] = {
+#if defined(CONFIG_DMI) && defined(CONFIG_X86)
+ {
+ .ident = "GPD P2 Max",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
+ },
+ .driver_data = &gpio_mapping_force_irq_active_high
+ },
+#endif
+ {}
+};
+#endif
+
/**
* goodix_i2c_read - read data from a register of the i2c slave device.
*
@@ -796,6 +824,15 @@ static int goodix_ts_probe(struct i2c_client *client,
struct goodix_ts_data *ts;
int error;
+#ifdef CONFIG_ACPI
+ struct dmi_system_id *dmi_match;
+
+ dmi_match = dmi_first_match(need_gpio_mapping);
+ if (dmi_match)
+ devm_acpi_dev_add_driver_gpios(&client->dev,
+ dmi_match->driver_data);
+#endif
+
dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
--
2.23.0
^ permalink raw reply related
* [PATCH 1/2] gpio: acpi: add quirk to override GpioInt polarity
From: Peter Cai @ 2019-08-30 0:00 UTC (permalink / raw)
Cc: Peter Cai, Mika Westerberg, Andy Shevchenko, Linus Walleij,
Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov,
Rafael J. Wysocki, Len Brown, linux-gpio, linux-acpi,
linux-kernel, linux-input
On GPD P2 Max, the firmware could not reset the touch panel correctly.
The kernel needs to take on the job instead, but the GpioInt definition
in DSDT specifies ActiveHigh while the GPIO pin should actually be
ActiveLow.
We need to override the polarity defined by DSDT. The GPIO driver
already allows defining polarity in acpi_gpio_params, but the option is
not applied to GpioInt.
This patch adds a new quirk that enables the polarity specified in
acpi_gpio_params to also be applied to GpioInt.
Signed-off-by: Peter Cai <peter@typeblog.net>
---
drivers/gpio/gpiolib-acpi.c | 10 +++++++++-
include/linux/acpi.h | 6 ++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 39f2f9035c11..1a07c79ca2de 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -583,13 +583,21 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
/*
* Polarity and triggering are only specified for GpioInt
* resource.
+ * Polarity specified by GpioInt may be ignored if
+ * ACPI_GPIO_QUIRK_OVERRIDE_POLARITY is set.
* Note: we expect here:
* - ACPI_ACTIVE_LOW == GPIO_ACTIVE_LOW
* - ACPI_ACTIVE_HIGH == GPIO_ACTIVE_HIGH
*/
if (lookup->info.gpioint) {
lookup->info.flags = GPIOD_IN;
- lookup->info.polarity = agpio->polarity;
+ if (lookup->info.quirks &
+ ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) {
+ dev_warn(&lookup->info.adev->dev, FW_BUG "Incorrect polarity specified by GpioInt, overriding.\n");
+ lookup->info.polarity = lookup->active_low;
+ } else {
+ lookup->info.polarity = agpio->polarity;
+ }
lookup->info.triggering = agpio->triggering;
} else {
lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 9426b9aaed86..6569773ceffd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1014,6 +1014,12 @@ struct acpi_gpio_mapping {
* get GpioIo type explicitly, this quirk may be used.
*/
#define ACPI_GPIO_QUIRK_ONLY_GPIOIO BIT(1)
+/*
+ * Use the GPIO polarity (ActiveHigh / ActiveLow) from acpi_gpio_params
+ * for GpioInt as well. The default behavior is to use the one specified
+ * by GpioInt, which can be incorrect on some devices.
+ */
+#define ACPI_GPIO_QUIRK_OVERRIDE_POLARITY BIT(2)
unsigned int quirks;
};
--
2.23.0
^ permalink raw reply related
* [PATCH RFC 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
A single controller can handle normal interrupts and wake-up interrupts
independently, with a different numbering space. It is thus crucial to
allow the driver for such a controller discriminate between the two.
A simple way to do so is to tag the wake-up irqdomain with a "bus token"
that indicates the wake-up domain. This slightly abuses the notion of
bus, but also radically simplifies the design of such a driver. Between
two evils, we choose the least damaging.
Suggested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
include/linux/irqdomain.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 07ec8b390161..cc846abeff28 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -83,6 +83,7 @@ enum irq_domain_bus_token {
DOMAIN_BUS_IPI,
DOMAIN_BUS_FSL_MC_MSI,
DOMAIN_BUS_TI_SCI_INTA_MSI,
+ DOMAIN_BUS_WAKEUP,
};
/**
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer, devicetree
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
In addition to configuring the PDC, additional registers that interface
the GIC have to be configured to match the GPIO type. The registers on
some QCOM SoCs are access restricted, while on other SoCs are not. They
SoCs with access restriction to these SPI registers need to be written
from the firmware using the SCM interface. Add a flag to indicate if the
register is to be written using SCM interface.
Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
.../bindings/interrupt-controller/qcom,pdc.txt | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
index 8e0797cb1487..852fcba98ea6 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
@@ -50,15 +50,22 @@ Properties:
The second element is the GIC hwirq number for the PDC port.
The third element is the number of interrupts in sequence.
+- qcom,scm-spi-cfg:
+ Usage: optional
+ Value type: <bool>
+ Definition: Specifies if the SPI configuration registers have to be
+ written from the firmware.
+
Example:
pdc: interrupt-controller@b220000 {
compatible = "qcom,sdm845-pdc";
- reg = <0xb220000 0x30000>;
+ reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
#interrupt-cells = <2>;
interrupt-parent = <&intc>;
interrupt-controller;
+ qcom,scm-spi-cfg;
};
DT binding of a device that wants to use the GIC SPI 514 as a wakeup
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 06/14] drivers: irqchip: pdc: additionally set type in SPI config registers
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
GPIOs that can be configured as wakeup are routed to the PDC wakeup
interrupt controller and from there to the GIC interrupt controller. On
some QCOM SoCs, the interface to the GIC for wakeup capable GPIOs have
additional hardware registers that need to be configured as well to
match the trigger type of the GPIO. This register interfaces the PDC to
the GIC and therefore updated from the PDC driver.
Typically, the firmware intializes the interface registers for the
wakeup capable GPIOs with commonly used GPIO trigger type, but it is
possible that a platform may want to use the GPIO differently. So, in
addition to configuring the PDC, configure the interface registers as
well.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
drivers/irqchip/qcom-pdc.c | 93 ++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index ad1faf634bcf..bf5f98bb4d2b 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -18,6 +18,8 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/qcom_scm.h>
+
#define PDC_MAX_IRQS 126
#define PDC_MAX_GPIO_IRQS 256
@@ -35,10 +37,20 @@ struct pdc_pin_region {
u32 cnt;
};
+struct spi_cfg_regs {
+ union {
+ u64 start;
+ void __iomem *base;
+ };
+ resource_size_t size;
+ bool scm_io;
+};
+
static DEFINE_RAW_SPINLOCK(pdc_lock);
static void __iomem *pdc_base;
static struct pdc_pin_region *pdc_region;
static int pdc_region_cnt;
+static struct spi_cfg_regs *spi_cfg;
static void pdc_reg_write(int reg, u32 i, u32 val)
{
@@ -100,6 +112,57 @@ static void qcom_pdc_gic_unmask(struct irq_data *d)
irq_chip_unmask_parent(d);
}
+static u32 __spi_pin_read(unsigned int pin)
+{
+ void __iomem *cfg_reg = spi_cfg->base + pin * 4;
+ u64 scm_cfg_reg = spi_cfg->start + pin * 4;
+
+ if (spi_cfg->scm_io) {
+ unsigned int val;
+
+ qcom_scm_io_readl(scm_cfg_reg, &val);
+ return val;
+ } else {
+ return readl(cfg_reg);
+ }
+}
+
+static void __spi_pin_write(unsigned int pin, unsigned int val)
+{
+ void __iomem *cfg_reg = spi_cfg->base + pin * 4;
+ u64 scm_cfg_reg = spi_cfg->start + pin * 4;
+
+ if (spi_cfg->scm_io)
+ qcom_scm_io_writel(scm_cfg_reg, val);
+ else
+ writel(val, cfg_reg);
+}
+
+static int spi_configure_type(irq_hw_number_t hwirq, unsigned int type)
+{
+ int spi = hwirq - 32;
+ u32 pin = spi / 32;
+ u32 mask = BIT(spi % 32);
+ u32 val;
+ unsigned long flags;
+
+ if (!spi_cfg)
+ return 0;
+
+ if (pin * 4 > spi_cfg->size)
+ return -EFAULT;
+
+ raw_spin_lock_irqsave(&pdc_lock, flags);
+ val = __spi_pin_read(pin);
+ val &= ~mask;
+ if (type & IRQ_TYPE_LEVEL_MASK)
+ val |= mask;
+ __spi_pin_write(pin, val);
+ raw_spin_unlock_irqrestore(&pdc_lock, flags);
+
+ return 0;
+}
+
/*
* GIC does not handle falling edge or active low. To allow falling edge and
* active low interrupts to be handled at GIC, PDC has an inverter that inverts
@@ -137,7 +200,9 @@ enum pdc_irq_config_bits {
static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
{
int pin_out = d->hwirq;
+ int parent_hwirq = d->parent_data->hwirq;
enum pdc_irq_config_bits pdc_type;
+ int ret;
if (pin_out == GPIO_NO_WAKE_IRQ)
return 0;
@@ -168,6 +233,11 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
+ /* Additionally, configure (only) the GPIO in the f/w */
+ ret = spi_configure_type(parent_hwirq, type);
+ if (ret)
+ return ret;
+
return irq_chip_set_type_parent(d, type);
}
@@ -354,6 +424,7 @@ static int pdc_setup_pin_mapping(struct device_node *np)
static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
{
struct irq_domain *parent_domain, *pdc_domain, *pdc_gpio_domain;
+ struct resource res;
int ret;
pdc_base = of_iomap(node, 0);
@@ -384,6 +455,27 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
goto fail;
}
+ ret = of_address_to_resource(node, 1, &res);
+ if (!ret) {
+ spi_cfg = kcalloc(1, sizeof(*spi_cfg), GFP_KERNEL);
+ if (!spi_cfg) {
+ ret = -ENOMEM;
+ goto remove;
+ }
+ spi_cfg->scm_io = of_find_property(node,
+ "qcom,scm-spi-cfg", NULL);
+ spi_cfg->size = resource_size(&res);
+ if (spi_cfg->scm_io) {
+ spi_cfg->start = res.start;
+ } else {
+ spi_cfg->base = ioremap(res.start, spi_cfg->size);
+ if (!spi_cfg->base) {
+ ret = -ENOMEM;
+ goto remove;
+ }
+ }
+ }
+
pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
PDC_MAX_GPIO_IRQS,
@@ -401,6 +493,7 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
remove:
irq_domain_remove(pdc_domain);
+ kfree(spi_cfg);
fail:
kfree(pdc_region);
iounmap(pdc_base);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
Some GPIOs are marked as wakeup capable and are routed to another
interrupt controller that is an always-domain and can detect interrupts
even most of the SoC is powered off. The wakeup interrupt controller
wakes up the GIC and replays the interrupt at the GIC.
Setup the TLMM irqchip in hierarchy with the wakeup interrupt controller
and ensure the wakeup GPIOs are handled correctly.
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 114 +++++++++++++++++++++++++++++
drivers/pinctrl/qcom/pinctrl-msm.h | 16 ++++
2 files changed, 130 insertions(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 76e8528e4d0a..d626264fe678 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -23,6 +23,8 @@
#include <linux/pm.h>
#include <linux/log2.h>
+#include <linux/soc/qcom/irq.h>
+
#include "../core.h"
#include "../pinconf.h"
#include "pinctrl-msm.h"
@@ -44,6 +46,7 @@
* @enabled_irqs: Bitmap of currently enabled irqs.
* @dual_edge_irqs: Bitmap of irqs that need sw emulated dual edge
* detection.
+ * @skip_wake_irqs: Skip IRQs that are handled by wakeup interrupt contrroller
* @soc; Reference to soc_data of platform specific data.
* @regs: Base addresses for the TLMM tiles.
*/
@@ -61,6 +64,7 @@ struct msm_pinctrl {
DECLARE_BITMAP(dual_edge_irqs, MAX_NR_GPIO);
DECLARE_BITMAP(enabled_irqs, MAX_NR_GPIO);
+ DECLARE_BITMAP(skip_wake_irqs, MAX_NR_GPIO);
const struct msm_pinctrl_soc_data *soc;
void __iomem *regs[MAX_NR_TILES];
@@ -708,6 +712,12 @@ static void msm_gpio_irq_mask(struct irq_data *d)
unsigned long flags;
u32 val;
+ if (d->parent_data)
+ irq_chip_mask_parent(d);
+
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return;
+
g = &pctrl->soc->groups[d->hwirq];
raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -752,6 +762,12 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear)
unsigned long flags;
u32 val;
+ if (d->parent_data)
+ irq_chip_unmask_parent(d);
+
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return;
+
g = &pctrl->soc->groups[d->hwirq];
raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -779,10 +795,43 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear)
static void msm_gpio_irq_enable(struct irq_data *d)
{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+
+ /*
+ * Clear the interrupt that may be pending before we enable
+ * the line.
+ * This is especially a problem with the GPIOs routed to the
+ * PDC. These GPIOs are direct-connect interrupts to the GIC.
+ * Disabling the interrupt line at the PDC does not prevent
+ * the interrupt from being latched at the GIC. The state at
+ * GIC needs to be cleared before enabling.
+ */
+ if (d->parent_data) {
+ irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, 0);
+ irq_chip_enable_parent(d);
+ }
+
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return;
msm_gpio_irq_clear_unmask(d, true);
}
+static void msm_gpio_irq_disable(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+
+ if (d->parent_data)
+ irq_chip_disable_parent(d);
+
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return;
+
+ msm_gpio_irq_mask(d);
+}
+
static void msm_gpio_irq_unmask(struct irq_data *d)
{
msm_gpio_irq_clear_unmask(d, false);
@@ -796,6 +845,9 @@ static void msm_gpio_irq_ack(struct irq_data *d)
unsigned long flags;
u32 val;
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return;
+
g = &pctrl->soc->groups[d->hwirq];
raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -821,6 +873,12 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
unsigned long flags;
u32 val;
+ if (d->parent_data)
+ irq_chip_set_type_parent(d, type);
+
+ if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+ return 0;
+
g = &pctrl->soc->groups[d->hwirq];
raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -913,6 +971,15 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
unsigned long flags;
+ if (d->parent_data)
+ irq_chip_set_wake_parent(d, on);
+
+ /*
+ * While they may not wake up when the TLMM is powered off,
+ * some GPIOs would like to wakeup the system from suspend
+ * when TLMM is powered on. To allow that, enable the GPIO
+ * summary line to be wakeup capable at GIC.
+ */
raw_spin_lock_irqsave(&pctrl->lock, flags);
irq_set_irq_wake(pctrl->irq, on);
@@ -991,6 +1058,30 @@ static void msm_gpio_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+static int msm_gpio_wakeirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
+{
+ struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+ const struct msm_gpio_wakeirq_map *map;
+ int i;
+
+ *parent = GPIO_NO_WAKE_IRQ;
+ *parent_type = IRQ_TYPE_EDGE_RISING;
+
+ for (i = 0; i < pctrl->soc->nwakeirq_map; i++) {
+ map = &pctrl->soc->wakeirq_map[i];
+ if (map->gpio == child) {
+ *parent = map->wakeirq;
+ break;
+ }
+ }
+
+ return 0;
+}
+
static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl)
{
if (pctrl->soc->reserved_gpios)
@@ -1004,6 +1095,7 @@ 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;
@@ -1019,14 +1111,36 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
pctrl->irq_chip.name = "msmgpio";
pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
+ pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
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;
+ pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
pctrl->irq_chip.irq_set_type = msm_gpio_irq_set_type;
pctrl->irq_chip.irq_set_wake = msm_gpio_irq_set_wake;
pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;
pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
+ dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
+ if (dn) {
+ int i;
+ bool skip;
+ unsigned int gpio;
+
+ 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->irq.child_to_parent_hwirq = msm_gpio_wakeirq;
+
+ skip = irq_domain_qcom_handle_wakeup(chip->irq.parent_domain);
+ for (i = 0; skip && i < pctrl->soc->nwakeirq_map; i++) {
+ gpio = pctrl->soc->wakeirq_map[i].gpio;
+ set_bit(gpio, pctrl->skip_wake_irqs);
+ }
+ }
+
chip->irq.chip = &pctrl->irq_chip;
chip->irq.default_type = IRQ_TYPE_NONE;
chip->irq.handler = handle_bad_irq;
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h
index 48569cda8471..15470203b446 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.h
+++ b/drivers/pinctrl/qcom/pinctrl-msm.h
@@ -5,6 +5,8 @@
#ifndef __PINCTRL_MSM_H__
#define __PINCTRL_MSM_H__
+#include <linux/gpio/driver.h>
+
struct pinctrl_pin_desc;
/**
@@ -91,6 +93,16 @@ struct msm_pingroup {
unsigned intr_detection_width:5;
};
+/**
+ * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins
+ * @gpio: The GPIOs that are wakeup capable
+ * @wakeirq: The interrupt at the always-on interrupt controller
+ */
+struct msm_gpio_wakeirq_map {
+ unsigned int gpio;
+ unsigned int wakeirq;
+};
+
/**
* struct msm_pinctrl_soc_data - Qualcomm pin controller driver configuration
* @pins: An array describing all pins the pin controller affects.
@@ -101,6 +113,8 @@ struct msm_pingroup {
* @ngroups: The numbmer of entries in @groups.
* @ngpio: The number of pingroups the driver should expose as GPIOs.
* @pull_no_keeper: The SoC does not support keeper bias.
+ * @wakeirq_map: The map of wakeup capable GPIOs and the pin at PDC/MPM
+ * @nwakeirq_map: The number of entries in @hierarchy_map
*/
struct msm_pinctrl_soc_data {
const struct pinctrl_pin_desc *pins;
@@ -114,6 +128,8 @@ struct msm_pinctrl_soc_data {
const char *const *tiles;
unsigned int ntiles;
const int *reserved_gpios;
+ const struct msm_gpio_wakeirq_map *wakeirq_map;
+ unsigned int nwakeirq_map;
};
extern const struct dev_pm_ops msm_pinctrl_dev_pm_ops;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 00/14] qcom: support wakeup capable GPIOs
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
This series is another attempt on adding wakeup capable GPIOs for QCOM
SoC. This patchset is based on Linus's support for hierarchical GPIOs
merged into linux-next [1]. The essense of the idea remains the same as
the previous submission [2]. GPIO irqchip TLMM is setup in hierarchy with
the PDC as the wakeup-parent. PDC's interrupt parent is the GIC. GPIOs
in QCOM SoC that are wakeup capable (when TLMM is powered off) are
routed to the PDC as well and can be detected at the always-on interrupt
controller (PDC). The idea is setup the irqchips in hierarchy and if the
interrupt is handled at the PDC, then TLMM relinquishes control and
configuration of the interrupt to the PDC.
There are few new additions in this submission. The first is the
additional SPI configuration that needs to be done to setup the GPIO
type in a register interface between the PDC and the GIC. This is needed
only for GPIOs. This registers in some QCOM SoCs is access restricted
and has to be written from the TZ. The DT bindings are also updated for
this new requirement. The second change is that with the new
hierarchical support in gpiolib, we could remove the .alloc and
.translate functions from the pinctrl driver. But to distinguish the
case where a wakeup interrupt controller needs the TLMM to configure the
GPIO interrupts (in the case of MPM interrupt controller), irqdomain
flags have been added. The third change is ensure the interrupt
controllers' interrupt pending bits are cleared when the GPIO is enabled
as an interrupt.
Please consider reviewing these patches.
Thanks,
Lina
Lina Iyer (12):
irqdomain: add bus token DOMAIN_BUS_WAKEUP
drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask
drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
of: irq: document properties for wakeup interrupt parent
dt-bindings/interrupt-controller: pdc: add SPI config register
drivers: irqchip: pdc: additionally set type in SPI config registers
drivers: pinctrl: msm: fix use of deprecated gpiolib APIs
drivers: pinctrl: msm: setup GPIO chip in hierarchy
drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs
arm64: dts: qcom: add PDC interrupt controller for SDM845
arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845
arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
Maulik Shah (2):
genirq: Introduce irq_chip_get/set_parent_state calls
drivers: irqchip: pdc: Add irqchip set/get state calls
.../interrupt-controller/interrupts.txt | 13 +
.../interrupt-controller/qcom,pdc.txt | 9 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 11 +
arch/arm64/configs/defconfig | 1 +
drivers/irqchip/qcom-pdc.c | 234 +++++++++++++++++-
drivers/pinctrl/qcom/pinctrl-msm.c | 142 +++++++++--
drivers/pinctrl/qcom/pinctrl-msm.h | 16 ++
drivers/pinctrl/qcom/pinctrl-sdm845.c | 83 ++++++-
include/linux/irq.h | 6 +
include/linux/irqdomain.h | 1 +
include/linux/soc/qcom/irq.h | 34 +++
kernel/irq/chip.c | 44 ++++
12 files changed, 566 insertions(+), 28 deletions(-)
create mode 100644 include/linux/soc/qcom/irq.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH RFC 13/14] arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845
From: Lina Iyer @ 2019-08-29 18:12 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
PDC always-on interrupt controller can detect certain GPIOs even when
the TLMM interrupt controller is powered off. Link the PDC as TLMM's
wakeup parent.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index ffe28b3e41d8..3002793ee688 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1358,6 +1358,7 @@
interrupt-controller;
#interrupt-cells = <2>;
gpio-ranges = <&tlmm 0 0 150>;
+ wakeup-parent = <&pdc_intc>;
qspi_clk: qspi-clk {
pinmux {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 04/14] of: irq: document properties for wakeup interrupt parent
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer, devicetree
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
Some interrupt controllers in a SoC, are always powered on and have a
select interrupts routed to them, so that they can wakeup the SoC from
suspend. Add wakeup-parent DT property to refer to these interrupt
controllers.
Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
.../bindings/interrupt-controller/interrupts.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
index 8a3c40829899..c10e31050dd2 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
@@ -108,3 +108,16 @@ commonly used:
sensitivity = <7>;
};
};
+
+3) Interrupt wakeup parent
+--------------------------
+
+Some interrupt controllers in a SoC, are always powered on and have a select
+interrupts routed to them, so that they can wakeup the SoC from suspend. These
+interrupt controllers do not fall into the category of a parent interrupt
+controller and can be specified by the "wakeup-parent" property and contain a
+single phandle referring to the wakeup capable interrupt controller.
+
+ Example:
+ wakeup-parent = <&pdc_intc>;
+
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 08/14] drivers: irqchip: pdc: Add irqchip set/get state calls
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
From: Maulik Shah <mkshah@codeaurora.org>
Add irqchip calls to set/get interrupt status from the parent interrupt
controller.
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
drivers/irqchip/qcom-pdc.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index bf5f98bb4d2b..ffd5f83d1023 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -5,6 +5,7 @@
#include <linux/err.h>
#include <linux/init.h>
+#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irqchip.h>
#include <linux/irqdomain.h>
@@ -87,6 +88,24 @@ static void qcom_pdc_gic_disable(struct irq_data *d)
irq_chip_disable_parent(d);
}
+static int qcom_pdc_gic_get_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which, bool *state)
+{
+ if (d->hwirq == GPIO_NO_WAKE_IRQ)
+ return 0;
+
+ return irq_chip_get_parent_state(d, which, state);
+}
+
+static int qcom_pdc_gic_set_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which, bool value)
+{
+ if (d->hwirq == GPIO_NO_WAKE_IRQ)
+ return 0;
+
+ return irq_chip_set_parent_state(d, which, value);
+}
+
static void qcom_pdc_gic_enable(struct irq_data *d)
{
if (d->hwirq == GPIO_NO_WAKE_IRQ)
@@ -248,6 +267,8 @@ static struct irq_chip qcom_pdc_gic_chip = {
.irq_unmask = qcom_pdc_gic_unmask,
.irq_disable = qcom_pdc_gic_disable,
.irq_enable = qcom_pdc_gic_enable,
+ .irq_get_irqchip_state = qcom_pdc_gic_get_irqchip_state,
+ .irq_set_irqchip_state = qcom_pdc_gic_set_irqchip_state,
.irq_retrigger = irq_chip_retrigger_hierarchy,
.irq_set_type = qcom_pdc_gic_set_type,
.flags = IRQCHIP_MASK_ON_SUSPEND |
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 07/14] genirq: Introduce irq_chip_get/set_parent_state calls
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
From: Maulik Shah <mkshah@codeaurora.org>
On certain QTI chipsets some GPIOs are direct-connect interrupts
to the GIC.
Even when GPIOs are not used for interrupt generation and interrupt
line is disabled, it does not prevent interrupt to get pending at
GIC_ISPEND. When drivers call enable_irq unwanted interrupt occures.
Introduce irq_chip_get/set_parent_state calls to clear pending irq
which can get called within irq_enable of child irq chip to clear
any pending irq before enabling.
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
include/linux/irq.h | 6 ++++++
kernel/irq/chip.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index fb301cf29148..7853eb9301f2 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -610,6 +610,12 @@ extern int irq_chip_pm_put(struct irq_data *data);
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
extern void handle_fasteoi_ack_irq(struct irq_desc *desc);
extern void handle_fasteoi_mask_irq(struct irq_desc *desc);
+extern int irq_chip_set_parent_state(struct irq_data *data,
+ enum irqchip_irq_state which,
+ bool val);
+extern int irq_chip_get_parent_state(struct irq_data *data,
+ enum irqchip_irq_state which,
+ bool *state);
extern void irq_chip_enable_parent(struct irq_data *data);
extern void irq_chip_disable_parent(struct irq_data *data);
extern void irq_chip_ack_parent(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b76703b2c0af..6bb5b22bb0a7 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1297,6 +1297,50 @@ EXPORT_SYMBOL_GPL(handle_fasteoi_mask_irq);
#endif /* CONFIG_IRQ_FASTEOI_HIERARCHY_HANDLERS */
+/**
+ * irq_chip_set_parent_state - set the state of a parent interrupt.
+ * @data: Pointer to interrupt specific data
+ * @which: State to be restored (one of IRQCHIP_STATE_*)
+ * @val: Value corresponding to @which
+ *
+ */
+int irq_chip_set_parent_state(struct irq_data *data,
+ enum irqchip_irq_state which,
+ bool val)
+{
+ data = data->parent_data;
+ if (!data)
+ return 0;
+
+ if (data->chip->irq_set_irqchip_state)
+ return data->chip->irq_set_irqchip_state(data, which, val);
+
+ return 0;
+}
+EXPORT_SYMBOL(irq_chip_set_parent_state);
+
+/**
+ * irq_chip_get_parent_state - get the state of a parent interrupt.
+ * @data: Pointer to interrupt specific data
+ * @which: one of IRQCHIP_STATE_* the caller wants to know
+ * @state: a pointer to a boolean where the state is to be stored
+ *
+ */
+int irq_chip_get_parent_state(struct irq_data *data,
+ enum irqchip_irq_state which,
+ bool *state)
+{
+ data = data->parent_data;
+ if (!data)
+ return 0;
+
+ if (data->chip->irq_get_irqchip_state)
+ return data->chip->irq_get_irqchip_state(data, which, state);
+
+ return 0;
+}
+EXPORT_SYMBOL(irq_chip_get_parent_state);
+
/**
* irq_chip_enable_parent - Enable the parent interrupt (defaults to unmask if
* NULL)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 09/14] drivers: pinctrl: msm: fix use of deprecated gpiolib APIs
From: Lina Iyer @ 2019-08-29 18:11 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
Replace gpiochip_irqchip_add() and gpiochip_set_chained_irqchip() calls
by populating the gpio_irq_chip data structures instead.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 7f35c196bb3e..76e8528e4d0a 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1027,7 +1027,19 @@ 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;
- ret = gpiochip_add_data(&pctrl->chip, pctrl);
+ chip->irq.chip = &pctrl->irq_chip;
+ chip->irq.default_type = IRQ_TYPE_NONE;
+ chip->irq.handler = handle_bad_irq;
+ chip->irq.fwnode = pctrl->dev->fwnode;
+ chip->irq.parent_handler = msm_gpio_irq_handler;
+ chip->irq.num_parents = 1;
+ chip->irq.parents = devm_kcalloc(pctrl->dev, 1,
+ sizeof(*chip->irq.parents),
+ GFP_KERNEL);
+ if (!chip->irq.parents)
+ return -ENOMEM;
+
+ ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
if (ret) {
dev_err(pctrl->dev, "Failed register gpiochip\n");
return ret;
@@ -1053,20 +1065,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
}
}
- 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;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 14/14] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
From: Lina Iyer @ 2019-08-29 18:12 UTC (permalink / raw)
To: swboyd, evgreen, marc.zyngier, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
rnayak, Lina Iyer
In-Reply-To: <20190829181203.2660-1-ilina@codeaurora.org>
Enable PDC interrupt controller for SDM845 devices. The interrupt
controller can detect wakeup capable interrupts when the SoC is in a low
power state.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0e58ef02880c..310b6048054a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -729,6 +729,7 @@ CONFIG_ARCH_R8A77970=y
CONFIG_ARCH_R8A77980=y
CONFIG_ARCH_R8A77990=y
CONFIG_ARCH_R8A77995=y
+CONFIG_QCOM_PDC=y
CONFIG_ROCKCHIP_PM_DOMAINS=y
CONFIG_ARCH_TEGRA_132_SOC=y
CONFIG_ARCH_TEGRA_210_SOC=y
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH AUTOSEL 5.2 23/76] gpio: Fix build error of function redefinition
From: Sasha Levin @ 2019-08-29 18:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: YueHaibing, Hulk Robot, Linus Walleij, Sasha Levin, linux-gpio
In-Reply-To: <20190829181311.7562-1-sashal@kernel.org>
From: YueHaibing <yuehaibing@huawei.com>
[ Upstream commit 68e03b85474a51ec1921b4d13204782594ef7223 ]
when do randbuilding, I got this error:
In file included from drivers/hwmon/pmbus/ucd9000.c:19:0:
./include/linux/gpio/driver.h:576:1: error: redefinition of gpiochip_add_pin_range
gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
^~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/hwmon/pmbus/ucd9000.c:18:0:
./include/linux/gpio.h:245:1: note: previous definition of gpiochip_add_pin_range was here
gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
^~~~~~~~~~~~~~~~~~~~~~
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 964cb341882f ("gpio: move pincontrol calls to <linux/gpio/driver.h>")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190731123814.46624-1-yuehaibing@huawei.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/linux/gpio.h | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 39745b8bdd65d..b3115d1a7d494 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -240,30 +240,6 @@ static inline int irq_to_gpio(unsigned irq)
return -EINVAL;
}
-static inline int
-gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
- unsigned int gpio_offset, unsigned int pin_offset,
- unsigned int npins)
-{
- WARN_ON(1);
- return -EINVAL;
-}
-
-static inline int
-gpiochip_add_pingroup_range(struct gpio_chip *chip,
- struct pinctrl_dev *pctldev,
- unsigned int gpio_offset, const char *pin_group)
-{
- WARN_ON(1);
- return -EINVAL;
-}
-
-static inline void
-gpiochip_remove_pin_ranges(struct gpio_chip *chip)
-{
- WARN_ON(1);
-}
-
static inline int devm_gpio_request(struct device *dev, unsigned gpio,
const char *label)
{
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox