Hi Dirk,
On Tue, Jul 2, 2024 at 6:56 AM Dirk Behme <dirk.behme@xxxxxxxxxxxx> 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>
Thanks for your patch!
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -1183,6 +1183,12 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> ret = platform_get_irq(pdev, 0);
> if (ret < 0)
> goto out_pm_put;
> +
> + /* reset master mode */
> + rcar_i2c_write(priv, ICMIER, 0);
> + rcar_i2c_write(priv, ICMCR, MDBS);
> + rcar_i2c_write(priv, ICMSR, 0);
LGTM, but I think you want to do the same for slave mode:
rcar_i2c_write(priv, ICSIER, 0);
rcar_i2c_write(priv, ICSCR, SDBS);
rcar_i2c_write(priv, ICSSR, 0);
> +
> priv->irq = ret;
> ret = devm_request_irq(dev, priv->irq, irqhandler, irqflags, dev_name(dev), priv);
> if (ret < 0) {
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
