Hi Dirk, thank you for another valuable bug report! On Tue, Jul 02, 2024 at 06:55:35AM +0200, Dirk Behme wrote: > We are getting crash reports where irqhandler crashes because it > uses priv->msg being NULL. This can happen if an interrupt is issued > before rcar_i2c_master_xfer() has initialized priv->msg. > > The rcar_i2c_probe() function assumes that the I2C hardware is > uninitialized and connects the interrupt handler via devm_request_irq(). > From this point in time irqhandler can be called. Normally, this is > just prevented by the I2C hardware being in reset state. > > However, there might be cases where rcar_i2c_probe() is called, but > the I2C hardware is *not* in reset state. E.g. if just the Linux > operating system was re-started by a supervisor. But the hardware didn't > get a reset. For cases like this make sure that the I2C hardware > interrupts are cleared properly before devm_request_irq() is called. > > This is inspired by rcar_i2c_init(). > > Signed-off-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx> > --- > drivers/i2c/busses/i2c-rcar.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > Notes: This is untested. Does this make sense and is acceptable? Yes, it makes sense to me. HW should be put to a sane state first in probe. IIRC this slipped through during some refactorization. Maybe it is also the reason for this code in the irq handler? 547 /* FIXME: sometimes, unknown interrupt happened. Do nothing */ 548 if (!(msr & MDE)) 549 return; Although this code existed even before the refactorization. Also, I will add a WARN there, otherwise we never know if this issue still exists. Anyhow, I think Geert's comment also makes sense. But instead of just adding the instructions, I'd prefer to reorganize the code. I hope it is okay for you if I send a "counterpatch". I can't really tell what outcome I actually prefer before I try out myself. Happy hacking, Wolfram
Attachment:
signature.asc
Description: PGP signature
