On Fri, 22 Feb 2019 09:25:04 -0800
Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> * Andreas Kemnade <andreas@xxxxxxxxxxxx> [190222 17:09]:
> > Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > > It's probaby some twl related code is (wrongly) trying to read/write
> > > registers at noirq suspend level. Sounds like some driver tagged with
> > > noirq suspend ops while it should not.
> > >
> > static irqreturn_t handle_twl4030_pih(int irq, void *devid)
> > in
> > drivers/mfd/twl4030-irq.c
> > is doing the failed i2c access.
>
> I wonder if tagging drivers/mfd/twl4030-irq.c request_threaded_irq
> with IRQF_NO_SUSPEND in addition to IRQF_ONESHOT might help if this
IRQF_NO_SUSPEND does not help.
> triggers right after resume_device_irqs()? Or do we already have
> IRQF_NO_SUSPEND set and some interrupt like USB battery charging
> is triggering during suspend?
well, if we are doing rtcwake, we could expect to have a twl4030 rtc
interrupt sitting behind the pih.
I would consider usb battery charging irq here a bug because
twl4030_charger and phy-twl4030_usb are modules and not loaded.
Some more research:
what failed is this:
static int
omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
{
struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
int i;
int r;
r = pm_runtime_get_sync(omap->dev);
if (r < 0) {
we get -EACCESS there.
It is indeed the case that
handle_twl4030_pih is called before i2c runtime resume.
Since we have threaded irq here, we can do some sleep in case of
problems. So this dirty hack helps:
diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index b16c16f194fd..6c729c4ec9b0 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -34,6 +34,7 @@
#include <linux/of.h>
#include <linux/irqdomain.h>
#include <linux/mfd/twl.h>
+#include <linux/delay.h>
#include "twl-core.h"
@@ -298,7 +299,11 @@ static irqreturn_t handle_twl4030_pih(int irq, void *devid)
REG_PIH_ISR_P1);
if (ret) {
pr_warn("twl4030: I2C error %d reading PIH ISR\n", ret);
- return IRQ_NONE;
+ msleep(10);
+ ret = twl_i2c_read_u8(TWL_MODULE_PIH, &pih_isr,
+ REG_PIH_ISR_P1);
+ if (ret)
+ return IRQ_NONE;
}
while (pih_isr) {
So if for some reason handle_twl4030_pih is called late enough, we will
not have those problems.
So maybe we just need to add a suspend/resume callback pair in
twl-core.c and disable/enable the pih there? So when the pih is run,
runtime_pm is ready, so you are allowed to do pm_runtime_get()
To wake up from suspend, it is sufficient to have
twl4030_pins: pinmux_twl4030_pins {
pinctrl-single,pins = <
OMAP3_CORE1_IOPAD(0x21e0, PIN_INPUT_PULLUP | PIN_OFF_WAKEUPENABLE | MUX_MODE0) /* sys_nirq.sys_nirq */
>;
};
I can even do
@ -752,7 +757,9 @@ int twl4030_init_irq(struct device *dev, int irq_num)
dev_err(dev, "could not claim irq%d: %d\n", irq_num, status);
goto fail_rqirq;
}
+#if 0
enable_irq_wake(irq_num);
+#endif
return irq_base;
fail_rqirq:
at least here. So disabling that irq on suspend might be an option.
Regards,
Andreas
Attachment:
pgpDARJ_8pPzj.pgp
Description: OpenPGP digital signature
