On 29/06/17 15:57, Jerome Brunet wrote:

On Thu, 2017-06-29 at 16:14 +0200, Linus Walleij wrote:

quoted

On Tue, Jun 27, 2017 at 8:25 PM, Jerome Brunet [off-list ref] wrote:

quoted

At the time Linus strongly rejected the idea of calling  irq_create_mapping
(or
any sleeping functions) in gpio_to_irq: please see the reply from Oct 26,
2016
(sorry for quoting such an old discussion but this is really the starting
point)

* Me: There is really a *lot* of gpio drivers which use irq_create_mapping
in
the to_irq callback, are these all wrong ?
* Linus: Yes they are all wrong. They should all be using
irq_find_mapping().

* Me: If this should not be used, what should we all do instead ?
* Linus: Call irq_create_mapping() in some other place.

gpio_prepare_irq is a proposition for this 'other place'.

There is a misunderstanding here.

I wrote (at the time):

quoted

Yes, but you want to call irq_create_mapping() in slowpath (irq setup)
and irq_find_mapping() in fastpath (irq handler). Else the first IRQ
may result in unwelcomed surprises.

This does not mean that irq_create_mapping() cannot be called
in irq context because I think it can.

What it means is that I think that is suboptimal from a performance
point of view if called from gpio_to_irq(), because nominally, at this
point, the mapping should already exist, since the GPIO and IRQ
frameworks are orthogonal.

Agreed. It should. In such case, irq_create_mapping is just a call to
irq_find_mapping which is fine. If, for whatever reason this is not the case,
this is going to call sleeping function in irq context. It should not happen but
it is not entirely impossible ...

quoted

But that may not apply to the case with many-to-few interrupt
mappings... so I think I was in some 1-to-1-mapping context
when I wrote this. Sorry :(

So I changed my mind, it is fine for this type of driver to call
irq_create_mapping() in gpio_to_irq(). Preferably with some comment
around the call.

What about disposing of the mapping ? there still is no counter part function to
gpio_to_irq. It seems weird to leave them lying around, don't you think ?

Here is a real use we will be having a meson:
* MMC driver load and call gpio_to_irq on its cd pin
  This is going to create a mapping 
* MMC driver request an irq with IRQ_EDGE_BOTH trigger (which we don't/can't
  support at the moment). request_irq fails.
* MMC defaults to polling the GPIO

Remember that we have only 8 possibles mappings. Now there is one (useless)
mapping lying around which can't get rid of.

Which should be dropped by a dispose_mapping() call in the MMC driver
(or ideally some gpio-specific wrapper around this function).

If there is more drivers doing this sort of tricks, we are going to exhaust the
ressource pretty quickly.

We can fix those drivers as we go. It's not like there's a huge variety
of potential HW on this particular platform of yours.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...