[PATCH] i2c: imx: add slave support

[PATCH] i2c: imx: add slave support

[Dmitriy Baranov]

Add I2C slave provider using the generic slave interface.
It also supports master transactions when the slave in the idle mode.
Changes work only in PIO mode (when driver doesn`t use DMA)
These changes weren`t tested with DMA is enabled.

Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
---
 drivers/i2c/busses/i2c-imx.c | 311 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 290 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a2b132c..3c286f1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -53,6 +53,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 /* This will be the driver name the kernel reports */
 #define DRIVER_NAME "imx-i2c"
@@ -171,6 +172,18 @@ enum imx_i2c_type {
 	VF610_I2C,
 };
 
+enum imx_i2c_mode {
+	I2C_IMX_SLAVE,
+	I2C_IMX_MASTER,
+	I2C_IMX_UNDEFINED
+};
+
+enum imx_i2c_slave_state {
+	I2C_IMX_SLAVE_IDLE,
+	I2C_IMX_SLAVE_IRQ,
+	I2C_IMX_SLAVE_POLLING
+};
+
 struct imx_i2c_hwdata {
 	enum imx_i2c_type	devtype;
 	unsigned		regshift;
@@ -193,10 +206,12 @@ struct imx_i2c_dma {
 
 struct imx_i2c_struct {
 	struct i2c_adapter	adapter;
+	struct i2c_client	*slave;
 	struct clk		*clk;
 	void __iomem		*base;
 	wait_queue_head_t	queue;
 	unsigned long		i2csr;
+	unsigned long		i2csr_slave;
 	unsigned int		disable_delay;
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
@@ -210,6 +225,11 @@ struct imx_i2c_struct {
 	struct pinctrl_state *pinctrl_pins_gpio;
 
 	struct imx_i2c_dma	*dma;
+
+	enum imx_i2c_mode	dev_mode;
+	atomic_t		slave_state;
+	struct task_struct	*slave_task;
+	wait_queue_head_t	slave_queue;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
@@ -510,39 +530,97 @@ static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx)
 #endif
 }
 
-static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
+static int i2c_imx_configure_clock(struct imx_i2c_struct *i2c_imx)
 {
-	unsigned int temp = 0;
 	int result;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
-
 	i2c_imx_set_clk(i2c_imx);
 
-	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
-	/* Enable I2C controller */
-	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
-	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx, IMX_I2C_I2CR);
+	result = clk_prepare_enable(i2c_imx->clk);
+	if (result == 0)
+		imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
+
+	return result;
+}
+
+static void i2c_imx_enable_i2c_controller(struct imx_i2c_struct *i2c_imx)
+{
+	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
+		IMX_I2C_I2SR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx,
+		IMX_I2C_I2CR);
 
 	/* Wait controller to be stable */
 	udelay(50);
+}
+
+static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
+{
+	unsigned int temp = 0;
+	int result;
+
+	i2c_imx->dev_mode = I2C_IMX_UNDEFINED;
+
+	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+
+	result = i2c_imx_configure_clock(i2c_imx);
+	if (result != 0)
+		return result;
+
+	i2c_imx_enable_i2c_controller(i2c_imx);
 
 	/* Start I2C transaction */
 	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 	temp |= I2CR_MSTA;
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
 	result = i2c_imx_bus_busy(i2c_imx, 1);
 	if (result)
 		return result;
 	i2c_imx->stopped = 0;
 
+	i2c_imx->dev_mode = I2C_IMX_MASTER;
+
 	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
 	temp &= ~I2CR_DMAEN;
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	return result;
 }
 
-static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
+static int i2c_imx_start_slave_mode(struct imx_i2c_struct *i2c_imx, bool enable)
+{
+	unsigned int temp;
+	int result;
+
+	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+
+	i2c_imx->dev_mode = I2C_IMX_UNDEFINED;
+
+	if (enable) {
+		result = i2c_imx_configure_clock(i2c_imx);
+		if (result != 0)
+			return result;
+	}
+
+	/* Set the Slave bit */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~I2CR_MSTA;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	/* Set the Slave address */
+	imx_i2c_write_reg((i2c_imx->slave->addr << 1), i2c_imx, IMX_I2C_IADR);
+
+	i2c_imx->dev_mode = I2C_IMX_SLAVE;
+
+	i2c_imx_enable_i2c_controller(i2c_imx);
+
+	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode | I2CR_IIEN, i2c_imx,
+		IMX_I2C_I2CR);
+
+	return 0;
+}
+
+static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx, bool disable)
 {
 	unsigned int temp = 0;
 
@@ -568,24 +646,152 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 		i2c_imx->stopped = 1;
 	}
 
-	/* Disable I2C controller */
-	temp = i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
+	temp = i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN;
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	if (disable)
+		clk_disable_unprepare(i2c_imx->clk);
+
+	i2c_imx->dev_mode = I2C_IMX_UNDEFINED;
+}
+
+static void i2c_imx_clear_isr_bit(struct imx_i2c_struct *i2c_imx,
+	unsigned int status)
+{
+	status &= ~I2SR_IIF;
+	status |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
+	imx_i2c_write_reg(status, i2c_imx, IMX_I2C_I2SR);
+}
+
+static void i2c_imx_master_isr_handler(struct imx_i2c_struct *i2c_imx,
+	unsigned int status)
+{
+	/* save status register */
+	i2c_imx->i2csr = status;
+	wake_up(&i2c_imx->queue);
+}
+
+static int i2c_imx_slave_threadfn(void *pdata)
+{
+	unsigned int ctl, status, timeout = HZ;
+	u8 data;
+	struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *) pdata;
+
+	do {
+		wait_event_timeout(i2c_imx->slave_queue,
+			atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_IRQ,
+			timeout);
+
+		if (atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_IRQ) {
+			atomic_set(&i2c_imx->slave_state,
+				I2C_IMX_SLAVE_POLLING);
+
+			timeout	= HZ/10;
+			status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+			ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+
+			if (status & I2SR_IAAS) {
+				if (status & I2SR_SRW) {
+					/* master wants to read from us */
+					i2c_slave_event(i2c_imx->slave,
+						I2C_SLAVE_READ_REQUESTED, &data);
+					ctl |= I2CR_MTX;
+					imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
+
+					/*send data */
+					imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
+				} else {
+					dev_dbg(&i2c_imx->adapter.dev, "write requested");
+					i2c_slave_event(i2c_imx->slave,
+						I2C_SLAVE_WRITE_REQUESTED, &data);
+					/*slave receive */
+					ctl &= ~I2CR_MTX;
+					imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
+
+					/*dummy read */
+					data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+				}
+			} else {
+				/* slave send */
+				if (ctl & I2CR_MTX) {
+					if (!(status & I2SR_RXAK)) {	/*ACK received */
+						i2c_slave_event(i2c_imx->slave,
+							I2C_SLAVE_READ_PROCESSED, &data);
+						ctl |= I2CR_MTX;
+						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
+						/*send data */
+						imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
+					} else {
+						/*no ACK. */
+						/*dummy read */
+						dev_dbg(&i2c_imx->adapter.dev, "read requested");
+						i2c_slave_event(i2c_imx->slave,
+							I2C_SLAVE_READ_REQUESTED, &data);
+
+						ctl &= ~I2CR_MTX;
+						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
+						imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+					}
+				} else {	/*read */
+					ctl &= ~I2CR_MTX;
+					imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
+
+					/*read */
+					data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+					dev_dbg(&i2c_imx->adapter.dev, "received %x",
+						(unsigned int) data);
+					i2c_slave_event(i2c_imx->slave,
+						I2C_SLAVE_WRITE_RECEIVED, &data);
+				}
+			}
+		}
+
+		if (atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_POLLING) {
+			udelay(50);
+			status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+
+			if ((status & I2SR_IBB) == 0) {
+				pr_debug("end of package");
+				i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
+				atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
+				timeout	= HZ;
+			}
+		}
+	} while (kthread_should_stop() == 0);
+
+	return 0;
+}
+
+static void i2c_imx_slave_isr_handler(struct imx_i2c_struct *i2c_imx,
+	unsigned int status)
+{
+	atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IRQ);
+	wake_up(&i2c_imx->slave_queue);
 }
 
 static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 {
 	struct imx_i2c_struct *i2c_imx = dev_id;
-	unsigned int temp;
+	unsigned int current_status;
+
+	current_status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+	if (current_status & I2SR_IIF) {
+		i2c_imx_clear_isr_bit(i2c_imx, current_status);
+
+		switch (i2c_imx->dev_mode) {
+		case I2C_IMX_SLAVE:
+			dev_dbg(&i2c_imx->adapter.dev, "slave interrupt");
+			i2c_imx_slave_isr_handler(i2c_imx, current_status);
+			break;
+		case I2C_IMX_MASTER:
+			dev_dbg(&i2c_imx->adapter.dev, "master interrupt");
+			i2c_imx_master_isr_handler(i2c_imx, current_status);
+			break;
+		case I2C_IMX_UNDEFINED:
+			dev_dbg(&i2c_imx->adapter.dev, "undefined interrupt");
+			break;
+		}
 
-	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
-	if (temp & I2SR_IIF) {
-		/* save status register */
-		i2c_imx->i2csr = temp;
-		temp &= ~I2SR_IIF;
-		temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
-		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
-		wake_up(&i2c_imx->queue);
 		return IRQ_HANDLED;
 	}
 
@@ -889,6 +1095,11 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
+	if (atomic_read(&i2c_imx->slave_state) != I2C_IMX_SLAVE_IDLE) {
+		dev_dbg(&i2c_imx->adapter.dev, "slave is working now\n");
+		return -EBUSY;
+	}
+
 	result = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
 	if (result < 0)
 		goto out;
@@ -954,7 +1165,14 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 
 fail0:
 	/* Stop I2C transfer */
-	i2c_imx_stop(i2c_imx);
+	if (i2c_imx->slave != NULL) {
+		/* Stop I2C transfer */
+		i2c_imx_stop(i2c_imx, false);
+
+		i2c_imx_start_slave_mode(i2c_imx, false);
+	} else {
+		i2c_imx_stop(i2c_imx, true);
+	}
 
 	pm_runtime_mark_last_busy(i2c_imx->adapter.dev.parent);
 	pm_runtime_put_autosuspend(i2c_imx->adapter.dev.parent);
@@ -1013,6 +1231,46 @@ static void i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
 	i2c_imx->adapter.bus_recovery_info = rinfo;
 }
 
+static int i2c_imx_reg_slave(struct i2c_client *slave)
+{
+	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(slave->adapter);
+
+	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+
+	if (i2c_imx->slave)
+		return -EBUSY;
+
+	if (slave->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	i2c_imx->slave = slave;
+
+	i2c_imx->slave_task = kthread_run(i2c_imx_slave_threadfn, (void *) i2c_imx, "i2c-slave-%s", i2c_imx->adapter.name);
+
+	return i2c_imx_start_slave_mode(i2c_imx, true);
+}
+
+static int i2c_imx_unreg_slave(struct i2c_client *slave)
+{
+	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(slave->adapter);
+
+	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+
+	if (i2c_imx->slave_task != NULL)
+		kthread_stop(i2c_imx->slave_task);
+	i2c_imx->slave_task = NULL;
+
+	/* slave_state is still tested by xfer() code */
+	atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
+	i2c_imx->dev_mode = I2C_IMX_UNDEFINED;
+
+	i2c_imx->slave = NULL;
+
+	i2c_imx_stop(i2c_imx, true);
+
+	return 0;
+}
+
 static u32 i2c_imx_func(struct i2c_adapter *adapter)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
@@ -1022,6 +1280,8 @@ static u32 i2c_imx_func(struct i2c_adapter *adapter)
 static struct i2c_algorithm i2c_imx_algo = {
 	.master_xfer	= i2c_imx_xfer,
 	.functionality	= i2c_imx_func,
+	.reg_slave	= i2c_imx_reg_slave,
+	.unreg_slave	= i2c_imx_unreg_slave,
 };
 
 static int i2c_imx_probe(struct platform_device *pdev)
@@ -1097,6 +1357,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 
 	/* Init queue */
 	init_waitqueue_head(&i2c_imx->queue);
+	init_waitqueue_head(&i2c_imx->slave_queue);
 
 	/* Set up adapter data */
 	i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
@@ -1146,6 +1407,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	/* Init DMA config if supported */
 	i2c_imx_dma_request(i2c_imx, phy_addr);
 
+	/* init slave_state to IDLE */
+	atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
+
+	i2c_imx->dev_mode = I2C_IMX_UNDEFINED;
+
 	return 0;   /* Return OK */
 
 rpm_disable:
@@ -1172,6 +1438,9 @@ static int i2c_imx_remove(struct platform_device *pdev)
 	dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
 	i2c_del_adapter(&i2c_imx->adapter);
 
+	if (i2c_imx->slave_task != NULL)
+		kthread_stop(i2c_imx->slave_task);
+
 	if (i2c_imx->dma)
 		i2c_imx_dma_free(i2c_imx);
 
-- 
2.5.0

Re: [PATCH] i2c: imx: add slave support

[kbuild test robot]

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

Hi Dmitriy,

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.5-rc1 next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Dmitriy-Baranov/i2c-imx-add-slave-support/20160125-225538
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
config: arm-imx_v6_v7_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-imx.c: In function 'i2c_imx_slave_threadfn':
>> drivers/i2c/busses/i2c-imx.c:696:6: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration]
         i2c_slave_event(i2c_imx->slave,
         ^
>> drivers/i2c/busses/i2c-imx.c:697:7: error: 'I2C_SLAVE_READ_REQUESTED' undeclared (first use in this function)
          I2C_SLAVE_READ_REQUESTED, &data);
          ^
   drivers/i2c/busses/i2c-imx.c:697:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/i2c/busses/i2c-imx.c:706:7: error: 'I2C_SLAVE_WRITE_REQUESTED' undeclared (first use in this function)
          I2C_SLAVE_WRITE_REQUESTED, &data);
          ^
>> drivers/i2c/busses/i2c-imx.c:719:8: error: 'I2C_SLAVE_READ_PROCESSED' undeclared (first use in this function)
           I2C_SLAVE_READ_PROCESSED, &data);
           ^
>> drivers/i2c/busses/i2c-imx.c:744:7: error: 'I2C_SLAVE_WRITE_RECEIVED' undeclared (first use in this function)
          I2C_SLAVE_WRITE_RECEIVED, &data);
          ^
>> drivers/i2c/busses/i2c-imx.c:755:37: error: 'I2C_SLAVE_STOP' undeclared (first use in this function)
        i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
                                        ^
   drivers/i2c/busses/i2c-imx.c: At top level:
>> drivers/i2c/busses/i2c-imx.c:1283:2: error: unknown field 'reg_slave' specified in initializer
     .reg_slave = i2c_imx_reg_slave,
     ^
>> drivers/i2c/busses/i2c-imx.c:1283:2: warning: excess elements in struct initializer
   drivers/i2c/busses/i2c-imx.c:1283:2: warning: (near initialization for 'i2c_imx_algo')
>> drivers/i2c/busses/i2c-imx.c:1284:2: error: unknown field 'unreg_slave' specified in initializer
     .unreg_slave = i2c_imx_unreg_slave,
     ^
   drivers/i2c/busses/i2c-imx.c:1284:2: warning: excess elements in struct initializer
   drivers/i2c/busses/i2c-imx.c:1284:2: warning: (near initialization for 'i2c_imx_algo')
   cc1: some warnings being treated as errors

vim +/i2c_slave_event +696 drivers/i2c/busses/i2c-imx.c

   690				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
   691				ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
   692	
   693				if (status & I2SR_IAAS) {
   694					if (status & I2SR_SRW) {
   695						/* master wants to read from us */
 > 696						i2c_slave_event(i2c_imx->slave,
 > 697							I2C_SLAVE_READ_REQUESTED, &data);
   698						ctl |= I2CR_MTX;
   699						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
   700	
   701						/*send data */
   702						imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
   703					} else {
   704						dev_dbg(&i2c_imx->adapter.dev, "write requested");
   705						i2c_slave_event(i2c_imx->slave,
 > 706							I2C_SLAVE_WRITE_REQUESTED, &data);
   707						/*slave receive */
   708						ctl &= ~I2CR_MTX;
   709						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
   710	
   711						/*dummy read */
   712						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
   713					}
   714				} else {
   715					/* slave send */
   716					if (ctl & I2CR_MTX) {
   717						if (!(status & I2SR_RXAK)) {	/*ACK received */
   718							i2c_slave_event(i2c_imx->slave,
 > 719								I2C_SLAVE_READ_PROCESSED, &data);
   720							ctl |= I2CR_MTX;
   721							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
   722							/*send data */
   723							imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
   724						} else {
   725							/*no ACK. */
   726							/*dummy read */
   727							dev_dbg(&i2c_imx->adapter.dev, "read requested");
   728							i2c_slave_event(i2c_imx->slave,
   729								I2C_SLAVE_READ_REQUESTED, &data);
   730	
   731							ctl &= ~I2CR_MTX;
   732							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
   733							imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
   734						}
   735					} else {	/*read */
   736						ctl &= ~I2CR_MTX;
   737						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
   738	
   739						/*read */
   740						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
   741						dev_dbg(&i2c_imx->adapter.dev, "received %x",
   742							(unsigned int) data);
   743						i2c_slave_event(i2c_imx->slave,
 > 744							I2C_SLAVE_WRITE_RECEIVED, &data);
   745					}
   746				}
   747			}
   748	
   749			if (atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_POLLING) {
   750				udelay(50);
   751				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
   752	
   753				if ((status & I2SR_IBB) == 0) {
   754					pr_debug("end of package");
 > 755					i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
   756					atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
   757					timeout	= HZ;
   758				}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 27224 bytes --]

Re: [PATCH] i2c: imx: add slave support

[Dmitriy Baranov]

Thank you for testing our patch.

Due to using the generic slave interface, It should be enabled in the 
config file.
Please add the following in the config file:
CONFIG_I2C_SLAVE=y



On 25.01.2016 21:09, kbuild test robot wrote:
> Hi Dmitriy,
>
> [auto build test ERROR on wsa/i2c/for-next]
> [also build test ERROR on v4.5-rc1 next-20160125]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Dmitriy-Baranov/i2c-imx-add-slave-support/20160125-225538
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
> config: arm-imx_v6_v7_defconfig (attached as .config)
> reproduce:
>          wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm
>
> All error/warnings (new ones prefixed by >>):
>
>     drivers/i2c/busses/i2c-imx.c: In function 'i2c_imx_slave_threadfn':
>>> drivers/i2c/busses/i2c-imx.c:696:6: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration]
>           i2c_slave_event(i2c_imx->slave,
>           ^
>>> drivers/i2c/busses/i2c-imx.c:697:7: error: 'I2C_SLAVE_READ_REQUESTED' undeclared (first use in this function)
>            I2C_SLAVE_READ_REQUESTED, &data);
>            ^
>     drivers/i2c/busses/i2c-imx.c:697:7: note: each undeclared identifier is reported only once for each function it appears in
>>> drivers/i2c/busses/i2c-imx.c:706:7: error: 'I2C_SLAVE_WRITE_REQUESTED' undeclared (first use in this function)
>            I2C_SLAVE_WRITE_REQUESTED, &data);
>            ^
>>> drivers/i2c/busses/i2c-imx.c:719:8: error: 'I2C_SLAVE_READ_PROCESSED' undeclared (first use in this function)
>             I2C_SLAVE_READ_PROCESSED, &data);
>             ^
>>> drivers/i2c/busses/i2c-imx.c:744:7: error: 'I2C_SLAVE_WRITE_RECEIVED' undeclared (first use in this function)
>            I2C_SLAVE_WRITE_RECEIVED, &data);
>            ^
>>> drivers/i2c/busses/i2c-imx.c:755:37: error: 'I2C_SLAVE_STOP' undeclared (first use in this function)
>          i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
>                                          ^
>     drivers/i2c/busses/i2c-imx.c: At top level:
>>> drivers/i2c/busses/i2c-imx.c:1283:2: error: unknown field 'reg_slave' specified in initializer
>       .reg_slave = i2c_imx_reg_slave,
>       ^
>>> drivers/i2c/busses/i2c-imx.c:1283:2: warning: excess elements in struct initializer
>     drivers/i2c/busses/i2c-imx.c:1283:2: warning: (near initialization for 'i2c_imx_algo')
>>> drivers/i2c/busses/i2c-imx.c:1284:2: error: unknown field 'unreg_slave' specified in initializer
>       .unreg_slave = i2c_imx_unreg_slave,
>       ^
>     drivers/i2c/busses/i2c-imx.c:1284:2: warning: excess elements in struct initializer
>     drivers/i2c/busses/i2c-imx.c:1284:2: warning: (near initialization for 'i2c_imx_algo')
>     cc1: some warnings being treated as errors
>
> vim +/i2c_slave_event +696 drivers/i2c/busses/i2c-imx.c
>
>     690				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>     691				ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>     692	
>     693				if (status & I2SR_IAAS) {
>     694					if (status & I2SR_SRW) {
>     695						/* master wants to read from us */
>   > 696						i2c_slave_event(i2c_imx->slave,
>   > 697							I2C_SLAVE_READ_REQUESTED, &data);
>     698						ctl |= I2CR_MTX;
>     699						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     700	
>     701						/*send data */
>     702						imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
>     703					} else {
>     704						dev_dbg(&i2c_imx->adapter.dev, "write requested");
>     705						i2c_slave_event(i2c_imx->slave,
>   > 706							I2C_SLAVE_WRITE_REQUESTED, &data);
>     707						/*slave receive */
>     708						ctl &= ~I2CR_MTX;
>     709						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     710	
>     711						/*dummy read */
>     712						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     713					}
>     714				} else {
>     715					/* slave send */
>     716					if (ctl & I2CR_MTX) {
>     717						if (!(status & I2SR_RXAK)) {	/*ACK received */
>     718							i2c_slave_event(i2c_imx->slave,
>   > 719								I2C_SLAVE_READ_PROCESSED, &data);
>     720							ctl |= I2CR_MTX;
>     721							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     722							/*send data */
>     723							imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
>     724						} else {
>     725							/*no ACK. */
>     726							/*dummy read */
>     727							dev_dbg(&i2c_imx->adapter.dev, "read requested");
>     728							i2c_slave_event(i2c_imx->slave,
>     729								I2C_SLAVE_READ_REQUESTED, &data);
>     730	
>     731							ctl &= ~I2CR_MTX;
>     732							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     733							imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     734						}
>     735					} else {	/*read */
>     736						ctl &= ~I2CR_MTX;
>     737						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     738	
>     739						/*read */
>     740						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     741						dev_dbg(&i2c_imx->adapter.dev, "received %x",
>     742							(unsigned int) data);
>     743						i2c_slave_event(i2c_imx->slave,
>   > 744							I2C_SLAVE_WRITE_RECEIVED, &data);
>     745					}
>     746				}
>     747			}
>     748	
>     749			if (atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_POLLING) {
>     750				udelay(50);
>     751				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>     752	
>     753				if ((status & I2SR_IBB) == 0) {
>     754					pr_debug("end of package");
>   > 755					i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
>     756					atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
>     757					timeout	= HZ;
>     758				}
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH] i2c: imx: add slave support

[Wolfram Sang]

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

On Tue, Jan 26, 2016 at 11:22:24AM +0300, Dmitriy Baranov wrote:
> Thank you for testing our patch.
> 
> Due to using the generic slave interface, It should be enabled in the config
> file.
> Please add the following in the config file:
> CONFIG_I2C_SLAVE=y

For now, you need to select it to make randconfigs built.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: imx: add slave support

[Dmitriy Baranov]

Sorry, we should have added selecting this in our patch.
The following fixes it:

Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the 
generic slave interface.

Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
---
  drivers/i2c/busses/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0299dfa..bc7cbfd 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -587,6 +587,7 @@ config I2C_IMG
  config I2C_IMX
      tristate "IMX I2C interface"
      depends on ARCH_MXC || ARCH_LAYERSCAPE
+    select I2C_SLAVE
      help
        Say Y here if you want to use the IIC bus controller on
        the Freescale i.MX/MXC or Layerscape processors.
-- 
2.5.0




On 26.01.2016 11:36, Wolfram Sang wrote:
> On Tue, Jan 26, 2016 at 11:22:24AM +0300, Dmitriy Baranov wrote:
>> Thank you for testing our patch.
>>
>> Due to using the generic slave interface, It should be enabled in the config
>> file.
>> Please add the following in the config file:
>> CONFIG_I2C_SLAVE=y
> For now, you need to select it to make randconfigs built.
>

Re: [PATCH] i2c: imx: add slave support

[Vladimir Zapolskiy]

On 26.01.2016 11:54, Dmitriy Baranov wrote:
> Sorry, we should have added selecting this in our patch.
> The following fixes it:
> 
> Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the 
> generic slave interface.
> 
> Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
> Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
> ---
>   drivers/i2c/busses/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 0299dfa..bc7cbfd 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -587,6 +587,7 @@ config I2C_IMG
>   config I2C_IMX
>       tristate "IMX I2C interface"
>       depends on ARCH_MXC || ARCH_LAYERSCAPE
> +    select I2C_SLAVE
>       help
>         Say Y here if you want to use the IIC bus controller on
>         the Freescale i.MX/MXC or Layerscape processors.
> 

Could you please squash the changes (care about indentation in Kconfig also)
and resend it as v2 (probably you may want to wait for review comments some
more time)?

Two more formal questions, why patch submitter's Signed-off-by: is not the
last in the list and why Maxim has Signed-off-by tag here? Who is the author
of the change?

See Documentation/SubmittingPatches "Sign your work" for details.

Best wishes,
Vladimir

Re: [PATCH] i2c: imx: add slave support

[Dmitriy Baranov]

Vladimir,

Thanks for the comment.
We are both the authors of this patch.

> Could you please squash the changes (care about indentation in Kconfig also)
> and resend it as v2 (probably you may want to wait for review comments some
> more time)?
I suppose that after review, there will be many other things to fix in 
this patch.
Thus, if you don`t mind, we wont send fixed version for now.




On 26.01.2016 16:37, Vladimir Zapolskiy wrote:
> On 26.01.2016 11:54, Dmitriy Baranov wrote:
>> Sorry, we should have added selecting this in our patch.
>> The following fixes it:
>>
>> Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the
>> generic slave interface.
>>
>> Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
>> Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
>> ---
>>    drivers/i2c/busses/Kconfig | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 0299dfa..bc7cbfd 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -587,6 +587,7 @@ config I2C_IMG
>>    config I2C_IMX
>>        tristate "IMX I2C interface"
>>        depends on ARCH_MXC || ARCH_LAYERSCAPE
>> +    select I2C_SLAVE
>>        help
>>          Say Y here if you want to use the IIC bus controller on
>>          the Freescale i.MX/MXC or Layerscape processors.
>>
> Could you please squash the changes (care about indentation in Kconfig also)
> and resend it as v2 (probably you may want to wait for review comments some
> more time)?
>
> Two more formal questions, why patch submitter's Signed-off-by: is not the
> last in the list and why Maxim has Signed-off-by tag here? Who is the author
> of the change?
>
> See Documentation/SubmittingPatches "Sign your work" for details.
>
> Best wishes,
> Vladimir

Re: [PATCH] i2c: imx: add slave support

[Wolfram Sang]

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

> Thus, if you don`t mind, we wont send fixed version for now.

Perfect!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]