Hi,
On 07-03-17 10:18, Thomas Gleixner wrote:
Hans,
On Tue, 7 Mar 2017, Hans de Goede wrote:
I've an "interesting" irq problem. I've an i2c pmic (Intel Cherry Trail
Whiskey Cove) which itself contains an i2c controller (adapter in Linux
terms) and has a pin dedicated for raising irqs by the external battery
charger ic attached to its i2c-adapter.
To be able to use the irq for the external-charger, the driver for the
PMIC needs to implement an irqchip and here things get interesting. This
irqchip can NOT use handle_nested_irq, because the i2c-client driver's
irq-handler will want to read/write to the external-charger which uses
the i2c-controller embedded in the PMIC which requires handling of new
(not arrived when started) PMIC irqs, which cannot be done if the client
irq-handler is running in handle_nested_irq, because then the PMIC's irq
handler is already / still running and blocked in the i2c-client's
irq-handler which is waiting for the new interrupt(s) to get processed to
signal completion of the i2c-transaction(s) it is doing.
Cute circular dependency.
I've solved this the following way, which works but
I wonder if it is the right way to solve this ?
Note this sits inside the threaded interrupt handler
for the PMIC irq (after reading and acking the irqs):
/*
* Do NOT use handle_nested_irq here, the client irq handler will
* likely want to do i2c transfers and the i2c controller uses this
* interrupt handler as well, so running the client irq handler from
* this thread will cause things to lock up.
*/
if (reg & CHT_WC_EXTCHGRIRQ_CLIENT_IRQ) {
/*
* generic_handle_irq expects local irqs to be disabled
* as normally it is called from interrupt context.
*/
local_irq_disable();
generic_handle_irq(adap->client_irq);
local_irq_enable();
}
Not really pretty, but it works well enough. I can
live with this if you can live with it too :)
I'm a bit worried about this being hardcoded for that particular use
case. That also means that you cannot use the generic regmap irq handling
stuff and need to have your own irq magic there.
Yeah, although the amount of irqchip code needed is pretty minimal
for this (otherwise) simple device.
Wouldn't it be smarter to
mark that interrupt in some way and have that as a generic option?
Such a generic option is what I was looking for before going this
route, so yeah +1 for that. I'm not familiar enough with the
irq code to feel comfortable adding that, but if you can provide
a patch, I can test it.
Can you point me at the relevant drivers/patches?
Here is my current wip stuff (I've pushed this to a for-irq-discussion
branch so that the commits won't go away when I rebase my wip
branch).
mfd patches:
https://github.com/jwrdegoede/linux-sunxi/commit/a0b9ee4ba7b8206880415351a53d5fbbd5969ba6
https://github.com/jwrdegoede/linux-sunxi/commit/a2bff50785c31239ec699a8b43e827c40089ee1f
https://github.com/jwrdegoede/linux-sunxi/commit/708985e8f33471976359c9ff52e2db9964e3ddf4
i2c-adapter patch:
https://github.com/jwrdegoede/linux-sunxi/commit/88a671b3543ffd1f5f4251f4028e4a5d67d88fda
Note the mfd driver has its own irqchip using the regmap-irq.c stuff (second commit)
and the trouble some irqchip driver is nested from that in the i2c-adapter driver
(the last commit).
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
