linux-kernel - Re: [PATCH 3/9] irq/irq_sim: provide irq_sim_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190124080914.knftob7s4hpgz27n@pengutronix.de>
Date:   Thu, 24 Jan 2019 09:09:14 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Bartosz Golaszewski <brgl@...ev.pl>
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()

On Thu, Jan 24, 2019 at 08:46:56AM +0100, Bartosz Golaszewski wrote:
> ś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.

How (if at all) do you intend to support level sensitive irqs with this
interface? It probably works (but I didn't thought it through
completely), but firing a LEVEL_HIGH sensitive irq on

	irq_sim_fire_type(EDGE_RISING)

might look at least surprising and needs proper comments and thoughts.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |