[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=Me58SFJOMxie5aTkOAHRqS7bjdOSAaZSY8wjPK7tntTfQ@mail.gmail.com>
Date: Thu, 24 Jan 2019 08:46:56 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <marc.zyngier@....com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH 3/9] irq/irq_sim: provide irq_sim_get_type()
śr., 23 sty 2019 o 20:18 Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> 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
Powered by blists - more mailing lists