Re: [PATCH v3 1/8] i2c: iproc: I2C read up to 255 bytes support

Re: [PATCH v3 1/8] i2c: iproc: I2C read up to 255 bytes support

[Ray Jui]

On 1/17/2019 10:04 PM, Rayagonda Kokatanur wrote:
> From: Shreesha Rajashekar <shreesha@broadcom.com>
> 
> Add support to allow I2C RX transfer up to 255 bytes.
> 
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
> ---
>  drivers/i2c/busses/i2c-bcm-iproc.c | 98 +++++++++++++++++++++++++++++---------
>  1 file changed, 76 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index 4c8c3bc4669c..eb7339a0280e 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -80,6 +80,10 @@
>  
>  #define I2C_TIMEOUT_MSEC             50000
>  #define M_TX_RX_FIFO_SIZE            64
> +#define M_RX_FIFO_MAX_THLD_VALUE     (M_TX_RX_FIFO_SIZE - 1)
> +
> +#define M_RX_MAX_READ_LEN            255
> +#define M_RX_FIFO_THLD_VALUE         50
>  
>  enum bus_speed_index {
>  	I2C_SPD_100K = 0,
> @@ -102,17 +106,41 @@ struct bcm_iproc_i2c_dev {
>  
>  	/* bytes that have been transferred */
>  	unsigned int tx_bytes;
> +	/* bytes that have been read */
> +	unsigned int rx_bytes;
> +	unsigned int thld_bytes;
>  };
>  
>  /*
>   * Can be expanded in the future if more interrupt status bits are utilized
>   */
> -#define ISR_MASK (BIT(IS_M_START_BUSY_SHIFT) | BIT(IS_M_TX_UNDERRUN_SHIFT))
> +#define ISR_MASK (BIT(IS_M_START_BUSY_SHIFT) | BIT(IS_M_TX_UNDERRUN_SHIFT)\
> +		| BIT(IS_M_RX_THLD_SHIFT))
> +
> +static void bcm_iproc_i2c_read_valid_bytes(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> +	struct i2c_msg *msg = iproc_i2c->msg;
> +
> +	/* Read valid data from RX FIFO */
> +	while (iproc_i2c->rx_bytes < msg->len) {
> +		if (!((readl(iproc_i2c->base +
> +		M_FIFO_CTRL_OFFSET) >>
> +		M_FIFO_RX_CNT_SHIFT) &
> +		M_FIFO_RX_CNT_MASK))
> +			break;
> +
> +		msg->buf[iproc_i2c->rx_bytes] =
> +			(readl(iproc_i2c->base + M_RX_OFFSET) >>
> +			M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
> +		iproc_i2c->rx_bytes++;
> +	}
> +}
>  
>  static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
>  {
>  	struct bcm_iproc_i2c_dev *iproc_i2c = data;
>  	u32 status = readl(iproc_i2c->base + IS_OFFSET);
> +	u32 tmp;
>  
>  	status &= ISR_MASK;
>  
> @@ -136,8 +164,6 @@ static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
>  
>  			/* mark the last byte */
>  			if (idx == msg->len - 1) {
> -				u32 tmp;
> -
>  				val |= BIT(M_TX_WR_STATUS_SHIFT);
>  
>  				/*
> @@ -156,6 +182,32 @@ static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
>  		iproc_i2c->tx_bytes += tx_bytes;
>  	}
>  
> +	if (status & BIT(IS_M_RX_THLD_SHIFT)) {
> +		struct i2c_msg *msg = iproc_i2c->msg;
> +		u32 bytes_left;
> +
> +		bcm_iproc_i2c_read_valid_bytes(iproc_i2c);
> +		bytes_left = msg->len - iproc_i2c->rx_bytes;
> +		if (bytes_left == 0) {
> +			/* finished reading all data, disable rx thld event */
> +			tmp = readl(iproc_i2c->base + IE_OFFSET);
> +			tmp &= ~BIT(IS_M_RX_THLD_SHIFT);
> +			writel(tmp, iproc_i2c->base + IE_OFFSET);
> +		} else if (bytes_left < iproc_i2c->thld_bytes) {
> +			/* set bytes left as threshold */
> +			tmp = readl(iproc_i2c->base + M_FIFO_CTRL_OFFSET);
> +			tmp &= ~(M_FIFO_RX_THLD_MASK << M_FIFO_RX_THLD_SHIFT);
> +			tmp |= (bytes_left << M_FIFO_RX_THLD_SHIFT);
> +			writel(tmp, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
> +			iproc_i2c->thld_bytes = bytes_left;
> +		}
> +		/*
> +		 * bytes_left >= iproc_i2c->thld_bytes,
> +		 * hence no need to change the THRESHOLD SET.
> +		 * It will remain as iproc_i2c->thld_bytes itself
> +		 */
> +	}
> +
>  	if (status & BIT(IS_M_START_BUSY_SHIFT)) {
>  		iproc_i2c->xfer_is_done = 1;
>  		complete(&iproc_i2c->done);
> @@ -253,7 +305,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  {
>  	int ret, i;
>  	u8 addr;
> -	u32 val;
> +	u32 val, tmp, val_intr_en;
>  	unsigned int tx_bytes;
>  	unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MSEC);
>  
> @@ -298,7 +350,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	 * transaction is done, i.e., the internal start_busy bit, transitions
>  	 * from 1 to 0.
>  	 */
> -	val = BIT(IE_M_START_BUSY_SHIFT);
> +	val_intr_en = BIT(IE_M_START_BUSY_SHIFT);
>  
>  	/*
>  	 * If TX data size is larger than the TX FIFO, need to enable TX
> @@ -307,9 +359,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	 */
>  	if (!(msg->flags & I2C_M_RD) &&
>  	    msg->len > iproc_i2c->tx_bytes)
> -		val |= BIT(IE_M_TX_UNDERRUN_SHIFT);
> -
> -	writel(val, iproc_i2c->base + IE_OFFSET);
> +		val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
>  
>  	/*
>  	 * Now we can activate the transfer. For a read operation, specify the
> @@ -317,11 +367,27 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	 */
>  	val = BIT(M_CMD_START_BUSY_SHIFT);
>  	if (msg->flags & I2C_M_RD) {
> +		iproc_i2c->rx_bytes = 0;
> +		if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
> +			iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
> +		else
> +			iproc_i2c->thld_bytes = msg->len;
> +
> +		/* set threshold value */
> +		tmp = readl(iproc_i2c->base + M_FIFO_CTRL_OFFSET);
> +		tmp &= ~(M_FIFO_RX_THLD_MASK << M_FIFO_RX_THLD_SHIFT);
> +		tmp |= iproc_i2c->thld_bytes << M_FIFO_RX_THLD_SHIFT;
> +		writel(tmp, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
> +
> +		/* enable the RX threshold interrupt */
> +		val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
> +
>  		val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
>  		       (msg->len << M_CMD_RD_CNT_SHIFT);
>  	} else {
>  		val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
>  	}
> +	writel(val_intr_en, iproc_i2c->base + IE_OFFSET);
>  	writel(val, iproc_i2c->base + M_CMD_OFFSET);
>  
>  	time_left = wait_for_completion_timeout(&iproc_i2c->done, time_left);
> @@ -353,17 +419,6 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		return ret;
>  	}
>  
> -	/*
> -	 * For a read operation, we now need to load the data from FIFO
> -	 * into the memory buffer
> -	 */
> -	if (msg->flags & I2C_M_RD) {
> -		for (i = 0; i < msg->len; i++) {
> -			msg->buf[i] = (readl(iproc_i2c->base + M_RX_OFFSET) >>
> -				      M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
> -		}
> -	}
> -
>  	return 0;
>  }
>  
> @@ -395,9 +450,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
>  	.functionality = bcm_iproc_i2c_functionality,
>  };
>  
> -static const struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> -	/* need to reserve one byte in the FIFO for the slave address */
> -	.max_read_len = M_TX_RX_FIFO_SIZE - 1,
> +static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> +	.max_read_len = M_RX_MAX_READ_LEN,
>  };
>  
>  static int bcm_iproc_i2c_cfg_speed(struct bcm_iproc_i2c_dev *iproc_i2c)
> 

Reviewed-by: Ray Jui <ray.jui@broadcom.com>

Re: [PATCH v3 2/8] i2c: iproc: Add Slave mode support

[Ray Jui]

On 1/17/2019 10:04 PM, Rayagonda Kokatanur wrote:
> From: Shreesha Rajashekar <shreesha.rajashekar@broadcom.com>
> 
> Add I2C slave mode support to the iProc I2C driver.
> I2C slave mode is supported in the iProc I2C block
> in all iProc based SoCs that include the I2C block.
> 
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Shreesha Rajashekar <shreesha.rajashekar@broadcom.com>
> Signed-off-by: Michael Cheng <ccheng@broadcom.com>
> ---
>  drivers/i2c/busses/Kconfig         |   1 +
>  drivers/i2c/busses/i2c-bcm-iproc.c | 315 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 310 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index f2c681971201..e0798b82b3bc 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -424,6 +424,7 @@ config I2C_BCM_IPROC
>  	tristate "Broadcom iProc I2C controller"
>  	depends on ARCH_BCM_IPROC || COMPILE_TEST
>  	default ARCH_BCM_IPROC
> +	select I2C_SLAVE
>  	help
>  	  If you say yes to this option, support will be included for the
>  	  Broadcom iProc I2C controller.
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index eb7339a0280e..4a4a052efcc9 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -23,11 +23,30 @@
>  #define CFG_OFFSET                   0x00
>  #define CFG_RESET_SHIFT              31
>  #define CFG_EN_SHIFT                 30
> +#define CFG_SLAVE_ADDR_0_SHIFT       28
>  #define CFG_M_RETRY_CNT_SHIFT        16
>  #define CFG_M_RETRY_CNT_MASK         0x0f
>  
>  #define TIM_CFG_OFFSET               0x04
>  #define TIM_CFG_MODE_400_SHIFT       31
> +#define TIM_RAND_SLAVE_STRETCH_SHIFT      24
> +#define TIM_RAND_SLAVE_STRETCH_MASK       0x7f
> +#define TIM_PERIODIC_SLAVE_STRETCH_SHIFT  16
> +#define TIM_PERIODIC_SLAVE_STRETCH_MASK   0x7f
> +
> +#define S_CFG_SMBUS_ADDR_OFFSET           0x08
> +#define S_CFG_EN_NIC_SMB_ADDR3_SHIFT      31
> +#define S_CFG_NIC_SMB_ADDR3_SHIFT         24
> +#define S_CFG_NIC_SMB_ADDR3_MASK          0x7f
> +#define S_CFG_EN_NIC_SMB_ADDR2_SHIFT      23
> +#define S_CFG_NIC_SMB_ADDR2_SHIFT         16
> +#define S_CFG_NIC_SMB_ADDR2_MASK          0x7f
> +#define S_CFG_EN_NIC_SMB_ADDR1_SHIFT      15
> +#define S_CFG_NIC_SMB_ADDR1_SHIFT         8
> +#define S_CFG_NIC_SMB_ADDR1_MASK          0x7f
> +#define S_CFG_EN_NIC_SMB_ADDR0_SHIFT      7
> +#define S_CFG_NIC_SMB_ADDR0_SHIFT         0
> +#define S_CFG_NIC_SMB_ADDR0_MASK          0x7f
>  
>  #define M_FIFO_CTRL_OFFSET           0x0c
>  #define M_FIFO_RX_FLUSH_SHIFT        31
> @@ -37,6 +56,14 @@
>  #define M_FIFO_RX_THLD_SHIFT         8
>  #define M_FIFO_RX_THLD_MASK          0x3f
>  
> +#define S_FIFO_CTRL_OFFSET           0x10
> +#define S_FIFO_RX_FLUSH_SHIFT        31
> +#define S_FIFO_TX_FLUSH_SHIFT        30
> +#define S_FIFO_RX_CNT_SHIFT          16
> +#define S_FIFO_RX_CNT_MASK           0x7f
> +#define S_FIFO_RX_THLD_SHIFT         8
> +#define S_FIFO_RX_THLD_MASK          0x3f
> +
>  #define M_CMD_OFFSET                 0x30
>  #define M_CMD_START_BUSY_SHIFT       31
>  #define M_CMD_STATUS_SHIFT           25
> @@ -46,6 +73,8 @@
>  #define M_CMD_STATUS_NACK_ADDR       0x2
>  #define M_CMD_STATUS_NACK_DATA       0x3
>  #define M_CMD_STATUS_TIMEOUT         0x4
> +#define M_CMD_STATUS_FIFO_UNDERRUN   0x5
> +#define M_CMD_STATUS_RX_FIFO_FULL    0x6
>  #define M_CMD_PROTOCOL_SHIFT         9
>  #define M_CMD_PROTOCOL_MASK          0xf
>  #define M_CMD_PROTOCOL_BLK_WR        0x7
> @@ -54,17 +83,36 @@
>  #define M_CMD_RD_CNT_SHIFT           0
>  #define M_CMD_RD_CNT_MASK            0xff
>  
> +#define S_CMD_OFFSET                 0x34
> +#define S_CMD_START_BUSY_SHIFT       31
> +#define S_CMD_STATUS_SHIFT           23
> +#define S_CMD_STATUS_MASK            0x07
> +#define S_CMD_STATUS_SUCCESS         0x0
> +#define S_CMD_STATUS_TIMEOUT         0x5
> +
>  #define IE_OFFSET                    0x38
>  #define IE_M_RX_FIFO_FULL_SHIFT      31
>  #define IE_M_RX_THLD_SHIFT           30
>  #define IE_M_START_BUSY_SHIFT        28
>  #define IE_M_TX_UNDERRUN_SHIFT       27
> +#define IE_S_RX_FIFO_FULL_SHIFT      26
> +#define IE_S_RX_THLD_SHIFT           25
> +#define IE_S_RX_EVENT_SHIFT          24
> +#define IE_S_START_BUSY_SHIFT        23
> +#define IE_S_TX_UNDERRUN_SHIFT       22
> +#define IE_S_RD_EVENT_SHIFT          21
>  
>  #define IS_OFFSET                    0x3c
>  #define IS_M_RX_FIFO_FULL_SHIFT      31
>  #define IS_M_RX_THLD_SHIFT           30
>  #define IS_M_START_BUSY_SHIFT        28
>  #define IS_M_TX_UNDERRUN_SHIFT       27
> +#define IS_S_RX_FIFO_FULL_SHIFT      26
> +#define IS_S_RX_THLD_SHIFT           25
> +#define IS_S_RX_EVENT_SHIFT          24
> +#define IS_S_START_BUSY_SHIFT        23
> +#define IS_S_TX_UNDERRUN_SHIFT       22
> +#define IS_S_RD_EVENT_SHIFT          21
>  
>  #define M_TX_OFFSET                  0x40
>  #define M_TX_WR_STATUS_SHIFT         31
> @@ -78,6 +126,18 @@
>  #define M_RX_DATA_SHIFT              0
>  #define M_RX_DATA_MASK               0xff
>  
> +#define S_TX_OFFSET                  0x48
> +#define S_TX_WR_STATUS_SHIFT         31
> +#define S_TX_DATA_SHIFT              0
> +#define S_TX_DATA_MASK               0xff
> +
> +#define S_RX_OFFSET                  0x4c
> +#define S_RX_STATUS_SHIFT            30
> +#define S_RX_STATUS_MASK             0x03
> +#define S_RX_PEC_ERR_SHIFT           29
> +#define S_RX_DATA_SHIFT              0
> +#define S_RX_DATA_MASK               0xff
> +
>  #define I2C_TIMEOUT_MSEC             50000
>  #define M_TX_RX_FIFO_SIZE            64
>  #define M_RX_FIFO_MAX_THLD_VALUE     (M_TX_RX_FIFO_SIZE - 1)
> @@ -85,6 +145,30 @@
>  #define M_RX_MAX_READ_LEN            255
>  #define M_RX_FIFO_THLD_VALUE         50
>  
> +#define IE_M_ALL_INTERRUPT_SHIFT     27
> +#define IE_M_ALL_INTERRUPT_MASK      0x1e
> +
> +#define SLAVE_READ_WRITE_BIT_MASK    0x1
> +#define SLAVE_READ_WRITE_BIT_SHIFT   0x1
> +#define SLAVE_MAX_SIZE_TRANSACTION   64
> +#define SLAVE_CLOCK_STRETCH_TIME     25
> +
> +#define IE_S_ALL_INTERRUPT_SHIFT     21
> +#define IE_S_ALL_INTERRUPT_MASK      0x3f
> +
> +enum i2c_slave_read_status {
> +	I2C_SLAVE_RX_FIFO_EMPTY = 0,
> +	I2C_SLAVE_RX_START,
> +	I2C_SLAVE_RX_DATA,
> +	I2C_SLAVE_RX_END,
> +};
> +
> +enum i2c_slave_xfer_dir {
> +	I2C_SLAVE_DIR_READ = 0,
> +	I2C_SLAVE_DIR_WRITE,
> +	I2C_SLAVE_DIR_NONE,
> +};
> +
>  enum bus_speed_index {
>  	I2C_SPD_100K = 0,
>  	I2C_SPD_400K,
> @@ -104,6 +188,9 @@ struct bcm_iproc_i2c_dev {
>  
>  	struct i2c_msg *msg;
>  
> +	struct i2c_client *slave;
> +	enum i2c_slave_xfer_dir xfer_dir;
> +
>  	/* bytes that have been transferred */
>  	unsigned int tx_bytes;
>  	/* bytes that have been read */
> @@ -117,6 +204,157 @@ struct bcm_iproc_i2c_dev {
>  #define ISR_MASK (BIT(IS_M_START_BUSY_SHIFT) | BIT(IS_M_TX_UNDERRUN_SHIFT)\
>  		| BIT(IS_M_RX_THLD_SHIFT))
>  
> +#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
> +		| BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT))
> +
> +static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
> +static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
> +static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
> +					 bool enable);
> +
> +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);
> +		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);
> +
> +	/* RANDOM SLAVE STRETCH time - 20ms*/
> +	val = readl(iproc_i2c->base + TIM_CFG_OFFSET);
> +	val &= ~(TIM_RAND_SLAVE_STRETCH_MASK << TIM_RAND_SLAVE_STRETCH_SHIFT);
> +	val |= (SLAVE_CLOCK_STRETCH_TIME << TIM_RAND_SLAVE_STRETCH_SHIFT);
> +	writel(val, iproc_i2c->base + TIM_CFG_OFFSET);
> +
> +	/* Configure the slave address */
> +	val = readl(iproc_i2c->base + S_CFG_SMBUS_ADDR_OFFSET);
> +	val |= BIT(S_CFG_EN_NIC_SMB_ADDR3_SHIFT);
> +	val &= ~(S_CFG_NIC_SMB_ADDR3_MASK << S_CFG_NIC_SMB_ADDR3_SHIFT);
> +	val |= (iproc_i2c->slave->addr << S_CFG_NIC_SMB_ADDR3_SHIFT);
> +	writel(val, iproc_i2c->base + S_CFG_SMBUS_ADDR_OFFSET);
> +
> +	/* clear all pending slave interrupts */
> +	writel(ISR_MASK_SLAVE, iproc_i2c->base + IS_OFFSET);
> +
> +	/* Enable interrupt register for any READ event */
> +	val = BIT(IE_S_RD_EVENT_SHIFT);
> +	/* Enable interrupt register to indicate a valid byte in receive fifo */
> +	val |= BIT(IE_S_RX_EVENT_SHIFT);
> +	/* Enable interrupt register for the Slave BUSY command */
> +	val |= BIT(IE_S_START_BUSY_SHIFT);
> +	writel(val, iproc_i2c->base + IE_OFFSET);
> +
> +	iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
> +}
> +
> +static void bcm_iproc_i2c_check_slave_status(
> +	struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> +	u32 val;
> +
> +	val = readl(iproc_i2c->base + S_CMD_OFFSET);
> +	val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
> +
> +	if (val == S_CMD_STATUS_TIMEOUT) {
> +		dev_err(iproc_i2c->device, "slave random stretch time timeout\n");
> +
> +		/* re-initialize i2c for recovery */
> +		bcm_iproc_i2c_enable_disable(iproc_i2c, false);
> +		bcm_iproc_i2c_slave_init(iproc_i2c, true);
> +		bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> +	}
> +}
> +
> +static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
> +				u32 status)
> +{
> +	u8 value;
> +	u32 val;
> +	u32 rd_status;
> +	u32 tmp;
> +
> +	/* Start of transaction. check address and populate the direction */
> +	if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_NONE) {
> +		tmp = readl(iproc_i2c->base + S_RX_OFFSET);
> +		rd_status = (tmp >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
> +		/* This condition checks whether the request is a new request */
> +		if (((rd_status == I2C_SLAVE_RX_START) &&
> +			(status & BIT(IS_S_RX_EVENT_SHIFT))) ||
> +			((rd_status == I2C_SLAVE_RX_END) &&
> +			(status & BIT(IS_S_RD_EVENT_SHIFT)))) {
> +
> +			/* Last bit is W/R bit.
> +			 * If 1 then its a read request(by master).
> +			 */
> +			iproc_i2c->xfer_dir = tmp & SLAVE_READ_WRITE_BIT_MASK;
> +			if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)
> +				i2c_slave_event(iproc_i2c->slave,
> +					I2C_SLAVE_READ_REQUESTED, &value);
> +			else
> +				i2c_slave_event(iproc_i2c->slave,
> +					I2C_SLAVE_WRITE_REQUESTED, &value);
> +		}
> +	}
> +
> +	/* read request from master */
> +	if ((status & BIT(IS_S_RD_EVENT_SHIFT)) &&
> +		(iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)) {
> +		i2c_slave_event(iproc_i2c->slave,
> +			I2C_SLAVE_READ_PROCESSED, &value);
> +		writel(value, iproc_i2c->base + S_TX_OFFSET);
> +
> +		val = BIT(S_CMD_START_BUSY_SHIFT);
> +		writel(val, iproc_i2c->base + S_CMD_OFFSET);
> +	}
> +
> +	/* write request from master */
> +	if ((status & BIT(IS_S_RX_EVENT_SHIFT)) &&
> +		(iproc_i2c->xfer_dir == I2C_SLAVE_DIR_READ)) {
> +		val = readl(iproc_i2c->base + S_RX_OFFSET);
> +		/* Its a write request by Master to Slave.
> +		 * We read data present in receive FIFO
> +		 */
> +		value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
> +		i2c_slave_event(iproc_i2c->slave,
> +			I2C_SLAVE_WRITE_RECEIVED, &value);
> +
> +		/* check the status for the last byte of the transaction */
> +		rd_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
> +		if (rd_status == I2C_SLAVE_RX_END)
> +			iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
> +
> +		dev_dbg(iproc_i2c->device, "\nread value = 0x%x\n", value);
> +	}
> +
> +	/* Stop */
> +	if (status & BIT(IS_S_START_BUSY_SHIFT)) {
> +		i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP, &value);
> +		iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
> +	}
> +
> +	/* clear interrupt status */
> +	writel(status, iproc_i2c->base + IS_OFFSET);
> +
> +	bcm_iproc_i2c_check_slave_status(iproc_i2c);
> +	return true;
> +}
> +
>  static void bcm_iproc_i2c_read_valid_bytes(struct bcm_iproc_i2c_dev *iproc_i2c)
>  {
>  	struct i2c_msg *msg = iproc_i2c->msg;
> @@ -142,6 +380,18 @@ static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
>  	u32 status = readl(iproc_i2c->base + IS_OFFSET);
>  	u32 tmp;
>  
> +
> +	bool ret;
> +	u32 sl_status = status & ISR_MASK_SLAVE;
> +
> +	if (sl_status) {
> +		ret = bcm_iproc_i2c_slave_isr(iproc_i2c, sl_status);
> +		if (ret)
> +			return IRQ_HANDLED;
> +		else
> +			return IRQ_NONE;
> +	}
> +
>  	status &= ISR_MASK;
>  
>  	if (!status)
> @@ -224,22 +474,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_RESET_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);
>  
>  	/* clear all pending interrupts */
>  	writel(0xffffffff, iproc_i2c->base + IS_OFFSET);
> @@ -288,6 +541,14 @@ static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		dev_dbg(iproc_i2c->device, "bus timeout\n");
>  		return -ETIMEDOUT;
>  
> +	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;
> +
>  	default:
>  		dev_dbg(iproc_i2c->device, "unknown error code=%d\n", val);
>  
> @@ -442,12 +703,14 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
>  
>  static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE;
>  }
>  
>  static const struct i2c_algorithm bcm_iproc_algo = {
>  	.master_xfer = bcm_iproc_i2c_xfer,
>  	.functionality = bcm_iproc_i2c_functionality,
> +	.reg_slave = bcm_iproc_i2c_reg_slave,
> +	.unreg_slave = bcm_iproc_i2c_unreg_slave,
>  };
>  
>  static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> @@ -612,6 +875,46 @@ static int bcm_iproc_i2c_resume(struct device *dev)
>  #define BCM_IPROC_I2C_PM_OPS NULL
>  #endif /* CONFIG_PM_SLEEP */
>  
> +
> +static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(slave->adapter);
> +
> +	if (iproc_i2c->slave)
> +		return -EBUSY;
> +
> +	if (slave->flags & I2C_CLIENT_TEN)
> +		return -EAFNOSUPPORT;
> +
> +	iproc_i2c->slave = slave;
> +	bcm_iproc_i2c_slave_init(iproc_i2c, false);
> +	return 0;
> +}
> +
> +static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave)
> +{
> +	u32 tmp;
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(slave->adapter);
> +
> +	if (!iproc_i2c->slave)
> +		return -EINVAL;
> +
> +	iproc_i2c->slave = NULL;
> +
> +	/* disable all slave interrupts */
> +	tmp = readl(iproc_i2c->base + IE_OFFSET);
> +	tmp &= ~(IE_S_ALL_INTERRUPT_MASK <<
> +			IE_S_ALL_INTERRUPT_SHIFT);
> +	writel(tmp, iproc_i2c->base + IE_OFFSET);
> +
> +	/* Erase the slave address programmed */
> +	tmp = readl(iproc_i2c->base + S_CFG_SMBUS_ADDR_OFFSET);
> +	tmp &= ~BIT(S_CFG_EN_NIC_SMB_ADDR3_SHIFT);
> +	writel(tmp, iproc_i2c->base + S_CFG_SMBUS_ADDR_OFFSET);
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id bcm_iproc_i2c_of_match[] = {
>  	{ .compatible = "brcm,iproc-i2c" },
>  	{ /* sentinel */ }
> 

Reviewed-by: Ray Jui <ray.jui@broadcom.com>

Thanks!

Re: [PATCH v3 0/8] Add IPROC I2C slave mode and NIC I2C support

[Ray Jui]

Hi Rayagonda/Wolfram,

I reviewed the entire patch series and added my Reviewed-by tag on 1/8
and 2/8. The rest of the patches in this series already had my
Signed-off-by tag.

Thanks,

Ray

On 1/17/2019 10:04 PM, Rayagonda Kokatanur wrote:
> Hi,
> This patchset contains support for IPROC I2C slave and NIC I2C support.
> Following are the brief changes done inorder to support both IPROC I2C
> slave mode and NIC I2C,
> 
> a. Add support to allow I2C rx transfer upto 255 bytes
> b. Add polling mode support to the iProc I2C driver. Polling mode is used
>    in certain SoCs where interrupt is not connected from the I2C block.
> c. Add warpper read/write access for register to support indirect register
>    access. This is required in certain SoCs with this iProc I2C
>    block registered.
> d. Remove the set RESET bit to LOW right after setting it to HIGH so that
>    wait duration can be met.
> 
> This patch series is based on kernel v5.0.0-rc2.
> 
> Changes from v2:
>  - Address Ray's review comments.
> 
> Changes from v1:
>  - Rebased to Linux v5.0.0-rc2
> 
> Ray Jui (1):
>   dt-bindings: i2c: iproc: make 'interrupts' optional
> 
> Rayagonda Kokatanur (5):
>   i2c: iproc: add polling support
>   i2c: iproc: use wrapper for read/write access
>   dt-bindings: i2c: iproc: add "brcm,iproc-nic-i2c" compatible string
>   i2c: iproc: add NIC I2C support
>   arm64: dts: Stingray: Add NIC i2c device node
> 
> Shreesha Rajashekar (2):
>   i2c: iproc: I2C read up to 255 bytes support
>   i2c: iproc: Add Slave mode support
> 
>  .../devicetree/bindings/i2c/brcm,iproc-i2c.txt     |  14 +-
>  .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  18 +
>  drivers/i2c/busses/Kconfig                         |   1 +
>  drivers/i2c/busses/i2c-bcm-iproc.c                 | 757 +++++++++++++++++----
>  4 files changed, 659 insertions(+), 131 deletions(-)
> 

Re: [PATCH v3 0/8] Add IPROC I2C slave mode and NIC I2C support

[Ray Jui]

Hi Wolfram,

Just want to check if you had a chance to review the following patch series?

Thanks,

Ray

On 1/20/2019 1:49 PM, Ray Jui wrote:
> Hi Rayagonda/Wolfram,
> 
> I reviewed the entire patch series and added my Reviewed-by tag on 1/8
> and 2/8. The rest of the patches in this series already had my
> Signed-off-by tag.
> 
> Thanks,
> 
> Ray
> 
> On 1/17/2019 10:04 PM, Rayagonda Kokatanur wrote:
>> Hi,
>> This patchset contains support for IPROC I2C slave and NIC I2C support.
>> Following are the brief changes done inorder to support both IPROC I2C
>> slave mode and NIC I2C,
>>
>> a. Add support to allow I2C rx transfer upto 255 bytes
>> b. Add polling mode support to the iProc I2C driver. Polling mode is used
>>    in certain SoCs where interrupt is not connected from the I2C block.
>> c. Add warpper read/write access for register to support indirect register
>>    access. This is required in certain SoCs with this iProc I2C
>>    block registered.
>> d. Remove the set RESET bit to LOW right after setting it to HIGH so that
>>    wait duration can be met.
>>
>> This patch series is based on kernel v5.0.0-rc2.
>>
>> Changes from v2:
>>  - Address Ray's review comments.
>>
>> Changes from v1:
>>  - Rebased to Linux v5.0.0-rc2
>>
>> Ray Jui (1):
>>   dt-bindings: i2c: iproc: make 'interrupts' optional
>>
>> Rayagonda Kokatanur (5):
>>   i2c: iproc: add polling support
>>   i2c: iproc: use wrapper for read/write access
>>   dt-bindings: i2c: iproc: add "brcm,iproc-nic-i2c" compatible string
>>   i2c: iproc: add NIC I2C support
>>   arm64: dts: Stingray: Add NIC i2c device node
>>
>> Shreesha Rajashekar (2):
>>   i2c: iproc: I2C read up to 255 bytes support
>>   i2c: iproc: Add Slave mode support
>>
>>  .../devicetree/bindings/i2c/brcm,iproc-i2c.txt     |  14 +-
>>  .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  18 +
>>  drivers/i2c/busses/Kconfig                         |   1 +
>>  drivers/i2c/busses/i2c-bcm-iproc.c                 | 757 +++++++++++++++++----
>>  4 files changed, 659 insertions(+), 131 deletions(-)
>>

Re: [PATCH v3 0/8] Add IPROC I2C slave mode and NIC I2C support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

Hi,

> Just want to check if you had a chance to review the following patch series?

I am sorry, but for some reason they are not listed in my patchwork
instance, so they fell through the cracks :( Can you please resend them?

I will double check they show up in patchwork then.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 0/8] Add IPROC I2C slave mode and NIC I2C support

[Ray Jui]

Hi Wolfram,

On 2/4/2019 3:12 AM, Wolfram Sang wrote:
> Hi,
> 
>> Just want to check if you had a chance to review the following patch series?
> 
> I am sorry, but for some reason they are not listed in my patchwork
> instance, so they fell through the cracks :( Can you please resend them?
> 
> I will double check they show up in patchwork then.

Thanks. I think the issue might have something to do with Rayagonda's
email client configuration. I will be helping to send out v4 of the
patch series.

Regards,

Ray

> 
> Thanks,
> 
>    Wolfram
>