irq-sim updates for 4.15
cover irq-sim updates for 4.15
I initially submitted patch 2/3 as a standalone, but it turned out during some development for iio that some additional functionalities would be useful as well.
The first patch pulls in slab.h directly instead of relying on the include from irq.h.
The second exposes an interface for accessing the base of the allocated interrupt range.
The third implements a function that allows to map the allocated interrupt number to the array offset within the irq sim object. This will be later (4.16) used in iio, where for 4.15 we needed to keep the irq base value and to calculate the offset manually in the driver.
Bartosz Golaszewski (3):
irq/irq_sim: explicitly pull in slab.h
irq/irq_sim: implement irq_sim_baseirq()
irq/irq_sim: implement irq_sim_irq2offset()
include/linux/irq_sim.h | 2 ++ kernel/irq/irq_sim.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
Signature · 1 line
2.13.2
PATCH 1/3 irq/irq_sim: explicitly pull in slab.h
We're calling kfree() in the irq_sim code but slab.h is pulled in indirectly via irq.h. Include it explicitly.
--- kernel/irq/irq_sim.c | 1 + 1 file changed, 1 insertion(+)
kernel/irq/irq_sim.c +1 -0
@@ -7,6 +7,7 @@* option) any later version. */ +#include <linux/slab.h> #include <linux/irq_sim.h> #include <linux/irq.h>
Signature · 1 line
2.13.2
PATCH 2/3 irq/irq_sim: implement irq_sim_baseirq()
In reply to Bartosz GolaszewskiSome users need to access the base of the allocated interrupt range. Although this can be calculated manually, it's more elegant to expose an interface for that.
--- include/linux/irq_sim.h | 1 + kernel/irq/irq_sim.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
include/linux/irq_sim.h +1 -0
@@ -40,5 +40,6 @@ int devm_irq_sim_init(struct device *dev, struct irq_sim *sim,void irq_sim_fini (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); +int irq_sim_baseirq (struct irq_sim *sim);#endif /* _LINUX_IRQ_SIM_H */
kernel/irq/irq_sim.c +11 -0
@@ -163,3 +163,14 @@ int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset)return sim->irqs[offset].irqnum; }EXPORT_SYMBOL_GPL (irq_sim_irqnum); + +/** +* irq_sim_baseirq - Get the base interrupt number. +* +* @sim: The interrupt simulator object. +*/ +int irq_sim_baseirq (struct irq_sim *sim) +{ +return irq_sim_irqnum (sim,0 ); +} +EXPORT_SYMBOL_GPL (irq_sim_baseirq);
Signature · 1 line
2.13.2
PATCH 3/3 irq/irq_sim: implement irq_sim_irq2offset()
In reply to Bartosz GolaszewskiAdd a routine allowing to retrieve the offset corresponding with an allocated interrupt number from an irq_sim object.
--- include/linux/irq_sim.h | 1 + kernel/irq/irq_sim.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)
include/linux/irq_sim.h +1 -0
@@ -41,5 +41,6 @@ void irq_sim_fini(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);int irq_sim_baseirq (struct irq_sim *sim); +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq);#endif /* _LINUX_IRQ_SIM_H */
kernel/irq/irq_sim.c +15 -0
@@ -174,3 +174,18 @@ int irq_sim_baseirq(struct irq_sim *sim)return irq_sim_irqnum (sim,0 ); }EXPORT_SYMBOL_GPL (irq_sim_baseirq); + +/** +* irq_sim_irq2offset - Get the offset of an allocated interrupt. +* +* @sim: The interrupt simulator object. +* irq: Allocated interrupt number. +* +* Perform a reverse lookup for the interrupt number and return its offset in +* the array allocated by sim_irq. +*/ +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq) +{ +return irq - sim->irq_base; +} +EXPORT_SYMBOL_GPL (irq_sim_irq2offset);
Signature · 1 line
2.13.2
Re: [PATCH 3/3] irq/irq_sim: implement irq_sim_irq2offset()
Quoted 34 lines · Bartosz Golaszewski
Add a routine allowing to retrieve the offset corresponding with an allocated interrupt number from an
irq_simobject.Signed-off-by Bartosz Golaszewski <[email protected]>--- include/linux/irq_sim.h | 1 + kernel/irq/irq_sim.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)include/linux/irq_sim.h +1
-0@@ -41,5 +41,6 @@ void irq_sim_fini(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);int irq_sim_baseirq (struct irq_sim *sim); +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq);#endif /* _LINUX_IRQ_SIM_H */ kernel/irq/irq_sim.c +5
-0@@ -174,3 +174,18 @@ int irq_sim_baseirq(struct irq_sim *sim)return irq_sim_irqnum (sim,0 ); }EXPORT_SYMBOL_GPL (irq_sim_baseirq); + +/** +* irq_sim_irq2offset - Get the offset of an allocated interrupt. +* +* @sim: The interrupt simulator object.
kernel/irq/irq_sim.c int irq_sim_baseirq(struct irq_sim *sim) · line 191
Quoted 10 lines
kernel/irq/irq_sim.c +10
-0+* irq: Allocated interrupt number. +* +* Perform a reverse lookup for the interrupt number and return its offset in +* the array allocated by sim_irq. +*/ +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq) +{ +return irq - sim->irq_base; +} +EXPORT_SYMBOL_GPL (irq_sim_irq2offset);
How is that useful in a random driver? The irq base and the offset are things that should be internal to an irqchip. Maybe you could explain the context you want to use this in.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Re: [PATCH 3/3] irq/irq_sim: implement irq_sim_irq2offset()
2017-10-18 10:58 GMT+02:00 Marc Zyngier <[email protected]>:
Quoted 41 lines · Bartosz Golaszewski
Quoted 40 lines
Add a routine allowing to retrieve the offset corresponding with an allocated interrupt number from an
irq_simobject.Signed-off-by Bartosz Golaszewski <[email protected]>--- include/linux/irq_sim.h | 1 + kernel/irq/irq_sim.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)include/linux/irq_sim.h +1
-0@@ -41,5 +41,6 @@ void irq_sim_fini(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);int irq_sim_baseirq (struct irq_sim *sim); +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq);#endif /* _LINUX_IRQ_SIM_H */ kernel/irq/irq_sim.c +12
-0@@ -174,3 +174,18 @@ int irq_sim_baseirq(struct irq_sim *sim)return irq_sim_irqnum (sim,0 ); }EXPORT_SYMBOL_GPL (irq_sim_baseirq); + +/** +* irq_sim_irq2offset - Get the offset of an allocated interrupt. +* +* @sim: The interrupt simulator object. +* irq: Allocated interrupt number. +* +* Perform a reverse lookup for the interrupt number and return its offset in +* the array allocated by sim_irq. +*/ +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq) +{
Quoted 9 lines
kernel/irq/irq_sim.c int irq_sim_baseirq(struct irq_sim *sim) · line 191
Quoted 4 lines
kernel/irq/irq_sim.c +3
-0+return irq - sim->irq_base; +} +EXPORT_SYMBOL_GPL (irq_sim_irq2offset);How is that useful in a random driver? The irq base and the offset are things that should be internal to an irqchip. Maybe you could explain the context you want to use this in.
The interrupts are allocated within the interrupt simulator code, but it's up to users to actually request these irqs. That's why we already have sim_irq_irqnum().
The IIO dummy driver (one of the users of irq_sim) uses these irq numbers retrieved from the simulator to also access private data stored in the iio_dummy_evgen module. Currently we store the base interrupt number in a separate variable in the dummy_evgen context struct and calculate the offset in the interrupt array manually. We could simplify that code by moving this logic into the interrupt simulator.
I hope it's clear enough.
Best regards,
Bartosz Golaszewski
Re: [PATCH 3/3] irq/irq_sim: implement irq_sim_irq2offset()
Quoted 56 lines · Bartosz Golaszewski
2017-10-18 10:58 GMT+02:00 Marc Zyngier <[email protected]>:
Quoted 41 lines · Bartosz Golaszewski
Quoted 40 lines
Add a routine allowing to retrieve the offset corresponding with an allocated interrupt number from an
irq_simobject.Signed-off-by Bartosz Golaszewski <[email protected]>--- include/linux/irq_sim.h | 1 + kernel/irq/irq_sim.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)include/linux/irq_sim.h +1
-0@@ -41,5 +41,6 @@ void irq_sim_fini(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);int irq_sim_baseirq (struct irq_sim *sim); +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq);#endif /* _LINUX_IRQ_SIM_H */ kernel/irq/irq_sim.c +12
-0@@ -174,3 +174,18 @@ int irq_sim_baseirq(struct irq_sim *sim)return irq_sim_irqnum (sim,0 ); }EXPORT_SYMBOL_GPL (irq_sim_baseirq); + +/** +* irq_sim_irq2offset - Get the offset of an allocated interrupt. +* +* @sim: The interrupt simulator object. +* irq: Allocated interrupt number. +* +* Perform a reverse lookup for the interrupt number and return its offset in +* the array allocated by sim_irq. +*/ +unsigned int irq_sim_irq2offset (struct irq_sim *sim,int irq) +{Quoted 9 lines
kernel/irq/irq_sim.c int irq_sim_baseirq(struct irq_sim *sim) · line 191
Quoted 4 lines
kernel/irq/irq_sim.c +3
-0+return irq - sim->irq_base; +} +EXPORT_SYMBOL_GPL (irq_sim_irq2offset);How is that useful in a random driver? The irq base and the offset are things that should be internal to an irqchip. Maybe you could explain the context you want to use this in.
The interrupts are allocated within the interrupt simulator code, but it's up to users to actually request these irqs. That's why we already have
sim_irq_irqnum().
Quoted 7 lines
The IIO dummy driver (one of the users of
irq_sim) uses these irq numbers retrieved from the simulator to also access private data stored in theiio_dummy_evgenmodule. Currently we store the base interrupt number in a separate variable in thedummy_evgencontext struct and calculate the offset in the interrupt array manually. We could simplify that code by moving this logic into the interrupt simulator.
It looks to me that this is all because the irq_sim creation is a bit awkward. You end-up with all kind of exotic interfaces because you don't know the base of the irq range at creation time.
How about something like this:
kernel/irq/irq_sim.c +3 -2
@@ -49,7 +49,8 @@ static void irq_sim_handle_irq(struct irq_work *work)* @sim: The interrupt simulator object to initialize. * @num_irqs: Number of interrupts to allocate * -* Returns 0 on success and a negative error number on failure. +* Returns the interrupt base on success and a negative error number +* on failure. */ int irq_sim_init (struct irq_sim *sim,unsigned int num_irqs) {
@@ -78,7 +79,7 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)init_irq_work (&sim->work_ctx.work, irq_sim_handle_irq); sim->irq_count = num_irqs; -return 0 ; +return sim->irq_base; }EXPORT_SYMBOL_GPL (irq_sim_init);
You can then deal with the offset directly in your driver, as you're guaranteed a 1:1 mapping.
Thoughts?
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Re: [PATCH 3/3] irq/irq_sim: implement irq_sim_irq2offset()
2017-10-18 12:13 GMT+02:00 Marc Zyngier <[email protected]>:
Quoted 1 line
[snip!]
Quoted 29 lines
It looks to me that this is all because the
irq_simcreation is a bit awkward. You end-up with all kind of exotic interfaces because you don't know the base of the irq range at creation time.How about something like this:
kernel/irq/irq_sim.c +3
-2@@ -49,7 +49,8 @@ static void irq_sim_handle_irq(struct irq_work *work)* @sim: The interrupt simulator object to initialize. * @num_irqs: Number of interrupts to allocate * -* Returns 0 on success and a negative error number on failure. +* Returns the interrupt base on success and a negative error number +* on failure. */ int irq_sim_init (struct irq_sim *sim,unsigned int num_irqs) {@@ -78,7 +79,7 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)init_irq_work (&sim->work_ctx.work, irq_sim_handle_irq); sim->irq_count = num_irqs; -return 0 ; +return sim->irq_base; }EXPORT_SYMBOL_GPL (irq_sim_init);
Quoted 3 lines
You can then deal with the offset directly in your driver, as you're guaranteed a 1:1 mapping.
This is what we have now in next for iio. The thing is we want to get rid of irq_base from the iio driver and store it (and relevant logic) in irq_sim to have less code in the caller. Otherwise we duplicate this info.
Thanks,
Bartosz
Re: [PATCH 3/3] irq/irq_sim: implement irq_sim_irq2offset()
Quoted 34 lines · Bartosz Golaszewski
2017-10-18 12:13 GMT+02:00 Marc Zyngier <[email protected]>:
Quoted 1 line
[snip!]
Quoted 29 lines
It looks to me that this is all because the
irq_simcreation is a bit awkward. You end-up with all kind of exotic interfaces because you don't know the base of the irq range at creation time.How about something like this:
kernel/irq/irq_sim.c +3
-2@@ -49,7 +49,8 @@ static void irq_sim_handle_irq(struct irq_work *work)* @sim: The interrupt simulator object to initialize. * @num_irqs: Number of interrupts to allocate * -* Returns 0 on success and a negative error number on failure. +* Returns the interrupt base on success and a negative error number +* on failure. */ int irq_sim_init (struct irq_sim *sim,unsigned int num_irqs) {@@ -78,7 +79,7 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)init_irq_work (&sim->work_ctx.work, irq_sim_handle_irq); sim->irq_count = num_irqs; -return 0 ; +return sim->irq_base; }EXPORT_SYMBOL_GPL (irq_sim_init);
Quoted 8 lines
Quoted 3 lines
You can then deal with the offset directly in your driver, as you're guaranteed a 1:1 mapping.
This is what we have now in next for iio. The thing is we want to get rid of
irq_basefrom the iio driver and store it (and relevant logic) inirq_simto have less code in the caller. Otherwise we duplicate this info.
The IIO subsystem caching this information is a side effect of the way it decides to deal with it. It is no different from the PCI layer requesting 16 MSIs and having to work out which ones it got. It doesn't go back to the irqchip to find out what's there.
How big is the saving on the IIO side?
Thanks,
M.
Signature · 1 line
Jazz is not dead, it just smell funny.
2017-10-05 14:44 GMT+02:00 Bartosz Golaszewski <[email protected]>:
Quoted 20 lines
I initially submitted patch 2/3 as a standalone, but it turned out during some development for iio that some additional functionalities would be useful as well.
The first patch pulls in
slab.hdirectly instead of relying on the include fromirq.h.The second exposes an interface for accessing the base of the allocated interrupt range.
The third implements a function that allows to map the allocated interrupt number to the array offset within the irq sim object. This will be later (4.16) used in iio, where for 4.15 we needed to keep the irq base value and to calculate the offset manually in the driver.
Bartosz Golaszewski (3):
irq/irq_sim: explicitly pull inslab.h
irq/irq_sim: implementirq_sim_baseirq()
irq/irq_sim: implementirq_sim_irq2offset()
Quoted 7 lines
include/linux/irq_sim.h | 2 ++ kernel/irq/irq_sim.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)--
2.13.2
Hi Thomas, Marc,
can these be merged or are there any issues with the series?
Thanks,
Bartosz