| Message ID | 20170328051226.21677-3-brendanhiggins@google.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
Hi Brendan, On 28/03/17 06:12, Brendan Higgins wrote: > The Aspeed 24XX/25XX chips share a single hardware interrupt across 14 > separate I2C busses. This adds a dummy irqchip which maps the single > hardware interrupt to software interrupts for each of the busses. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > --- > Added in v6: > - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is > what it actually does. > --- > drivers/irqchip/Makefile | 2 +- > drivers/irqchip/irq-aspeed-i2c-ic.c | 102 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 103 insertions(+), 1 deletion(-) > create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 152bc40b6762..c136c2bd1761 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -74,6 +74,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o > obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o > 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_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o > obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o > diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c > new file mode 100644 > index 000000000000..59c50b28dec0 > --- /dev/null > +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c > @@ -0,0 +1,102 @@ > +/* > + * Aspeed 24XX/25XX I2C Interrupt Controller. > + * > + * Copyright (C) 2012-2017 ASPEED Technology Inc. > + * Copyright 2017 IBM Corporation > + * Copyright 2017 Google, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/irq.h> > +#include <linux/irqchip.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irqdomain.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/io.h> > + > + > +#define ASPEED_I2C_IC_NUM_BUS 14 > + > +struct aspeed_i2c_ic { > + void __iomem *base; > + int parent_irq; > + struct irq_domain *irq_domain; > +}; > + > +/* > + * The aspeed chip provides a single hardware interrupt for all of the I2C > + * busses, so we use a dummy interrupt chip to translate this single interrupt > + * into multiple interrupts, each associated with a single I2C bus. > + */ > +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc) > +{ > + struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long bit, status; > + unsigned int bus_irq; > + > + chained_irq_enter(chip, desc); > + status = readl(i2c_ic->base); > + for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) { > + bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit); > + generic_handle_irq(bus_irq); > + } > + chained_irq_exit(chip, desc); > +} > + > +/* > + * Set simple handler and mark IRQ as valid. Nothing interesting to do here > + * since we are using a dummy interrupt chip. > + */ > +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain, > + unsigned int irq, irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); > + irq_set_chip_data(irq, domain->host_data); > + > + return 0; > +} I'm a bit concerned by this. It means that you can't even mask an interrupt. Is that really what you intend to do? Or all that the HW can do? If you cannot mask an interrupt, you're at the mercy of a screaming device... Thanks, M.
On Mon, 2017-03-27 at 22:12 -0700, Brendan Higgins wrote: > The Aspeed 24XX/25XX chips share a single hardware interrupt across > 14 > separate I2C busses. This adds a dummy irqchip which maps the single > hardware interrupt to software interrupts for each of the busses. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> I do think as I said earlier that is' a tiny bit overkill, I do worry about the overhead of the added layer of indirections on a 400Mhz ARMv9 (AST2400) core but otherwise: Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > Added in v6: > - Pulled "aspeed_i2c_controller" out into a interrupt controller > since that is > what it actually does. > --- > drivers/irqchip/Makefile | 2 +- > drivers/irqchip/irq-aspeed-i2c-ic.c | 102 > ++++++++++++++++++++++++++++++++++++ > 2 files changed, 103 insertions(+), 1 deletion(-) > create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 152bc40b6762..c136c2bd1761 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -74,6 +74,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq- > mvebu-odmi.o > obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o > 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_ARCH_ASPEED) += irq-aspeed-vic.o irq- > aspeed-i2c-ic.o > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o > obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq- > combiner.o > diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c > b/drivers/irqchip/irq-aspeed-i2c-ic.c > new file mode 100644 > index 000000000000..59c50b28dec0 > --- /dev/null > +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c > @@ -0,0 +1,102 @@ > +/* > + * Aspeed 24XX/25XX I2C Interrupt Controller. > + * > + * Copyright (C) 2012-2017 ASPEED Technology Inc. > + * Copyright 2017 IBM Corporation > + * Copyright 2017 Google, Inc. > + * > + * This program is free software; you can redistribute it and/or > modify > + * it under the terms of the GNU General Public License version 2 > as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/irq.h> > +#include <linux/irqchip.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irqdomain.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/io.h> > + > + > +#define ASPEED_I2C_IC_NUM_BUS 14 > + > +struct aspeed_i2c_ic { > + void __iomem *base; > + int parent_irq; > + struct irq_domain *irq_domain; > +}; > + > +/* > + * The aspeed chip provides a single hardware interrupt for all of > the I2C > + * busses, so we use a dummy interrupt chip to translate this single > interrupt > + * into multiple interrupts, each associated with a single I2C bus. > + */ > +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc) > +{ > + struct aspeed_i2c_ic *i2c_ic = > irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long bit, status; > + unsigned int bus_irq; > + > + chained_irq_enter(chip, desc); > + status = readl(i2c_ic->base); > + for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) { > + bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit); > + generic_handle_irq(bus_irq); > + } > + chained_irq_exit(chip, desc); > +} > + > +/* > + * Set simple handler and mark IRQ as valid. Nothing interesting to > do here > + * since we are using a dummy interrupt chip. > + */ > +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain, > + unsigned int irq, > irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &dummy_irq_chip, > handle_simple_irq); > + irq_set_chip_data(irq, domain->host_data); > + > + return 0; > +} > + > +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = { > + .map = aspeed_i2c_ic_map_irq_domain, > +}; > + > +static int __init aspeed_i2c_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct aspeed_i2c_ic *i2c_ic; > + > + i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL); > + if (!i2c_ic) > + return -ENOMEM; > + > + i2c_ic->base = of_iomap(node, 0); > + if (IS_ERR(i2c_ic->base)) > + return PTR_ERR(i2c_ic->base); > + > + i2c_ic->parent_irq = irq_of_parse_and_map(node, 0); > + if (i2c_ic->parent_irq < 0) > + return i2c_ic->parent_irq; > + > + i2c_ic->irq_domain = irq_domain_add_linear( > + node, ASPEED_I2C_IC_NUM_BUS, > + &aspeed_i2c_ic_irq_domain_ops, NULL); > + if (!i2c_ic->irq_domain) > + return -ENOMEM; > + > + i2c_ic->irq_domain->name = "ast-i2c-domain"; > + > + irq_set_chained_handler_and_data(i2c_ic->parent_irq, > + aspeed_i2c_ic_irq_handler, > i2c_ic); > + > + pr_info("i2c controller registered, irq %d\n", i2c_ic- > >parent_irq); > + > + return 0; > +} > + > +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", > aspeed_i2c_ic_of_init); > +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", > aspeed_i2c_ic_of_init);
On Tue, 2017-03-28 at 09:32 +0100, Marc Zyngier wrote: > I'm a bit concerned by this. It means that you can't even mask an > interrupt. Is that really what you intend to do? Or all that the HW can > do? If you cannot mask an interrupt, you're at the mercy of a screaming > device... This is not really an interrupt controller. It's a "summary" register that reflects the state of the 14 i2c controller interrupts. This approach does have the advantage of providing separate counters in /proc/interrupts which is rather nice, but it does have overhead. On those shittly little ARMv9 400Mhz cores it can be significant. I would personally have some kind of trick to register a single interrupt handler that calls directly the handlers of the respective i2c busses via a simple indirection for speed, maybe adding my custom sysfs or debugfs statistics. But that's just me trying to suck the last cycle out of the bloody thing ;-) Cheers, Ben.
On 28/03/17 10:12, Benjamin Herrenschmidt wrote: > On Tue, 2017-03-28 at 09:32 +0100, Marc Zyngier wrote: >> I'm a bit concerned by this. It means that you can't even mask an >> interrupt. Is that really what you intend to do? Or all that the HW can >> do? If you cannot mask an interrupt, you're at the mercy of a screaming >> device... > > This is not really an interrupt controller. It's a "summary" register > that reflects the state of the 14 i2c controller interrupts. > > This approach does have the advantage of providing separate counters in > /proc/interrupts which is rather nice, but it does have overhead. On > those shittly little ARMv9 400Mhz cores it can be significant. <pedantic> s/ARMv9/ARM9/, as we're still on variations of the ARMv8 architecture ;-) </pedantic> A 400MHz ARM9 (which is either ARMv4 or ARMv5) is not too bad (hey, we still have a couple of Versatile-ABs here...). Caches are pretty small though. > I would personally have some kind of trick to register a single > interrupt handler that calls directly the handlers of the respective > i2c busses via a simple indirection for speed, maybe adding my custom > sysfs or debugfs statistics. But that's just me trying to suck the last > cycle out of the bloody thing ;-) I'd hope the irqdomain itself to be pretty light (the revmap should help here), but of course you're going to do more work. Counters also come at a cost. It'd be interesting to see if Brendan has any overhead data about this. Cheers, M.
On Tue, 2017-03-28 at 10:40 +0100, Marc Zyngier wrote: > On 28/03/17 10:12, Benjamin Herrenschmidt wrote: > > On Tue, 2017-03-28 at 09:32 +0100, Marc Zyngier wrote: > > > I'm a bit concerned by this. It means that you can't even mask an > > > interrupt. Is that really what you intend to do? Or all that the HW can > > > do? If you cannot mask an interrupt, you're at the mercy of a screaming > > > device... > > > > This is not really an interrupt controller. It's a "summary" register > > that reflects the state of the 14 i2c controller interrupts. > > > > This approach does have the advantage of providing separate counters in > > /proc/interrupts which is rather nice, but it does have overhead. On > > those shittly little ARMv9 400Mhz cores it can be significant. > > <pedantic> > s/ARMv9/ARM9/, as we're still on variations of the ARMv8 architecture ;-) > </pedantic> It was a typo, I meant ARM9/ARMv5 :-) The 2 SOC families we are talking about (Aspeed 24xx and 25xx) are based on a ARM926EJ at 400Mhz and an ARM1176JZFS at 800Mhz respectively, so cycles do count :-) > A 400MHz ARM9 (which is either ARMv4 or ARMv5) is not too bad (hey, we > still have a couple of Versatile-ABs here...). Caches are pretty small > though. 16K/16K, no L2 :) > > I would personally have some kind of trick to register a single > > interrupt handler that calls directly the handlers of the respective > > i2c busses via a simple indirection for speed, maybe adding my custom > > sysfs or debugfs statistics. But that's just me trying to suck the last > > cycle out of the bloody thing ;-) > > I'd hope the irqdomain itself to be pretty light (the revmap should help > here), but of course you're going to do more work. Counters also come at > a cost. It'd be interesting to see if Brendan has any overhead data > about this. Thankfully, the HW supports buffered sends/receive or even DMA. The current patch doesn't yet support these but they would be a good way to alleviate the cost of the interrupts if it becomes a problem. Cheers, Ben. > Cheers, > > M.
The main reason I took this approach is just because I thought it was cleaner from the perspective of the busses which are totally independent (except for the fact that they share a single hardware interrupt). I did not make any measurements, so I doubt that I have anything to add that you don't already know. I saw other usages of chained interrupts that do the same thing (scan a "status" register and use them to make software interrupts) and I thought that is basically what the dummy irq chip code is for. The only thing I thought I was doing that was novel was actually breaking out the dummy irqchip into its own driver; it is not my idea, but I do think makes it a lot cleaner. Nevertheless, it should be cheap in terms of number of instructions; the most expensive part looks like looking up the mapping. In any case, I think the low hanging fruit here is supporting buffering or DMA, like Ben suggested. To address the comment on being over engineered: outside of the init function (which would exist regardless of how we do this, if not here then in the I2C driver); the code is actually pretty small and generic. All that being said, it would not be very hard to do this without using the dummy irqchip code and it would definitely be smaller in terms of indirection and space used, but I think the code would actually be more complicated to read. We would be going back to having an I2C controller along with the I2C busses; where all the I2C controller does is read the IRQ register and then call the appropriate bus irq handler, which looks a lot like a dummy irqchip. Cheers
On 29/03/17 10:59, Brendan Higgins wrote: > The main reason I took this approach is just because I thought it was > cleaner from the perspective of the busses which are totally > independent (except for the fact that they share a single hardware > interrupt). > > I did not make any measurements, so I doubt that I have anything to > add that you don't already know. I saw other usages of chained > interrupts that do the same thing (scan a "status" register and use > them to make software interrupts) and I thought that is basically what > the dummy irq chip code is for. The only thing I thought I was doing > that was novel was actually breaking out the dummy irqchip into its > own driver; it is not my idea, but I do think makes it a lot cleaner. > Nevertheless, it should be cheap in terms of number of instructions; > the most expensive part looks like looking up the mapping. In any > case, I think the low hanging fruit here is supporting buffering or > DMA, like Ben suggested. > > To address the comment on being over engineered: outside of the init > function (which would exist regardless of how we do this, if not here > then in the I2C driver); the code is actually pretty small and > generic. > > All that being said, it would not be very hard to do this without > using the dummy irqchip code and it would definitely be smaller in > terms of indirection and space used, but I think the code would > actually be more complicated to read. We would be going back to having > an I2C controller along with the I2C busses; where all the I2C > controller does is read the IRQ register and then call the appropriate > bus irq handler, which looks a lot like a dummy irqchip. As long as you're happy with the performance and the restrictions that come attached to the HW, I'm happy to take the irqchip patches. Thanks, M.
On Tue, Mar 28, 2017 at 3:42 PM, Brendan Higgins <brendanhiggins@google.com> wrote: > +static int __init aspeed_i2c_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct aspeed_i2c_ic *i2c_ic; > + > + i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL); > + if (!i2c_ic) > + return -ENOMEM; > + > + i2c_ic->base = of_iomap(node, 0); > + if (IS_ERR(i2c_ic->base)) > + return PTR_ERR(i2c_ic->base); > + > + i2c_ic->parent_irq = irq_of_parse_and_map(node, 0); > + if (i2c_ic->parent_irq < 0) > + return i2c_ic->parent_irq; > + > + i2c_ic->irq_domain = irq_domain_add_linear( > + node, ASPEED_I2C_IC_NUM_BUS, > + &aspeed_i2c_ic_irq_domain_ops, NULL); > + if (!i2c_ic->irq_domain) > + return -ENOMEM; > + > + i2c_ic->irq_domain->name = "ast-i2c-domain"; Nit: Make this aspeed-i2c-domain to make this consistent with the other Aspeed drivers in the kernel tree. Could this irq code be embedded in the i2c driver? We took a similar approach for the Aspeed GPIO driver, which has a similar IRQ structure of one hardware IRQ that tells the driver to check status registers for the precise irq source. The upside being all of the i2c code is in the same place in the kernel tree. Cheers, Joel
> Nit: Make this aspeed-i2c-domain to make this consistent with the > other Aspeed drivers in the kernel tree. > > Could this irq code be embedded in the i2c driver? We took a similar > approach for the Aspeed GPIO driver, which has a similar IRQ structure > of one hardware IRQ that tells the driver to check status registers > for the precise irq source. The upside being all of the i2c code is in > the same place in the kernel tree. In the previous version of the patch, this code was embedded in the I2C driver as the "struct aspeed_i2c_controller;" I really did not change anything about it other than rename some stuff and change the init method to match what irqchip code wants. I pulled it out into a separate driver because I was asked to by Vladimir; nevertheless, it does turn the I2C driver into a normal platforms driver, which is nice. Another benefit: if we put our dummy irqchip code in with the other irqchips, it makes it easier for the irqchip people to recognize when we are reusing the same patterns; for example, I would not at all be surprised if there are other dummy irqchips which have the same exact map(...) operation (I looked and did not see anything), but it is quite possibly something that other people want to do. If we put this stuff in drivers/irqchip, it is more likely that the irqchip people would recognize this as a common use case. I do not think either of these reasons I provided are particularly compelling, but I do not think the reasons to move it out are particularly compelling either (unless we decide we do not want make our own irq_domain). Cheers
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 152bc40b6762..c136c2bd1761 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -74,6 +74,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o 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_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c new file mode 100644 index 000000000000..59c50b28dec0 --- /dev/null +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c @@ -0,0 +1,102 @@ +/* + * Aspeed 24XX/25XX I2C Interrupt Controller. + * + * Copyright (C) 2012-2017 ASPEED Technology Inc. + * Copyright 2017 IBM Corporation + * Copyright 2017 Google, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/io.h> + + +#define ASPEED_I2C_IC_NUM_BUS 14 + +struct aspeed_i2c_ic { + void __iomem *base; + int parent_irq; + struct irq_domain *irq_domain; +}; + +/* + * The aspeed chip provides a single hardware interrupt for all of the I2C + * busses, so we use a dummy interrupt chip to translate this single interrupt + * into multiple interrupts, each associated with a single I2C bus. + */ +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc) +{ + struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long bit, status; + unsigned int bus_irq; + + chained_irq_enter(chip, desc); + status = readl(i2c_ic->base); + for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) { + bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit); + generic_handle_irq(bus_irq); + } + chained_irq_exit(chip, desc); +} + +/* + * Set simple handler and mark IRQ as valid. Nothing interesting to do here + * since we are using a dummy interrupt chip. + */ +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain, + unsigned int irq, irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); + irq_set_chip_data(irq, domain->host_data); + + return 0; +} + +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = { + .map = aspeed_i2c_ic_map_irq_domain, +}; + +static int __init aspeed_i2c_ic_of_init(struct device_node *node, + struct device_node *parent) +{ + struct aspeed_i2c_ic *i2c_ic; + + i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL); + if (!i2c_ic) + return -ENOMEM; + + i2c_ic->base = of_iomap(node, 0); + if (IS_ERR(i2c_ic->base)) + return PTR_ERR(i2c_ic->base); + + i2c_ic->parent_irq = irq_of_parse_and_map(node, 0); + if (i2c_ic->parent_irq < 0) + return i2c_ic->parent_irq; + + i2c_ic->irq_domain = irq_domain_add_linear( + node, ASPEED_I2C_IC_NUM_BUS, + &aspeed_i2c_ic_irq_domain_ops, NULL); + if (!i2c_ic->irq_domain) + return -ENOMEM; + + i2c_ic->irq_domain->name = "ast-i2c-domain"; + + irq_set_chained_handler_and_data(i2c_ic->parent_irq, + aspeed_i2c_ic_irq_handler, i2c_ic); + + pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq); + + return 0; +} + +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init); +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
The Aspeed 24XX/25XX chips share a single hardware interrupt across 14 separate I2C busses. This adds a dummy irqchip which maps the single hardware interrupt to software interrupts for each of the busses. Signed-off-by: Brendan Higgins <brendanhiggins@google.com> --- Added in v6: - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is what it actually does. --- drivers/irqchip/Makefile | 2 +- drivers/irqchip/irq-aspeed-i2c-ic.c | 102 ++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c