| Message ID | 20190212131600.18960-2-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: mockup: improve the user-space testing interface | expand |
On Tue, 12 Feb 2019 13:15:54 +0000, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Provide a helper that allows users to retrieve the current irq type > of dummy interrupts. > > Internally: store the type in the line context when the irq_set_type() > callback is called. > > This is required by the gpio-mockup module which wants to track the > state of GPIO lines and simulate rising and falling edge interrupts. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > include/linux/irq_sim.h | 2 ++ > kernel/irq/irq_sim.c | 23 +++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h > index 4500d453a63e..c73be0a2c15c 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 type; > }; > > struct irq_sim { > @@ -37,5 +38,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); > +unsigned int irq_sim_get_type(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..74561a0dca65 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->type = type; > + > + return 0; > +} In the cover letter, you say you only support edge. And yet you don't reject any of the level configuration. That's not right. > + > 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) > @@ -180,3 +190,16 @@ int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset) > return sim->irqs[offset].irqnum; > } > EXPORT_SYMBOL_GPL(irq_sim_irqnum); > + > +/** > + * irq_sim_get_type - Get the type configuration for a dummy interrupt. > + * > + * @sim: The interrupt simulator object. > + * @offset: Offset of the simulated interrupt for which to retrieve > + * the type. > + */ > +unsigned int irq_sim_get_type(struct irq_sim *sim, unsigned int offset) > +{ > + return sim->irqs[offset].type; > +} > +EXPORT_SYMBOL_GPL(irq_sim_get_type); This just shows that the way you have architected the whole thing is backward. The irq_set_type callback should push the configuration wherever it makes sense instead of offering a retrieving API exposing random bits of the data structures. Thanks, M.
diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h index 4500d453a63e..c73be0a2c15c 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 type; }; struct irq_sim { @@ -37,5 +38,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); +unsigned int irq_sim_get_type(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..74561a0dca65 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->type = type; + + 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) @@ -180,3 +190,16 @@ int irq_sim_irqnum(struct irq_sim *sim, unsigned int offset) return sim->irqs[offset].irqnum; } EXPORT_SYMBOL_GPL(irq_sim_irqnum); + +/** + * irq_sim_get_type - Get the type configuration for a dummy interrupt. + * + * @sim: The interrupt simulator object. + * @offset: Offset of the simulated interrupt for which to retrieve + * the type. + */ +unsigned int irq_sim_get_type(struct irq_sim *sim, unsigned int offset) +{ + return sim->irqs[offset].type; +} +EXPORT_SYMBOL_GPL(irq_sim_get_type);