On Tue, 24 Apr 2018, Jarkko Nikula wrote: > > --- a/drivers/i2c/busses/i2c-designware-master.c > > +++ b/drivers/i2c/busses/i2c-designware-master.c > > @@ -181,6 +181,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev) > > /* Disable the adapter */ > > __i2c_dw_enable_and_wait(dev, false); > > + disable_irq(dev->irq); > > /* If the slave address is ten bit address, enable 10BITADDR */ > > ic_con = dw_readl(dev, DW_IC_CON); > > @@ -214,6 +215,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev) > > /* Clear and enable interrupts */ > > dw_readl(dev, DW_IC_CLR_INTR); > > dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK); > > + enable_irq(dev->irq); > > Are you seeing an issue here? Not sure about low level irq handling but > thinking here can we miss an interrupt between unmasking and enable_irq()? Right, sorry, enable_irq should be between clearing and unmasking. > I'm also thinking is there actually any issue if i2c_dw_isr() runs while > executing the i2c_dw_xfer_init() e.g. if the IRQ line is shared since the > i2c_dw_isr() is checking the DW_IC_ENABLE which is not enabled until near at > the end of i2c_dw_isr(). Imagine that i2c_dw_isr and i2c_dw_xfer_init begin running concurrently. First the isr observes non-zero DW_IC_ENABLE and proceeds. Then xfer_init waits until DW_IC_ENABLE becomes zero and also proceeds. At this point both the isr and xfer_init may access the adapter registers concurrently. Alexander
