Hi Wolfram, On 2019-08-08 21:54:17 +0200, Wolfram Sang wrote: > After we disabled interrupts, there might still be an active one > running. Sync before clearing the pointer to the slave device. > > Fixes: c31d0a00021d ("i2c: emev2: add slave support") > Reported-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx> > Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > --- > > Not tested on hardware yet. If someone has the board set up, testing if > standard I2C communication works would be nice. That would mean irq > setup did not regress. The actual race is more complicated to trigger. > If noone has the board, I will fetch it from my repository. It is packed > away currently. I do have the hardware but similar situation as you, packed away in its box. But the box is also packed in a larger box as I'm preparing for the move. I'm not sure in what box I put the box in, and I'm not super keen to look thru all of them before I unpack them on the other end. > > drivers/i2c/busses/i2c-emev2.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c > index 35b302d983e0..959d4912ec0d 100644 > --- a/drivers/i2c/busses/i2c-emev2.c > +++ b/drivers/i2c/busses/i2c-emev2.c > @@ -69,6 +69,7 @@ struct em_i2c_device { > struct completion msg_done; > struct clk *sclk; > struct i2c_client *slave; > + int irq; > }; > > static inline void em_clear_set_bit(struct em_i2c_device *priv, u8 clear, u8 set, u8 reg) > @@ -339,6 +340,12 @@ static int em_i2c_unreg_slave(struct i2c_client *slave) > > writeb(0, priv->base + I2C_OFS_SVA0); > > + /* > + * Wait for interrupt to finish. New slave irqs cannot happen because we > + * cleared the slave address and, thus, only extension codes will be > + * detected which do not use the slave ptr. > + */ > + synchronize_irq(priv->irq); > priv->slave = NULL; > > return 0; > @@ -355,7 +362,7 @@ static int em_i2c_probe(struct platform_device *pdev) > { > struct em_i2c_device *priv; > struct resource *r; > - int irq, ret; > + int ret; > > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > @@ -390,8 +397,8 @@ static int em_i2c_probe(struct platform_device *pdev) > > em_i2c_reset(&priv->adap); > > - irq = platform_get_irq(pdev, 0); > - ret = devm_request_irq(&pdev->dev, irq, em_i2c_irq_handler, 0, > + priv->irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, priv->irq, em_i2c_irq_handler, 0, > "em_i2c", priv); > if (ret) > goto err_clk; > @@ -401,7 +408,8 @@ static int em_i2c_probe(struct platform_device *pdev) > if (ret) > goto err_clk; > > - dev_info(&pdev->dev, "Added i2c controller %d, irq %d\n", priv->adap.nr, irq); > + dev_info(&pdev->dev, "Added i2c controller %d, irq %d\n", priv->adap.nr, > + priv->irq); > > return 0; > > -- > 2.20.1 > -- Regards, Niklas Söderlund