> +static void bcm_iproc_i2c_slave_init( > + struct bcm_iproc_i2c_dev *iproc_i2c, bool need_reset) > +{ > + u32 val; > + > + if (need_reset) { > + /* put controller in reset */ > + val = readl(iproc_i2c->base + CFG_OFFSET); > + val |= BIT(CFG_RESET_SHIFT); > + writel(val, iproc_i2c->base + CFG_OFFSET); > + > + /* wait 100 usec per spec */ > + udelay(100); > + > + /* bring controller out of reset */ > + val &= ~(BIT(CFG_RESET_SHIFT)); > + writel(val, iproc_i2c->base + CFG_OFFSET); > + } > + > + /* flush TX/RX FIFOs */ > + val = (BIT(S_FIFO_RX_FLUSH_SHIFT) | BIT(S_FIFO_TX_FLUSH_SHIFT)); > + writel(val, iproc_i2c->base + S_FIFO_CTRL_OFFSET); Will flushing FIFOs work when a slave is register while a master transfer is on-going at the same time? > + > + /* RANDOM SLAVE STRETCH time - 20ms*/ What is a "random stretch time"? 20ms sounds like a lot. Also, missing space before comment terminator. > @@ -224,22 +473,25 @@ static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *iproc_i2c) > > /* put controller in reset */ > val = readl(iproc_i2c->base + CFG_OFFSET); > - val |= 1 << CFG_RESET_SHIFT; > - val &= ~(1 << CFG_EN_SHIFT); > + val |= BIT(CFG_RESET_SHIFT); > + val &= ~(BIT(CFG_EN_SHIFT)); > writel(val, iproc_i2c->base + CFG_OFFSET); > > /* wait 100 usec per spec */ > udelay(100); > > /* bring controller out of reset */ > - val &= ~(1 << CFG_RESET_SHIFT); > + val &= ~(BIT(CFG_RESET_SHIFT)); > writel(val, iproc_i2c->base + CFG_OFFSET); > > /* flush TX/RX FIFOs and set RX FIFO threshold to zero */ > - val = (1 << M_FIFO_RX_FLUSH_SHIFT) | (1 << M_FIFO_TX_FLUSH_SHIFT); > + val = (BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT)); > writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET); > /* disable all interrupts */ > - writel(0, iproc_i2c->base + IE_OFFSET); > + val = readl(iproc_i2c->base + IE_OFFSET); > + val &= ~(IE_M_ALL_INTERRUPT_MASK << > + IE_M_ALL_INTERRUPT_SHIFT); > + writel(val, iproc_i2c->base + IE_OFFSET); This block looks unrelated, but I won't be too strict here... > + case M_CMD_STATUS_FIFO_UNDERRUN: > + dev_dbg(iproc_i2c->device, "FIFO under-run\n"); > + return -ENXIO; > + > + case M_CMD_STATUS_RX_FIFO_FULL: > + dev_dbg(iproc_i2c->device, "Master Rx FIFO full > 10ms\n"); > + return -ETIMEDOUT; > + ... however, this looks really unrelated to me. This is about master transmission, or? Rest looks OK.