* [PATCH] i2c: imx: add slave support
@ 2016-01-25 14:53 Dmitriy Baranov
2016-01-25 18:09 ` kbuild test robot
0 siblings, 1 reply; 8+ messages in thread
From: Dmitriy Baranov @ 2016-01-25 14:53 UTC (permalink / raw)
To: wsa; +Cc: linux-i2c, linux-kernel, Dmitriy Baranov, Maxim Syrchin
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-25 14:53 [PATCH] i2c: imx: add slave support Dmitriy Baranov
@ 2016-01-25 18:09 ` kbuild test robot
2016-01-26 8:22 ` Dmitriy Baranov
0 siblings, 1 reply; 8+ messages in thread
From: kbuild test robot @ 2016-01-25 18:09 UTC (permalink / raw)
Cc: kbuild-all, wsa, linux-i2c, linux-kernel, Dmitriy Baranov,
Maxim Syrchin
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-25 18:09 ` kbuild test robot
@ 2016-01-26 8:22 ` Dmitriy Baranov
2016-01-26 8:36 ` Wolfram Sang
0 siblings, 1 reply; 8+ messages in thread
From: Dmitriy Baranov @ 2016-01-26 8:22 UTC (permalink / raw)
Cc: wsa, linux-i2c, linux-kernel, Maxim Syrchin
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-26 8:22 ` Dmitriy Baranov
@ 2016-01-26 8:36 ` Wolfram Sang
2016-01-26 9:54 ` Dmitriy Baranov
0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2016-01-26 8:36 UTC (permalink / raw)
To: Dmitriy Baranov; +Cc: linux-i2c, linux-kernel, Maxim Syrchin
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-26 8:36 ` Wolfram Sang
@ 2016-01-26 9:54 ` Dmitriy Baranov
2016-01-26 13:37 ` Vladimir Zapolskiy
0 siblings, 1 reply; 8+ messages in thread
From: Dmitriy Baranov @ 2016-01-26 9:54 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c, linux-kernel, Maxim Syrchin
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.
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-26 9:54 ` Dmitriy Baranov
@ 2016-01-26 13:37 ` Vladimir Zapolskiy
2016-01-26 14:13 ` Dmitriy Baranov
0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Zapolskiy @ 2016-01-26 13:37 UTC (permalink / raw)
To: Dmitriy Baranov, Maxim Syrchin; +Cc: Wolfram Sang, linux-i2c, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i2c: imx: add slave support
2016-01-26 13:37 ` Vladimir Zapolskiy
@ 2016-01-26 14:13 ` Dmitriy Baranov
2016-01-26 15:09 ` Wolfram Sang
0 siblings, 1 reply; 8+ messages in thread
From: Dmitriy Baranov @ 2016-01-26 14:13 UTC (permalink / raw)
To: Vladimir Zapolskiy, Maxim Syrchin; +Cc: Wolfram Sang, linux-i2c, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-26 15:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-25 14:53 [PATCH] i2c: imx: add slave support Dmitriy Baranov
2016-01-25 18:09 ` kbuild test robot
2016-01-26 8:22 ` Dmitriy Baranov
2016-01-26 8:36 ` Wolfram Sang
2016-01-26 9:54 ` Dmitriy Baranov
2016-01-26 13:37 ` Vladimir Zapolskiy
2016-01-26 14:13 ` Dmitriy Baranov
2016-01-26 15:09 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).