On Mon, Mar 02, 2015 at 09:49:11PM +0100, Geert Uytterhoeven wrote: > Hi Wolfram, > > On Mon, Mar 2, 2015 at 7:32 PM, Wolfram Sang <[email protected]> wrote: > > On Mon, Mar 02, 2015 at 06:28:43PM +0100, Geert Uytterhoeven wrote: > >> The r8a7791/koelsch development board has da9063 and da9210 regulators. > >> Both regulators have their interrupt request lines tied to the same > >> interrupt pin (IRQ2) on the SoC. > >> > >> After boot-up, both the da9063 and da9210 seem to assert their interrupt > >> request lines. Hence as soon as one driver requests this irq, it gets > >> stuck in an interrupt storm, as it only manages to deassert its own > >> interrupt request line, and the other driver hasn't installed an > >> interrupt handler yet. > >> > >> To handle this, install a quirk that masks the interrupts in both the > >> da9063 and da9210. This quirk has to run after the i2c master driver > >> has been initialized, but before the i2c slave drivers are initialized. > >> > >> On koelsch, the following happens: > >> > >> - Cold boot or reboot using the da9063 restart handler: > >> > >> IRQ2 is asserted, installing da9063/da9210 regulator quirk > >> ... > >> i2c i2c-6: regulator_quirk_notify: 1, IRQC_MONITOR = 0x3fb > >> i2c 6-0058: regulator_quirk_notify: 1, IRQC_MONITOR = 0x3fb > >> i2c 6-0058: Detected da9063 > >> i2c 6-0058: Masking da9063 interrupt sources > >> i2c 6-0068: regulator_quirk_notify: 1, IRQC_MONITOR = 0x3fb > >> i2c 6-0068: Detected da9210 > >> i2c 6-0068: Masking da9210 interrupt sources > >> i2c 6-0068: IRQ2 is not asserted, removing quirk > >> > >> - Warm boot (reset button): > >> > >> rcar_gen2_regulator_quirk: IRQ2 is not asserted, not installing quirk > >> > >> Not-yet-signed-off-by: Geert Uytterhoeven <[email protected]> > >> --- > >> Based on the schematics, I believe r8a7790/lager is also affected. > > > > Boot log from Lager (cold boot & watchdog reboot): > > > > [ 0.117037] IRQ2 is asserted, installing da9063/da9210 regulator quirk > > [ 0.162803] i2c i2c-4: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.163050] i2c-sh_mobile e6500000.i2c: I2C adapter 4, bus speed 100000 > > Hz > > [ 0.164283] i2c i2c-5: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.164481] i2c-sh_mobile e6510000.i2c: I2C adapter 5, bus speed 100000 > > Hz > > [ 0.165836] i2c i2c-6: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.166352] i2c 6-0012: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.166425] i2c 6-0012: Detected ak4643 > > [ 0.166891] i2c 6-0020: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.166963] i2c 6-0020: Detected adv7180 > > [ 0.167421] i2c 6-0039: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.167494] i2c 6-0039: Detected adv7511w > > [ 0.167630] i2c-sh_mobile e6520000.i2c: I2C adapter 6, bus speed 100000 > > Hz > > [ 0.168805] i2c i2c-7: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.169343] i2c 7-0058: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.169415] i2c 7-0058: Detected da9063 > > [ 0.169479] i2c 7-0058: Masking da9063 interrupt sources > > [ 0.175641] i2c 7-0068: regulator_quirk_notify: 1, IRQC_MONITOR = 0xb > > [ 0.175715] i2c 7-0068: Detected da9210 > > [ 0.175778] i2c 7-0068: Masking da9210 interrupt sources > > [ 0.176757] i2c 7-0068: IRQ2 is not asserted, removing quirk > > > > Reboot using reset: > > > > [ 0.117819] rcar_gen2_regulator_quirk: IRQ2 is not asserted, not > > installing quirk > > > > So, not surprisingly, no difference to Koelsch. For completeness, I disabled > > installing the notifier and got the interrupt storm again. So: > > > > Tested-by: Wolfram Sang <[email protected]> > > Thanks for testing! > > >> +#ifdef CONFIG_I2C > > > > Is it a realistic scenario that I2C and slave drivers are modules? > > Yes. > > Unfortunately that's more difficult to support, as the code references > i2c_bus_type and i2c_adapter_type. > > i2c_bus_type is needed to register the notifier. > i2c_adapter_type is needed to distinguish between i2c adapters and clients, > as the notifier is called for both with action BUS_NOTIFY_ADD_DEVICE. > > >> +static int regulator_quirk_notify(struct notifier_block *nb, > >> + unsigned long action, void *data); > > > > This forward declaration can be skipped... > > > >> + > >> +static struct notifier_block regulator_quirk_nb = { > >> + .notifier_call = regulator_quirk_notify > >> +}; > > > > ... if you move this after the notfier function. > > regulator_quirk_nb and regulator_quirk_notify reference each other. > But regulator_quirk_notify() can probably just use the passed > struct notifier_block pointer. Will fix. > > > Other than that the code looks okay to me (given that this is a quirk > > workaround).
Hi Geert, this seems reasonable enough to me, however, I'd like to let it hang out for a bit longer to see if there is any further review. To that end could you consider posting a non-RFC version for review if you wish to pursue this approach further? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
