diff mbox

[V9,3/3] irqchip: qcom: Add IRQ combiner driver

Message ID 1481753438-3905-4-git-send-email-agustinv@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Agustin Vega-Frias Dec. 14, 2016, 10:10 p.m. UTC 
Driver for interrupt combiners in the Top-level Control and Status
Registers (TCSR) hardware block in Qualcomm Technologies chips.

An interrupt combiner in this block combines a set of interrupts by
OR'ing the individual interrupt signals into a summary interrupt
signal routed to a parent interrupt controller, and provides read-
only, 32-bit registers to query the status of individual interrupts.
The status bit for IRQ n is bit (n % 32) within register (n / 32)
of the given combiner. Thus, each combiner can be described as a set
of register offsets and the number of IRQs managed.

Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
---
 drivers/irqchip/Kconfig             |   9 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/qcom-irq-combiner.c | 322 ++++++++++++++++++++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/irqchip/qcom-irq-combiner.c

Comments

Marc Zyngier Jan. 5, 2017, 4:48 p.m. UTC | #1 
Hi Agustin,

On 14/12/16 22:10, Agustin Vega-Frias wrote:
> Driver for interrupt combiners in the Top-level Control and Status
> Registers (TCSR) hardware block in Qualcomm Technologies chips.
> 
> An interrupt combiner in this block combines a set of interrupts by
> OR'ing the individual interrupt signals into a summary interrupt
> signal routed to a parent interrupt controller, and provides read-
> only, 32-bit registers to query the status of individual interrupts.
> The status bit for IRQ n is bit (n % 32) within register (n / 32)
> of the given combiner. Thus, each combiner can be described as a set
> of register offsets and the number of IRQs managed.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/irqchip/Kconfig             |   9 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/qcom-irq-combiner.c | 322 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 332 insertions(+)
>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index bc0af33..3e3430c 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -279,3 +279,12 @@ config EZNPS_GIC
>  config STM32_EXTI
>  	bool
>  	select IRQ_DOMAIN
> +
> +config QCOM_IRQ_COMBINER
> +	bool "QCOM IRQ combiner support"
> +	depends on ARCH_QCOM && ACPI
> +	select IRQ_DOMAIN
> +	select IRQ_DOMAIN_HIERARCHY
> +	help
> +	  Say yes here to add support for the IRQ combiner devices embedded
> +	  in Qualcomm Technologies chips.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e4dbfc8..1818a0b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
>  obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
>  obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
>  obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
> +obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
> diff --git a/drivers/irqchip/qcom-irq-combiner.c b/drivers/irqchip/qcom-irq-combiner.c
> new file mode 100644
> index 0000000..0055e08
> --- /dev/null
> +++ b/drivers/irqchip/qcom-irq-combiner.c
> @@ -0,0 +1,322 @@
> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/*
> + * Driver for interrupt combiners in the Top-level Control and Status
> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
> + * An interrupt combiner in this block combines a set of interrupts by
> + * OR'ing the individual interrupt signals into a summary interrupt
> + * signal routed to a parent interrupt controller, and provides read-
> + * only, 32-bit registers to query the status of individual interrupts.
> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
> + * of the given combiner. Thus, each combiner can be described as a set
> + * of register offsets and the number of IRQs managed.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/platform_device.h>
> +
> +#define REG_SIZE 32
> +
> +struct combiner_reg {
> +	void __iomem *addr;
> +	unsigned long mask;
> +};
> +
> +struct combiner {
> +	struct irq_domain   *domain;
> +	int                 parent_irq;
> +	u32                 nirqs;
> +	u32                 nregs;
> +	struct combiner_reg regs[0];
> +};
> +
> +static inline u32 irq_register(int irq)
> +{
> +	return irq / REG_SIZE;
> +}
> +
> +static inline u32 irq_bit(int irq)
> +{
> +	return irq % REG_SIZE;
> +
> +}
> +
> +static inline int irq_nr(u32 reg, u32 bit)
> +{
> +	return reg * REG_SIZE + bit;
> +}
> +
> +/*
> + * Handler for the cascaded IRQ.
> + */
> +static void combiner_handle_irq(struct irq_desc *desc)
> +{
> +	struct combiner *combiner = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	u32 reg;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	for (reg = 0; reg < combiner->nregs; reg++) {
> +		int virq;
> +		int hwirq;
> +		u32 bit;
> +		u32 status;
> +
> +		if (combiner->regs[reg].mask == 0)
> +			continue;

I'm a bit worried by this. If I understand it well, this is a pure
software construct (controlled from combiner_irq_chip_{un,}mask_irq) and
there is nothing that actually masks the interrupt at the HW level.

So if a device asserts its interrupt line, what mechanism do we have to
make sure that we don't end-up with the CPU pegged in interrupt context?

> +
> +		status = readl_relaxed(combiner->regs[reg].addr);
> +		status &= combiner->regs[reg].mask;
> +
> +		while (status) {
> +			bit = __ffs(status);
> +			status &= ~(1 << bit);
> +			hwirq = irq_nr(reg, bit);
> +			virq = irq_find_mapping(combiner->domain, hwirq);
> +			if (virq >= 0)

Small bug: virq == 0 shouldn't happen, since this would be an indication
that the hwirq doesn't have a mapping.

> +				generic_handle_irq(virq);
> +
> +		}
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * irqchip callbacks
> + */
> +
> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
> +
> +	clear_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
> +
> +	set_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +static struct irq_chip irq_chip = {
> +	.irq_mask = combiner_irq_chip_mask_irq,
> +	.irq_unmask = combiner_irq_chip_unmask_irq,
> +	.name = "qcom-irq-combiner"
> +};
> +
> +/*
> + * irq_domain_ops callbacks
> + */
> +
> +static int combiner_irq_map(struct irq_domain *domain, unsigned int irq,
> +				   irq_hw_number_t hwirq)
> +{
> +	struct combiner *combiner = domain->host_data;
> +
> +	if (hwirq >= combiner->nirqs)
> +		return -EINVAL;

Given that this should have come from the translate function, can we
move the check there instead, so that we validate everything early?

> +
> +	irq_set_chip_and_handler(irq, &irq_chip, handle_level_irq);
> +	irq_set_chip_data(irq, combiner);
> +	irq_set_noprobe(irq);
> +	return 0;
> +}
> +
> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned int irq)
> +{
> +	struct irq_data *data = irq_get_irq_data(irq);
> +
> +	if (WARN_ON(!data))
> +		return;

Can this happen?

> +	irq_domain_reset_irq_data(data);
> +}
> +
> +static int combiner_irq_translate(struct irq_domain *d, struct irq_fwspec *fws,
> +				  unsigned long *hwirq, unsigned int *type)
> +{
> +	if (is_acpi_node(fws->fwnode)) {
> +		if (WARN_ON((fws->param_count != 2) ||
> +			    (fws->param[1] & IORESOURCE_IRQ_LOWEDGE) ||
> +			    (fws->param[1] & IORESOURCE_IRQ_HIGHEDGE)))
> +			return -EINVAL;
> +
> +		*hwirq = fws->param[0];
> +		*type = fws->param[1];
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct irq_domain_ops domain_ops = {
> +	.map = combiner_irq_map,
> +	.unmap = combiner_irq_unmap,
> +	.translate = combiner_irq_translate
> +};
> +
> +/*
> + * Device probing
> + */
> +
> +static acpi_status count_registers_cb(struct acpi_resource *ares, void *context)
> +{
> +	int *count = context;
> +
> +	if (ares->type == ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
> +		++(*count);
> +	return AE_OK;
> +}
> +
> +static int count_registers(struct platform_device *pdev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> +	acpi_status status;
> +	int count = 0;
> +
> +	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
> +		return -EINVAL;
> +
> +	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> +				     count_registers_cb, &count);
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;
> +	return count;
> +}
> +
> +struct get_registers_context {
> +	struct device *dev;
> +	struct combiner *combiner;
> +	int err;
> +};
> +
> +static acpi_status get_registers_cb(struct acpi_resource *ares, void *context)
> +{
> +	struct get_registers_context *ctx = context;
> +	struct acpi_resource_generic_register *reg;
> +	phys_addr_t paddr;
> +	void __iomem *vaddr;
> +
> +	if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
> +		return AE_OK;
> +
> +	reg = &ares->data.generic_reg;
> +	paddr = reg->address;
> +	if ((reg->space_id != ACPI_SPACE_MEM) ||
> +	    (reg->bit_offset != 0) ||
> +	    (reg->bit_width > REG_SIZE)) {
> +		dev_err(ctx->dev, "Bad register resource @%pa\n", &paddr);
> +		ctx->err = -EINVAL;
> +		return AE_ERROR;
> +	}
> +
> +	vaddr = devm_ioremap(ctx->dev, reg->address, REG_SIZE);
> +	if (IS_ERR(vaddr)) {
> +		dev_err(ctx->dev, "Can't map register @%pa\n", &paddr);
> +		ctx->err = PTR_ERR(vaddr);
> +		return AE_ERROR;
> +	}
> +
> +	ctx->combiner->regs[ctx->combiner->nregs].addr = vaddr;
> +	ctx->combiner->nirqs += reg->bit_width;
> +	ctx->combiner->nregs++;
> +	return AE_OK;
> +}
> +
> +static int get_registers(struct platform_device *pdev, struct combiner *comb)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> +	acpi_status status;
> +	struct get_registers_context ctx;
> +
> +	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
> +		return -EINVAL;
> +
> +	ctx.dev = &pdev->dev;
> +	ctx.combiner = comb;
> +	ctx.err = 0;
> +
> +	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> +				     get_registers_cb, &ctx);
> +	if (ACPI_FAILURE(status))
> +		return ctx.err;
> +	return 0;
> +}
> +
> +static int __init combiner_probe(struct platform_device *pdev)
> +{
> +	struct combiner *combiner;
> +	size_t alloc_sz;
> +	u32 nregs;
> +	int err;
> +
> +	nregs = count_registers(pdev);
> +	if (nregs <= 0) {
> +		dev_err(&pdev->dev, "Error reading register resources\n");
> +		return -EINVAL;
> +	}
> +
> +	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
> +	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
> +	if (!combiner)
> +		return -ENOMEM;
> +
> +	err = get_registers(pdev, combiner);
> +	if (err < 0)
> +		return err;
> +
> +	combiner->parent_irq = platform_get_irq(pdev, 0);
> +	if (combiner->parent_irq <= 0) {
> +		dev_err(&pdev->dev, "Error getting IRQ resource\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, combiner->nirqs,
> +						    &domain_ops, combiner);
> +	if (!combiner->domain)
> +		/* Errors printed by irq_domain_create_linear */
> +		return -ENODEV;
> +
> +	irq_set_chained_handler_and_data(combiner->parent_irq,
> +					 combiner_handle_irq, combiner);
> +
> +	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
> +		 combiner->parent_irq, combiner->nirqs, combiner->regs[0].addr);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
> +	{ "QCOM80B1", },
> +	{ }
> +};
> +
> +static struct platform_driver qcom_irq_combiner_probe = {
> +	.driver = {
> +		.name = "qcom-irq-combiner",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
> +	},
> +	.probe = combiner_probe,
> +};
> +
> +static int __init register_qcom_irq_combiner(void)
> +{
> +	return platform_driver_register(&qcom_irq_combiner_probe);
> +}
> +device_initcall(register_qcom_irq_combiner);
> 

Other than the questions I have above, this now looks in a much better
shape. Hopefully Rafael will soon come back will his conclusions on the
first two patches, as I'd like to see this code to get some -next for a
while.

Thanks,

	M.
Agustin Vega-Frias Jan. 6, 2017, 1:17 p.m. UTC | #2 
Hey Marc,

On 2017-01-05 11:48, Marc Zyngier wrote:
> Hi Agustin,
> 
> On 14/12/16 22:10, Agustin Vega-Frias wrote:
>> Driver for interrupt combiners in the Top-level Control and Status
>> Registers (TCSR) hardware block in Qualcomm Technologies chips.
>> 
>> An interrupt combiner in this block combines a set of interrupts by
>> OR'ing the individual interrupt signals into a summary interrupt
>> signal routed to a parent interrupt controller, and provides read-
>> only, 32-bit registers to query the status of individual interrupts.
>> The status bit for IRQ n is bit (n % 32) within register (n / 32)
>> of the given combiner. Thus, each combiner can be described as a set
>> of register offsets and the number of IRQs managed.
>> 
>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>> ---
>>  drivers/irqchip/Kconfig             |   9 +
>>  drivers/irqchip/Makefile            |   1 +
>>  drivers/irqchip/qcom-irq-combiner.c | 322 
>> ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 332 insertions(+)
>>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
>> 
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index bc0af33..3e3430c 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -279,3 +279,12 @@ config EZNPS_GIC
>>  config STM32_EXTI
>>  	bool
>>  	select IRQ_DOMAIN
>> +
>> +config QCOM_IRQ_COMBINER
>> +	bool "QCOM IRQ combiner support"
>> +	depends on ARCH_QCOM && ACPI
>> +	select IRQ_DOMAIN
>> +	select IRQ_DOMAIN_HIERARCHY
>> +	help
>> +	  Say yes here to add support for the IRQ combiner devices embedded
>> +	  in Qualcomm Technologies chips.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index e4dbfc8..1818a0b 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
>>  obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
>>  obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
>>  obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
>> +obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
>> diff --git a/drivers/irqchip/qcom-irq-combiner.c 
>> b/drivers/irqchip/qcom-irq-combiner.c
>> new file mode 100644
>> index 0000000..0055e08
>> --- /dev/null
>> +++ b/drivers/irqchip/qcom-irq-combiner.c
>> @@ -0,0 +1,322 @@
>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights 
>> reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or 
>> modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +/*
>> + * Driver for interrupt combiners in the Top-level Control and Status
>> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
>> + * An interrupt combiner in this block combines a set of interrupts 
>> by
>> + * OR'ing the individual interrupt signals into a summary interrupt
>> + * signal routed to a parent interrupt controller, and provides read-
>> + * only, 32-bit registers to query the status of individual 
>> interrupts.
>> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
>> + * of the given combiner. Thus, each combiner can be described as a 
>> set
>> + * of register offsets and the number of IRQs managed.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define REG_SIZE 32
>> +
>> +struct combiner_reg {
>> +	void __iomem *addr;
>> +	unsigned long mask;
>> +};
>> +
>> +struct combiner {
>> +	struct irq_domain   *domain;
>> +	int                 parent_irq;
>> +	u32                 nirqs;
>> +	u32                 nregs;
>> +	struct combiner_reg regs[0];
>> +};
>> +
>> +static inline u32 irq_register(int irq)
>> +{
>> +	return irq / REG_SIZE;
>> +}
>> +
>> +static inline u32 irq_bit(int irq)
>> +{
>> +	return irq % REG_SIZE;
>> +
>> +}
>> +
>> +static inline int irq_nr(u32 reg, u32 bit)
>> +{
>> +	return reg * REG_SIZE + bit;
>> +}
>> +
>> +/*
>> + * Handler for the cascaded IRQ.
>> + */
>> +static void combiner_handle_irq(struct irq_desc *desc)
>> +{
>> +	struct combiner *combiner = irq_desc_get_handler_data(desc);
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	u32 reg;
>> +
>> +	chained_irq_enter(chip, desc);
>> +
>> +	for (reg = 0; reg < combiner->nregs; reg++) {
>> +		int virq;
>> +		int hwirq;
>> +		u32 bit;
>> +		u32 status;
>> +
>> +		if (combiner->regs[reg].mask == 0)
>> +			continue;
> 
> I'm a bit worried by this. If I understand it well, this is a pure
> software construct (controlled from combiner_irq_chip_{un,}mask_irq) 
> and
> there is nothing that actually masks the interrupt at the HW level.
> 
> So if a device asserts its interrupt line, what mechanism do we have to
> make sure that we don't end-up with the CPU pegged in interrupt 
> context?
> 

Yes, unfortunately this is a dumb hardware combiner; however, the real 
use
of mask here is to optimize IRQ handling if the combiner instance has 
its
IRQ statuses across more than one register. Currently all active 
instances
only use one register, but we are getting close to 32 in one case, so I
wanted to accommodate a general case where an instance can combine more
than 32 IRQs.

Having said that, what I'm inclined to do is to remove the use of mask
on the status read form the register on the next two lines, and then let
irq_find_mapping fail if we are getting an unexpected IRQ, then call
handle_bad_irq(desc).

Do you have any other suggestions to handle the scenario? E.g., can we
safely disable the parent IRQ from this context if we see too many
spurious interrupts?

>> +
>> +		status = readl_relaxed(combiner->regs[reg].addr);
>> +		status &= combiner->regs[reg].mask;
>> +
>> +		while (status) {
>> +			bit = __ffs(status);
>> +			status &= ~(1 << bit);
>> +			hwirq = irq_nr(reg, bit);
>> +			virq = irq_find_mapping(combiner->domain, hwirq);
>> +			if (virq >= 0)
> 
> Small bug: virq == 0 shouldn't happen, since this would be an 
> indication
> that the hwirq doesn't have a mapping.
> 

Will fix this on V10.

>> +				generic_handle_irq(virq);
>> +
>> +		}
>> +	}
>> +
>> +	chained_irq_exit(chip, desc);
>> +}
>> +
>> +/*
>> + * irqchip callbacks
>> + */
>> +
>> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
>> +{
>> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
>> +	struct combiner_reg *reg = combiner->regs + 
>> irq_register(data->hwirq);
>> +
>> +	clear_bit(irq_bit(data->hwirq), &reg->mask);
>> +}
>> +
>> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
>> +{
>> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
>> +	struct combiner_reg *reg = combiner->regs + 
>> irq_register(data->hwirq);
>> +
>> +	set_bit(irq_bit(data->hwirq), &reg->mask);
>> +}
>> +
>> +static struct irq_chip irq_chip = {
>> +	.irq_mask = combiner_irq_chip_mask_irq,
>> +	.irq_unmask = combiner_irq_chip_unmask_irq,
>> +	.name = "qcom-irq-combiner"
>> +};
>> +
>> +/*
>> + * irq_domain_ops callbacks
>> + */
>> +
>> +static int combiner_irq_map(struct irq_domain *domain, unsigned int 
>> irq,
>> +				   irq_hw_number_t hwirq)
>> +{
>> +	struct combiner *combiner = domain->host_data;
>> +
>> +	if (hwirq >= combiner->nirqs)
>> +		return -EINVAL;
> 
> Given that this should have come from the translate function, can we
> move the check there instead, so that we validate everything early?
> 

Will fix this on V10.

>> +
>> +	irq_set_chip_and_handler(irq, &irq_chip, handle_level_irq);
>> +	irq_set_chip_data(irq, combiner);
>> +	irq_set_noprobe(irq);
>> +	return 0;
>> +}
>> +
>> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned 
>> int irq)
>> +{
>> +	struct irq_data *data = irq_get_irq_data(irq);
>> +
>> +	if (WARN_ON(!data))
>> +		return;
> 
> Can this happen?
> 

I will remove this check on V10 given that irq_domain_disassociate 
already
has a check for a NULL irq_data.

>> +	irq_domain_reset_irq_data(data);
>> +}
>> +
>> +static int combiner_irq_translate(struct irq_domain *d, struct 
>> irq_fwspec *fws,
>> +				  unsigned long *hwirq, unsigned int *type)
>> +{
>> +	if (is_acpi_node(fws->fwnode)) {
>> +		if (WARN_ON((fws->param_count != 2) ||
>> +			    (fws->param[1] & IORESOURCE_IRQ_LOWEDGE) ||
>> +			    (fws->param[1] & IORESOURCE_IRQ_HIGHEDGE)))
>> +			return -EINVAL;
>> +
>> +		*hwirq = fws->param[0];
>> +		*type = fws->param[1];
>> +		return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct irq_domain_ops domain_ops = {
>> +	.map = combiner_irq_map,
>> +	.unmap = combiner_irq_unmap,
>> +	.translate = combiner_irq_translate
>> +};
>> +
>> +/*
>> + * Device probing
>> + */
>> +
>> +static acpi_status count_registers_cb(struct acpi_resource *ares, 
>> void *context)
>> +{
>> +	int *count = context;
>> +
>> +	if (ares->type == ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
>> +		++(*count);
>> +	return AE_OK;
>> +}
>> +
>> +static int count_registers(struct platform_device *pdev)
>> +{
>> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> +	acpi_status status;
>> +	int count = 0;
>> +
>> +	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
>> +		return -EINVAL;
>> +
>> +	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>> +				     count_registers_cb, &count);
>> +	if (ACPI_FAILURE(status))
>> +		return -EINVAL;
>> +	return count;
>> +}
>> +
>> +struct get_registers_context {
>> +	struct device *dev;
>> +	struct combiner *combiner;
>> +	int err;
>> +};
>> +
>> +static acpi_status get_registers_cb(struct acpi_resource *ares, void 
>> *context)
>> +{
>> +	struct get_registers_context *ctx = context;
>> +	struct acpi_resource_generic_register *reg;
>> +	phys_addr_t paddr;
>> +	void __iomem *vaddr;
>> +
>> +	if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
>> +		return AE_OK;
>> +
>> +	reg = &ares->data.generic_reg;
>> +	paddr = reg->address;
>> +	if ((reg->space_id != ACPI_SPACE_MEM) ||
>> +	    (reg->bit_offset != 0) ||
>> +	    (reg->bit_width > REG_SIZE)) {
>> +		dev_err(ctx->dev, "Bad register resource @%pa\n", &paddr);
>> +		ctx->err = -EINVAL;
>> +		return AE_ERROR;
>> +	}
>> +
>> +	vaddr = devm_ioremap(ctx->dev, reg->address, REG_SIZE);
>> +	if (IS_ERR(vaddr)) {
>> +		dev_err(ctx->dev, "Can't map register @%pa\n", &paddr);
>> +		ctx->err = PTR_ERR(vaddr);
>> +		return AE_ERROR;
>> +	}
>> +
>> +	ctx->combiner->regs[ctx->combiner->nregs].addr = vaddr;
>> +	ctx->combiner->nirqs += reg->bit_width;
>> +	ctx->combiner->nregs++;
>> +	return AE_OK;
>> +}
>> +
>> +static int get_registers(struct platform_device *pdev, struct 
>> combiner *comb)
>> +{
>> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> +	acpi_status status;
>> +	struct get_registers_context ctx;
>> +
>> +	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
>> +		return -EINVAL;
>> +
>> +	ctx.dev = &pdev->dev;
>> +	ctx.combiner = comb;
>> +	ctx.err = 0;
>> +
>> +	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>> +				     get_registers_cb, &ctx);
>> +	if (ACPI_FAILURE(status))
>> +		return ctx.err;
>> +	return 0;
>> +}
>> +
>> +static int __init combiner_probe(struct platform_device *pdev)
>> +{
>> +	struct combiner *combiner;
>> +	size_t alloc_sz;
>> +	u32 nregs;
>> +	int err;
>> +
>> +	nregs = count_registers(pdev);
>> +	if (nregs <= 0) {
>> +		dev_err(&pdev->dev, "Error reading register resources\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
>> +	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
>> +	if (!combiner)
>> +		return -ENOMEM;
>> +
>> +	err = get_registers(pdev, combiner);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	combiner->parent_irq = platform_get_irq(pdev, 0);
>> +	if (combiner->parent_irq <= 0) {
>> +		dev_err(&pdev->dev, "Error getting IRQ resource\n");
>> +		return -EPROBE_DEFER;
>> +	}
>> +
>> +	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, 
>> combiner->nirqs,
>> +						    &domain_ops, combiner);
>> +	if (!combiner->domain)
>> +		/* Errors printed by irq_domain_create_linear */
>> +		return -ENODEV;
>> +
>> +	irq_set_chained_handler_and_data(combiner->parent_irq,
>> +					 combiner_handle_irq, combiner);
>> +
>> +	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
>> +		 combiner->parent_irq, combiner->nirqs, combiner->regs[0].addr);
>> +	return 0;
>> +}
>> +
>> +static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
>> +	{ "QCOM80B1", },
>> +	{ }
>> +};
>> +
>> +static struct platform_driver qcom_irq_combiner_probe = {
>> +	.driver = {
>> +		.name = "qcom-irq-combiner",
>> +		.owner = THIS_MODULE,
>> +		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
>> +	},
>> +	.probe = combiner_probe,
>> +};
>> +
>> +static int __init register_qcom_irq_combiner(void)
>> +{
>> +	return platform_driver_register(&qcom_irq_combiner_probe);
>> +}
>> +device_initcall(register_qcom_irq_combiner);
>> 
> 
> Other than the questions I have above, this now looks in a much better
> shape. Hopefully Rafael will soon come back will his conclusions on the
> first two patches, as I'd like to see this code to get some -next for a
> while.

Yes, hopefully we can get to that in the following weeks.

Thanks,
Agustin

> 
> Thanks,
> 
> 	M.
> --
> Jazz is not dead. It just smells funny...
Marc Zyngier Jan. 9, 2017, 3:25 p.m. UTC | #3 
On 06/01/17 13:17, Agustin Vega-Frias wrote:
> Hey Marc,
> 
> On 2017-01-05 11:48, Marc Zyngier wrote:
>> Hi Agustin,
>>
>> On 14/12/16 22:10, Agustin Vega-Frias wrote:
>>> Driver for interrupt combiners in the Top-level Control and Status
>>> Registers (TCSR) hardware block in Qualcomm Technologies chips.
>>>
>>> An interrupt combiner in this block combines a set of interrupts by
>>> OR'ing the individual interrupt signals into a summary interrupt
>>> signal routed to a parent interrupt controller, and provides read-
>>> only, 32-bit registers to query the status of individual interrupts.
>>> The status bit for IRQ n is bit (n % 32) within register (n / 32)
>>> of the given combiner. Thus, each combiner can be described as a set
>>> of register offsets and the number of IRQs managed.
>>>
>>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>>> ---
>>>  drivers/irqchip/Kconfig             |   9 +
>>>  drivers/irqchip/Makefile            |   1 +
>>>  drivers/irqchip/qcom-irq-combiner.c | 322 
>>> ++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 332 insertions(+)
>>>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
>>>
>>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>> index bc0af33..3e3430c 100644
>>> --- a/drivers/irqchip/Kconfig
>>> +++ b/drivers/irqchip/Kconfig
>>> @@ -279,3 +279,12 @@ config EZNPS_GIC
>>>  config STM32_EXTI
>>>  	bool
>>>  	select IRQ_DOMAIN
>>> +
>>> +config QCOM_IRQ_COMBINER
>>> +	bool "QCOM IRQ combiner support"
>>> +	depends on ARCH_QCOM && ACPI
>>> +	select IRQ_DOMAIN
>>> +	select IRQ_DOMAIN_HIERARCHY
>>> +	help
>>> +	  Say yes here to add support for the IRQ combiner devices embedded
>>> +	  in Qualcomm Technologies chips.
>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>> index e4dbfc8..1818a0b 100644
>>> --- a/drivers/irqchip/Makefile
>>> +++ b/drivers/irqchip/Makefile
>>> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
>>>  obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
>>>  obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
>>>  obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
>>> +obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
>>> diff --git a/drivers/irqchip/qcom-irq-combiner.c 
>>> b/drivers/irqchip/qcom-irq-combiner.c
>>> new file mode 100644
>>> index 0000000..0055e08
>>> --- /dev/null
>>> +++ b/drivers/irqchip/qcom-irq-combiner.c
>>> @@ -0,0 +1,322 @@
>>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights 
>>> reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or 
>>> modify
>>> + * it under the terms of the GNU General Public License version 2 and
>>> + * only version 2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +/*
>>> + * Driver for interrupt combiners in the Top-level Control and Status
>>> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
>>> + * An interrupt combiner in this block combines a set of interrupts 
>>> by
>>> + * OR'ing the individual interrupt signals into a summary interrupt
>>> + * signal routed to a parent interrupt controller, and provides read-
>>> + * only, 32-bit registers to query the status of individual 
>>> interrupts.
>>> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
>>> + * of the given combiner. Thus, each combiner can be described as a 
>>> set
>>> + * of register offsets and the number of IRQs managed.
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/irqchip/chained_irq.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define REG_SIZE 32
>>> +
>>> +struct combiner_reg {
>>> +	void __iomem *addr;
>>> +	unsigned long mask;
>>> +};
>>> +
>>> +struct combiner {
>>> +	struct irq_domain   *domain;
>>> +	int                 parent_irq;
>>> +	u32                 nirqs;
>>> +	u32                 nregs;
>>> +	struct combiner_reg regs[0];
>>> +};
>>> +
>>> +static inline u32 irq_register(int irq)
>>> +{
>>> +	return irq / REG_SIZE;
>>> +}
>>> +
>>> +static inline u32 irq_bit(int irq)
>>> +{
>>> +	return irq % REG_SIZE;
>>> +
>>> +}
>>> +
>>> +static inline int irq_nr(u32 reg, u32 bit)
>>> +{
>>> +	return reg * REG_SIZE + bit;
>>> +}
>>> +
>>> +/*
>>> + * Handler for the cascaded IRQ.
>>> + */
>>> +static void combiner_handle_irq(struct irq_desc *desc)
>>> +{
>>> +	struct combiner *combiner = irq_desc_get_handler_data(desc);
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	u32 reg;
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +
>>> +	for (reg = 0; reg < combiner->nregs; reg++) {
>>> +		int virq;
>>> +		int hwirq;
>>> +		u32 bit;
>>> +		u32 status;
>>> +
>>> +		if (combiner->regs[reg].mask == 0)
>>> +			continue;
>>
>> I'm a bit worried by this. If I understand it well, this is a pure
>> software construct (controlled from combiner_irq_chip_{un,}mask_irq) 
>> and
>> there is nothing that actually masks the interrupt at the HW level.
>>
>> So if a device asserts its interrupt line, what mechanism do we have to
>> make sure that we don't end-up with the CPU pegged in interrupt 
>> context?
>>
> 
> Yes, unfortunately this is a dumb hardware combiner; however, the
> real use of mask here is to optimize IRQ handling if the combiner
> instance has its IRQ statuses across more than one register.
> Currently all active instances only use one register, but we are
> getting close to 32 in one case, so I wanted to accommodate a general
> case where an instance can combine more than 32 IRQs.
> 
> Having said that, what I'm inclined to do is to remove the use of mask
> on the status read form the register on the next two lines, and then let
> irq_find_mapping fail if we are getting an unexpected IRQ, then call
> handle_bad_irq(desc).

Unfortunately, having a mapping in place is not an indication that
someone can handle the interrupt. Also, you still need to handle
mask/unmask, and I don't see how you manage that, given that this
interrupt combiner seems to be a glorified OR gate.

> Do you have any other suggestions to handle the scenario? E.g., can we
> safely disable the parent IRQ from this context if we see too many
> spurious interrupts?

I don't think so. There is two issues here:
- If a device is stuck with an active interrupt, you won't be able to
reliably detect that this is an invalid condition (the mapping trick
above is not reliable, and generic_handle_irq doesn't return anything
useful)
- Even if you could detect it, what do you do? Killing the cascade
interrupt would also kill all the other devices. Is that something
acceptable in your setup? Can you implement mask/unmask the same way?

Thanks,

	M.
diff mbox

Patch

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bc0af33..3e3430c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -279,3 +279,12 @@  config EZNPS_GIC
 config STM32_EXTI
 	bool
 	select IRQ_DOMAIN
+
+config QCOM_IRQ_COMBINER
+	bool "QCOM IRQ combiner support"
+	depends on ARCH_QCOM && ACPI
+	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  Say yes here to add support for the IRQ combiner devices embedded
+	  in Qualcomm Technologies chips.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc8..1818a0b 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -74,3 +74,4 @@  obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
 obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
+obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
diff --git a/drivers/irqchip/qcom-irq-combiner.c b/drivers/irqchip/qcom-irq-combiner.c
new file mode 100644
index 0000000..0055e08
--- /dev/null
+++ b/drivers/irqchip/qcom-irq-combiner.c
@@ -0,0 +1,322 @@ 
+/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * Driver for interrupt combiners in the Top-level Control and Status
+ * Registers (TCSR) hardware block in Qualcomm Technologies chips.
+ * An interrupt combiner in this block combines a set of interrupts by
+ * OR'ing the individual interrupt signals into a summary interrupt
+ * signal routed to a parent interrupt controller, and provides read-
+ * only, 32-bit registers to query the status of individual interrupts.
+ * The status bit for IRQ n is bit (n % 32) within register (n / 32)
+ * of the given combiner. Thus, each combiner can be described as a set
+ * of register offsets and the number of IRQs managed.
+ */
+
+#include <linux/acpi.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/platform_device.h>
+
+#define REG_SIZE 32
+
+struct combiner_reg {
+	void __iomem *addr;
+	unsigned long mask;
+};
+
+struct combiner {
+	struct irq_domain   *domain;
+	int                 parent_irq;
+	u32                 nirqs;
+	u32                 nregs;
+	struct combiner_reg regs[0];
+};
+
+static inline u32 irq_register(int irq)
+{
+	return irq / REG_SIZE;
+}
+
+static inline u32 irq_bit(int irq)
+{
+	return irq % REG_SIZE;
+
+}
+
+static inline int irq_nr(u32 reg, u32 bit)
+{
+	return reg * REG_SIZE + bit;
+}
+
+/*
+ * Handler for the cascaded IRQ.
+ */
+static void combiner_handle_irq(struct irq_desc *desc)
+{
+	struct combiner *combiner = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 reg;
+
+	chained_irq_enter(chip, desc);
+
+	for (reg = 0; reg < combiner->nregs; reg++) {
+		int virq;
+		int hwirq;
+		u32 bit;
+		u32 status;
+
+		if (combiner->regs[reg].mask == 0)
+			continue;
+
+		status = readl_relaxed(combiner->regs[reg].addr);
+		status &= combiner->regs[reg].mask;
+
+		while (status) {
+			bit = __ffs(status);
+			status &= ~(1 << bit);
+			hwirq = irq_nr(reg, bit);
+			virq = irq_find_mapping(combiner->domain, hwirq);
+			if (virq >= 0)
+				generic_handle_irq(virq);
+
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+/*
+ * irqchip callbacks
+ */
+
+static void combiner_irq_chip_mask_irq(struct irq_data *data)
+{
+	struct combiner *combiner = irq_data_get_irq_chip_data(data);
+	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
+
+	clear_bit(irq_bit(data->hwirq), &reg->mask);
+}
+
+static void combiner_irq_chip_unmask_irq(struct irq_data *data)
+{
+	struct combiner *combiner = irq_data_get_irq_chip_data(data);
+	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
+
+	set_bit(irq_bit(data->hwirq), &reg->mask);
+}
+
+static struct irq_chip irq_chip = {
+	.irq_mask = combiner_irq_chip_mask_irq,
+	.irq_unmask = combiner_irq_chip_unmask_irq,
+	.name = "qcom-irq-combiner"
+};
+
+/*
+ * irq_domain_ops callbacks
+ */
+
+static int combiner_irq_map(struct irq_domain *domain, unsigned int irq,
+				   irq_hw_number_t hwirq)
+{
+	struct combiner *combiner = domain->host_data;
+
+	if (hwirq >= combiner->nirqs)
+		return -EINVAL;
+
+	irq_set_chip_and_handler(irq, &irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, combiner);
+	irq_set_noprobe(irq);
+	return 0;
+}
+
+static void combiner_irq_unmap(struct irq_domain *domain, unsigned int irq)
+{
+	struct irq_data *data = irq_get_irq_data(irq);
+
+	if (WARN_ON(!data))
+		return;
+	irq_domain_reset_irq_data(data);
+}
+
+static int combiner_irq_translate(struct irq_domain *d, struct irq_fwspec *fws,
+				  unsigned long *hwirq, unsigned int *type)
+{
+	if (is_acpi_node(fws->fwnode)) {
+		if (WARN_ON((fws->param_count != 2) ||
+			    (fws->param[1] & IORESOURCE_IRQ_LOWEDGE) ||
+			    (fws->param[1] & IORESOURCE_IRQ_HIGHEDGE)))
+			return -EINVAL;
+
+		*hwirq = fws->param[0];
+		*type = fws->param[1];
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct irq_domain_ops domain_ops = {
+	.map = combiner_irq_map,
+	.unmap = combiner_irq_unmap,
+	.translate = combiner_irq_translate
+};
+
+/*
+ * Device probing
+ */
+
+static acpi_status count_registers_cb(struct acpi_resource *ares, void *context)
+{
+	int *count = context;
+
+	if (ares->type == ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
+		++(*count);
+	return AE_OK;
+}
+
+static int count_registers(struct platform_device *pdev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	acpi_status status;
+	int count = 0;
+
+	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
+		return -EINVAL;
+
+	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+				     count_registers_cb, &count);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+	return count;
+}
+
+struct get_registers_context {
+	struct device *dev;
+	struct combiner *combiner;
+	int err;
+};
+
+static acpi_status get_registers_cb(struct acpi_resource *ares, void *context)
+{
+	struct get_registers_context *ctx = context;
+	struct acpi_resource_generic_register *reg;
+	phys_addr_t paddr;
+	void __iomem *vaddr;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
+		return AE_OK;
+
+	reg = &ares->data.generic_reg;
+	paddr = reg->address;
+	if ((reg->space_id != ACPI_SPACE_MEM) ||
+	    (reg->bit_offset != 0) ||
+	    (reg->bit_width > REG_SIZE)) {
+		dev_err(ctx->dev, "Bad register resource @%pa\n", &paddr);
+		ctx->err = -EINVAL;
+		return AE_ERROR;
+	}
+
+	vaddr = devm_ioremap(ctx->dev, reg->address, REG_SIZE);
+	if (IS_ERR(vaddr)) {
+		dev_err(ctx->dev, "Can't map register @%pa\n", &paddr);
+		ctx->err = PTR_ERR(vaddr);
+		return AE_ERROR;
+	}
+
+	ctx->combiner->regs[ctx->combiner->nregs].addr = vaddr;
+	ctx->combiner->nirqs += reg->bit_width;
+	ctx->combiner->nregs++;
+	return AE_OK;
+}
+
+static int get_registers(struct platform_device *pdev, struct combiner *comb)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	acpi_status status;
+	struct get_registers_context ctx;
+
+	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
+		return -EINVAL;
+
+	ctx.dev = &pdev->dev;
+	ctx.combiner = comb;
+	ctx.err = 0;
+
+	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+				     get_registers_cb, &ctx);
+	if (ACPI_FAILURE(status))
+		return ctx.err;
+	return 0;
+}
+
+static int __init combiner_probe(struct platform_device *pdev)
+{
+	struct combiner *combiner;
+	size_t alloc_sz;
+	u32 nregs;
+	int err;
+
+	nregs = count_registers(pdev);
+	if (nregs <= 0) {
+		dev_err(&pdev->dev, "Error reading register resources\n");
+		return -EINVAL;
+	}
+
+	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
+	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
+	if (!combiner)
+		return -ENOMEM;
+
+	err = get_registers(pdev, combiner);
+	if (err < 0)
+		return err;
+
+	combiner->parent_irq = platform_get_irq(pdev, 0);
+	if (combiner->parent_irq <= 0) {
+		dev_err(&pdev->dev, "Error getting IRQ resource\n");
+		return -EPROBE_DEFER;
+	}
+
+	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, combiner->nirqs,
+						    &domain_ops, combiner);
+	if (!combiner->domain)
+		/* Errors printed by irq_domain_create_linear */
+		return -ENODEV;
+
+	irq_set_chained_handler_and_data(combiner->parent_irq,
+					 combiner_handle_irq, combiner);
+
+	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
+		 combiner->parent_irq, combiner->nirqs, combiner->regs[0].addr);
+	return 0;
+}
+
+static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
+	{ "QCOM80B1", },
+	{ }
+};
+
+static struct platform_driver qcom_irq_combiner_probe = {
+	.driver = {
+		.name = "qcom-irq-combiner",
+		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
+	},
+	.probe = combiner_probe,
+};
+
+static int __init register_qcom_irq_combiner(void)
+{
+	return platform_driver_register(&qcom_irq_combiner_probe);
+}
+device_initcall(register_qcom_irq_combiner);