| Message ID | 20181120134032.31645-2-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] irq/irq_sim: provide irq_sim_fire_edge() | expand |
On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > ("gpiolib: Don't support irq sharing for userspace") which made it > impossible for gpio-mockup to ignore certain events (e.g. only receive > notifications about rising edge events). > > Introduce a specialized variant of irq_sim_fire() which takes another > argument called edge. allowing to specify the trigger type for the > dummy interrupt. I wonder if it's worth the effort to fix irq_sim. If you take a look in my gpio-simulator patch, it is trivial to get it right without external help with an amount of code that is usual for a driver that handles irqs. > @@ -161,12 +171,28 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init); > */ > void irq_sim_fire(struct irq_sim *sim, unsigned int offset) > { > - if (sim->irqs[offset].enabled) { > + /* Don't care about the edge. */ > + irq_sim_fire_edge(sim, offset, IRQ_TYPE_EDGE_BOTH); Conceptually irq_sim_fire_edge should be defined above irq_sim_fire. > +EXPORT_SYMBOL_GPL(irq_sim_fire); > + > +/** > + * irq_sim_fire_edge - Enqueue an interrupt, specify the edge. > + * > + * @sim: The interrupt simulator object. > + * @offset: Offset of the simulated interrupt which should be fired. > + * edge: Edge of the interrupt (IRQ_TYPE_EDGE_RISING/FALLING). > + */ > +void irq_sim_fire_edge(struct irq_sim *sim, unsigned int offset, int edge) > +{ > + struct irq_sim_irq_ctx *irq = &sim->irqs[offset]; > + > + if (irq->enabled && (irq->edge & edge)) { > set_bit(offset, sim->work_ctx.pending); > irq_work_queue(&sim->work_ctx.work); > } > } > -EXPORT_SYMBOL_GPL(irq_sim_fire); > +EXPORT_SYMBOL_GPL(irq_sim_fire_edge); Best regards Uwe
wt., 20 lis 2018 o 18:17 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > > ("gpiolib: Don't support irq sharing for userspace") which made it > > impossible for gpio-mockup to ignore certain events (e.g. only receive > > notifications about rising edge events). > > > > Introduce a specialized variant of irq_sim_fire() which takes another > > argument called edge. allowing to specify the trigger type for the > > dummy interrupt. > > I wonder if it's worth the effort to fix irq_sim. If you take a look in > my gpio-simulator patch, it is trivial to get it right without external > help with an amount of code that is usual for a driver that handles > irqs. > You're basically recommending handcrafting another local piece of code for simulating interrupts - something that multiple users may be interested in. You did that in your proposed gpio-simulator and I still can't understand why you couldn't reuse the existing solution. Even if it's broken for your use-case, it's surely easier to fix it than to rewrite and duplicate it? There are very few cases where code consolidation is not a good thing and I don't think this is one of them. The interrupt simulator exists and is used by two testing drivers currently. One of them needs some more flexibility and this is what this patch is trying to add. I believe it is worth the effort. Best regards, Bartosz > > @@ -161,12 +171,28 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init); > > */ > > void irq_sim_fire(struct irq_sim *sim, unsigned int offset) > > { > > - if (sim->irqs[offset].enabled) { > > + /* Don't care about the edge. */ > > + irq_sim_fire_edge(sim, offset, IRQ_TYPE_EDGE_BOTH); > > Conceptually irq_sim_fire_edge should be defined above irq_sim_fire. > > > +EXPORT_SYMBOL_GPL(irq_sim_fire); > > + > > +/** > > + * irq_sim_fire_edge - Enqueue an interrupt, specify the edge. > > + * > > + * @sim: The interrupt simulator object. > > + * @offset: Offset of the simulated interrupt which should be fired. > > + * edge: Edge of the interrupt (IRQ_TYPE_EDGE_RISING/FALLING). > > + */ > > +void irq_sim_fire_edge(struct irq_sim *sim, unsigned int offset, int edge) > > +{ > > + struct irq_sim_irq_ctx *irq = &sim->irqs[offset]; > > + > > + if (irq->enabled && (irq->edge & edge)) { > > set_bit(offset, sim->work_ctx.pending); > > irq_work_queue(&sim->work_ctx.work); > > } > > } > > -EXPORT_SYMBOL_GPL(irq_sim_fire); > > +EXPORT_SYMBOL_GPL(irq_sim_fire_edge); > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |
Hello Bartosz, On Wed, Nov 21, 2018 at 05:34:32PM +0100, Bartosz Golaszewski wrote: > wt., 20 lis 2018 o 18:17 Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > > > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > > > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > > > ("gpiolib: Don't support irq sharing for userspace") which made it > > > impossible for gpio-mockup to ignore certain events (e.g. only receive > > > notifications about rising edge events). > > > > > > Introduce a specialized variant of irq_sim_fire() which takes another > > > argument called edge. allowing to specify the trigger type for the > > > dummy interrupt. > > > > I wonder if it's worth the effort to fix irq_sim. If you take a look in > > my gpio-simulator patch, it is trivial to get it right without external > > help with an amount of code that is usual for a driver that handles > > irqs. > > You're basically recommending handcrafting another local piece of code > for simulating interrupts - something that multiple users may be > interested in. You did that in your proposed gpio-simulator and I > still can't understand why you couldn't reuse the existing solution. > Even if it's broken for your use-case, it's surely easier to fix it > than to rewrite and duplicate it? There are very few cases where code > consolidation is not a good thing and I don't think this is one of > them. I don't say that factoring out common stuff is bad. But if in the end you call irq_sim_something(some, parameters, offset); with the simulator and if you don't use the irq simulator you do irq = irq_find_mapping(irqdomain, offset); generic_handle_irq(irq); I prefer the latter because it's only a single additional line and in return it's more obvious what it does because it's the same that many other drivers (for real hardware) also do. Best regards Uwe
śr., 21 lis 2018 o 20:15 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > Hello Bartosz, > > On Wed, Nov 21, 2018 at 05:34:32PM +0100, Bartosz Golaszewski wrote: > > wt., 20 lis 2018 o 18:17 Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > > > > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > > > > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > > > > ("gpiolib: Don't support irq sharing for userspace") which made it > > > > impossible for gpio-mockup to ignore certain events (e.g. only receive > > > > notifications about rising edge events). > > > > > > > > Introduce a specialized variant of irq_sim_fire() which takes another > > > > argument called edge. allowing to specify the trigger type for the > > > > dummy interrupt. > > > > > > I wonder if it's worth the effort to fix irq_sim. If you take a look in > > > my gpio-simulator patch, it is trivial to get it right without external > > > help with an amount of code that is usual for a driver that handles > > > irqs. > > > > You're basically recommending handcrafting another local piece of code > > for simulating interrupts - something that multiple users may be > > interested in. You did that in your proposed gpio-simulator and I > > still can't understand why you couldn't reuse the existing solution. > > Even if it's broken for your use-case, it's surely easier to fix it > > than to rewrite and duplicate it? There are very few cases where code > > consolidation is not a good thing and I don't think this is one of > > them. > > I don't say that factoring out common stuff is bad. But if in the end > you call > > irq_sim_something(some, parameters, offset); > > with the simulator and if you don't use the irq simulator you do > > irq = irq_find_mapping(irqdomain, offset); > generic_handle_irq(irq); > > I prefer the latter because it's only a single additional line and in > return it's more obvious what it does because it's the same that many > other drivers (for real hardware) also do. > I'm not sure I'm following you. You still need to add ~150 LOC for the gpio_simulator_irqtrigger() worker and gpio_simulator_irq_*() routines locally as you did in your gpio-simulator patch. A generic simulator + using the irq_work saves you that. Bart
On Fri, Nov 23, 2018 at 04:59:46PM +0100, Bartosz Golaszewski wrote: > śr., 21 lis 2018 o 20:15 Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > Hello Bartosz, > > > > On Wed, Nov 21, 2018 at 05:34:32PM +0100, Bartosz Golaszewski wrote: > > > wt., 20 lis 2018 o 18:17 Uwe Kleine-König > > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > > > On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > > > > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > > > > > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > > > > > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > > > > > ("gpiolib: Don't support irq sharing for userspace") which made it > > > > > impossible for gpio-mockup to ignore certain events (e.g. only receive > > > > > notifications about rising edge events). > > > > > > > > > > Introduce a specialized variant of irq_sim_fire() which takes another > > > > > argument called edge. allowing to specify the trigger type for the > > > > > dummy interrupt. > > > > > > > > I wonder if it's worth the effort to fix irq_sim. If you take a look in > > > > my gpio-simulator patch, it is trivial to get it right without external > > > > help with an amount of code that is usual for a driver that handles > > > > irqs. > > > > > > You're basically recommending handcrafting another local piece of code > > > for simulating interrupts - something that multiple users may be > > > interested in. You did that in your proposed gpio-simulator and I > > > still can't understand why you couldn't reuse the existing solution. > > > Even if it's broken for your use-case, it's surely easier to fix it > > > than to rewrite and duplicate it? There are very few cases where code > > > consolidation is not a good thing and I don't think this is one of > > > them. > > > > I don't say that factoring out common stuff is bad. But if in the end > > you call > > > > irq_sim_something(some, parameters, offset); > > > > with the simulator and if you don't use the irq simulator you do > > > > irq = irq_find_mapping(irqdomain, offset); > > generic_handle_irq(irq); > > > > I prefer the latter because it's only a single additional line and in > > return it's more obvious what it does because it's the same that many > > other drivers (for real hardware) also do. > > I'm not sure I'm following you. You still need to add ~150 LOC for the > gpio_simulator_irqtrigger() worker and gpio_simulator_irq_*() routines > locally as you did in your gpio-simulator patch. A generic simulator + > using the irq_work saves you that. If you teach the irq-sim driver everything that the gpio-simulator does in the functions you pointed out then this is for sure functionality that other users of the irq-sim code won't make use of. This is about tracking the level of the gpio/irq line and the interrupt enable and raw status bits that usually happen in hardware. The dummy iio driver won't need that for sure as it only cares about triggering an irq and doesn't even specify an irq type. Best regards Uwe
niedz., 25 lis 2018 o 22:18 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Fri, Nov 23, 2018 at 04:59:46PM +0100, Bartosz Golaszewski wrote: > > śr., 21 lis 2018 o 20:15 Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > Hello Bartosz, > > > > > > On Wed, Nov 21, 2018 at 05:34:32PM +0100, Bartosz Golaszewski wrote: > > > > wt., 20 lis 2018 o 18:17 Uwe Kleine-König > > > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > > > > > On Tue, Nov 20, 2018 at 02:40:31PM +0100, Bartosz Golaszewski wrote: > > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > > > > > > > The irq_sim irqchip doesn't allow to configure the sensitivity so every > > > > > > call to irq_sim_fire() fires a dummy interrupt. This used to not matter > > > > > > for gpio-mockup (one of the irq_sim users) until commit fa38869b0161 > > > > > > ("gpiolib: Don't support irq sharing for userspace") which made it > > > > > > impossible for gpio-mockup to ignore certain events (e.g. only receive > > > > > > notifications about rising edge events). > > > > > > > > > > > > Introduce a specialized variant of irq_sim_fire() which takes another > > > > > > argument called edge. allowing to specify the trigger type for the > > > > > > dummy interrupt. > > > > > > > > > > I wonder if it's worth the effort to fix irq_sim. If you take a look in > > > > > my gpio-simulator patch, it is trivial to get it right without external > > > > > help with an amount of code that is usual for a driver that handles > > > > > irqs. > > > > > > > > You're basically recommending handcrafting another local piece of code > > > > for simulating interrupts - something that multiple users may be > > > > interested in. You did that in your proposed gpio-simulator and I > > > > still can't understand why you couldn't reuse the existing solution. > > > > Even if it's broken for your use-case, it's surely easier to fix it > > > > than to rewrite and duplicate it? There are very few cases where code > > > > consolidation is not a good thing and I don't think this is one of > > > > them. > > > > > > I don't say that factoring out common stuff is bad. But if in the end > > > you call > > > > > > irq_sim_something(some, parameters, offset); > > > > > > with the simulator and if you don't use the irq simulator you do > > > > > > irq = irq_find_mapping(irqdomain, offset); > > > generic_handle_irq(irq); > > > > > > I prefer the latter because it's only a single additional line and in > > > return it's more obvious what it does because it's the same that many > > > other drivers (for real hardware) also do. > > > > I'm not sure I'm following you. You still need to add ~150 LOC for the > > gpio_simulator_irqtrigger() worker and gpio_simulator_irq_*() routines > > locally as you did in your gpio-simulator patch. A generic simulator + > > using the irq_work saves you that. > > If you teach the irq-sim driver everything that the gpio-simulator does > in the functions you pointed out then this is for sure functionality > that other users of the irq-sim code won't make use of. This is about > tracking the level of the gpio/irq line and the interrupt enable and raw > status bits that usually happen in hardware. The dummy iio driver won't > need that for sure as it only cares about triggering an irq and doesn't > even specify an irq type. > We're getting too much into details of how to handle simulated interrupts and we can continue discussing it, but meanwhile I'd like to address a different thing: Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq sharing for userspace") some libgpiod tests are failing because we can no longer depend on reading the value of a dummy GPIO after detecting an interrupt to know the edge of the interrupt. While these interrupts are triggered from debugfs and debugfs is not required to maintain compatibility, I thing having a working test suite for the GPIO subsystem and uAPI is worth applying these two patches and also the previous one[1]. Can we have them applied for 4.20 or are there any objections? Best regards, Bartosz Golaszewski [1] https://lkml.org/lkml/2018/11/9/1418
On Thu, Nov 29, 2018 at 7:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > sharing for userspace") some libgpiod tests are failing because we can > no longer depend on reading the value of a dummy GPIO after detecting > an interrupt to know the edge of the interrupt. While these interrupts > are triggered from debugfs and debugfs is not required to maintain > compatibility, I thing having a working test suite for the GPIO > subsystem and uAPI is worth applying these two patches and also the > previous one[1]. > > Can we have them applied for 4.20 or are there any objections? I'm fine with applying them if I can just get an ACK from one of the IRQ maintainers (Thomas, Marc). Yours, Linus Walleij
On Fri, 30 Nov 2018 23:26:25 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Nov 29, 2018 at 7:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > sharing for userspace") some libgpiod tests are failing because we can > > no longer depend on reading the value of a dummy GPIO after detecting > > an interrupt to know the edge of the interrupt. While these interrupts > > are triggered from debugfs and debugfs is not required to maintain > > compatibility, I thing having a working test suite for the GPIO > > subsystem and uAPI is worth applying these two patches and also the > > previous one[1]. > > > > Can we have them applied for 4.20 or are there any objections? > > I'm fine with applying them if I can just get an ACK from one of the IRQ > maintainers (Thomas, Marc). I'm fine with that patch, with the provision that (nitpick) the edge field is turned into a bool instead of an int. With that, Acked-by: Marc Zyngier <marc.zyngier@arm.com> Thanks, M.
niedz., 2 gru 2018 o 13:29 Marc Zyngier <marc.zyngier@arm.com> napisał(a): > > On Fri, 30 Nov 2018 23:26:25 +0100 > Linus Walleij <linus.walleij@linaro.org> wrote: > > > On Thu, Nov 29, 2018 at 7:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > > sharing for userspace") some libgpiod tests are failing because we can > > > no longer depend on reading the value of a dummy GPIO after detecting > > > an interrupt to know the edge of the interrupt. While these interrupts > > > are triggered from debugfs and debugfs is not required to maintain > > > compatibility, I thing having a working test suite for the GPIO > > > subsystem and uAPI is worth applying these two patches and also the > > > previous one[1]. > > > > > > Can we have them applied for 4.20 or are there any objections? > > > > I'm fine with applying them if I can just get an ACK from one of the IRQ > > maintainers (Thomas, Marc). > > I'm fine with that patch, with the provision that (nitpick) the edge > field is turned into a bool instead of an int. > > With that, > > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > I wanted to reuse the already existing IRQ_TYPE_EDGE_RISING/FALLING defines which I think makes more sense here than a boolean. Bart
Hello, On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > We're getting too much into details of how to handle simulated > interrupts and we can continue discussing it, but meanwhile I'd like > to address a different thing: > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > sharing for userspace") some libgpiod tests are failing because we can > no longer depend on reading the value of a dummy GPIO after detecting > an interrupt to know the edge of the interrupt. While these interrupts > are triggered from debugfs and debugfs is not required to maintain > compatibility, I thing having a working test suite for the GPIO > subsystem and uAPI is worth applying these two patches and also the > previous one[1]. > > Can we have them applied for 4.20 or are there any objections? Just for the record: I objected the patch, Bartosz agrees to discuss further and but because this is too much detail the patch should now be applied anyhow to fix the test suite of an external project. This seems wrong to me. Best regards Uwe
niedz., 2 gru 2018 o 22:56 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > Hello, > > On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > > We're getting too much into details of how to handle simulated > > interrupts and we can continue discussing it, but meanwhile I'd like > > to address a different thing: > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > sharing for userspace") some libgpiod tests are failing because we can > > no longer depend on reading the value of a dummy GPIO after detecting > > an interrupt to know the edge of the interrupt. While these interrupts > > are triggered from debugfs and debugfs is not required to maintain > > compatibility, I thing having a working test suite for the GPIO > > subsystem and uAPI is worth applying these two patches and also the > > previous one[1]. > > > > Can we have them applied for 4.20 or are there any objections? > > Just for the record: I objected the patch, Bartosz agrees to discuss > further and but because this is too much detail the patch should now be > applied anyhow to fix the test suite of an external project. This seems > wrong to me. > Just to look at it from a different perspective: we have a project whose tests rely on a behavior that was changed by Uwe's patch. While the patch is fine, we need to find a correct way of testing the GPIO user API. This may take a long time. In order to not break the tests of an external project in 4.20 I propose to patch the interrupt simulator (a component only used for testing) for now and to revisit it later without time pressure. Best regards, Bartosz
niedz., 2 gru 2018 o 23:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > niedz., 2 gru 2018 o 22:56 Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > Hello, > > > > On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > > > We're getting too much into details of how to handle simulated > > > interrupts and we can continue discussing it, but meanwhile I'd like > > > to address a different thing: > > > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > > sharing for userspace") some libgpiod tests are failing because we can > > > no longer depend on reading the value of a dummy GPIO after detecting > > > an interrupt to know the edge of the interrupt. While these interrupts > > > are triggered from debugfs and debugfs is not required to maintain > > > compatibility, I thing having a working test suite for the GPIO > > > subsystem and uAPI is worth applying these two patches and also the > > > previous one[1]. > > > > > > Can we have them applied for 4.20 or are there any objections? > > > > Just for the record: I objected the patch, Bartosz agrees to discuss > > further and but because this is too much detail the patch should now be > > applied anyhow to fix the test suite of an external project. This seems > > wrong to me. > > > > Just to look at it from a different perspective: we have a project > whose tests rely on a behavior that was changed by Uwe's patch. While > the patch is fine, we need to find a correct way of testing the GPIO > user API. This may take a long time. In order to not break the tests > of an external project in 4.20 I propose to patch the interrupt > simulator (a component only used for testing) for now and to revisit > it later without time pressure. > > Best regards, > Bartosz In fact after re-reading this conversation I'm still not sure what your objection is exactly. You're proposing a solution that may well be nicely engineered but it's specific to your gpio-simulator. Meanwhile I'm trying to provide a more generalized API for more testing modules to use. Why exactly would you not merge this fix? Bart
On Mon, Dec 03, 2018 at 11:23:38AM +0100, Bartosz Golaszewski wrote: > niedz., 2 gru 2018 o 23:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > > > niedz., 2 gru 2018 o 22:56 Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > Hello, > > > > > > On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > > > > We're getting too much into details of how to handle simulated > > > > interrupts and we can continue discussing it, but meanwhile I'd like > > > > to address a different thing: > > > > > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > > > sharing for userspace") some libgpiod tests are failing because we can > > > > no longer depend on reading the value of a dummy GPIO after detecting > > > > an interrupt to know the edge of the interrupt. While these interrupts > > > > are triggered from debugfs and debugfs is not required to maintain > > > > compatibility, I thing having a working test suite for the GPIO > > > > subsystem and uAPI is worth applying these two patches and also the > > > > previous one[1]. > > > > > > > > Can we have them applied for 4.20 or are there any objections? > > > > > > Just for the record: I objected the patch, Bartosz agrees to discuss > > > further and but because this is too much detail the patch should now be > > > applied anyhow to fix the test suite of an external project. This seems > > > wrong to me. > > > > > > > Just to look at it from a different perspective: we have a project > > whose tests rely on a behavior that was changed by Uwe's patch. While > > the patch is fine, we need to find a correct way of testing the GPIO > > user API. This may take a long time. In order to not break the tests > > of an external project in 4.20 I propose to patch the interrupt > > simulator (a component only used for testing) for now and to revisit > > it later without time pressure. > > > > Best regards, > > Bartosz > > In fact after re-reading this conversation I'm still not sure what > your objection is exactly. You're proposing a solution that may well > be nicely engineered but it's specific to your gpio-simulator. > Meanwhile I'm trying to provide a more generalized API for more > testing modules to use. I think you're generalizing something that won't find any user apart from your mockup driver. So I'd say the generalisation is wrong and the added code could better live in the mockup driver directly. Best regards Uwe
pon., 3 gru 2018 o 11:49 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Mon, Dec 03, 2018 at 11:23:38AM +0100, Bartosz Golaszewski wrote: > > niedz., 2 gru 2018 o 23:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > > > > > niedz., 2 gru 2018 o 22:56 Uwe Kleine-König > > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > > > Hello, > > > > > > > > On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > > > > > We're getting too much into details of how to handle simulated > > > > > interrupts and we can continue discussing it, but meanwhile I'd like > > > > > to address a different thing: > > > > > > > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > > > > sharing for userspace") some libgpiod tests are failing because we can > > > > > no longer depend on reading the value of a dummy GPIO after detecting > > > > > an interrupt to know the edge of the interrupt. While these interrupts > > > > > are triggered from debugfs and debugfs is not required to maintain > > > > > compatibility, I thing having a working test suite for the GPIO > > > > > subsystem and uAPI is worth applying these two patches and also the > > > > > previous one[1]. > > > > > > > > > > Can we have them applied for 4.20 or are there any objections? > > > > > > > > Just for the record: I objected the patch, Bartosz agrees to discuss > > > > further and but because this is too much detail the patch should now be > > > > applied anyhow to fix the test suite of an external project. This seems > > > > wrong to me. > > > > > > > > > > Just to look at it from a different perspective: we have a project > > > whose tests rely on a behavior that was changed by Uwe's patch. While > > > the patch is fine, we need to find a correct way of testing the GPIO > > > user API. This may take a long time. In order to not break the tests > > > of an external project in 4.20 I propose to patch the interrupt > > > simulator (a component only used for testing) for now and to revisit > > > it later without time pressure. > > > > > > Best regards, > > > Bartosz > > > > In fact after re-reading this conversation I'm still not sure what > > your objection is exactly. You're proposing a solution that may well > > be nicely engineered but it's specific to your gpio-simulator. > > Meanwhile I'm trying to provide a more generalized API for more > > testing modules to use. > > I think you're generalizing something that won't find any user apart > from your mockup driver. So I'd say the generalisation is wrong and the > added code could better live in the mockup driver directly. > It used to live in the gpio-mockup driver and I generalized it precisely because there was another driver - iio evgen - which was doing basically the same thing. While I don't know if there'll be more users (I'd guess it would be useful for testing purposes of other subsystems) having the same functionality implemented once is better than twice. Bart
On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > pon., 3 gru 2018 o 11:49 Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > On Mon, Dec 03, 2018 at 11:23:38AM +0100, Bartosz Golaszewski wrote: > > > niedz., 2 gru 2018 o 23:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > > > > > > > niedz., 2 gru 2018 o 22:56 Uwe Kleine-König > > > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > > > > > Hello, > > > > > > > > > > On Thu, Nov 29, 2018 at 07:14:45PM +0100, Bartosz Golaszewski wrote: > > > > > > We're getting too much into details of how to handle simulated > > > > > > interrupts and we can continue discussing it, but meanwhile I'd like > > > > > > to address a different thing: > > > > > > > > > > > > Thomas, Linus: after commit fa38869b0161 ("gpiolib: Don't support irq > > > > > > sharing for userspace") some libgpiod tests are failing because we can > > > > > > no longer depend on reading the value of a dummy GPIO after detecting > > > > > > an interrupt to know the edge of the interrupt. While these interrupts > > > > > > are triggered from debugfs and debugfs is not required to maintain > > > > > > compatibility, I thing having a working test suite for the GPIO > > > > > > subsystem and uAPI is worth applying these two patches and also the > > > > > > previous one[1]. > > > > > > > > > > > > Can we have them applied for 4.20 or are there any objections? > > > > > > > > > > Just for the record: I objected the patch, Bartosz agrees to discuss > > > > > further and but because this is too much detail the patch should now be > > > > > applied anyhow to fix the test suite of an external project. This seems > > > > > wrong to me. > > > > > > > > > > > > > Just to look at it from a different perspective: we have a project > > > > whose tests rely on a behavior that was changed by Uwe's patch. While > > > > the patch is fine, we need to find a correct way of testing the GPIO > > > > user API. This may take a long time. In order to not break the tests > > > > of an external project in 4.20 I propose to patch the interrupt > > > > simulator (a component only used for testing) for now and to revisit > > > > it later without time pressure. > > > > > > > > Best regards, > > > > Bartosz > > > > > > In fact after re-reading this conversation I'm still not sure what > > > your objection is exactly. You're proposing a solution that may well > > > be nicely engineered but it's specific to your gpio-simulator. > > > Meanwhile I'm trying to provide a more generalized API for more > > > testing modules to use. > > > > I think you're generalizing something that won't find any user apart > > from your mockup driver. So I'd say the generalisation is wrong and the > > added code could better live in the mockup driver directly. > > > > It used to live in the gpio-mockup driver and I generalized it > precisely because there was another driver - iio evgen - which was > doing basically the same thing. While I don't know if there'll be more > users (I'd guess it would be useful for testing purposes of other > subsystems) having the same functionality implemented once is better > than twice. The iio testing driver only needs the trigger and relies on an irq that then calls the registerd handler. The iio driver doesn't need to tune the edge sensitivity though and if your mockup driver just only calls the fire routine if the configured sensitivity justifies that, everything should work as expected. Best regards Uwe
On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > It used to live in the gpio-mockup driver and I generalized it > > precisely because there was another driver - iio evgen - which was > > doing basically the same thing. While I don't know if there'll be more > > users (I'd guess it would be useful for testing purposes of other > > subsystems) having the same functionality implemented once is better > > than twice. > > The iio testing driver only needs the trigger and relies on an irq that > then calls the registerd handler. The iio driver doesn't need to tune > the edge sensitivity though and if your mockup driver just only calls > the fire routine if the configured sensitivity justifies that, > everything should work as expected. Simulating edges in the generic IRQ simulator codes seems generally useful to me, even if there is just one user now. Certainly for any kind of IRQ testing, it could be interesting to throw several low-to-high and high-to-low transitions on a driver and see how it reacts. But it is up to the irqchip maintainers to state whether they agree. Yours, Linus Walleij
śr., 5 gru 2018 o 13:20 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > > > It used to live in the gpio-mockup driver and I generalized it > > > precisely because there was another driver - iio evgen - which was > > > doing basically the same thing. While I don't know if there'll be more > > > users (I'd guess it would be useful for testing purposes of other > > > subsystems) having the same functionality implemented once is better > > > than twice. > > > > The iio testing driver only needs the trigger and relies on an irq that > > then calls the registerd handler. The iio driver doesn't need to tune > > the edge sensitivity though and if your mockup driver just only calls > > the fire routine if the configured sensitivity justifies that, > > everything should work as expected. > > Simulating edges in the generic IRQ simulator codes seems > generally useful to me, even if there is just one user now. > > Certainly for any kind of IRQ testing, it could be interesting to > throw several low-to-high and high-to-low transitions > on a driver and see how it reacts. > > But it is up to the irqchip maintainers to state whether they > agree. > All that would be great, but at this point I just want to fix broken tests in user-space. After that we can think about how to improve/approach simulating interrupts all we want. Marc: is my explanation for using an int instead of bool for irq_sim_fire_edge() fine? Can Linus pick this up for fixes? Bart
On Wed, Dec 5, 2018 at 1:38 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > All that would be great, but at this point I just want to fix broken > tests in user-space. After that we can think about how to > improve/approach simulating interrupts all we want. That is true. The other Linus is known to steamroll maintainers who knowingly break userspace, so we have a broad consensus not to do that, and I think that holds no matter if the ABI user is a test or not, debugfs or not. Yours, Linus Walleij
On Wed, Dec 05, 2018 at 01:19:54PM +0100, Linus Walleij wrote: > On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > > > It used to live in the gpio-mockup driver and I generalized it > > > precisely because there was another driver - iio evgen - which was > > > doing basically the same thing. While I don't know if there'll be more > > > users (I'd guess it would be useful for testing purposes of other > > > subsystems) having the same functionality implemented once is better > > > than twice. > > > > The iio testing driver only needs the trigger and relies on an irq that > > then calls the registerd handler. The iio driver doesn't need to tune > > the edge sensitivity though and if your mockup driver just only calls > > the fire routine if the configured sensitivity justifies that, > > everything should work as expected. > > Simulating edges in the generic IRQ simulator codes seems > generally useful to me, even if there is just one user now. I cannot imagine another potential user. Which kind of driver could use that that is not a gpio simulator? Best regards Uwe
On Wed, Dec 5, 2018 at 2:20 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Wed, Dec 05, 2018 at 01:19:54PM +0100, Linus Walleij wrote: > > > The iio testing driver only needs the trigger and relies on an irq that > > > then calls the registerd handler. The iio driver doesn't need to tune > > > the edge sensitivity though and if your mockup driver just only calls > > > the fire routine if the configured sensitivity justifies that, > > > everything should work as expected. > > > > Simulating edges in the generic IRQ simulator codes seems > > generally useful to me, even if there is just one user now. > > I cannot imagine another potential user. Which kind of driver could use > that that is not a gpio simulator? I suppose anything that can generate an IRQ and wants to generate some test IRQs where edge matters, drivers/irqchips/? Regmap IRQ drivers/base/regmap/regmap-irq.c does support edges, and regmap already expose all registers it handles in debugfs, so it'd be really neat of you can also use debugfs to insert IRQs from a device using regmap. Yours, Linus Walleij
śr., 5 gru 2018 o 13:38 Bartosz Golaszewski <bgolaszewski@baylibre.com> napisał(a): > > śr., 5 gru 2018 o 13:20 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > > > On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> wrote: > > > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > > > > > It used to live in the gpio-mockup driver and I generalized it > > > > precisely because there was another driver - iio evgen - which was > > > > doing basically the same thing. While I don't know if there'll be more > > > > users (I'd guess it would be useful for testing purposes of other > > > > subsystems) having the same functionality implemented once is better > > > > than twice. > > > > > > The iio testing driver only needs the trigger and relies on an irq that > > > then calls the registerd handler. The iio driver doesn't need to tune > > > the edge sensitivity though and if your mockup driver just only calls > > > the fire routine if the configured sensitivity justifies that, > > > everything should work as expected. > > > > Simulating edges in the generic IRQ simulator codes seems > > generally useful to me, even if there is just one user now. > > > > Certainly for any kind of IRQ testing, it could be interesting to > > throw several low-to-high and high-to-low transitions > > on a driver and see how it reacts. > > > > But it is up to the irqchip maintainers to state whether they > > agree. > > > > All that would be great, but at this point I just want to fix broken > tests in user-space. After that we can think about how to > improve/approach simulating interrupts all we want. > > Marc: is my explanation for using an int instead of bool for > irq_sim_fire_edge() fine? Can Linus pick this up for fixes? > Ping. We have only this week left to fix the regression - can we get your Ack Marc? NOTE: this patch must go into v4.20, and will probably conflict with the bitmap patch you picked up for 4.21. I can take it into my PR for Linus Walleij and then fix the conflict in next. Bart
On Fri, Dec 14, 2018 at 03:07:37PM +0100, Linus Walleij wrote: > On Wed, Dec 5, 2018 at 2:20 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Wed, Dec 05, 2018 at 01:19:54PM +0100, Linus Walleij wrote: > > > > > The iio testing driver only needs the trigger and relies on an irq that > > > > then calls the registerd handler. The iio driver doesn't need to tune > > > > the edge sensitivity though and if your mockup driver just only calls > > > > the fire routine if the configured sensitivity justifies that, > > > > everything should work as expected. > > > > > > Simulating edges in the generic IRQ simulator codes seems > > > generally useful to me, even if there is just one user now. > > > > I cannot imagine another potential user. Which kind of driver could use > > that that is not a gpio simulator? > > I suppose anything that can generate an IRQ and wants to generate > some test IRQs where edge matters, drivers/irqchips/? Should the irqchip be the consumer or the provider of the irq? I would have said the provider as it is an irqchip. What would be the gain of such an irq chip as all users could use a (flexible enough) gpio simulator instead? Best regards Uwe
On Mon, Dec 17, 2018 at 11:32:45AM +0100, Bartosz Golaszewski wrote: > śr., 5 gru 2018 o 13:38 Bartosz Golaszewski > <bgolaszewski@baylibre.com> napisał(a): > > > > śr., 5 gru 2018 o 13:20 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König > > > <u.kleine-koenig@pengutronix.de> wrote: > > > > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > > > > > > > It used to live in the gpio-mockup driver and I generalized it > > > > > precisely because there was another driver - iio evgen - which was > > > > > doing basically the same thing. While I don't know if there'll be more > > > > > users (I'd guess it would be useful for testing purposes of other > > > > > subsystems) having the same functionality implemented once is better > > > > > than twice. > > > > > > > > The iio testing driver only needs the trigger and relies on an irq that > > > > then calls the registerd handler. The iio driver doesn't need to tune > > > > the edge sensitivity though and if your mockup driver just only calls > > > > the fire routine if the configured sensitivity justifies that, > > > > everything should work as expected. > > > > > > Simulating edges in the generic IRQ simulator codes seems > > > generally useful to me, even if there is just one user now. > > > > > > Certainly for any kind of IRQ testing, it could be interesting to > > > throw several low-to-high and high-to-low transitions > > > on a driver and see how it reacts. > > > > > > But it is up to the irqchip maintainers to state whether they > > > agree. > > > > > > > All that would be great, but at this point I just want to fix broken > > tests in user-space. After that we can think about how to > > improve/approach simulating interrupts all we want. > > > > Marc: is my explanation for using an int instead of bool for > > irq_sim_fire_edge() fine? Can Linus pick this up for fixes? > > > > Ping. We have only this week left to fix the regression - can we get > your Ack Marc? I don't understand the urge. The problem is that libgpiod's test is failing. And that is because when userspace requested IRQF_TRIGGER_FALLING the mockup driver also fires if the line rised and with my change libgpiod now sees that and wonders about it. Apart from the test failure both libgpiod and the gpio framework are entirely fine (AFAICT). The "fix" under discussion is to modify the mockup driver to only report a falling irq if the output is set to 0. But it also fires if the value is already 0 and is set to 0 again. So the problem isn't addressed completely, but only enough to make libgpiod's test suite report success. In my eyes this is not how test-driven development works. Tests are here to bring breakage into the light. That worked just fine here. And now as a test fails, the goal is to fix the breakage, but not to change just enough details to get the test to pass and then urge them through because "we're already at -rc7 and userspace broke!" And no, the right fix isn't hard. My concerns were expressed Tuesday last week, and the problem wasn't important enough since then to fix the patch set. But maybe it's just me and the Linux development process changed since I learned about it and today the demand on quality isn't that high any more. Best regards Uwe
pon., 17 gru 2018 o 13:59 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Mon, Dec 17, 2018 at 11:32:45AM +0100, Bartosz Golaszewski wrote: > > śr., 5 gru 2018 o 13:38 Bartosz Golaszewski > > <bgolaszewski@baylibre.com> napisał(a): > > > > > > śr., 5 gru 2018 o 13:20 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Uwe Kleine-König > > > > <u.kleine-koenig@pengutronix.de> wrote: > > > > > On Mon, Dec 03, 2018 at 11:57:26AM +0100, Bartosz Golaszewski wrote: > > > > > > > > > > It used to live in the gpio-mockup driver and I generalized it > > > > > > precisely because there was another driver - iio evgen - which was > > > > > > doing basically the same thing. While I don't know if there'll be more > > > > > > users (I'd guess it would be useful for testing purposes of other > > > > > > subsystems) having the same functionality implemented once is better > > > > > > than twice. > > > > > > > > > > The iio testing driver only needs the trigger and relies on an irq that > > > > > then calls the registerd handler. The iio driver doesn't need to tune > > > > > the edge sensitivity though and if your mockup driver just only calls > > > > > the fire routine if the configured sensitivity justifies that, > > > > > everything should work as expected. > > > > > > > > Simulating edges in the generic IRQ simulator codes seems > > > > generally useful to me, even if there is just one user now. > > > > > > > > Certainly for any kind of IRQ testing, it could be interesting to > > > > throw several low-to-high and high-to-low transitions > > > > on a driver and see how it reacts. > > > > > > > > But it is up to the irqchip maintainers to state whether they > > > > agree. > > > > > > > > > > All that would be great, but at this point I just want to fix broken > > > tests in user-space. After that we can think about how to > > > improve/approach simulating interrupts all we want. > > > > > > Marc: is my explanation for using an int instead of bool for > > > irq_sim_fire_edge() fine? Can Linus pick this up for fixes? > > > > > > > Ping. We have only this week left to fix the regression - can we get > > your Ack Marc? > > I don't understand the urge. The problem is that libgpiod's test is > failing. And that is because when userspace requested > IRQF_TRIGGER_FALLING the mockup driver also fires if the line rised and > with my change libgpiod now sees that and wonders about it. Apart from > the test failure both libgpiod and the gpio framework are entirely fine > (AFAICT). > > The "fix" under discussion is to modify the mockup driver to only report > a falling irq if the output is set to 0. But it also fires if the value > is already 0 and is set to 0 again. So the problem isn't addressed > completely, but only enough to make libgpiod's test suite report > success. > The problem with your approach is that you're treating gpio-mockup as a regular driver the goal of which is self-contained correctness. I treat it as a tool to test the userspace API. Of course libgpiod works correctly - it requests and receives the correct type of interrupts. So does the gpiolib in-kernel part. The problem indeed lies with the kernel testing module. But it doesn't matter - we want to make sure the uAPI works correctly i.e. it behaves the same with gpio-mockup as it would with a real driver for actual HW behind. We're not testing gpio-mockup(!) here. > In my eyes this is not how test-driven development works. Tests are > here to bring breakage into the light. That worked just fine here. And > now as a test fails, the goal is to fix the breakage, but not to change > just enough details to get the test to pass and then urge them through > because "we're already at -rc7 and userspace broke!" > The tests here are to find regressions in a) libgpiod and b) gpiolib kernel-to-userspace interface. The mockup module isn't part of either. The breakage in gpio-mockup/irq_sim is really not all that important. Whether the userspace API works is. And with a breakage like this we're now unable to check if it behaves correctly for events of specified types. So yes: for 4.20 I want to fix the gpio-mockup module just enough to keep the tests passing. > And no, the right fix isn't hard. My concerns were expressed Tuesday > last week, and the problem wasn't important enough since then to fix the > patch set. > I already told you on several occasions that I *will* address certain issues in irq_sim. It will *not* make you happy however because it will not use the mechanism you suggested in gpio-simulator as I still want to keep this relatively generic for others to use. I will fix other problems though - among others the one with multiple subsequent events of the same edge. I don't want to be in a hurry to propose something fast, so I want to patch the tests now and then have time to come up with a better solution. Linus agrees. I see no objections to that from neither Marc nor Thomas. I would really like to stop discussing this over and over again after my every e-mail in this thread. > But maybe it's just me and the Linux development process changed since I > learned about it and today the demand on quality isn't that high any > more. > First rule has always been "don't break the userspace" and everything else came after that. Best regards, Bartosz Golaszewski
diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h index 4500d453a63e..f55148e77792 100644 --- a/include/linux/irq_sim.h +++ b/include/linux/irq_sim.h @@ -22,6 +22,7 @@ struct irq_sim_work_ctx { struct irq_sim_irq_ctx { int irqnum; bool enabled; + int edge; }; struct irq_sim { @@ -36,6 +37,7 @@ int devm_irq_sim_init(struct device *dev, struct irq_sim *sim, unsigned int num_irqs); void irq_sim_fini(struct irq_sim *sim); void irq_sim_fire(struct irq_sim *sim, unsigned int offset); +void irq_sim_fire_edge(struct irq_sim *sim, unsigned int offset, int edge); int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset); #endif /* _LINUX_IRQ_SIM_H */ diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c index 98a20e1594ce..a9330050f751 100644 --- a/kernel/irq/irq_sim.c +++ b/kernel/irq/irq_sim.c @@ -25,10 +25,20 @@ static void irq_sim_irqunmask(struct irq_data *data) irq_ctx->enabled = true; } +static int irq_sim_set_type(struct irq_data *data, unsigned int type) +{ + struct irq_sim_irq_ctx *irq_ctx = irq_data_get_irq_chip_data(data); + + irq_ctx->edge = type & IRQ_TYPE_EDGE_BOTH; + + return 0; +} + static struct irq_chip irq_sim_irqchip = { .name = "irq_sim", .irq_mask = irq_sim_irqmask, .irq_unmask = irq_sim_irqunmask, + .irq_set_type = irq_sim_set_type, }; static void irq_sim_handle_irq(struct irq_work *work) @@ -161,12 +171,28 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init); */ void irq_sim_fire(struct irq_sim *sim, unsigned int offset) { - if (sim->irqs[offset].enabled) { + /* Don't care about the edge. */ + irq_sim_fire_edge(sim, offset, IRQ_TYPE_EDGE_BOTH); +} +EXPORT_SYMBOL_GPL(irq_sim_fire); + +/** + * irq_sim_fire_edge - Enqueue an interrupt, specify the edge. + * + * @sim: The interrupt simulator object. + * @offset: Offset of the simulated interrupt which should be fired. + * edge: Edge of the interrupt (IRQ_TYPE_EDGE_RISING/FALLING). + */ +void irq_sim_fire_edge(struct irq_sim *sim, unsigned int offset, int edge) +{ + struct irq_sim_irq_ctx *irq = &sim->irqs[offset]; + + if (irq->enabled && (irq->edge & edge)) { set_bit(offset, sim->work_ctx.pending); irq_work_queue(&sim->work_ctx.work); } } -EXPORT_SYMBOL_GPL(irq_sim_fire); +EXPORT_SYMBOL_GPL(irq_sim_fire_edge); /** * irq_sim_irqnum - Get the allocated number of a dummy interrupt.