Re: [PATCH 3/9] irq/irq_sim: provide irq_sim_get_type()

[Bartosz Golaszewski]

År., 23 sty 2019 o 20:18 Uwe Kleine-KÃnig
<u.kleine-koenig@xxxxxxxxxxxxxx> napisaÅ(a):
>
> Hello Bartosz,
>
> On Wed, Jan 23, 2019 at 03:15:32PM +0100, Bartosz Golaszewski wrote:
> > Provide a helper that allows users to retrieve the configured flow type
> > of dummy interrupts. That allows certain users to decide whether an irq
> > needs to be fired depending on its edge/level/... configuration.
>
> You don't talk about the .set_type callback here; is this intended?
>
> I wonder how you think this should be used. Assume the mockup-driver is
> directed to pull up a certain line, does it do something like that:
>
>         def mockup_setval(self, val):
>                 irqtype = irq_sim_get_type(...)
>                 if irqtype == LEVEL_HIGH:
>                         if val:
>                                 fire_irq()
>
>                 else if irqtype == EDGE_RISING:
>                         if val and not prev_val:
>                                 fire_irq()
>
>                 else if irqtype == LEVEL_LOW:
>                         if not val:
>                                 fire_irq()
>
>                 else if irqtype == EDGE_FALLING:
>                         if not val and prev_val:
>                                 fire_irq()
>
> I wonder if that logic should be done in the same place as where the irq
> type is stored. Otherwise that .type member is only a data store
> provided by the irq simulator. So I suggest to either move the variable
> that mirrors the current level of the line into the irq simulator, or
> keep the irqtype variable in the mockup driver. Both approaches would
> make it unnecessary to provide an accessor function for the type member.
>

Yeah, might be better to go back to my previous idea of adding
irq_sim_fire_edge(), but maybe it should be irq_sim_fire_type()
instead, so that irq_sim_fire() fires unconditionally and
irq_sim_fire_type() would fire only if the passed flag is the same as
the one previously configured by the set_type() callback.

Thanks,
Bartosz