* [PATCH] I2C: EXYNOS: Add slave support to i2c [not found] <1354354961-28833-1-git-send-email-giridhar.maruthy@linaro.org> @ 2012-12-03 12:16 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 12:16 UTC (permalink / raw) To: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 8434 bytes --] This patch adds slave support to i2c. The dt entry i2c-mode decides at probe time if the controller needs to work in slave mode and the controller is accordingly programmed. Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> --- drivers/i2c/busses/i2c-s3c2410.c | 100 ++++++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index e93e7d6..d83a6d7 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -53,6 +53,9 @@ /* Max time to wait for bus to become idle after a xfer (in us) */ #define S3C2410_IDLE_TIMEOUT 5000 +/* To find the master/slave mode of current controller */ +#define is_master(i2c) (!i2c->i2c_mode) + /* i2c controller state */ enum s3c24xx_i2c_state { STATE_IDLE, @@ -89,6 +92,8 @@ struct s3c24xx_i2c { #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition; #endif + /* i2c_mode: 0 is for master; and 1 is for slave */ + unsigned int i2c_mode; }; static struct platform_device_id s3c24xx_driver_ids[] = { @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, stat = 0; stat |= S3C2410_IICSTAT_TXRXEN; - if (msg->flags & I2C_M_RD) { - stat |= S3C2410_IICSTAT_MASTER_RX; - addr |= 1; - } else - stat |= S3C2410_IICSTAT_MASTER_TX; + if (is_master(i2c)) { + /* Master mode */ + if (msg->flags & I2C_M_RD) { + stat |= S3C2410_IICSTAT_MASTER_RX; + addr |= 1; + } else + stat |= S3C2410_IICSTAT_MASTER_TX; + } else { + /* Slave mode */ + if (msg->flags & I2C_M_RD) { + stat |= S3C2410_IICSTAT_SLAVE_RX; + addr |= 1; + } else + stat |= S3C2410_IICSTAT_SLAVE_TX; + } if (msg->flags & I2C_M_REV_DIR_ADDR) addr ^= 1; @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); writel(iiccon, i2c->regs + S3C2410_IICCON); - stat |= S3C2410_IICSTAT_START; - writel(stat, i2c->regs + S3C2410_IICSTAT); + if (is_master(i2c)) { + stat |= S3C2410_IICSTAT_START; + writel(stat, i2c->regs + S3C2410_IICSTAT); + } } static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) * devices, the host as Master and the HDMIPHY device as the slave. * Skipping the STOP condition has been tested on this bus and works. */ - if (i2c->quirks & QUIRK_HDMIPHY) { - /* Stop driving the I2C pins */ - iicstat &= ~S3C2410_IICSTAT_TXRXEN; - } else { - /* stop the transfer */ - iicstat &= ~S3C2410_IICSTAT_START; + if (is_master(i2c)) { + if (i2c->quirks & QUIRK_HDMIPHY) { + /* Stop driving the I2C pins */ + iicstat &= ~S3C2410_IICSTAT_TXRXEN; + } else { + /* stop the transfer */ + if (is_master(i2c)) { + /* Start/Stop required only for master */ + iicstat &= ~S3C2410_IICSTAT_START; + } + } + writel(iicstat, i2c->regs + S3C2410_IICSTAT); } - writel(iicstat, i2c->regs + S3C2410_IICSTAT); i2c->state = STATE_STOP; @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) */ if (iicstat & S3C2410_IICSTAT_LASTBIT && - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && + is_master(i2c)) { /* ack was not received... */ dev_dbg(i2c->dev, "ack was not received\n"); @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) * end of the message, and if so, work out what to do */ - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) { if (iicstat & S3C2410_IICSTAT_LASTBIT) { dev_dbg(i2c->dev, "WRITE: No Ack\n"); @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) } else { /* send stop */ - s3c24xx_i2c_stop(i2c, 0); } break; @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) i2c->msg->buf[i2c->msg_ptr++] = byte; prepare_read: - if (is_msglast(i2c)) { + if (is_msglast(i2c) && is_master(i2c)) { /* last byte of buffer */ if (is_lastmsg(i2c)) @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, if (i2c->suspended) return -EIO; - ret = s3c24xx_i2c_set_master(i2c); - if (ret != 0) { - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); - ret = -EAGAIN; - goto out; + if (is_master(i2c)) { + ret = s3c24xx_i2c_set_master(i2c); + if (ret != 0) { + dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); + ret = -EAGAIN; + goto out; + } } i2c->msg = msgs; @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, s3c24xx_i2c_enable_irq(i2c); s3c24xx_i2c_message_start(i2c, msgs); - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); + if (is_master(i2c)) + timeout = wait_event_timeout(i2c->wait,\ + i2c->msg_num == 0, HZ * 5); + else + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); ret = i2c->msg_idx; /* having these next two as dev_err() makes life very * noisy when doing an i2cdetect */ - if (timeout == 0) - dev_dbg(i2c->dev, "timeout\n"); - else if (ret != num) - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); + if (is_master(i2c)) { + if (timeout == 0) + dev_dbg(i2c->dev, "timeout\n"); + else if (ret != num) + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); - /* For QUIRK_HDMIPHY, bus is already disabled */ - if (i2c->quirks & QUIRK_HDMIPHY) - goto out; + /* For QUIRK_HDMIPHY, bus is already disabled */ + if (i2c->quirks & QUIRK_HDMIPHY) + goto out; - s3c24xx_i2c_wait_idle(i2c); + s3c24xx_i2c_wait_idle(i2c); + } out: return ret; @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr); of_property_read_u32(np, "samsung,i2c-max-bus-freq", (u32 *)&pdata->frequency); + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); } #else static void @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) goto err_noclk; } + /* By default, i2c works in master mode */ + /* This currently will be updated using DT */ + i2c->i2c_mode = 0; + i2c->quirks = s3c24xx_get_device_quirks(pdev); if (pdata) memcpy(i2c->pdata, pdata, sizeof(*pdata)); @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; i2c->tx_setup = 50; + init_waitqueue_head(&i2c->wait); /* find the clock and enable it */ -- 1.7.9.5 [-- Attachment #1.2: Type: text/html, Size: 9727 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-03 12:16 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 12:16 UTC (permalink / raw) To: linux-arm-kernel This patch adds slave support to i2c. The dt entry i2c-mode decides at probe time if the controller needs to work in slave mode and the controller is accordingly programmed. Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> --- drivers/i2c/busses/i2c-s3c2410.c | 100 ++++++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index e93e7d6..d83a6d7 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -53,6 +53,9 @@ /* Max time to wait for bus to become idle after a xfer (in us) */ #define S3C2410_IDLE_TIMEOUT 5000 +/* To find the master/slave mode of current controller */ +#define is_master(i2c) (!i2c->i2c_mode) + /* i2c controller state */ enum s3c24xx_i2c_state { STATE_IDLE, @@ -89,6 +92,8 @@ struct s3c24xx_i2c { #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition; #endif + /* i2c_mode: 0 is for master; and 1 is for slave */ + unsigned int i2c_mode; }; static struct platform_device_id s3c24xx_driver_ids[] = { @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, stat = 0; stat |= S3C2410_IICSTAT_TXRXEN; - if (msg->flags & I2C_M_RD) { - stat |= S3C2410_IICSTAT_MASTER_RX; - addr |= 1; - } else - stat |= S3C2410_IICSTAT_MASTER_TX; + if (is_master(i2c)) { + /* Master mode */ + if (msg->flags & I2C_M_RD) { + stat |= S3C2410_IICSTAT_MASTER_RX; + addr |= 1; + } else + stat |= S3C2410_IICSTAT_MASTER_TX; + } else { + /* Slave mode */ + if (msg->flags & I2C_M_RD) { + stat |= S3C2410_IICSTAT_SLAVE_RX; + addr |= 1; + } else + stat |= S3C2410_IICSTAT_SLAVE_TX; + } if (msg->flags & I2C_M_REV_DIR_ADDR) addr ^= 1; @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); writel(iiccon, i2c->regs + S3C2410_IICCON); - stat |= S3C2410_IICSTAT_START; - writel(stat, i2c->regs + S3C2410_IICSTAT); + if (is_master(i2c)) { + stat |= S3C2410_IICSTAT_START; + writel(stat, i2c->regs + S3C2410_IICSTAT); + } } static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) * devices, the host as Master and the HDMIPHY device as the slave. * Skipping the STOP condition has been tested on this bus and works. */ - if (i2c->quirks & QUIRK_HDMIPHY) { - /* Stop driving the I2C pins */ - iicstat &= ~S3C2410_IICSTAT_TXRXEN; - } else { - /* stop the transfer */ - iicstat &= ~S3C2410_IICSTAT_START; + if (is_master(i2c)) { + if (i2c->quirks & QUIRK_HDMIPHY) { + /* Stop driving the I2C pins */ + iicstat &= ~S3C2410_IICSTAT_TXRXEN; + } else { + /* stop the transfer */ + if (is_master(i2c)) { + /* Start/Stop required only for master */ + iicstat &= ~S3C2410_IICSTAT_START; + } + } + writel(iicstat, i2c->regs + S3C2410_IICSTAT); } - writel(iicstat, i2c->regs + S3C2410_IICSTAT); i2c->state = STATE_STOP; @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) */ if (iicstat & S3C2410_IICSTAT_LASTBIT && - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && + is_master(i2c)) { /* ack was not received... */ dev_dbg(i2c->dev, "ack was not received\n"); @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) * end of the message, and if so, work out what to do */ - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) { if (iicstat & S3C2410_IICSTAT_LASTBIT) { dev_dbg(i2c->dev, "WRITE: No Ack\n"); @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) } else { /* send stop */ - s3c24xx_i2c_stop(i2c, 0); } break; @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) i2c->msg->buf[i2c->msg_ptr++] = byte; prepare_read: - if (is_msglast(i2c)) { + if (is_msglast(i2c) && is_master(i2c)) { /* last byte of buffer */ if (is_lastmsg(i2c)) @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, if (i2c->suspended) return -EIO; - ret = s3c24xx_i2c_set_master(i2c); - if (ret != 0) { - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); - ret = -EAGAIN; - goto out; + if (is_master(i2c)) { + ret = s3c24xx_i2c_set_master(i2c); + if (ret != 0) { + dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); + ret = -EAGAIN; + goto out; + } } i2c->msg = msgs; @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, s3c24xx_i2c_enable_irq(i2c); s3c24xx_i2c_message_start(i2c, msgs); - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); + if (is_master(i2c)) + timeout = wait_event_timeout(i2c->wait,\ + i2c->msg_num == 0, HZ * 5); + else + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); ret = i2c->msg_idx; /* having these next two as dev_err() makes life very * noisy when doing an i2cdetect */ - if (timeout == 0) - dev_dbg(i2c->dev, "timeout\n"); - else if (ret != num) - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); + if (is_master(i2c)) { + if (timeout == 0) + dev_dbg(i2c->dev, "timeout\n"); + else if (ret != num) + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); - /* For QUIRK_HDMIPHY, bus is already disabled */ - if (i2c->quirks & QUIRK_HDMIPHY) - goto out; + /* For QUIRK_HDMIPHY, bus is already disabled */ + if (i2c->quirks & QUIRK_HDMIPHY) + goto out; - s3c24xx_i2c_wait_idle(i2c); + s3c24xx_i2c_wait_idle(i2c); + } out: return ret; @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr); of_property_read_u32(np, "samsung,i2c-max-bus-freq", (u32 *)&pdata->frequency); + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); } #else static void @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) goto err_noclk; } + /* By default, i2c works in master mode */ + /* This currently will be updated using DT */ + i2c->i2c_mode = 0; + i2c->quirks = s3c24xx_get_device_quirks(pdev); if (pdata) memcpy(i2c->pdata, pdata, sizeof(*pdata)); @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; i2c->tx_setup = 50; + init_waitqueue_head(&i2c->wait); /* find the clock and enable it */ -- 1.7.9.5 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121203/a4a4ec03/attachment-0001.html> ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-03 12:16 ` Giridhar Maruthy @ 2012-12-03 13:28 ` Kyungmin Park -1 siblings, 0 replies; 23+ messages in thread From: Kyungmin Park @ 2012-12-03 13:28 UTC (permalink / raw) To: Giridhar Maruthy Cc: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Hi, On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy <giridhar.maruthy@linaro.org> wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; If it's used for master or not, doesn't better to use 'is_master' or 'is_slave'? what's the meaning of 'i2c_mode'? and #define is_master(i2c) (i2c->is_master) Thank you, Kyungmin Park > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c > *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) > { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-03 13:28 ` Kyungmin Park 0 siblings, 0 replies; 23+ messages in thread From: Kyungmin Park @ 2012-12-03 13:28 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy <giridhar.maruthy@linaro.org> wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; If it's used for master or not, doesn't better to use 'is_master' or 'is_slave'? what's the meaning of 'i2c_mode'? and #define is_master(i2c) (i2c->is_master) Thank you, Kyungmin Park > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c > *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) > { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-03 13:28 ` Kyungmin Park @ 2012-12-03 22:46 ` Giridhar Maruthy -1 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 22:46 UTC (permalink / raw) To: Kyungmin Park Cc: linux-samsung-soc, w.sang, linux-kernel, kgene.kim, linux-i2c, ben-linux, khali, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 10112 bytes --] Thank you Kyungmin Park for the review. On 3 December 2012 22:28, Kyungmin Park <kmpark@infradead.org> wrote: > Hi, > > On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy > <giridhar.maruthy@linaro.org> wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. > > > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > > --- > > drivers/i2c/busses/i2c-s3c2410.c | 100 > > ++++++++++++++++++++++++++------------ > > 1 file changed, 68 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > > b/drivers/i2c/busses/i2c-s3c2410.c > > index e93e7d6..d83a6d7 100644 > > --- a/drivers/i2c/busses/i2c-s3c2410.c > > +++ b/drivers/i2c/busses/i2c-s3c2410.c > > @@ -53,6 +53,9 @@ > > /* Max time to wait for bus to become idle after a xfer (in us) */ > > #define S3C2410_IDLE_TIMEOUT 5000 > > > > +/* To find the master/slave mode of current controller */ > > +#define is_master(i2c) (!i2c->i2c_mode) > > + > > /* i2c controller state */ > > enum s3c24xx_i2c_state { > > STATE_IDLE, > > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > > #ifdef CONFIG_CPU_FREQ > > struct notifier_block freq_transition; > > #endif > > + /* i2c_mode: 0 is for master; and 1 is for slave */ > > + unsigned int i2c_mode; > If it's used for master or not, doesn't better to use 'is_master' or > 'is_slave'? what's the meaning of 'i2c_mode'? > and > #define is_master(i2c) (i2c->is_master) > I thought mode indicates master mode or slave mode, but I agree 'is_master' name is better. Will change that. > > Thank you, > Kyungmin Park > > }; > > > > static struct platform_device_id s3c24xx_driver_ids[] = { > > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > stat = 0; > > stat |= S3C2410_IICSTAT_TXRXEN; > > > > - if (msg->flags & I2C_M_RD) { > > - stat |= S3C2410_IICSTAT_MASTER_RX; > > - addr |= 1; > > - } else > > - stat |= S3C2410_IICSTAT_MASTER_TX; > > + if (is_master(i2c)) { > > + /* Master mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_MASTER_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_MASTER_TX; > > + } else { > > + /* Slave mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_SLAVE_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_SLAVE_TX; > > + } > > > > if (msg->flags & I2C_M_REV_DIR_ADDR) > > addr ^= 1; > > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > > writel(iiccon, i2c->regs + S3C2410_IICCON); > > > > - stat |= S3C2410_IICSTAT_START; > > - writel(stat, i2c->regs + S3C2410_IICSTAT); > > + if (is_master(i2c)) { > > + stat |= S3C2410_IICSTAT_START; > > + writel(stat, i2c->regs + S3C2410_IICSTAT); > > + } > > } > > > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c > > *i2c, int ret) > > * devices, the host as Master and the HDMIPHY device as the > slave. > > * Skipping the STOP condition has been tested on this bus and > > works. > > */ > > - if (i2c->quirks & QUIRK_HDMIPHY) { > > - /* Stop driving the I2C pins */ > > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > - } else { > > - /* stop the transfer */ > > - iicstat &= ~S3C2410_IICSTAT_START; > > + if (is_master(i2c)) { > > + if (i2c->quirks & QUIRK_HDMIPHY) { > > + /* Stop driving the I2C pins */ > > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > + } else { > > + /* stop the transfer */ > > + if (is_master(i2c)) { > > + /* Start/Stop required only for master */ > > + iicstat &= ~S3C2410_IICSTAT_START; > > + } > > + } > > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > } > > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > > i2c->state = STATE_STOP; > > > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > */ > > > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > > + is_master(i2c)) { > > /* ack was not received... */ > > > > dev_dbg(i2c->dev, "ack was not received\n"); > > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > * end of the message, and if so, work out what to do > > */ > > > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) > > { > > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > > > } else { > > /* send stop */ > > - > > s3c24xx_i2c_stop(i2c, 0); > > } > > break; > > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > i2c->msg->buf[i2c->msg_ptr++] = byte; > > > > prepare_read: > > - if (is_msglast(i2c)) { > > + if (is_msglast(i2c) && is_master(i2c)) { > > /* last byte of buffer */ > > > > if (is_lastmsg(i2c)) > > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c > *i2c, > > if (i2c->suspended) > > return -EIO; > > > > - ret = s3c24xx_i2c_set_master(i2c); > > - if (ret != 0) { > > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > > - ret = -EAGAIN; > > - goto out; > > + if (is_master(i2c)) { > > + ret = s3c24xx_i2c_set_master(i2c); > > + if (ret != 0) { > > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > > ret); > > + ret = -EAGAIN; > > + goto out; > > + } > > } > > > > i2c->msg = msgs; > > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c > *i2c, > > s3c24xx_i2c_enable_irq(i2c); > > s3c24xx_i2c_message_start(i2c, msgs); > > > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * > 5); > > + if (is_master(i2c)) > > + timeout = wait_event_timeout(i2c->wait,\ > > + i2c->msg_num == 0, HZ * 5); > > + else > > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > > > ret = i2c->msg_idx; > > > > /* having these next two as dev_err() makes life very > > * noisy when doing an i2cdetect */ > > > > - if (timeout == 0) > > - dev_dbg(i2c->dev, "timeout\n"); > > - else if (ret != num) > > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > + if (is_master(i2c)) { > > + if (timeout == 0) > > + dev_dbg(i2c->dev, "timeout\n"); > > + else if (ret != num) > > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > > - if (i2c->quirks & QUIRK_HDMIPHY) > > - goto out; > > + /* For QUIRK_HDMIPHY, bus is already disabled */ > > + if (i2c->quirks & QUIRK_HDMIPHY) > > + goto out; > > > > - s3c24xx_i2c_wait_idle(i2c); > > + s3c24xx_i2c_wait_idle(i2c); > > + } > > > > out: > > return ret; > > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > > s3c24xx_i2c *i2c) > > of_property_read_u32(np, "samsung,i2c-slave-addr", > > &pdata->slave_addr); > > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > > (u32 *)&pdata->frequency); > > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > > } > > #else > > static void > > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct > platform_device > > *pdev) > > goto err_noclk; > > } > > > > + /* By default, i2c works in master mode */ > > + /* This currently will be updated using DT */ > > + i2c->i2c_mode = 0; > > + > > i2c->quirks = s3c24xx_get_device_quirks(pdev); > > if (pdata) > > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > > i2c->tx_setup = 50; > > > > + > > init_waitqueue_head(&i2c->wait); > > > > /* find the clock and enable it */ > > -- > > 1.7.9.5 > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > [-- Attachment #1.2: Type: text/html, Size: 12357 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-03 22:46 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 22:46 UTC (permalink / raw) To: linux-arm-kernel Thank you Kyungmin Park for the review. On 3 December 2012 22:28, Kyungmin Park <kmpark@infradead.org> wrote: > Hi, > > On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy > <giridhar.maruthy@linaro.org> wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. > > > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > > --- > > drivers/i2c/busses/i2c-s3c2410.c | 100 > > ++++++++++++++++++++++++++------------ > > 1 file changed, 68 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > > b/drivers/i2c/busses/i2c-s3c2410.c > > index e93e7d6..d83a6d7 100644 > > --- a/drivers/i2c/busses/i2c-s3c2410.c > > +++ b/drivers/i2c/busses/i2c-s3c2410.c > > @@ -53,6 +53,9 @@ > > /* Max time to wait for bus to become idle after a xfer (in us) */ > > #define S3C2410_IDLE_TIMEOUT 5000 > > > > +/* To find the master/slave mode of current controller */ > > +#define is_master(i2c) (!i2c->i2c_mode) > > + > > /* i2c controller state */ > > enum s3c24xx_i2c_state { > > STATE_IDLE, > > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > > #ifdef CONFIG_CPU_FREQ > > struct notifier_block freq_transition; > > #endif > > + /* i2c_mode: 0 is for master; and 1 is for slave */ > > + unsigned int i2c_mode; > If it's used for master or not, doesn't better to use 'is_master' or > 'is_slave'? what's the meaning of 'i2c_mode'? > and > #define is_master(i2c) (i2c->is_master) > I thought mode indicates master mode or slave mode, but I agree 'is_master' name is better. Will change that. > > Thank you, > Kyungmin Park > > }; > > > > static struct platform_device_id s3c24xx_driver_ids[] = { > > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > stat = 0; > > stat |= S3C2410_IICSTAT_TXRXEN; > > > > - if (msg->flags & I2C_M_RD) { > > - stat |= S3C2410_IICSTAT_MASTER_RX; > > - addr |= 1; > > - } else > > - stat |= S3C2410_IICSTAT_MASTER_TX; > > + if (is_master(i2c)) { > > + /* Master mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_MASTER_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_MASTER_TX; > > + } else { > > + /* Slave mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_SLAVE_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_SLAVE_TX; > > + } > > > > if (msg->flags & I2C_M_REV_DIR_ADDR) > > addr ^= 1; > > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > > writel(iiccon, i2c->regs + S3C2410_IICCON); > > > > - stat |= S3C2410_IICSTAT_START; > > - writel(stat, i2c->regs + S3C2410_IICSTAT); > > + if (is_master(i2c)) { > > + stat |= S3C2410_IICSTAT_START; > > + writel(stat, i2c->regs + S3C2410_IICSTAT); > > + } > > } > > > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c > > *i2c, int ret) > > * devices, the host as Master and the HDMIPHY device as the > slave. > > * Skipping the STOP condition has been tested on this bus and > > works. > > */ > > - if (i2c->quirks & QUIRK_HDMIPHY) { > > - /* Stop driving the I2C pins */ > > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > - } else { > > - /* stop the transfer */ > > - iicstat &= ~S3C2410_IICSTAT_START; > > + if (is_master(i2c)) { > > + if (i2c->quirks & QUIRK_HDMIPHY) { > > + /* Stop driving the I2C pins */ > > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > + } else { > > + /* stop the transfer */ > > + if (is_master(i2c)) { > > + /* Start/Stop required only for master */ > > + iicstat &= ~S3C2410_IICSTAT_START; > > + } > > + } > > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > } > > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > > i2c->state = STATE_STOP; > > > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > */ > > > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > > + is_master(i2c)) { > > /* ack was not received... */ > > > > dev_dbg(i2c->dev, "ack was not received\n"); > > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > * end of the message, and if so, work out what to do > > */ > > > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) > > { > > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > > > } else { > > /* send stop */ > > - > > s3c24xx_i2c_stop(i2c, 0); > > } > > break; > > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, > > unsigned long iicstat) > > i2c->msg->buf[i2c->msg_ptr++] = byte; > > > > prepare_read: > > - if (is_msglast(i2c)) { > > + if (is_msglast(i2c) && is_master(i2c)) { > > /* last byte of buffer */ > > > > if (is_lastmsg(i2c)) > > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c > *i2c, > > if (i2c->suspended) > > return -EIO; > > > > - ret = s3c24xx_i2c_set_master(i2c); > > - if (ret != 0) { > > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > > - ret = -EAGAIN; > > - goto out; > > + if (is_master(i2c)) { > > + ret = s3c24xx_i2c_set_master(i2c); > > + if (ret != 0) { > > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > > ret); > > + ret = -EAGAIN; > > + goto out; > > + } > > } > > > > i2c->msg = msgs; > > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c > *i2c, > > s3c24xx_i2c_enable_irq(i2c); > > s3c24xx_i2c_message_start(i2c, msgs); > > > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * > 5); > > + if (is_master(i2c)) > > + timeout = wait_event_timeout(i2c->wait,\ > > + i2c->msg_num == 0, HZ * 5); > > + else > > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > > > ret = i2c->msg_idx; > > > > /* having these next two as dev_err() makes life very > > * noisy when doing an i2cdetect */ > > > > - if (timeout == 0) > > - dev_dbg(i2c->dev, "timeout\n"); > > - else if (ret != num) > > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > + if (is_master(i2c)) { > > + if (timeout == 0) > > + dev_dbg(i2c->dev, "timeout\n"); > > + else if (ret != num) > > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > > - if (i2c->quirks & QUIRK_HDMIPHY) > > - goto out; > > + /* For QUIRK_HDMIPHY, bus is already disabled */ > > + if (i2c->quirks & QUIRK_HDMIPHY) > > + goto out; > > > > - s3c24xx_i2c_wait_idle(i2c); > > + s3c24xx_i2c_wait_idle(i2c); > > + } > > > > out: > > return ret; > > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > > s3c24xx_i2c *i2c) > > of_property_read_u32(np, "samsung,i2c-slave-addr", > > &pdata->slave_addr); > > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > > (u32 *)&pdata->frequency); > > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > > } > > #else > > static void > > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct > platform_device > > *pdev) > > goto err_noclk; > > } > > > > + /* By default, i2c works in master mode */ > > + /* This currently will be updated using DT */ > > + i2c->i2c_mode = 0; > > + > > i2c->quirks = s3c24xx_get_device_quirks(pdev); > > if (pdata) > > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > > i2c->tx_setup = 50; > > > > + > > init_waitqueue_head(&i2c->wait); > > > > /* find the clock and enable it */ > > -- > > 1.7.9.5 > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121204/8fa2b6e8/attachment-0001.html> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-03 13:28 ` Kyungmin Park @ 2012-12-03 22:53 ` Giridhar Maruthy -1 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 22:53 UTC (permalink / raw) To: Kyungmin Park Cc: linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Thank you Kyungmin Park for the review. On 3 December 2012 22:28, Kyungmin Park <kmpark@infradead.org> wrote: > > Hi, > > On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy > <giridhar.maruthy@linaro.org> wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. > > > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > > --- > > drivers/i2c/busses/i2c-s3c2410.c | 100 > > ++++++++++++++++++++++++++------------ > > 1 file changed, 68 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > > b/drivers/i2c/busses/i2c-s3c2410.c > > index e93e7d6..d83a6d7 100644 > > --- a/drivers/i2c/busses/i2c-s3c2410.c > > +++ b/drivers/i2c/busses/i2c-s3c2410.c > > @@ -53,6 +53,9 @@ > > /* Max time to wait for bus to become idle after a xfer (in us) */ > > #define S3C2410_IDLE_TIMEOUT 5000 > > > > +/* To find the master/slave mode of current controller */ > > +#define is_master(i2c) (!i2c->i2c_mode) > > + > > /* i2c controller state */ > > enum s3c24xx_i2c_state { > > STATE_IDLE, > > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > > #ifdef CONFIG_CPU_FREQ > > struct notifier_block freq_transition; > > #endif > > + /* i2c_mode: 0 is for master; and 1 is for slave */ > > + unsigned int i2c_mode; > If it's used for master or not, doesn't better to use 'is_master' or > 'is_slave'? what's the meaning of 'i2c_mode'? > and > #define is_master(i2c) (i2c->is_master) I thought mode indicates master mode or slave mode, but I agree 'is_master' name is better. Will change that. > > Thank you, > Kyungmin Park > > }; > > > > static struct platform_device_id s3c24xx_driver_ids[] = { > > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > stat = 0; > > stat |= S3C2410_IICSTAT_TXRXEN; > > > > - if (msg->flags & I2C_M_RD) { > > - stat |= S3C2410_IICSTAT_MASTER_RX; > > - addr |= 1; > > - } else > > - stat |= S3C2410_IICSTAT_MASTER_TX; > > + if (is_master(i2c)) { > > + /* Master mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_MASTER_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_MASTER_TX; > > + } else { > > + /* Slave mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_SLAVE_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_SLAVE_TX; > > + } > > > > if (msg->flags & I2C_M_REV_DIR_ADDR) > > addr ^= 1; > > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > > writel(iiccon, i2c->regs + S3C2410_IICCON); > > > > - stat |= S3C2410_IICSTAT_START; > > - writel(stat, i2c->regs + S3C2410_IICSTAT); > > + if (is_master(i2c)) { > > + stat |= S3C2410_IICSTAT_START; > > + writel(stat, i2c->regs + S3C2410_IICSTAT); > > + } > > } > > > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c > > *i2c, int ret) > > * devices, the host as Master and the HDMIPHY device as the slave. > > * Skipping the STOP condition has been tested on this bus and > > works. > > */ > > - if (i2c->quirks & QUIRK_HDMIPHY) { > > - /* Stop driving the I2C pins */ > > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > - } else { > > - /* stop the transfer */ > > - iicstat &= ~S3C2410_IICSTAT_START; > > + if (is_master(i2c)) { > > + if (i2c->quirks & QUIRK_HDMIPHY) { > > + /* Stop driving the I2C pins */ > > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > + } else { > > + /* stop the transfer */ > > + if (is_master(i2c)) { > > + /* Start/Stop required only for master */ > > + iicstat &= ~S3C2410_IICSTAT_START; > > + } > > + } > > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > } > > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > > i2c->state = STATE_STOP; > > > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > */ > > > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > > + is_master(i2c)) { > > /* ack was not received... */ > > > > dev_dbg(i2c->dev, "ack was not received\n"); > > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > * end of the message, and if so, work out what to do > > */ > > > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) > > { > > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > > > } else { > > /* send stop */ > > - > > s3c24xx_i2c_stop(i2c, 0); > > } > > break; > > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > i2c->msg->buf[i2c->msg_ptr++] = byte; > > > > prepare_read: > > - if (is_msglast(i2c)) { > > + if (is_msglast(i2c) && is_master(i2c)) { > > /* last byte of buffer */ > > > > if (is_lastmsg(i2c)) > > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > > if (i2c->suspended) > > return -EIO; > > > > - ret = s3c24xx_i2c_set_master(i2c); > > - if (ret != 0) { > > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > > - ret = -EAGAIN; > > - goto out; > > + if (is_master(i2c)) { > > + ret = s3c24xx_i2c_set_master(i2c); > > + if (ret != 0) { > > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > > ret); > > + ret = -EAGAIN; > > + goto out; > > + } > > } > > > > i2c->msg = msgs; > > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > > s3c24xx_i2c_enable_irq(i2c); > > s3c24xx_i2c_message_start(i2c, msgs); > > > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > > + if (is_master(i2c)) > > + timeout = wait_event_timeout(i2c->wait,\ > > + i2c->msg_num == 0, HZ * 5); > > + else > > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > > > ret = i2c->msg_idx; > > > > /* having these next two as dev_err() makes life very > > * noisy when doing an i2cdetect */ > > > > - if (timeout == 0) > > - dev_dbg(i2c->dev, "timeout\n"); > > - else if (ret != num) > > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > + if (is_master(i2c)) { > > + if (timeout == 0) > > + dev_dbg(i2c->dev, "timeout\n"); > > + else if (ret != num) > > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > > - if (i2c->quirks & QUIRK_HDMIPHY) > > - goto out; > > + /* For QUIRK_HDMIPHY, bus is already disabled */ > > + if (i2c->quirks & QUIRK_HDMIPHY) > > + goto out; > > > > - s3c24xx_i2c_wait_idle(i2c); > > + s3c24xx_i2c_wait_idle(i2c); > > + } > > > > out: > > return ret; > > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > > s3c24xx_i2c *i2c) > > of_property_read_u32(np, "samsung,i2c-slave-addr", > > &pdata->slave_addr); > > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > > (u32 *)&pdata->frequency); > > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > > } > > #else > > static void > > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > goto err_noclk; > > } > > > > + /* By default, i2c works in master mode */ > > + /* This currently will be updated using DT */ > > + i2c->i2c_mode = 0; > > + > > i2c->quirks = s3c24xx_get_device_quirks(pdev); > > if (pdata) > > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > > i2c->tx_setup = 50; > > > > + > > init_waitqueue_head(&i2c->wait); > > > > /* find the clock and enable it */ > > -- > > 1.7.9.5 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-03 22:53 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-03 22:53 UTC (permalink / raw) To: linux-arm-kernel Thank you Kyungmin Park for the review. On 3 December 2012 22:28, Kyungmin Park <kmpark@infradead.org> wrote: > > Hi, > > On Mon, Dec 3, 2012 at 9:16 PM, Giridhar Maruthy > <giridhar.maruthy@linaro.org> wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. > > > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > > --- > > drivers/i2c/busses/i2c-s3c2410.c | 100 > > ++++++++++++++++++++++++++------------ > > 1 file changed, 68 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > > b/drivers/i2c/busses/i2c-s3c2410.c > > index e93e7d6..d83a6d7 100644 > > --- a/drivers/i2c/busses/i2c-s3c2410.c > > +++ b/drivers/i2c/busses/i2c-s3c2410.c > > @@ -53,6 +53,9 @@ > > /* Max time to wait for bus to become idle after a xfer (in us) */ > > #define S3C2410_IDLE_TIMEOUT 5000 > > > > +/* To find the master/slave mode of current controller */ > > +#define is_master(i2c) (!i2c->i2c_mode) > > + > > /* i2c controller state */ > > enum s3c24xx_i2c_state { > > STATE_IDLE, > > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > > #ifdef CONFIG_CPU_FREQ > > struct notifier_block freq_transition; > > #endif > > + /* i2c_mode: 0 is for master; and 1 is for slave */ > > + unsigned int i2c_mode; > If it's used for master or not, doesn't better to use 'is_master' or > 'is_slave'? what's the meaning of 'i2c_mode'? > and > #define is_master(i2c) (i2c->is_master) I thought mode indicates master mode or slave mode, but I agree 'is_master' name is better. Will change that. > > Thank you, > Kyungmin Park > > }; > > > > static struct platform_device_id s3c24xx_driver_ids[] = { > > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > stat = 0; > > stat |= S3C2410_IICSTAT_TXRXEN; > > > > - if (msg->flags & I2C_M_RD) { > > - stat |= S3C2410_IICSTAT_MASTER_RX; > > - addr |= 1; > > - } else > > - stat |= S3C2410_IICSTAT_MASTER_TX; > > + if (is_master(i2c)) { > > + /* Master mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_MASTER_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_MASTER_TX; > > + } else { > > + /* Slave mode */ > > + if (msg->flags & I2C_M_RD) { > > + stat |= S3C2410_IICSTAT_SLAVE_RX; > > + addr |= 1; > > + } else > > + stat |= S3C2410_IICSTAT_SLAVE_TX; > > + } > > > > if (msg->flags & I2C_M_REV_DIR_ADDR) > > addr ^= 1; > > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > > s3c24xx_i2c *i2c, > > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > > writel(iiccon, i2c->regs + S3C2410_IICCON); > > > > - stat |= S3C2410_IICSTAT_START; > > - writel(stat, i2c->regs + S3C2410_IICSTAT); > > + if (is_master(i2c)) { > > + stat |= S3C2410_IICSTAT_START; > > + writel(stat, i2c->regs + S3C2410_IICSTAT); > > + } > > } > > > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c > > *i2c, int ret) > > * devices, the host as Master and the HDMIPHY device as the slave. > > * Skipping the STOP condition has been tested on this bus and > > works. > > */ > > - if (i2c->quirks & QUIRK_HDMIPHY) { > > - /* Stop driving the I2C pins */ > > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > - } else { > > - /* stop the transfer */ > > - iicstat &= ~S3C2410_IICSTAT_START; > > + if (is_master(i2c)) { > > + if (i2c->quirks & QUIRK_HDMIPHY) { > > + /* Stop driving the I2C pins */ > > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > > + } else { > > + /* stop the transfer */ > > + if (is_master(i2c)) { > > + /* Start/Stop required only for master */ > > + iicstat &= ~S3C2410_IICSTAT_START; > > + } > > + } > > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > } > > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > > i2c->state = STATE_STOP; > > > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > */ > > > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > > + is_master(i2c)) { > > /* ack was not received... */ > > > > dev_dbg(i2c->dev, "ack was not received\n"); > > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > * end of the message, and if so, work out what to do > > */ > > > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && is_master(i2c)) > > { > > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > > > } else { > > /* send stop */ > > - > > s3c24xx_i2c_stop(i2c, 0); > > } > > break; > > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, > > unsigned long iicstat) > > i2c->msg->buf[i2c->msg_ptr++] = byte; > > > > prepare_read: > > - if (is_msglast(i2c)) { > > + if (is_msglast(i2c) && is_master(i2c)) { > > /* last byte of buffer */ > > > > if (is_lastmsg(i2c)) > > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > > if (i2c->suspended) > > return -EIO; > > > > - ret = s3c24xx_i2c_set_master(i2c); > > - if (ret != 0) { > > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > > - ret = -EAGAIN; > > - goto out; > > + if (is_master(i2c)) { > > + ret = s3c24xx_i2c_set_master(i2c); > > + if (ret != 0) { > > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > > ret); > > + ret = -EAGAIN; > > + goto out; > > + } > > } > > > > i2c->msg = msgs; > > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > > s3c24xx_i2c_enable_irq(i2c); > > s3c24xx_i2c_message_start(i2c, msgs); > > > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > > + if (is_master(i2c)) > > + timeout = wait_event_timeout(i2c->wait,\ > > + i2c->msg_num == 0, HZ * 5); > > + else > > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > > > ret = i2c->msg_idx; > > > > /* having these next two as dev_err() makes life very > > * noisy when doing an i2cdetect */ > > > > - if (timeout == 0) > > - dev_dbg(i2c->dev, "timeout\n"); > > - else if (ret != num) > > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > + if (is_master(i2c)) { > > + if (timeout == 0) > > + dev_dbg(i2c->dev, "timeout\n"); > > + else if (ret != num) > > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > > - if (i2c->quirks & QUIRK_HDMIPHY) > > - goto out; > > + /* For QUIRK_HDMIPHY, bus is already disabled */ > > + if (i2c->quirks & QUIRK_HDMIPHY) > > + goto out; > > > > - s3c24xx_i2c_wait_idle(i2c); > > + s3c24xx_i2c_wait_idle(i2c); > > + } > > > > out: > > return ret; > > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > > s3c24xx_i2c *i2c) > > of_property_read_u32(np, "samsung,i2c-slave-addr", > > &pdata->slave_addr); > > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > > (u32 *)&pdata->frequency); > > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > > } > > #else > > static void > > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > goto err_noclk; > > } > > > > + /* By default, i2c works in master mode */ > > + /* This currently will be updated using DT */ > > + i2c->i2c_mode = 0; > > + > > i2c->quirks = s3c24xx_get_device_quirks(pdev); > > if (pdata) > > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > > *pdev) > > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > > i2c->tx_setup = 50; > > > > + > > init_waitqueue_head(&i2c->wait); > > > > /* find the clock and enable it */ > > -- > > 1.7.9.5 ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CAHuqX+EQ0pZnm27yAVqC=NRX8huQ5EvbC8PGn2G3xYw4D71gQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-03 12:16 ` Giridhar Maruthy (?) @ 2012-12-06 14:05 ` Subash Patel -1 siblings, 0 replies; 23+ messages in thread From: Subash Patel @ 2012-12-06 14:05 UTC (permalink / raw) To: Giridhar Maruthy Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg, kgene.kim-Sze3O3UU22JBDgjK7y7TUQ, khali-PUYAD+kWke1g9hUCZPvPmw, w.sang-bIcnvbaLZ9MEGnE8C9+IrQ, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Hi Giridhar, On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org > <mailto:giridhar.maruthy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); I don't see i2c controller for HDMIPHY working in slave mode. So do we need to check if its master and proceed? Cant the quirks check enough for it? Even if it is configured as slave, there is no error indication for this here. > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > Regards, Subash ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-06 14:05 ` Subash Patel 0 siblings, 0 replies; 23+ messages in thread From: Subash Patel @ 2012-12-06 14:05 UTC (permalink / raw) To: Giridhar Maruthy Cc: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Hi Giridhar, On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org > <mailto:giridhar.maruthy@linaro.org>> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); I don't see i2c controller for HDMIPHY working in slave mode. So do we need to check if its master and proceed? Cant the quirks check enough for it? Even if it is configured as slave, there is no error indication for this here. > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > Regards, Subash ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-06 14:05 ` Subash Patel 0 siblings, 0 replies; 23+ messages in thread From: Subash Patel @ 2012-12-06 14:05 UTC (permalink / raw) To: linux-arm-kernel Hi Giridhar, On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org > <mailto:giridhar.maruthy@linaro.org>> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); I don't see i2c controller for HDMIPHY working in slave mode. So do we need to check if its master and proceed? Cant the quirks check enough for it? Even if it is configured as slave, there is no error indication for this here. > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct > platform_device *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > Regards, Subash ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-06 14:05 ` Subash Patel @ 2012-12-07 11:36 ` Giridhar Maruthy -1 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-07 11:36 UTC (permalink / raw) To: Subash Patel Cc: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Thanks for the review Subash. Please find my reply below. On 6 December 2012 23:05, Subash Patel <subashrp@gmail.com> wrote: > Hi Giridhar, > > > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> >> This patch adds slave support to i2c. The dt entry i2c-mode >> decides at probe time if the controller needs to work in >> slave mode and the controller is accordingly programmed. >> >> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org >> <mailto:giridhar.maruthy@linaro.org>> >> >> --- >> drivers/i2c/busses/i2c-s3c2410.c | 100 >> ++++++++++++++++++++++++++------------ >> 1 file changed, 68 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-s3c2410.c >> b/drivers/i2c/busses/i2c-s3c2410.c >> index e93e7d6..d83a6d7 100644 >> --- a/drivers/i2c/busses/i2c-s3c2410.c >> +++ b/drivers/i2c/busses/i2c-s3c2410.c >> @@ -53,6 +53,9 @@ >> /* Max time to wait for bus to become idle after a xfer (in us) */ >> #define S3C2410_IDLE_TIMEOUT 5000 >> >> +/* To find the master/slave mode of current controller */ >> +#define is_master(i2c) (!i2c->i2c_mode) >> + >> /* i2c controller state */ >> enum s3c24xx_i2c_state { >> STATE_IDLE, >> @@ -89,6 +92,8 @@ struct s3c24xx_i2c { >> #ifdef CONFIG_CPU_FREQ >> struct notifier_block freq_transition; >> #endif >> + /* i2c_mode: 0 is for master; and 1 is for slave */ >> + unsigned int i2c_mode; >> }; >> >> static struct platform_device_id s3c24xx_driver_ids[] = { >> @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> stat = 0; >> stat |= S3C2410_IICSTAT_TXRXEN; >> >> - if (msg->flags & I2C_M_RD) { >> - stat |= S3C2410_IICSTAT_MASTER_RX; >> - addr |= 1; >> - } else >> - stat |= S3C2410_IICSTAT_MASTER_TX; >> + if (is_master(i2c)) { >> + /* Master mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_MASTER_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_MASTER_TX; >> + } else { >> + /* Slave mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_SLAVE_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_SLAVE_TX; >> + } >> >> if (msg->flags & I2C_M_REV_DIR_ADDR) >> addr ^= 1; >> @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); >> writel(iiccon, i2c->regs + S3C2410_IICCON); >> >> - stat |= S3C2410_IICSTAT_START; >> - writel(stat, i2c->regs + S3C2410_IICSTAT); >> + if (is_master(i2c)) { >> + stat |= S3C2410_IICSTAT_START; >> + writel(stat, i2c->regs + S3C2410_IICSTAT); >> + } >> } >> >> static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) >> @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct >> s3c24xx_i2c *i2c, int ret) >> * devices, the host as Master and the HDMIPHY device as the >> slave. >> * Skipping the STOP condition has been tested on this bus and >> works. >> */ >> - if (i2c->quirks & QUIRK_HDMIPHY) { >> - /* Stop driving the I2C pins */ >> - iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> - } else { >> - /* stop the transfer */ >> - iicstat &= ~S3C2410_IICSTAT_START; >> + if (is_master(i2c)) { >> + if (i2c->quirks & QUIRK_HDMIPHY) { >> + /* Stop driving the I2C pins */ >> + iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> + } else { >> + /* stop the transfer */ >> + if (is_master(i2c)) { >> + /* Start/Stop required only for master */ >> + iicstat &= ~S3C2410_IICSTAT_START; >> + } >> + } >> + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > I don't see i2c controller for HDMIPHY working in slave mode. So do we need > to check if its master and proceed? Cant the quirks check enough for it? > Even if it is configured as slave, there is no error indication for this > here. My understanding is that the HDMIPHY always commands and hence, HDMIPHY does not need to be slave at all. But if you think it is a policy decision which need not be implemented in driver, I agree with you. > >> } >> - writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> >> i2c->state = STATE_STOP; >> >> @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> */ >> >> if (iicstat & S3C2410_IICSTAT_LASTBIT && >> - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> + is_master(i2c)) { >> /* ack was not received... */ >> >> dev_dbg(i2c->dev, "ack was not received\n"); >> @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> * end of the message, and if so, work out what to do >> */ >> >> - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> is_master(i2c)) { >> if (iicstat & S3C2410_IICSTAT_LASTBIT) { >> dev_dbg(i2c->dev, "WRITE: No Ack\n"); >> >> @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> >> } else { >> /* send stop */ >> - >> s3c24xx_i2c_stop(i2c, 0); >> } >> break; >> @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> i2c->msg->buf[i2c->msg_ptr++] = byte; >> >> prepare_read: >> - if (is_msglast(i2c)) { >> + if (is_msglast(i2c) && is_master(i2c)) { >> /* last byte of buffer */ >> >> if (is_lastmsg(i2c)) >> @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c >> *i2c, >> if (i2c->suspended) >> return -EIO; >> >> - ret = s3c24xx_i2c_set_master(i2c); >> - if (ret != 0) { >> - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); >> - ret = -EAGAIN; >> - goto out; >> + if (is_master(i2c)) { >> + ret = s3c24xx_i2c_set_master(i2c); >> + if (ret != 0) { >> + dev_err(i2c->dev, "cannot get bus (error %d)\n", >> ret); >> + ret = -EAGAIN; >> + goto out; >> + } >> } >> >> i2c->msg = msgs; >> @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c >> *i2c, >> s3c24xx_i2c_enable_irq(i2c); >> s3c24xx_i2c_message_start(i2c, msgs); >> >> - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * >> 5); >> + if (is_master(i2c)) >> + timeout = wait_event_timeout(i2c->wait,\ >> + i2c->msg_num == 0, HZ * 5); >> + else >> + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); >> >> ret = i2c->msg_idx; >> >> /* having these next two as dev_err() makes life very >> * noisy when doing an i2cdetect */ >> >> - if (timeout == 0) >> - dev_dbg(i2c->dev, "timeout\n"); >> - else if (ret != num) >> - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> + if (is_master(i2c)) { >> + if (timeout == 0) >> + dev_dbg(i2c->dev, "timeout\n"); >> + else if (ret != num) >> + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> >> - /* For QUIRK_HDMIPHY, bus is already disabled */ >> - if (i2c->quirks & QUIRK_HDMIPHY) >> - goto out; >> + /* For QUIRK_HDMIPHY, bus is already disabled */ >> + if (i2c->quirks & QUIRK_HDMIPHY) >> + goto out; >> >> - s3c24xx_i2c_wait_idle(i2c); >> + s3c24xx_i2c_wait_idle(i2c); >> + } >> >> out: >> return ret; >> @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct >> s3c24xx_i2c *i2c) >> of_property_read_u32(np, "samsung,i2c-slave-addr", >> &pdata->slave_addr); >> of_property_read_u32(np, "samsung,i2c-max-bus-freq", >> (u32 *)&pdata->frequency); >> + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); >> } >> #else >> static void >> @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct >> platform_device *pdev) >> goto err_noclk; >> } >> >> + /* By default, i2c works in master mode */ >> + /* This currently will be updated using DT */ >> + i2c->i2c_mode = 0; >> + >> i2c->quirks = s3c24xx_get_device_quirks(pdev); >> if (pdata) >> memcpy(i2c->pdata, pdata, sizeof(*pdata)); >> @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct >> platform_device *pdev) >> i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; >> i2c->tx_setup = 50; >> >> + >> init_waitqueue_head(&i2c->wait); >> >> /* find the clock and enable it */ >> -- >> 1.7.9.5 >> >> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > Regards, > Subash ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-07 11:36 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-07 11:36 UTC (permalink / raw) To: linux-arm-kernel Thanks for the review Subash. Please find my reply below. On 6 December 2012 23:05, Subash Patel <subashrp@gmail.com> wrote: > Hi Giridhar, > > > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> >> This patch adds slave support to i2c. The dt entry i2c-mode >> decides at probe time if the controller needs to work in >> slave mode and the controller is accordingly programmed. >> >> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org >> <mailto:giridhar.maruthy@linaro.org>> >> >> --- >> drivers/i2c/busses/i2c-s3c2410.c | 100 >> ++++++++++++++++++++++++++------------ >> 1 file changed, 68 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-s3c2410.c >> b/drivers/i2c/busses/i2c-s3c2410.c >> index e93e7d6..d83a6d7 100644 >> --- a/drivers/i2c/busses/i2c-s3c2410.c >> +++ b/drivers/i2c/busses/i2c-s3c2410.c >> @@ -53,6 +53,9 @@ >> /* Max time to wait for bus to become idle after a xfer (in us) */ >> #define S3C2410_IDLE_TIMEOUT 5000 >> >> +/* To find the master/slave mode of current controller */ >> +#define is_master(i2c) (!i2c->i2c_mode) >> + >> /* i2c controller state */ >> enum s3c24xx_i2c_state { >> STATE_IDLE, >> @@ -89,6 +92,8 @@ struct s3c24xx_i2c { >> #ifdef CONFIG_CPU_FREQ >> struct notifier_block freq_transition; >> #endif >> + /* i2c_mode: 0 is for master; and 1 is for slave */ >> + unsigned int i2c_mode; >> }; >> >> static struct platform_device_id s3c24xx_driver_ids[] = { >> @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> stat = 0; >> stat |= S3C2410_IICSTAT_TXRXEN; >> >> - if (msg->flags & I2C_M_RD) { >> - stat |= S3C2410_IICSTAT_MASTER_RX; >> - addr |= 1; >> - } else >> - stat |= S3C2410_IICSTAT_MASTER_TX; >> + if (is_master(i2c)) { >> + /* Master mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_MASTER_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_MASTER_TX; >> + } else { >> + /* Slave mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_SLAVE_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_SLAVE_TX; >> + } >> >> if (msg->flags & I2C_M_REV_DIR_ADDR) >> addr ^= 1; >> @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); >> writel(iiccon, i2c->regs + S3C2410_IICCON); >> >> - stat |= S3C2410_IICSTAT_START; >> - writel(stat, i2c->regs + S3C2410_IICSTAT); >> + if (is_master(i2c)) { >> + stat |= S3C2410_IICSTAT_START; >> + writel(stat, i2c->regs + S3C2410_IICSTAT); >> + } >> } >> >> static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) >> @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct >> s3c24xx_i2c *i2c, int ret) >> * devices, the host as Master and the HDMIPHY device as the >> slave. >> * Skipping the STOP condition has been tested on this bus and >> works. >> */ >> - if (i2c->quirks & QUIRK_HDMIPHY) { >> - /* Stop driving the I2C pins */ >> - iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> - } else { >> - /* stop the transfer */ >> - iicstat &= ~S3C2410_IICSTAT_START; >> + if (is_master(i2c)) { >> + if (i2c->quirks & QUIRK_HDMIPHY) { >> + /* Stop driving the I2C pins */ >> + iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> + } else { >> + /* stop the transfer */ >> + if (is_master(i2c)) { >> + /* Start/Stop required only for master */ >> + iicstat &= ~S3C2410_IICSTAT_START; >> + } >> + } >> + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > > I don't see i2c controller for HDMIPHY working in slave mode. So do we need > to check if its master and proceed? Cant the quirks check enough for it? > Even if it is configured as slave, there is no error indication for this > here. My understanding is that the HDMIPHY always commands and hence, HDMIPHY does not need to be slave at all. But if you think it is a policy decision which need not be implemented in driver, I agree with you. > >> } >> - writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> >> i2c->state = STATE_STOP; >> >> @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> */ >> >> if (iicstat & S3C2410_IICSTAT_LASTBIT && >> - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> + is_master(i2c)) { >> /* ack was not received... */ >> >> dev_dbg(i2c->dev, "ack was not received\n"); >> @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> * end of the message, and if so, work out what to do >> */ >> >> - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> is_master(i2c)) { >> if (iicstat & S3C2410_IICSTAT_LASTBIT) { >> dev_dbg(i2c->dev, "WRITE: No Ack\n"); >> >> @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> >> } else { >> /* send stop */ >> - >> s3c24xx_i2c_stop(i2c, 0); >> } >> break; >> @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> i2c->msg->buf[i2c->msg_ptr++] = byte; >> >> prepare_read: >> - if (is_msglast(i2c)) { >> + if (is_msglast(i2c) && is_master(i2c)) { >> /* last byte of buffer */ >> >> if (is_lastmsg(i2c)) >> @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c >> *i2c, >> if (i2c->suspended) >> return -EIO; >> >> - ret = s3c24xx_i2c_set_master(i2c); >> - if (ret != 0) { >> - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); >> - ret = -EAGAIN; >> - goto out; >> + if (is_master(i2c)) { >> + ret = s3c24xx_i2c_set_master(i2c); >> + if (ret != 0) { >> + dev_err(i2c->dev, "cannot get bus (error %d)\n", >> ret); >> + ret = -EAGAIN; >> + goto out; >> + } >> } >> >> i2c->msg = msgs; >> @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c >> *i2c, >> s3c24xx_i2c_enable_irq(i2c); >> s3c24xx_i2c_message_start(i2c, msgs); >> >> - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * >> 5); >> + if (is_master(i2c)) >> + timeout = wait_event_timeout(i2c->wait,\ >> + i2c->msg_num == 0, HZ * 5); >> + else >> + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); >> >> ret = i2c->msg_idx; >> >> /* having these next two as dev_err() makes life very >> * noisy when doing an i2cdetect */ >> >> - if (timeout == 0) >> - dev_dbg(i2c->dev, "timeout\n"); >> - else if (ret != num) >> - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> + if (is_master(i2c)) { >> + if (timeout == 0) >> + dev_dbg(i2c->dev, "timeout\n"); >> + else if (ret != num) >> + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> >> - /* For QUIRK_HDMIPHY, bus is already disabled */ >> - if (i2c->quirks & QUIRK_HDMIPHY) >> - goto out; >> + /* For QUIRK_HDMIPHY, bus is already disabled */ >> + if (i2c->quirks & QUIRK_HDMIPHY) >> + goto out; >> >> - s3c24xx_i2c_wait_idle(i2c); >> + s3c24xx_i2c_wait_idle(i2c); >> + } >> >> out: >> return ret; >> @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct >> s3c24xx_i2c *i2c) >> of_property_read_u32(np, "samsung,i2c-slave-addr", >> &pdata->slave_addr); >> of_property_read_u32(np, "samsung,i2c-max-bus-freq", >> (u32 *)&pdata->frequency); >> + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); >> } >> #else >> static void >> @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct >> platform_device *pdev) >> goto err_noclk; >> } >> >> + /* By default, i2c works in master mode */ >> + /* This currently will be updated using DT */ >> + i2c->i2c_mode = 0; >> + >> i2c->quirks = s3c24xx_get_device_quirks(pdev); >> if (pdata) >> memcpy(i2c->pdata, pdata, sizeof(*pdata)); >> @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct >> platform_device *pdev) >> i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; >> i2c->tx_setup = 50; >> >> + >> init_waitqueue_head(&i2c->wait); >> >> /* find the clock and enable it */ >> -- >> 1.7.9.5 >> >> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > Regards, > Subash ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-03 12:16 ` Giridhar Maruthy @ 2012-12-07 12:03 ` Tushar Behera -1 siblings, 0 replies; 23+ messages in thread From: Tushar Behera @ 2012-12-07 12:03 UTC (permalink / raw) To: Giridhar Maruthy Cc: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { This is executed only if is_master(i2c) is true, so there seems no need to checking it again. > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Tushar Behera ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-07 12:03 ` Tushar Behera 0 siblings, 0 replies; 23+ messages in thread From: Tushar Behera @ 2012-12-07 12:03 UTC (permalink / raw) To: linux-arm-kernel On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > This patch adds slave support to i2c. The dt entry i2c-mode > decides at probe time if the controller needs to work in > slave mode and the controller is accordingly programmed. > > Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> > --- > drivers/i2c/busses/i2c-s3c2410.c | 100 > ++++++++++++++++++++++++++------------ > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index e93e7d6..d83a6d7 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -53,6 +53,9 @@ > /* Max time to wait for bus to become idle after a xfer (in us) */ > #define S3C2410_IDLE_TIMEOUT 5000 > > +/* To find the master/slave mode of current controller */ > +#define is_master(i2c) (!i2c->i2c_mode) > + > /* i2c controller state */ > enum s3c24xx_i2c_state { > STATE_IDLE, > @@ -89,6 +92,8 @@ struct s3c24xx_i2c { > #ifdef CONFIG_CPU_FREQ > struct notifier_block freq_transition; > #endif > + /* i2c_mode: 0 is for master; and 1 is for slave */ > + unsigned int i2c_mode; > }; > > static struct platform_device_id s3c24xx_driver_ids[] = { > @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > stat = 0; > stat |= S3C2410_IICSTAT_TXRXEN; > > - if (msg->flags & I2C_M_RD) { > - stat |= S3C2410_IICSTAT_MASTER_RX; > - addr |= 1; > - } else > - stat |= S3C2410_IICSTAT_MASTER_TX; > + if (is_master(i2c)) { > + /* Master mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_MASTER_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_MASTER_TX; > + } else { > + /* Slave mode */ > + if (msg->flags & I2C_M_RD) { > + stat |= S3C2410_IICSTAT_SLAVE_RX; > + addr |= 1; > + } else > + stat |= S3C2410_IICSTAT_SLAVE_TX; > + } > > if (msg->flags & I2C_M_REV_DIR_ADDR) > addr ^= 1; > @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); > writel(iiccon, i2c->regs + S3C2410_IICCON); > > - stat |= S3C2410_IICSTAT_START; > - writel(stat, i2c->regs + S3C2410_IICSTAT); > + if (is_master(i2c)) { > + stat |= S3C2410_IICSTAT_START; > + writel(stat, i2c->regs + S3C2410_IICSTAT); > + } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct > s3c24xx_i2c *i2c, int ret) > * devices, the host as Master and the HDMIPHY device as the slave. > * Skipping the STOP condition has been tested on this bus and > works. > */ > - if (i2c->quirks & QUIRK_HDMIPHY) { > - /* Stop driving the I2C pins */ > - iicstat &= ~S3C2410_IICSTAT_TXRXEN; > - } else { > - /* stop the transfer */ > - iicstat &= ~S3C2410_IICSTAT_START; > + if (is_master(i2c)) { > + if (i2c->quirks & QUIRK_HDMIPHY) { > + /* Stop driving the I2C pins */ > + iicstat &= ~S3C2410_IICSTAT_TXRXEN; > + } else { > + /* stop the transfer */ > + if (is_master(i2c)) { This is executed only if is_master(i2c) is true, so there seems no need to checking it again. > + /* Start/Stop required only for master */ > + iicstat &= ~S3C2410_IICSTAT_START; > + } > + } > + writel(iicstat, i2c->regs + S3C2410_IICSTAT); > } > - writel(iicstat, i2c->regs + S3C2410_IICSTAT); > > i2c->state = STATE_STOP; > > @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > */ > > if (iicstat & S3C2410_IICSTAT_LASTBIT && > - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && > + is_master(i2c)) { > /* ack was not received... */ > > dev_dbg(i2c->dev, "ack was not received\n"); > @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > * end of the message, and if so, work out what to do > */ > > - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { > + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && > is_master(i2c)) { > if (iicstat & S3C2410_IICSTAT_LASTBIT) { > dev_dbg(i2c->dev, "WRITE: No Ack\n"); > > @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > > } else { > /* send stop */ > - > s3c24xx_i2c_stop(i2c, 0); > } > break; > @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c > *i2c, unsigned long iicstat) > i2c->msg->buf[i2c->msg_ptr++] = byte; > > prepare_read: > - if (is_msglast(i2c)) { > + if (is_msglast(i2c) && is_master(i2c)) { > /* last byte of buffer */ > > if (is_lastmsg(i2c)) > @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > if (i2c->suspended) > return -EIO; > > - ret = s3c24xx_i2c_set_master(i2c); > - if (ret != 0) { > - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); > - ret = -EAGAIN; > - goto out; > + if (is_master(i2c)) { > + ret = s3c24xx_i2c_set_master(i2c); > + if (ret != 0) { > + dev_err(i2c->dev, "cannot get bus (error %d)\n", > ret); > + ret = -EAGAIN; > + goto out; > + } > } > > i2c->msg = msgs; > @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > s3c24xx_i2c_enable_irq(i2c); > s3c24xx_i2c_message_start(i2c, msgs); > > - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > + if (is_master(i2c)) > + timeout = wait_event_timeout(i2c->wait,\ > + i2c->msg_num == 0, HZ * 5); > + else > + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); > > ret = i2c->msg_idx; > > /* having these next two as dev_err() makes life very > * noisy when doing an i2cdetect */ > > - if (timeout == 0) > - dev_dbg(i2c->dev, "timeout\n"); > - else if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + if (is_master(i2c)) { > + if (timeout == 0) > + dev_dbg(i2c->dev, "timeout\n"); > + else if (ret != num) > + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > > - /* For QUIRK_HDMIPHY, bus is already disabled */ > - if (i2c->quirks & QUIRK_HDMIPHY) > - goto out; > + /* For QUIRK_HDMIPHY, bus is already disabled */ > + if (i2c->quirks & QUIRK_HDMIPHY) > + goto out; > > - s3c24xx_i2c_wait_idle(i2c); > + s3c24xx_i2c_wait_idle(i2c); > + } > > out: > return ret; > @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct > s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", > &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); > } > #else > static void > @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > goto err_noclk; > } > > + /* By default, i2c works in master mode */ > + /* This currently will be updated using DT */ > + i2c->i2c_mode = 0; > + > i2c->quirks = s3c24xx_get_device_quirks(pdev); > if (pdata) > memcpy(i2c->pdata, pdata, sizeof(*pdata)); > @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > i2c->tx_setup = 50; > > + > init_waitqueue_head(&i2c->wait); > > /* find the clock and enable it */ > -- > 1.7.9.5 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Tushar Behera ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-07 12:03 ` Tushar Behera @ 2012-12-07 12:33 ` Russell King - ARM Linux -1 siblings, 0 replies; 23+ messages in thread From: Russell King - ARM Linux @ 2012-12-07 12:33 UTC (permalink / raw) To: Tushar Behera Cc: Giridhar Maruthy, linux-samsung-soc, w.sang, linux-kernel, kgene.kim, linux-i2c, ben-linux, khali, linux-arm-kernel On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. (I don't have the original patches.) Hmm. How has slave-mode support been tested? Remembering that I2C slave devices do not initiate bus accesses, all accesses will be started by some other master. How are you dealing with the bytes received from the master, and how are you returning a response to the master in reply to a read request? We had support for this on PXA I2C through a callback from the driver into PXA code (used for the Psion Teklogix Netbook device) and it worked really well, but what you can't do is use the standard I2C interfaces for slave mode. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-07 12:33 ` Russell King - ARM Linux 0 siblings, 0 replies; 23+ messages in thread From: Russell King - ARM Linux @ 2012-12-07 12:33 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > > This patch adds slave support to i2c. The dt entry i2c-mode > > decides at probe time if the controller needs to work in > > slave mode and the controller is accordingly programmed. (I don't have the original patches.) Hmm. How has slave-mode support been tested? Remembering that I2C slave devices do not initiate bus accesses, all accesses will be started by some other master. How are you dealing with the bytes received from the master, and how are you returning a response to the master in reply to a read request? We had support for this on PXA I2C through a callback from the driver into PXA code (used for the Psion Teklogix Netbook device) and it worked really well, but what you can't do is use the standard I2C interfaces for slave mode. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-07 12:33 ` Russell King - ARM Linux @ 2012-12-10 9:32 ` Giridhar Maruthy -1 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-10 9:32 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Tushar Behera, linux-samsung-soc, w.sang, linux-kernel, kgene.kim, linux-i2c, ben-linux, khali, linux-arm-kernel Hi Russel, Thanks for review and please find my replies below. On 7 December 2012 18:03, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: >> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> > This patch adds slave support to i2c. The dt entry i2c-mode >> > decides at probe time if the controller needs to work in >> > slave mode and the controller is accordingly programmed. > > (I don't have the original patches.) > > Hmm. How has slave-mode support been tested? I have taken two I2C controllers in exynos5250. I configured one as slave and the other as master port. Then physically connected the pins of the two ports. > > Remembering that I2C slave devices do not initiate bus accesses, all > accesses will be started by some other master. How are you dealing > with the bytes received from the master, I run the slave read application in background. The resulting slave read will sleep till all bytes are received (wait_event_interruptible) once the master sends the data, an ack is given out by the slave controller in the ISR and the data is cached in the buffer( buffer sent by slave receive application). After all data is received, the read wakes up and the slave receive program gets the data. > and how are you returning a response to the master in reply to a read request? Similarl logic works in slave transmit mode (read request). Slave sleeps till the master initiates the transfer. > > We had support for this on PXA I2C through a callback from the driver > into PXA code (used for the Psion Teklogix Netbook device) and it worked > really well, but what you can't do is use the standard I2C interfaces > for slave mode. I have a question here. Since the same framework can work for both master and slave, is there any technical limitations I have overseen which prevents the slave mode to work? ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-10 9:32 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-10 9:32 UTC (permalink / raw) To: linux-arm-kernel Hi Russel, Thanks for review and please find my replies below. On 7 December 2012 18:03, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: >> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> > This patch adds slave support to i2c. The dt entry i2c-mode >> > decides at probe time if the controller needs to work in >> > slave mode and the controller is accordingly programmed. > > (I don't have the original patches.) > > Hmm. How has slave-mode support been tested? I have taken two I2C controllers in exynos5250. I configured one as slave and the other as master port. Then physically connected the pins of the two ports. > > Remembering that I2C slave devices do not initiate bus accesses, all > accesses will be started by some other master. How are you dealing > with the bytes received from the master, I run the slave read application in background. The resulting slave read will sleep till all bytes are received (wait_event_interruptible) once the master sends the data, an ack is given out by the slave controller in the ISR and the data is cached in the buffer( buffer sent by slave receive application). After all data is received, the read wakes up and the slave receive program gets the data. > and how are you returning a response to the master in reply to a read request? Similarl logic works in slave transmit mode (read request). Slave sleeps till the master initiates the transfer. > > We had support for this on PXA I2C through a callback from the driver > into PXA code (used for the Psion Teklogix Netbook device) and it worked > really well, but what you can't do is use the standard I2C interfaces > for slave mode. I have a question here. Since the same framework can work for both master and slave, is there any technical limitations I have overseen which prevents the slave mode to work? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-10 9:32 ` Giridhar Maruthy @ 2012-12-10 12:44 ` Russell King - ARM Linux -1 siblings, 0 replies; 23+ messages in thread From: Russell King - ARM Linux @ 2012-12-10 12:44 UTC (permalink / raw) To: Giridhar Maruthy Cc: Tushar Behera, linux-samsung-soc, w.sang, linux-kernel, kgene.kim, linux-i2c, ben-linux, khali, linux-arm-kernel On Mon, Dec 10, 2012 at 03:02:28PM +0530, Giridhar Maruthy wrote: > Hi Russel, > > Thanks for review and please find my replies below. > > On 7 December 2012 18:03, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: > >> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > >> > This patch adds slave support to i2c. The dt entry i2c-mode > >> > decides at probe time if the controller needs to work in > >> > slave mode and the controller is accordingly programmed. > > > > (I don't have the original patches.) > > > > Hmm. How has slave-mode support been tested? > > I have taken two I2C controllers in exynos5250. > I configured one as slave and the other as master port. > Then physically connected the pins of the two ports. > > > > Remembering that I2C slave devices do not initiate bus accesses, all > > accesses will be started by some other master. How are you dealing > > with the bytes received from the master, > > I run the slave read application in background. > The resulting slave read will sleep till all bytes are received > (wait_event_interruptible) Oh god no, not more broken interruptible waits in I2C code. I've seen already the results of crap like this with a kernel I2C peripheral driver falling over because the I2C layer returns -ERESTARTcrap with the Marvell I2C bus adapter. > once the master sends the data, an ack is given out by the slave controller > in the ISR and the data is cached in the buffer( > buffer sent by slave receive application). > After all data is received, the read wakes up and the > slave receive program gets the data. > > > and how are you returning a response to the master in reply to a read request? > > Similarl logic works in slave transmit mode (read request). Slave > sleeps till the master initiates the transfer. > > > > We had support for this on PXA I2C through a callback from the driver > > into PXA code (used for the Psion Teklogix Netbook device) and it worked > > really well, but what you can't do is use the standard I2C interfaces > > for slave mode. > > I have a question here. Since the same framework can work for both > master and slave, is there any technical limitations I have overseen > which prevents the slave mode to work? I don't really call your description above as "working" - it sounds like a bodge - it sounds like trying to bend a layer designed for master mode to sort-of-maybe-if-the-wind-is-in-the-right-direction-and-the-cows-are- all-standing-up-work. What if the application is trying to read in slave mode while the master is also trying to read? How do you deal with that? What about the converse? What if the application is trying to write data and the master also issues a write request? Finally, what about a multi-master situation? I can see no way for your implementation to have any hope of working there because there is no distinction between operating the interface in master mode and slave mode. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-10 12:44 ` Russell King - ARM Linux 0 siblings, 0 replies; 23+ messages in thread From: Russell King - ARM Linux @ 2012-12-10 12:44 UTC (permalink / raw) To: linux-arm-kernel On Mon, Dec 10, 2012 at 03:02:28PM +0530, Giridhar Maruthy wrote: > Hi Russel, > > Thanks for review and please find my replies below. > > On 7 December 2012 18:03, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Fri, Dec 07, 2012 at 05:33:17PM +0530, Tushar Behera wrote: > >> On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: > >> > This patch adds slave support to i2c. The dt entry i2c-mode > >> > decides at probe time if the controller needs to work in > >> > slave mode and the controller is accordingly programmed. > > > > (I don't have the original patches.) > > > > Hmm. How has slave-mode support been tested? > > I have taken two I2C controllers in exynos5250. > I configured one as slave and the other as master port. > Then physically connected the pins of the two ports. > > > > Remembering that I2C slave devices do not initiate bus accesses, all > > accesses will be started by some other master. How are you dealing > > with the bytes received from the master, > > I run the slave read application in background. > The resulting slave read will sleep till all bytes are received > (wait_event_interruptible) Oh god no, not more broken interruptible waits in I2C code. I've seen already the results of crap like this with a kernel I2C peripheral driver falling over because the I2C layer returns -ERESTARTcrap with the Marvell I2C bus adapter. > once the master sends the data, an ack is given out by the slave controller > in the ISR and the data is cached in the buffer( > buffer sent by slave receive application). > After all data is received, the read wakes up and the > slave receive program gets the data. > > > and how are you returning a response to the master in reply to a read request? > > Similarl logic works in slave transmit mode (read request). Slave > sleeps till the master initiates the transfer. > > > > We had support for this on PXA I2C through a callback from the driver > > into PXA code (used for the Psion Teklogix Netbook device) and it worked > > really well, but what you can't do is use the standard I2C interfaces > > for slave mode. > > I have a question here. Since the same framework can work for both > master and slave, is there any technical limitations I have overseen > which prevents the slave mode to work? I don't really call your description above as "working" - it sounds like a bodge - it sounds like trying to bend a layer designed for master mode to sort-of-maybe-if-the-wind-is-in-the-right-direction-and-the-cows-are- all-standing-up-work. What if the application is trying to read in slave mode while the master is also trying to read? How do you deal with that? What about the converse? What if the application is trying to write data and the master also issues a write request? Finally, what about a multi-master situation? I can see no way for your implementation to have any hope of working there because there is no distinction between operating the interface in master mode and slave mode. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] I2C: EXYNOS: Add slave support to i2c 2012-12-07 12:03 ` Tushar Behera @ 2012-12-10 8:10 ` Giridhar Maruthy -1 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-10 8:10 UTC (permalink / raw) To: Tushar Behera Cc: ben-linux, kgene.kim, khali, w.sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Thanks Tushar, On 7 December 2012 17:33, Tushar Behera <tushar.behera@linaro.org> wrote: > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> This patch adds slave support to i2c. The dt entry i2c-mode >> decides at probe time if the controller needs to work in >> slave mode and the controller is accordingly programmed. >> >> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> >> --- >> drivers/i2c/busses/i2c-s3c2410.c | 100 >> ++++++++++++++++++++++++++------------ >> 1 file changed, 68 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-s3c2410.c >> b/drivers/i2c/busses/i2c-s3c2410.c >> index e93e7d6..d83a6d7 100644 >> --- a/drivers/i2c/busses/i2c-s3c2410.c >> +++ b/drivers/i2c/busses/i2c-s3c2410.c >> @@ -53,6 +53,9 @@ >> /* Max time to wait for bus to become idle after a xfer (in us) */ >> #define S3C2410_IDLE_TIMEOUT 5000 >> >> +/* To find the master/slave mode of current controller */ >> +#define is_master(i2c) (!i2c->i2c_mode) >> + >> /* i2c controller state */ >> enum s3c24xx_i2c_state { >> STATE_IDLE, >> @@ -89,6 +92,8 @@ struct s3c24xx_i2c { >> #ifdef CONFIG_CPU_FREQ >> struct notifier_block freq_transition; >> #endif >> + /* i2c_mode: 0 is for master; and 1 is for slave */ >> + unsigned int i2c_mode; >> }; >> >> static struct platform_device_id s3c24xx_driver_ids[] = { >> @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> stat = 0; >> stat |= S3C2410_IICSTAT_TXRXEN; >> >> - if (msg->flags & I2C_M_RD) { >> - stat |= S3C2410_IICSTAT_MASTER_RX; >> - addr |= 1; >> - } else >> - stat |= S3C2410_IICSTAT_MASTER_TX; >> + if (is_master(i2c)) { >> + /* Master mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_MASTER_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_MASTER_TX; >> + } else { >> + /* Slave mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_SLAVE_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_SLAVE_TX; >> + } >> >> if (msg->flags & I2C_M_REV_DIR_ADDR) >> addr ^= 1; >> @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); >> writel(iiccon, i2c->regs + S3C2410_IICCON); >> >> - stat |= S3C2410_IICSTAT_START; >> - writel(stat, i2c->regs + S3C2410_IICSTAT); >> + if (is_master(i2c)) { >> + stat |= S3C2410_IICSTAT_START; >> + writel(stat, i2c->regs + S3C2410_IICSTAT); >> + } >> } >> >> static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) >> @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct >> s3c24xx_i2c *i2c, int ret) >> * devices, the host as Master and the HDMIPHY device as the slave. >> * Skipping the STOP condition has been tested on this bus and >> works. >> */ >> - if (i2c->quirks & QUIRK_HDMIPHY) { >> - /* Stop driving the I2C pins */ >> - iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> - } else { >> - /* stop the transfer */ >> - iicstat &= ~S3C2410_IICSTAT_START; >> + if (is_master(i2c)) { >> + if (i2c->quirks & QUIRK_HDMIPHY) { >> + /* Stop driving the I2C pins */ >> + iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> + } else { >> + /* stop the transfer */ >> + if (is_master(i2c)) { > > This is executed only if is_master(i2c) is true, so there seems no need > to checking it again. > Agreed, I will correct this. >> + /* Start/Stop required only for master */ >> + iicstat &= ~S3C2410_IICSTAT_START; >> + } >> + } >> + writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> } >> - writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> >> i2c->state = STATE_STOP; >> >> @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> */ >> >> if (iicstat & S3C2410_IICSTAT_LASTBIT && >> - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> + is_master(i2c)) { >> /* ack was not received... */ >> >> dev_dbg(i2c->dev, "ack was not received\n"); >> @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> * end of the message, and if so, work out what to do >> */ >> >> - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> is_master(i2c)) { >> if (iicstat & S3C2410_IICSTAT_LASTBIT) { >> dev_dbg(i2c->dev, "WRITE: No Ack\n"); >> >> @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> >> } else { >> /* send stop */ >> - >> s3c24xx_i2c_stop(i2c, 0); >> } >> break; >> @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> i2c->msg->buf[i2c->msg_ptr++] = byte; >> >> prepare_read: >> - if (is_msglast(i2c)) { >> + if (is_msglast(i2c) && is_master(i2c)) { >> /* last byte of buffer */ >> >> if (is_lastmsg(i2c)) >> @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, >> if (i2c->suspended) >> return -EIO; >> >> - ret = s3c24xx_i2c_set_master(i2c); >> - if (ret != 0) { >> - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); >> - ret = -EAGAIN; >> - goto out; >> + if (is_master(i2c)) { >> + ret = s3c24xx_i2c_set_master(i2c); >> + if (ret != 0) { >> + dev_err(i2c->dev, "cannot get bus (error %d)\n", >> ret); >> + ret = -EAGAIN; >> + goto out; >> + } >> } >> >> i2c->msg = msgs; >> @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, >> s3c24xx_i2c_enable_irq(i2c); >> s3c24xx_i2c_message_start(i2c, msgs); >> >> - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); >> + if (is_master(i2c)) >> + timeout = wait_event_timeout(i2c->wait,\ >> + i2c->msg_num == 0, HZ * 5); >> + else >> + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); >> >> ret = i2c->msg_idx; >> >> /* having these next two as dev_err() makes life very >> * noisy when doing an i2cdetect */ >> >> - if (timeout == 0) >> - dev_dbg(i2c->dev, "timeout\n"); >> - else if (ret != num) >> - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> + if (is_master(i2c)) { >> + if (timeout == 0) >> + dev_dbg(i2c->dev, "timeout\n"); >> + else if (ret != num) >> + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> >> - /* For QUIRK_HDMIPHY, bus is already disabled */ >> - if (i2c->quirks & QUIRK_HDMIPHY) >> - goto out; >> + /* For QUIRK_HDMIPHY, bus is already disabled */ >> + if (i2c->quirks & QUIRK_HDMIPHY) >> + goto out; >> >> - s3c24xx_i2c_wait_idle(i2c); >> + s3c24xx_i2c_wait_idle(i2c); >> + } >> >> out: >> return ret; >> @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct >> s3c24xx_i2c *i2c) >> of_property_read_u32(np, "samsung,i2c-slave-addr", >> &pdata->slave_addr); >> of_property_read_u32(np, "samsung,i2c-max-bus-freq", >> (u32 *)&pdata->frequency); >> + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); >> } >> #else >> static void >> @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device >> *pdev) >> goto err_noclk; >> } >> >> + /* By default, i2c works in master mode */ >> + /* This currently will be updated using DT */ >> + i2c->i2c_mode = 0; >> + >> i2c->quirks = s3c24xx_get_device_quirks(pdev); >> if (pdata) >> memcpy(i2c->pdata, pdata, sizeof(*pdata)); >> @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device >> *pdev) >> i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; >> i2c->tx_setup = 50; >> >> + >> init_waitqueue_head(&i2c->wait); >> >> /* find the clock and enable it */ >> -- >> 1.7.9.5 >> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > > > -- > Tushar Behera ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] I2C: EXYNOS: Add slave support to i2c @ 2012-12-10 8:10 ` Giridhar Maruthy 0 siblings, 0 replies; 23+ messages in thread From: Giridhar Maruthy @ 2012-12-10 8:10 UTC (permalink / raw) To: linux-arm-kernel Thanks Tushar, On 7 December 2012 17:33, Tushar Behera <tushar.behera@linaro.org> wrote: > On 12/03/2012 05:46 PM, Giridhar Maruthy wrote: >> This patch adds slave support to i2c. The dt entry i2c-mode >> decides at probe time if the controller needs to work in >> slave mode and the controller is accordingly programmed. >> >> Signed-off-by: Giridhar Maruthy <giridhar.maruthy@linaro.org> >> --- >> drivers/i2c/busses/i2c-s3c2410.c | 100 >> ++++++++++++++++++++++++++------------ >> 1 file changed, 68 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-s3c2410.c >> b/drivers/i2c/busses/i2c-s3c2410.c >> index e93e7d6..d83a6d7 100644 >> --- a/drivers/i2c/busses/i2c-s3c2410.c >> +++ b/drivers/i2c/busses/i2c-s3c2410.c >> @@ -53,6 +53,9 @@ >> /* Max time to wait for bus to become idle after a xfer (in us) */ >> #define S3C2410_IDLE_TIMEOUT 5000 >> >> +/* To find the master/slave mode of current controller */ >> +#define is_master(i2c) (!i2c->i2c_mode) >> + >> /* i2c controller state */ >> enum s3c24xx_i2c_state { >> STATE_IDLE, >> @@ -89,6 +92,8 @@ struct s3c24xx_i2c { >> #ifdef CONFIG_CPU_FREQ >> struct notifier_block freq_transition; >> #endif >> + /* i2c_mode: 0 is for master; and 1 is for slave */ >> + unsigned int i2c_mode; >> }; >> >> static struct platform_device_id s3c24xx_driver_ids[] = { >> @@ -202,11 +207,21 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> stat = 0; >> stat |= S3C2410_IICSTAT_TXRXEN; >> >> - if (msg->flags & I2C_M_RD) { >> - stat |= S3C2410_IICSTAT_MASTER_RX; >> - addr |= 1; >> - } else >> - stat |= S3C2410_IICSTAT_MASTER_TX; >> + if (is_master(i2c)) { >> + /* Master mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_MASTER_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_MASTER_TX; >> + } else { >> + /* Slave mode */ >> + if (msg->flags & I2C_M_RD) { >> + stat |= S3C2410_IICSTAT_SLAVE_RX; >> + addr |= 1; >> + } else >> + stat |= S3C2410_IICSTAT_SLAVE_TX; >> + } >> >> if (msg->flags & I2C_M_REV_DIR_ADDR) >> addr ^= 1; >> @@ -228,8 +243,10 @@ static void s3c24xx_i2c_message_start(struct >> s3c24xx_i2c *i2c, >> dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); >> writel(iiccon, i2c->regs + S3C2410_IICCON); >> >> - stat |= S3C2410_IICSTAT_START; >> - writel(stat, i2c->regs + S3C2410_IICSTAT); >> + if (is_master(i2c)) { >> + stat |= S3C2410_IICSTAT_START; >> + writel(stat, i2c->regs + S3C2410_IICSTAT); >> + } >> } >> >> static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) >> @@ -272,14 +289,19 @@ static inline void s3c24xx_i2c_stop(struct >> s3c24xx_i2c *i2c, int ret) >> * devices, the host as Master and the HDMIPHY device as the slave. >> * Skipping the STOP condition has been tested on this bus and >> works. >> */ >> - if (i2c->quirks & QUIRK_HDMIPHY) { >> - /* Stop driving the I2C pins */ >> - iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> - } else { >> - /* stop the transfer */ >> - iicstat &= ~S3C2410_IICSTAT_START; >> + if (is_master(i2c)) { >> + if (i2c->quirks & QUIRK_HDMIPHY) { >> + /* Stop driving the I2C pins */ >> + iicstat &= ~S3C2410_IICSTAT_TXRXEN; >> + } else { >> + /* stop the transfer */ >> + if (is_master(i2c)) { > > This is executed only if is_master(i2c) is true, so there seems no need > to checking it again. > Agreed, I will correct this. >> + /* Start/Stop required only for master */ >> + iicstat &= ~S3C2410_IICSTAT_START; >> + } >> + } >> + writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> } >> - writel(iicstat, i2c->regs + S3C2410_IICSTAT); >> >> i2c->state = STATE_STOP; >> >> @@ -348,7 +370,8 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> */ >> >> if (iicstat & S3C2410_IICSTAT_LASTBIT && >> - !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + !(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> + is_master(i2c)) { >> /* ack was not received... */ >> >> dev_dbg(i2c->dev, "ack was not received\n"); >> @@ -380,7 +403,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> * end of the message, and if so, work out what to do >> */ >> >> - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { >> + if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) && >> is_master(i2c)) { >> if (iicstat & S3C2410_IICSTAT_LASTBIT) { >> dev_dbg(i2c->dev, "WRITE: No Ack\n"); >> >> @@ -432,7 +455,6 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> >> } else { >> /* send stop */ >> - >> s3c24xx_i2c_stop(i2c, 0); >> } >> break; >> @@ -447,7 +469,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c >> *i2c, unsigned long iicstat) >> i2c->msg->buf[i2c->msg_ptr++] = byte; >> >> prepare_read: >> - if (is_msglast(i2c)) { >> + if (is_msglast(i2c) && is_master(i2c)) { >> /* last byte of buffer */ >> >> if (is_lastmsg(i2c)) >> @@ -612,11 +634,13 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, >> if (i2c->suspended) >> return -EIO; >> >> - ret = s3c24xx_i2c_set_master(i2c); >> - if (ret != 0) { >> - dev_err(i2c->dev, "cannot get bus (error %d)\n", ret); >> - ret = -EAGAIN; >> - goto out; >> + if (is_master(i2c)) { >> + ret = s3c24xx_i2c_set_master(i2c); >> + if (ret != 0) { >> + dev_err(i2c->dev, "cannot get bus (error %d)\n", >> ret); >> + ret = -EAGAIN; >> + goto out; >> + } >> } >> >> i2c->msg = msgs; >> @@ -628,23 +652,29 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, >> s3c24xx_i2c_enable_irq(i2c); >> s3c24xx_i2c_message_start(i2c, msgs); >> >> - timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); >> + if (is_master(i2c)) >> + timeout = wait_event_timeout(i2c->wait,\ >> + i2c->msg_num == 0, HZ * 5); >> + else >> + wait_event_interruptible(i2c->wait, i2c->msg_num == 0); >> >> ret = i2c->msg_idx; >> >> /* having these next two as dev_err() makes life very >> * noisy when doing an i2cdetect */ >> >> - if (timeout == 0) >> - dev_dbg(i2c->dev, "timeout\n"); >> - else if (ret != num) >> - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> + if (is_master(i2c)) { >> + if (timeout == 0) >> + dev_dbg(i2c->dev, "timeout\n"); >> + else if (ret != num) >> + dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); >> >> - /* For QUIRK_HDMIPHY, bus is already disabled */ >> - if (i2c->quirks & QUIRK_HDMIPHY) >> - goto out; >> + /* For QUIRK_HDMIPHY, bus is already disabled */ >> + if (i2c->quirks & QUIRK_HDMIPHY) >> + goto out; >> >> - s3c24xx_i2c_wait_idle(i2c); >> + s3c24xx_i2c_wait_idle(i2c); >> + } >> >> out: >> return ret; >> @@ -963,6 +993,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct >> s3c24xx_i2c *i2c) >> of_property_read_u32(np, "samsung,i2c-slave-addr", >> &pdata->slave_addr); >> of_property_read_u32(np, "samsung,i2c-max-bus-freq", >> (u32 *)&pdata->frequency); >> + of_property_read_u32(np, "samsung,i2c-mode", &i2c->i2c_mode); >> } >> #else >> static void >> @@ -1004,6 +1035,10 @@ static int s3c24xx_i2c_probe(struct platform_device >> *pdev) >> goto err_noclk; >> } >> >> + /* By default, i2c works in master mode */ >> + /* This currently will be updated using DT */ >> + i2c->i2c_mode = 0; >> + >> i2c->quirks = s3c24xx_get_device_quirks(pdev); >> if (pdata) >> memcpy(i2c->pdata, pdata, sizeof(*pdata)); >> @@ -1017,6 +1052,7 @@ static int s3c24xx_i2c_probe(struct platform_device >> *pdev) >> i2c->adap.class = I2C_CLASS_HWMON | I2C_CLASS_SPD; >> i2c->tx_setup = 50; >> >> + >> init_waitqueue_head(&i2c->wait); >> >> /* find the clock and enable it */ >> -- >> 1.7.9.5 >> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > > > -- > Tushar Behera ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2012-12-10 12:44 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1354354961-28833-1-git-send-email-giridhar.maruthy@linaro.org>
2012-12-03 12:16 ` [PATCH] I2C: EXYNOS: Add slave support to i2c Giridhar Maruthy
2012-12-03 12:16 ` Giridhar Maruthy
2012-12-03 13:28 ` Kyungmin Park
2012-12-03 13:28 ` Kyungmin Park
2012-12-03 22:46 ` Giridhar Maruthy
2012-12-03 22:46 ` Giridhar Maruthy
2012-12-03 22:53 ` Giridhar Maruthy
2012-12-03 22:53 ` Giridhar Maruthy
[not found] ` <CAHuqX+EQ0pZnm27yAVqC=NRX8huQ5EvbC8PGn2G3xYw4D71gQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-12-06 14:05 ` Subash Patel
2012-12-06 14:05 ` Subash Patel
2012-12-06 14:05 ` Subash Patel
2012-12-07 11:36 ` Giridhar Maruthy
2012-12-07 11:36 ` Giridhar Maruthy
2012-12-07 12:03 ` Tushar Behera
2012-12-07 12:03 ` Tushar Behera
2012-12-07 12:33 ` Russell King - ARM Linux
2012-12-07 12:33 ` Russell King - ARM Linux
2012-12-10 9:32 ` Giridhar Maruthy
2012-12-10 9:32 ` Giridhar Maruthy
2012-12-10 12:44 ` Russell King - ARM Linux
2012-12-10 12:44 ` Russell King - ARM Linux
2012-12-10 8:10 ` Giridhar Maruthy
2012-12-10 8:10 ` Giridhar Maruthy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.