| Message ID | 20190812125256.9690-3-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irq/irq_sim: try to improve the API | expand |
On Mon, 12 Aug 2019 14:52:56 +0200 Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > We currently have a dedicated function to map the interrupt simulator > offsets to global interrupt numbers. This is something that irq_domain > should handle. > > Create a linear irq_domain when initializing the interrupt simulator > and modify the irq_sim_fire() function to only take as parameter the > global interrupt number. > > Convert both users in the same patch to using the new interface. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Hmm. Again, in principle looks fine, but what advantages do we get from using an irq domain? Pure code wise, it's longer and only a little bit simpler. Jonathan > --- > drivers/gpio/gpio-mockup.c | 11 ++- > drivers/iio/dummy/iio_dummy_evgen.c | 22 ++++-- > include/linux/irq_sim.h | 5 +- > kernel/irq/Kconfig | 1 + > kernel/irq/irq_sim.c | 110 +++++++++++++++++----------- > 5 files changed, 94 insertions(+), 55 deletions(-) > > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c > index 9b28ffec5826..4cf594f0e7cd 100644 > --- a/drivers/gpio/gpio-mockup.c > +++ b/drivers/gpio/gpio-mockup.c > @@ -186,7 +186,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset) > { > struct gpio_mockup_chip *chip = gpiochip_get_data(gc); > > - return irq_sim_irqnum(chip->irqsim, offset); > + return irq_create_mapping(irq_sim_get_domain(chip->irqsim), offset); > } > > static void gpio_mockup_free(struct gpio_chip *gc, unsigned int offset) > @@ -228,6 +228,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > struct gpio_mockup_dbgfs_private *priv; > int rv, val, curr, irq, irq_type; > struct gpio_mockup_chip *chip; > + struct irq_domain *domain; > struct seq_file *sfile; > struct gpio_desc *desc; > struct gpio_chip *gc; > @@ -248,6 +249,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > gc = &chip->gc; > desc = &gc->gpiodev->descs[priv->offset]; > sim = chip->irqsim; > + domain = irq_sim_get_domain(sim); > > mutex_lock(&chip->lock); > > @@ -257,12 +259,15 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > if (curr == val) > goto out; > > - irq = irq_sim_irqnum(sim, priv->offset); > + irq = irq_find_mapping(domain, priv->offset); > + if (!irq) > + return -ENOENT; > + > irq_type = irq_get_trigger_type(irq); > > if ((val == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) || > (val == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING))) > - irq_sim_fire(sim, priv->offset); > + irq_sim_fire(irq); > } > > /* Change the value unless we're actively driving the line. */ > diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c > index efbcd4a5609e..cc827f60a535 100644 > --- a/drivers/iio/dummy/iio_dummy_evgen.c > +++ b/drivers/iio/dummy/iio_dummy_evgen.c > @@ -31,14 +31,13 @@ > * @lock: protect the evgen state > * @inuse: mask of which irqs are connected > * @irq_sim: interrupt simulator > - * @base: base of irq range > */ > struct iio_dummy_eventgen { > struct iio_dummy_regs regs[IIO_EVENTGEN_NO]; > struct mutex lock; > bool inuse[IIO_EVENTGEN_NO]; > struct irq_sim *irq_sim; > - int base; > + struct irq_domain *domain; > }; > > /* We can only ever have one instance of this 'device' */ > @@ -56,7 +55,7 @@ static int iio_dummy_evgen_create(void) > return PTR_ERR(iio_evgen->irq_sim); > } > > - iio_evgen->base = irq_sim_irqnum(iio_evgen->irq_sim, 0); > + iio_evgen->domain = irq_sim_get_domain(iio_evgen->irq_sim); > mutex_init(&iio_evgen->lock); > > return 0; > @@ -78,7 +77,7 @@ int iio_dummy_evgen_get_irq(void) > mutex_lock(&iio_evgen->lock); > for (i = 0; i < IIO_EVENTGEN_NO; i++) { > if (!iio_evgen->inuse[i]) { > - ret = irq_sim_irqnum(iio_evgen->irq_sim, i); > + ret = irq_create_mapping(iio_evgen->domain, i); > iio_evgen->inuse[i] = true; > break; > } > @@ -99,15 +98,21 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq); > */ > void iio_dummy_evgen_release_irq(int irq) > { > + struct irq_data *irqd; > + > + irqd = irq_get_irq_data(irq); > + > mutex_lock(&iio_evgen->lock); > - iio_evgen->inuse[irq - iio_evgen->base] = false; > + iio_evgen->inuse[irqd_to_hwirq(irqd)] = false; > mutex_unlock(&iio_evgen->lock); > } > EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq); > > struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq) > { > - return &iio_evgen->regs[irq - iio_evgen->base]; > + struct irq_data *irqd = irq_get_irq_data(irq); > + > + return &iio_evgen->regs[irqd_to_hwirq(irqd)]; > } > EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs); > > @@ -129,7 +134,7 @@ static ssize_t iio_evgen_poke(struct device *dev, > { > struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); > unsigned long event; > - int ret; > + int ret, irq; > > ret = kstrtoul(buf, 10, &event); > if (ret) > @@ -138,7 +143,8 @@ static ssize_t iio_evgen_poke(struct device *dev, > iio_evgen->regs[this_attr->address].reg_id = this_attr->address; > iio_evgen->regs[this_attr->address].reg_data = event; > > - irq_sim_fire(iio_evgen->irq_sim, this_attr->address); > + irq = irq_create_mapping(iio_evgen->domain, this_attr->address); > + irq_sim_fire(irq); > > return len; > } > diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h > index 4bbf036145e2..4056d0e7f0b4 100644 > --- a/include/linux/irq_sim.h > +++ b/include/linux/irq_sim.h > @@ -7,6 +7,7 @@ > #define _LINUX_IRQ_SIM_H > > #include <linux/irq_work.h> > +#include <linux/irqdomain.h> > #include <linux/device.h> > > /* > @@ -19,7 +20,7 @@ struct irq_sim; > struct irq_sim *irq_sim_new(unsigned int num_irqs); > struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs); > void irq_sim_free(struct irq_sim *sim); > -void irq_sim_fire(struct irq_sim *sim, unsigned int offset); > -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset); > +void irq_sim_fire(int virq); > +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim); > > #endif /* _LINUX_IRQ_SIM_H */ > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > index f92d9a687372..d0890f7729d4 100644 > --- a/kernel/irq/Kconfig > +++ b/kernel/irq/Kconfig > @@ -68,6 +68,7 @@ config IRQ_DOMAIN > config IRQ_SIM > bool > select IRQ_WORK > + select IRQ_DOMAIN > > # Support for hierarchical irq domains > config IRQ_DOMAIN_HIERARCHY > diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c > index 79f0a6494b6c..a1c91aefb6cd 100644 > --- a/kernel/irq/irq_sim.c > +++ b/kernel/irq/irq_sim.c > @@ -15,13 +15,14 @@ struct irq_sim_work_ctx { > struct irq_sim_irq_ctx { > int irqnum; > bool enabled; > + struct irq_sim_work_ctx *work_ctx; > }; > > struct irq_sim { > struct irq_sim_work_ctx work_ctx; > int irq_base; > unsigned int irq_count; > - struct irq_sim_irq_ctx *irqs; > + struct irq_domain *domain; > }; > > struct irq_sim_devres { > @@ -74,11 +75,46 @@ static void irq_sim_handle_irq(struct irq_work *work) > offset = find_next_bit(work_ctx->pending, > sim->irq_count, offset); > clear_bit(offset, work_ctx->pending); > - irqnum = irq_sim_irqnum(sim, offset); > + irqnum = irq_find_mapping(sim->domain, offset); > handle_simple_irq(irq_to_desc(irqnum)); > } > } > > +static int irq_sim_domain_map(struct irq_domain *domain, > + unsigned int virq, irq_hw_number_t hw) > +{ > + struct irq_sim *sim = domain->host_data; > + struct irq_sim_irq_ctx *ctx; > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + irq_set_chip(virq, &irq_sim_irqchip); > + irq_set_chip_data(virq, ctx); > + irq_set_handler(virq, handle_simple_irq); > + irq_modify_status(virq, IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); > + ctx->work_ctx = &sim->work_ctx; > + > + return 0; > +} > + > +static void irq_sim_domain_unmap(struct irq_domain *domain, unsigned int virq) > +{ > + struct irq_sim_irq_ctx *ctx; > + struct irq_data *irqd; > + > + irqd = irq_domain_get_irq_data(domain, virq); > + ctx = irq_data_get_irq_chip_data(irqd); > + > + kfree(ctx); > +} > + > +static const struct irq_domain_ops irq_sim_domain_ops = { > + .map = irq_sim_domain_map, > + .unmap = irq_sim_domain_unmap, > +}; > + > /** > * irq_sim_new - Create a new interrupt simulator: allocate a range of > * dummy interrupts. > @@ -91,42 +127,21 @@ static void irq_sim_handle_irq(struct irq_work *work) > struct irq_sim *irq_sim_new(unsigned int num_irqs) > { > struct irq_sim *sim; > - int i; > > sim = kmalloc(sizeof(*sim), GFP_KERNEL); > if (!sim) > return ERR_PTR(-ENOMEM); > > - sim->irqs = kmalloc_array(num_irqs, sizeof(*sim->irqs), GFP_KERNEL); > - if (!sim->irqs) { > - kfree(sim); > - return ERR_PTR(-ENOMEM); > - } > - > - sim->irq_base = irq_alloc_descs(-1, 0, num_irqs, 0); > - if (sim->irq_base < 0) { > - kfree(sim->irqs); > - kfree(sim); > - return ERR_PTR(sim->irq_base); > - } > - > sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL); > if (!sim->work_ctx.pending) { > - kfree(sim->irqs); > kfree(sim); > - irq_free_descs(sim->irq_base, num_irqs); > return ERR_PTR(-ENOMEM); > } > > - for (i = 0; i < num_irqs; i++) { > - sim->irqs[i].irqnum = sim->irq_base + i; > - sim->irqs[i].enabled = false; > - irq_set_chip(sim->irq_base + i, &irq_sim_irqchip); > - irq_set_chip_data(sim->irq_base + i, &sim->irqs[i]); > - irq_set_handler(sim->irq_base + i, &handle_simple_irq); > - irq_modify_status(sim->irq_base + i, > - IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); > - } > + sim->domain = irq_domain_create_linear(NULL, num_irqs, > + &irq_sim_domain_ops, sim); > + if (!sim->domain) > + return ERR_PTR(-ENOMEM); > > init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq); > sim->irq_count = num_irqs; > @@ -143,10 +158,17 @@ EXPORT_SYMBOL_GPL(irq_sim_new); > */ > void irq_sim_free(struct irq_sim *sim) > { > + int i, irq; > + > + for (i = 0; i < sim->irq_count; i++) { > + irq = irq_find_mapping(sim->domain, i); > + if (irq) > + irq_dispose_mapping(irq); > + } > + > + irq_domain_remove(sim->domain); > irq_work_sync(&sim->work_ctx.work); > bitmap_free(sim->work_ctx.pending); > - irq_free_descs(sim->irq_base, sim->irq_count); > - kfree(sim->irqs); > kfree(sim); > } > EXPORT_SYMBOL_GPL(irq_sim_free); > @@ -189,27 +211,31 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_new); > /** > * irq_sim_fire - Enqueue an interrupt. > * > - * @sim: The interrupt simulator object. > - * @offset: Offset of the simulated interrupt which should be fired. > + * @virq: Virtual interrupt number to fire. It must be associated with > + * an existing interrupt simulator. > */ > -void irq_sim_fire(struct irq_sim *sim, unsigned int offset) > +void irq_sim_fire(int virq) > { > - if (sim->irqs[offset].enabled) { > - set_bit(offset, sim->work_ctx.pending); > - irq_work_queue(&sim->work_ctx.work); > + struct irq_sim_irq_ctx *ctx; > + struct irq_data *irqd; > + > + irqd = irq_get_irq_data(virq); > + ctx = irq_data_get_irq_chip_data(irqd); > + > + if (ctx->enabled) { > + set_bit(irqd_to_hwirq(irqd), ctx->work_ctx->pending); > + irq_work_queue(&ctx->work_ctx->work); > } > } > EXPORT_SYMBOL_GPL(irq_sim_fire); > > /** > - * irq_sim_irqnum - Get the allocated number of a dummy interrupt. > + * irq_sim_get_domain - Retrieve the interrupt domain for this simulator. > * > - * @sim: The interrupt simulator object. > - * @offset: Offset of the simulated interrupt for which to retrieve > - * the number. > + * @sim: The interrupt simulator the domain of which we retrieve. > */ > -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset) > +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim) > { > - return sim->irqs[offset].irqnum; > + return sim->domain; > } > -EXPORT_SYMBOL_GPL(irq_sim_irqnum); > +EXPORT_SYMBOL(irq_sim_get_domain);
On Mon, 12 Aug 2019 13:52:56 +0100, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > We currently have a dedicated function to map the interrupt simulator > offsets to global interrupt numbers. This is something that irq_domain > should handle. > > Create a linear irq_domain when initializing the interrupt simulator > and modify the irq_sim_fire() function to only take as parameter the > global interrupt number. > > Convert both users in the same patch to using the new interface. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > drivers/gpio/gpio-mockup.c | 11 ++- > drivers/iio/dummy/iio_dummy_evgen.c | 22 ++++-- > include/linux/irq_sim.h | 5 +- > kernel/irq/Kconfig | 1 + > kernel/irq/irq_sim.c | 110 +++++++++++++++++----------- > 5 files changed, 94 insertions(+), 55 deletions(-) > > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c > index 9b28ffec5826..4cf594f0e7cd 100644 > --- a/drivers/gpio/gpio-mockup.c > +++ b/drivers/gpio/gpio-mockup.c > @@ -186,7 +186,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset) > { > struct gpio_mockup_chip *chip = gpiochip_get_data(gc); > > - return irq_sim_irqnum(chip->irqsim, offset); > + return irq_create_mapping(irq_sim_get_domain(chip->irqsim), offset); > } > > static void gpio_mockup_free(struct gpio_chip *gc, unsigned int offset) > @@ -228,6 +228,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > struct gpio_mockup_dbgfs_private *priv; > int rv, val, curr, irq, irq_type; > struct gpio_mockup_chip *chip; > + struct irq_domain *domain; > struct seq_file *sfile; > struct gpio_desc *desc; > struct gpio_chip *gc; > @@ -248,6 +249,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > gc = &chip->gc; > desc = &gc->gpiodev->descs[priv->offset]; > sim = chip->irqsim; > + domain = irq_sim_get_domain(sim); > > mutex_lock(&chip->lock); > > @@ -257,12 +259,15 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, > if (curr == val) > goto out; > > - irq = irq_sim_irqnum(sim, priv->offset); > + irq = irq_find_mapping(domain, priv->offset); > + if (!irq) > + return -ENOENT; > + > irq_type = irq_get_trigger_type(irq); > > if ((val == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) || > (val == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING))) > - irq_sim_fire(sim, priv->offset); > + irq_sim_fire(irq); > } > > /* Change the value unless we're actively driving the line. */ > diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c > index efbcd4a5609e..cc827f60a535 100644 > --- a/drivers/iio/dummy/iio_dummy_evgen.c > +++ b/drivers/iio/dummy/iio_dummy_evgen.c > @@ -31,14 +31,13 @@ > * @lock: protect the evgen state > * @inuse: mask of which irqs are connected > * @irq_sim: interrupt simulator > - * @base: base of irq range > */ > struct iio_dummy_eventgen { > struct iio_dummy_regs regs[IIO_EVENTGEN_NO]; > struct mutex lock; > bool inuse[IIO_EVENTGEN_NO]; > struct irq_sim *irq_sim; > - int base; > + struct irq_domain *domain; > }; > > /* We can only ever have one instance of this 'device' */ > @@ -56,7 +55,7 @@ static int iio_dummy_evgen_create(void) > return PTR_ERR(iio_evgen->irq_sim); > } > > - iio_evgen->base = irq_sim_irqnum(iio_evgen->irq_sim, 0); > + iio_evgen->domain = irq_sim_get_domain(iio_evgen->irq_sim); > mutex_init(&iio_evgen->lock); > > return 0; > @@ -78,7 +77,7 @@ int iio_dummy_evgen_get_irq(void) > mutex_lock(&iio_evgen->lock); > for (i = 0; i < IIO_EVENTGEN_NO; i++) { > if (!iio_evgen->inuse[i]) { > - ret = irq_sim_irqnum(iio_evgen->irq_sim, i); > + ret = irq_create_mapping(iio_evgen->domain, i); > iio_evgen->inuse[i] = true; > break; > } > @@ -99,15 +98,21 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq); > */ > void iio_dummy_evgen_release_irq(int irq) > { > + struct irq_data *irqd; > + > + irqd = irq_get_irq_data(irq); > + > mutex_lock(&iio_evgen->lock); > - iio_evgen->inuse[irq - iio_evgen->base] = false; > + iio_evgen->inuse[irqd_to_hwirq(irqd)] = false; > mutex_unlock(&iio_evgen->lock); > } > EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq); > > struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq) > { > - return &iio_evgen->regs[irq - iio_evgen->base]; > + struct irq_data *irqd = irq_get_irq_data(irq); > + > + return &iio_evgen->regs[irqd_to_hwirq(irqd)]; > } > EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs); > > @@ -129,7 +134,7 @@ static ssize_t iio_evgen_poke(struct device *dev, > { > struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); > unsigned long event; > - int ret; > + int ret, irq; > > ret = kstrtoul(buf, 10, &event); > if (ret) > @@ -138,7 +143,8 @@ static ssize_t iio_evgen_poke(struct device *dev, > iio_evgen->regs[this_attr->address].reg_id = this_attr->address; > iio_evgen->regs[this_attr->address].reg_data = event; > > - irq_sim_fire(iio_evgen->irq_sim, this_attr->address); > + irq = irq_create_mapping(iio_evgen->domain, this_attr->address); > + irq_sim_fire(irq); That's not the way the API is intended to be used. irq_create_mapping() is something that should only happen once, or at least as rarely as possible. At interrupt handling time, you should call irq_find_mapping() instead. You need to rethink the way this driver is using interrupts. > > return len; > } > diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h > index 4bbf036145e2..4056d0e7f0b4 100644 > --- a/include/linux/irq_sim.h > +++ b/include/linux/irq_sim.h > @@ -7,6 +7,7 @@ > #define _LINUX_IRQ_SIM_H > > #include <linux/irq_work.h> > +#include <linux/irqdomain.h> > #include <linux/device.h> > > /* > @@ -19,7 +20,7 @@ struct irq_sim; > struct irq_sim *irq_sim_new(unsigned int num_irqs); > struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs); > void irq_sim_free(struct irq_sim *sim); > -void irq_sim_fire(struct irq_sim *sim, unsigned int offset); > -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset); > +void irq_sim_fire(int virq); > +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim); > > #endif /* _LINUX_IRQ_SIM_H */ > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > index f92d9a687372..d0890f7729d4 100644 > --- a/kernel/irq/Kconfig > +++ b/kernel/irq/Kconfig > @@ -68,6 +68,7 @@ config IRQ_DOMAIN > config IRQ_SIM > bool > select IRQ_WORK > + select IRQ_DOMAIN > > # Support for hierarchical irq domains > config IRQ_DOMAIN_HIERARCHY > diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c > index 79f0a6494b6c..a1c91aefb6cd 100644 > --- a/kernel/irq/irq_sim.c > +++ b/kernel/irq/irq_sim.c > @@ -15,13 +15,14 @@ struct irq_sim_work_ctx { > struct irq_sim_irq_ctx { > int irqnum; > bool enabled; > + struct irq_sim_work_ctx *work_ctx; > }; > > struct irq_sim { > struct irq_sim_work_ctx work_ctx; > int irq_base; > unsigned int irq_count; > - struct irq_sim_irq_ctx *irqs; > + struct irq_domain *domain; > }; > > struct irq_sim_devres { > @@ -74,11 +75,46 @@ static void irq_sim_handle_irq(struct irq_work *work) > offset = find_next_bit(work_ctx->pending, > sim->irq_count, offset); > clear_bit(offset, work_ctx->pending); > - irqnum = irq_sim_irqnum(sim, offset); > + irqnum = irq_find_mapping(sim->domain, offset); > handle_simple_irq(irq_to_desc(irqnum)); > } > } > > +static int irq_sim_domain_map(struct irq_domain *domain, > + unsigned int virq, irq_hw_number_t hw) > +{ > + struct irq_sim *sim = domain->host_data; > + struct irq_sim_irq_ctx *ctx; > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + irq_set_chip(virq, &irq_sim_irqchip); > + irq_set_chip_data(virq, ctx); > + irq_set_handler(virq, handle_simple_irq); > + irq_modify_status(virq, IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); > + ctx->work_ctx = &sim->work_ctx; > + > + return 0; > +} > + > +static void irq_sim_domain_unmap(struct irq_domain *domain, unsigned int virq) > +{ > + struct irq_sim_irq_ctx *ctx; > + struct irq_data *irqd; > + > + irqd = irq_domain_get_irq_data(domain, virq); > + ctx = irq_data_get_irq_chip_data(irqd); > + > + kfree(ctx); I'd expect something like: irq_set_handler(virq, NULL); irq_domain_reset_irq_data(irqd); in addition to the above. > +} > + > +static const struct irq_domain_ops irq_sim_domain_ops = { > + .map = irq_sim_domain_map, > + .unmap = irq_sim_domain_unmap, > +}; > + > /** > * irq_sim_new - Create a new interrupt simulator: allocate a range of > * dummy interrupts. > @@ -91,42 +127,21 @@ static void irq_sim_handle_irq(struct irq_work *work) > struct irq_sim *irq_sim_new(unsigned int num_irqs) > { > struct irq_sim *sim; > - int i; > > sim = kmalloc(sizeof(*sim), GFP_KERNEL); > if (!sim) > return ERR_PTR(-ENOMEM); > > - sim->irqs = kmalloc_array(num_irqs, sizeof(*sim->irqs), GFP_KERNEL); > - if (!sim->irqs) { > - kfree(sim); > - return ERR_PTR(-ENOMEM); > - } > - > - sim->irq_base = irq_alloc_descs(-1, 0, num_irqs, 0); > - if (sim->irq_base < 0) { > - kfree(sim->irqs); > - kfree(sim); > - return ERR_PTR(sim->irq_base); > - } > - > sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL); > if (!sim->work_ctx.pending) { > - kfree(sim->irqs); > kfree(sim); > - irq_free_descs(sim->irq_base, num_irqs); > return ERR_PTR(-ENOMEM); > } > > - for (i = 0; i < num_irqs; i++) { > - sim->irqs[i].irqnum = sim->irq_base + i; > - sim->irqs[i].enabled = false; > - irq_set_chip(sim->irq_base + i, &irq_sim_irqchip); > - irq_set_chip_data(sim->irq_base + i, &sim->irqs[i]); > - irq_set_handler(sim->irq_base + i, &handle_simple_irq); > - irq_modify_status(sim->irq_base + i, > - IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); > - } > + sim->domain = irq_domain_create_linear(NULL, num_irqs, > + &irq_sim_domain_ops, sim); > + if (!sim->domain) > + return ERR_PTR(-ENOMEM); Aren't you now leaking memory from the sim structure? > > init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq); > sim->irq_count = num_irqs; > @@ -143,10 +158,17 @@ EXPORT_SYMBOL_GPL(irq_sim_new); > */ > void irq_sim_free(struct irq_sim *sim) > { > + int i, irq; > + > + for (i = 0; i < sim->irq_count; i++) { > + irq = irq_find_mapping(sim->domain, i); > + if (irq) > + irq_dispose_mapping(irq); > + } > + > + irq_domain_remove(sim->domain); > irq_work_sync(&sim->work_ctx.work); > bitmap_free(sim->work_ctx.pending); > - irq_free_descs(sim->irq_base, sim->irq_count); > - kfree(sim->irqs); > kfree(sim); > } > EXPORT_SYMBOL_GPL(irq_sim_free); > @@ -189,27 +211,31 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_new); > /** > * irq_sim_fire - Enqueue an interrupt. > * > - * @sim: The interrupt simulator object. > - * @offset: Offset of the simulated interrupt which should be fired. > + * @virq: Virtual interrupt number to fire. It must be associated with > + * an existing interrupt simulator. > */ > -void irq_sim_fire(struct irq_sim *sim, unsigned int offset) > +void irq_sim_fire(int virq) > { > - if (sim->irqs[offset].enabled) { > - set_bit(offset, sim->work_ctx.pending); > - irq_work_queue(&sim->work_ctx.work); > + struct irq_sim_irq_ctx *ctx; > + struct irq_data *irqd; > + > + irqd = irq_get_irq_data(virq); > + ctx = irq_data_get_irq_chip_data(irqd); > + > + if (ctx->enabled) { > + set_bit(irqd_to_hwirq(irqd), ctx->work_ctx->pending); > + irq_work_queue(&ctx->work_ctx->work); > } > } > EXPORT_SYMBOL_GPL(irq_sim_fire); > > /** > - * irq_sim_irqnum - Get the allocated number of a dummy interrupt. > + * irq_sim_get_domain - Retrieve the interrupt domain for this simulator. > * > - * @sim: The interrupt simulator object. > - * @offset: Offset of the simulated interrupt for which to retrieve > - * the number. > + * @sim: The interrupt simulator the domain of which we retrieve. > */ > -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset) > +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim) > { > - return sim->irqs[offset].irqnum; > + return sim->domain; > } > -EXPORT_SYMBOL_GPL(irq_sim_irqnum); > +EXPORT_SYMBOL(irq_sim_get_domain); This seems to be reinventing irq_find_matching_fwnode(). Please consider allocating a fwnode as part of the irq_sim structure, and use this fwnode to lookup the domain instead. M.
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c index 9b28ffec5826..4cf594f0e7cd 100644 --- a/drivers/gpio/gpio-mockup.c +++ b/drivers/gpio/gpio-mockup.c @@ -186,7 +186,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset) { struct gpio_mockup_chip *chip = gpiochip_get_data(gc); - return irq_sim_irqnum(chip->irqsim, offset); + return irq_create_mapping(irq_sim_get_domain(chip->irqsim), offset); } static void gpio_mockup_free(struct gpio_chip *gc, unsigned int offset) @@ -228,6 +228,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, struct gpio_mockup_dbgfs_private *priv; int rv, val, curr, irq, irq_type; struct gpio_mockup_chip *chip; + struct irq_domain *domain; struct seq_file *sfile; struct gpio_desc *desc; struct gpio_chip *gc; @@ -248,6 +249,7 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, gc = &chip->gc; desc = &gc->gpiodev->descs[priv->offset]; sim = chip->irqsim; + domain = irq_sim_get_domain(sim); mutex_lock(&chip->lock); @@ -257,12 +259,15 @@ static ssize_t gpio_mockup_debugfs_write(struct file *file, if (curr == val) goto out; - irq = irq_sim_irqnum(sim, priv->offset); + irq = irq_find_mapping(domain, priv->offset); + if (!irq) + return -ENOENT; + irq_type = irq_get_trigger_type(irq); if ((val == 1 && (irq_type & IRQ_TYPE_EDGE_RISING)) || (val == 0 && (irq_type & IRQ_TYPE_EDGE_FALLING))) - irq_sim_fire(sim, priv->offset); + irq_sim_fire(irq); } /* Change the value unless we're actively driving the line. */ diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c index efbcd4a5609e..cc827f60a535 100644 --- a/drivers/iio/dummy/iio_dummy_evgen.c +++ b/drivers/iio/dummy/iio_dummy_evgen.c @@ -31,14 +31,13 @@ * @lock: protect the evgen state * @inuse: mask of which irqs are connected * @irq_sim: interrupt simulator - * @base: base of irq range */ struct iio_dummy_eventgen { struct iio_dummy_regs regs[IIO_EVENTGEN_NO]; struct mutex lock; bool inuse[IIO_EVENTGEN_NO]; struct irq_sim *irq_sim; - int base; + struct irq_domain *domain; }; /* We can only ever have one instance of this 'device' */ @@ -56,7 +55,7 @@ static int iio_dummy_evgen_create(void) return PTR_ERR(iio_evgen->irq_sim); } - iio_evgen->base = irq_sim_irqnum(iio_evgen->irq_sim, 0); + iio_evgen->domain = irq_sim_get_domain(iio_evgen->irq_sim); mutex_init(&iio_evgen->lock); return 0; @@ -78,7 +77,7 @@ int iio_dummy_evgen_get_irq(void) mutex_lock(&iio_evgen->lock); for (i = 0; i < IIO_EVENTGEN_NO; i++) { if (!iio_evgen->inuse[i]) { - ret = irq_sim_irqnum(iio_evgen->irq_sim, i); + ret = irq_create_mapping(iio_evgen->domain, i); iio_evgen->inuse[i] = true; break; } @@ -99,15 +98,21 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq); */ void iio_dummy_evgen_release_irq(int irq) { + struct irq_data *irqd; + + irqd = irq_get_irq_data(irq); + mutex_lock(&iio_evgen->lock); - iio_evgen->inuse[irq - iio_evgen->base] = false; + iio_evgen->inuse[irqd_to_hwirq(irqd)] = false; mutex_unlock(&iio_evgen->lock); } EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq); struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq) { - return &iio_evgen->regs[irq - iio_evgen->base]; + struct irq_data *irqd = irq_get_irq_data(irq); + + return &iio_evgen->regs[irqd_to_hwirq(irqd)]; } EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs); @@ -129,7 +134,7 @@ static ssize_t iio_evgen_poke(struct device *dev, { struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); unsigned long event; - int ret; + int ret, irq; ret = kstrtoul(buf, 10, &event); if (ret) @@ -138,7 +143,8 @@ static ssize_t iio_evgen_poke(struct device *dev, iio_evgen->regs[this_attr->address].reg_id = this_attr->address; iio_evgen->regs[this_attr->address].reg_data = event; - irq_sim_fire(iio_evgen->irq_sim, this_attr->address); + irq = irq_create_mapping(iio_evgen->domain, this_attr->address); + irq_sim_fire(irq); return len; } diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h index 4bbf036145e2..4056d0e7f0b4 100644 --- a/include/linux/irq_sim.h +++ b/include/linux/irq_sim.h @@ -7,6 +7,7 @@ #define _LINUX_IRQ_SIM_H #include <linux/irq_work.h> +#include <linux/irqdomain.h> #include <linux/device.h> /* @@ -19,7 +20,7 @@ struct irq_sim; struct irq_sim *irq_sim_new(unsigned int num_irqs); struct irq_sim *devm_irq_sim_new(struct device *dev, unsigned int num_irqs); void irq_sim_free(struct irq_sim *sim); -void irq_sim_fire(struct irq_sim *sim, unsigned int offset); -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset); +void irq_sim_fire(int virq); +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim); #endif /* _LINUX_IRQ_SIM_H */ diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index f92d9a687372..d0890f7729d4 100644 --- a/kernel/irq/Kconfig +++ b/kernel/irq/Kconfig @@ -68,6 +68,7 @@ config IRQ_DOMAIN config IRQ_SIM bool select IRQ_WORK + select IRQ_DOMAIN # Support for hierarchical irq domains config IRQ_DOMAIN_HIERARCHY diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c index 79f0a6494b6c..a1c91aefb6cd 100644 --- a/kernel/irq/irq_sim.c +++ b/kernel/irq/irq_sim.c @@ -15,13 +15,14 @@ struct irq_sim_work_ctx { struct irq_sim_irq_ctx { int irqnum; bool enabled; + struct irq_sim_work_ctx *work_ctx; }; struct irq_sim { struct irq_sim_work_ctx work_ctx; int irq_base; unsigned int irq_count; - struct irq_sim_irq_ctx *irqs; + struct irq_domain *domain; }; struct irq_sim_devres { @@ -74,11 +75,46 @@ static void irq_sim_handle_irq(struct irq_work *work) offset = find_next_bit(work_ctx->pending, sim->irq_count, offset); clear_bit(offset, work_ctx->pending); - irqnum = irq_sim_irqnum(sim, offset); + irqnum = irq_find_mapping(sim->domain, offset); handle_simple_irq(irq_to_desc(irqnum)); } } +static int irq_sim_domain_map(struct irq_domain *domain, + unsigned int virq, irq_hw_number_t hw) +{ + struct irq_sim *sim = domain->host_data; + struct irq_sim_irq_ctx *ctx; + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + irq_set_chip(virq, &irq_sim_irqchip); + irq_set_chip_data(virq, ctx); + irq_set_handler(virq, handle_simple_irq); + irq_modify_status(virq, IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); + ctx->work_ctx = &sim->work_ctx; + + return 0; +} + +static void irq_sim_domain_unmap(struct irq_domain *domain, unsigned int virq) +{ + struct irq_sim_irq_ctx *ctx; + struct irq_data *irqd; + + irqd = irq_domain_get_irq_data(domain, virq); + ctx = irq_data_get_irq_chip_data(irqd); + + kfree(ctx); +} + +static const struct irq_domain_ops irq_sim_domain_ops = { + .map = irq_sim_domain_map, + .unmap = irq_sim_domain_unmap, +}; + /** * irq_sim_new - Create a new interrupt simulator: allocate a range of * dummy interrupts. @@ -91,42 +127,21 @@ static void irq_sim_handle_irq(struct irq_work *work) struct irq_sim *irq_sim_new(unsigned int num_irqs) { struct irq_sim *sim; - int i; sim = kmalloc(sizeof(*sim), GFP_KERNEL); if (!sim) return ERR_PTR(-ENOMEM); - sim->irqs = kmalloc_array(num_irqs, sizeof(*sim->irqs), GFP_KERNEL); - if (!sim->irqs) { - kfree(sim); - return ERR_PTR(-ENOMEM); - } - - sim->irq_base = irq_alloc_descs(-1, 0, num_irqs, 0); - if (sim->irq_base < 0) { - kfree(sim->irqs); - kfree(sim); - return ERR_PTR(sim->irq_base); - } - sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL); if (!sim->work_ctx.pending) { - kfree(sim->irqs); kfree(sim); - irq_free_descs(sim->irq_base, num_irqs); return ERR_PTR(-ENOMEM); } - for (i = 0; i < num_irqs; i++) { - sim->irqs[i].irqnum = sim->irq_base + i; - sim->irqs[i].enabled = false; - irq_set_chip(sim->irq_base + i, &irq_sim_irqchip); - irq_set_chip_data(sim->irq_base + i, &sim->irqs[i]); - irq_set_handler(sim->irq_base + i, &handle_simple_irq); - irq_modify_status(sim->irq_base + i, - IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE); - } + sim->domain = irq_domain_create_linear(NULL, num_irqs, + &irq_sim_domain_ops, sim); + if (!sim->domain) + return ERR_PTR(-ENOMEM); init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq); sim->irq_count = num_irqs; @@ -143,10 +158,17 @@ EXPORT_SYMBOL_GPL(irq_sim_new); */ void irq_sim_free(struct irq_sim *sim) { + int i, irq; + + for (i = 0; i < sim->irq_count; i++) { + irq = irq_find_mapping(sim->domain, i); + if (irq) + irq_dispose_mapping(irq); + } + + irq_domain_remove(sim->domain); irq_work_sync(&sim->work_ctx.work); bitmap_free(sim->work_ctx.pending); - irq_free_descs(sim->irq_base, sim->irq_count); - kfree(sim->irqs); kfree(sim); } EXPORT_SYMBOL_GPL(irq_sim_free); @@ -189,27 +211,31 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_new); /** * irq_sim_fire - Enqueue an interrupt. * - * @sim: The interrupt simulator object. - * @offset: Offset of the simulated interrupt which should be fired. + * @virq: Virtual interrupt number to fire. It must be associated with + * an existing interrupt simulator. */ -void irq_sim_fire(struct irq_sim *sim, unsigned int offset) +void irq_sim_fire(int virq) { - if (sim->irqs[offset].enabled) { - set_bit(offset, sim->work_ctx.pending); - irq_work_queue(&sim->work_ctx.work); + struct irq_sim_irq_ctx *ctx; + struct irq_data *irqd; + + irqd = irq_get_irq_data(virq); + ctx = irq_data_get_irq_chip_data(irqd); + + if (ctx->enabled) { + set_bit(irqd_to_hwirq(irqd), ctx->work_ctx->pending); + irq_work_queue(&ctx->work_ctx->work); } } EXPORT_SYMBOL_GPL(irq_sim_fire); /** - * irq_sim_irqnum - Get the allocated number of a dummy interrupt. + * irq_sim_get_domain - Retrieve the interrupt domain for this simulator. * - * @sim: The interrupt simulator object. - * @offset: Offset of the simulated interrupt for which to retrieve - * the number. + * @sim: The interrupt simulator the domain of which we retrieve. */ -int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset) +struct irq_domain *irq_sim_get_domain(struct irq_sim *sim) { - return sim->irqs[offset].irqnum; + return sim->domain; } -EXPORT_SYMBOL_GPL(irq_sim_irqnum); +EXPORT_SYMBOL(irq_sim_get_domain);