Re: [PATCH v4 2/2] irq/irq_sim: simplify the API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Bartosz,

On 2020-04-30 15:30, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>

The interrupt simulator API exposes a lot of custom data structures and
functions and doesn't reuse the interfaces already exposed by the irq
subsystem. This patch tries to address it.

We hide all the simulator-related data structures from users and instead rely on the well-known irq domain. When creating the interrupt simulator the user receives a pointer to a newly created irq_domain and can use it
to create mappings for simulated interrupts.

It is also possible to pass a handle to fwnode when creating the simulator
domain and retrieve it using irq_find_matching_fwnode().

The irq_sim_fire() function now only takes the virtual interrupt number
as argument - there's no need anymore to pass it any data structure linked
to the simulator.

We modify the two modules that use the simulator at the same time as
adding these changes in order to reduce the intermediate bloat that would
result when trying to migrate the drivers in separate patches.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
Acked-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> #for IIO
---
 drivers/gpio/gpio-mockup.c          |  47 ++++--
 drivers/iio/dummy/iio_dummy_evgen.c |  32 ++--
 include/linux/irq_sim.h             |  34 ++---
 kernel/irq/Kconfig                  |   1 +
 kernel/irq/irq_sim.c                | 225 +++++++++++++++++-----------
 5 files changed, 202 insertions(+), 137 deletions(-)

[...]

 /**
  * 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 *irq_ctx;
+	struct irq_data *irqd;
+
+	irqd = irq_get_irq_data(virq);
+	if (!irqd) {
+		pr_warn_ratelimited("%s: invalid irq number\n", __func__);
+		return;
 	}
-}
-EXPORT_SYMBOL_GPL(irq_sim_fire);

-/**
- * irq_sim_irqnum - Get the allocated number of a dummy interrupt.
- *
- * @sim:        The interrupt simulator object.
- * @offset: Offset of the simulated interrupt for which to retrieve
- *              the number.
- */
-int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset)
-{
-	return sim->irqs[offset].irqnum;
+	irq_ctx = irq_data_get_irq_chip_data(irqd);
+
+	if (irq_ctx->enabled) {
+		set_bit(irqd_to_hwirq(irqd), irq_ctx->work_ctx->pending);
+		irq_work_queue(&irq_ctx->work_ctx->work);
+	}
 }
-EXPORT_SYMBOL_GPL(irq_sim_irqnum);
+EXPORT_SYMBOL_GPL(irq_sim_fire);

Rather than using an ad-hoc API to queue an interrupt, why don't you
actually implement the interface that already exists for this at
the irqchip level (irq_set_irqchip_state, which allows the pending
state to be set)?

Thanks,

        M.
--
Jazz is not dead. It just smells funny...



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux