* [PATCH 1/3 v3] exynos: i2c: Fix i2c driver to handle NACKs properly
[not found] <1380799354-14460-1-git-send-email-ch.naveen@samsung.com>
@ 2013-10-15 5:12 ` Naveen Krishna Chatradhi
2013-10-15 6:06 ` Heiko Schocher
0 siblings, 1 reply; 3+ messages in thread
From: Naveen Krishna Chatradhi @ 2013-10-15 5:12 UTC (permalink / raw)
To: linux-i2c, linux-kernel, linux-samsung-soc, hs
Cc: khali, ben-linux, grant.likely, sjg, naveenkrishna.ch, d.mueller,
trini, mk7.kang, cpgs
The Exynos5 i2c driver does not handle NACKs properly. This change:
- fixes the NACK processing problem (do not continue transaction if
address cycle was NACKed)
- eliminates a fair amount of duplicate code
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Simon Glass <sjg@google.com>
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---
Changes since v1:
Fixed complilation warning in function i2c_init()
Changes since v2:
None
drivers/i2c/s3c24x0_i2c.c | 214 +++++++++++++++++++--------------------------
1 file changed, 90 insertions(+), 124 deletions(-)
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index cd09c78..c65360d 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -43,7 +43,7 @@
#define I2C_START_STOP 0x20 /* START / STOP */
#define I2C_TXRX_ENA 0x10 /* I2C Tx/Rx enable */
-#define I2C_TIMEOUT 1 /* 1 second */
+#define I2C_TIMEOUT_MS 1000 /* 1 second */
/*
@@ -84,22 +84,26 @@ static void SetI2CSCL(int x)
}
#endif
+/*
+ * Wait til the byte transfer is completed.
+ *
+ * @param i2c- pointer to the appropriate i2c register bank.
+ * @return I2C_OK, if transmission was ACKED
+ * I2C_NACK, if transmission was NACKED
+ * I2C_NOK_TIMEOUT, if transaction did not complete in I2C_TIMEOUT_MS
+ */
+
static int WaitForXfer(struct s3c24x0_i2c *i2c)
{
- int i;
+ ulong start_time = get_timer(0);
- i = I2C_TIMEOUT * 10000;
- while (!(readl(&i2c->iiccon) & I2CCON_IRPND) && (i > 0)) {
- udelay(100);
- i--;
- }
+ do {
+ if (readl(&i2c->iiccon) & I2CCON_IRPND)
+ return (readl(&i2c->iicstat) & I2CSTAT_NACK) ?
+ I2C_NACK : I2C_OK;
+ } while (get_timer(start_time) < I2C_TIMEOUT_MS);
- return (readl(&i2c->iiccon) & I2CCON_IRPND) ? I2C_OK : I2C_NOK_TOUT;
-}
-
-static int IsACK(struct s3c24x0_i2c *i2c)
-{
- return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
+ return I2C_NOK_TOUT;
}
static void ReadWriteByte(struct s3c24x0_i2c *i2c)
@@ -180,21 +184,27 @@ unsigned int i2c_get_bus_num(void)
void i2c_init(int speed, int slaveadd)
{
+ int i;
struct s3c24x0_i2c *i2c;
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
struct s3c24x0_gpio *gpio = s3c24x0_get_base_gpio();
#endif
- int i;
+ ulong start_time = get_timer(0);
/* By default i2c channel 0 is the current bus */
g_current_bus = 0;
i2c = get_base_i2c();
- /* wait for some time to give previous transfer a chance to finish */
- i = I2C_TIMEOUT * 1000;
- while ((readl(&i2c->iicstat) & I2CSTAT_BSY) && (i > 0)) {
- udelay(1000);
- i--;
+ /*
+ * In case the previous transfer is still going, wait to give it a
+ * chance to finish.
+ */
+ while (readl(&i2c->iicstat) & I2CSTAT_BSY) {
+ if (get_timer(start_time) > I2C_TIMEOUT_MS) {
+ printf("%s: I2C bus busy for %p\n", __func__,
+ &i2c->iicstat);
+ return;
+ }
}
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
@@ -260,7 +270,8 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
unsigned char data[],
unsigned short data_len)
{
- int i, result;
+ int i = 0, result;
+ ulong start_time = get_timer(0);
if (data == 0 || data_len == 0) {
/*Don't support data transfer of no length or to address 0 */
@@ -268,128 +279,78 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
return I2C_NOK;
}
- /* Check I2C bus idle */
- i = I2C_TIMEOUT * 1000;
- while ((readl(&i2c->iicstat) & I2CSTAT_BSY) && (i > 0)) {
- udelay(1000);
- i--;
+ while (readl(&i2c->iicstat) & I2CSTAT_BSY) {
+ if (get_timer(start_time) > I2C_TIMEOUT_MS)
+ return I2C_NOK_TOUT;
}
- if (readl(&i2c->iicstat) & I2CSTAT_BSY)
- return I2C_NOK_TOUT;
-
writel(readl(&i2c->iiccon) | I2CCON_ACKGEN, &i2c->iiccon);
- result = I2C_OK;
- switch (cmd_type) {
- case I2C_WRITE:
- if (addr && addr_len) {
- writel(chip, &i2c->iicds);
- /* send START */
- writel(I2C_MODE_MT | I2C_TXRX_ENA | I2C_START_STOP,
- &i2c->iicstat);
- i = 0;
- while ((i < addr_len) && (result == I2C_OK)) {
- result = WaitForXfer(i2c);
- writel(addr[i], &i2c->iicds);
- ReadWriteByte(i2c);
- i++;
- }
- i = 0;
- while ((i < data_len) && (result == I2C_OK)) {
- result = WaitForXfer(i2c);
- writel(data[i], &i2c->iicds);
- ReadWriteByte(i2c);
- i++;
- }
- } else {
- writel(chip, &i2c->iicds);
- /* send START */
- writel(I2C_MODE_MT | I2C_TXRX_ENA | I2C_START_STOP,
- &i2c->iicstat);
- i = 0;
- while ((i < data_len) && (result == I2C_OK)) {
- result = WaitForXfer(i2c);
- writel(data[i], &i2c->iicds);
- ReadWriteByte(i2c);
- i++;
- }
+ /* Get the slave chip address going */
+ writel(chip, &i2c->iicds);
+ if ((cmd_type == I2C_WRITE) || (addr && addr_len))
+ writel(I2C_MODE_MT | I2C_TXRX_ENA | I2C_START_STOP,
+ &i2c->iicstat);
+ else
+ writel(I2C_MODE_MR | I2C_TXRX_ENA | I2C_START_STOP,
+ &i2c->iicstat);
+
+ /* Wait for chip address to transmit. */
+ result = WaitForXfer(i2c);
+ if (result != I2C_OK)
+ goto bailout;
+
+ /* If register address needs to be transmitted - do it now. */
+ if (addr && addr_len) {
+ while ((i < addr_len) && (result == I2C_OK)) {
+ writel(addr[i++], &i2c->iicds);
+ ReadWriteByte(i2c);
+ result = WaitForXfer(i2c);
}
+ i = 0;
+ if (result != I2C_OK)
+ goto bailout;
+ }
- if (result == I2C_OK)
+ switch (cmd_type) {
+ case I2C_WRITE:
+ while ((i < data_len) && (result == I2C_OK)) {
+ writel(data[i++], &i2c->iicds);
+ ReadWriteByte(i2c);
result = WaitForXfer(i2c);
-
- /* send STOP */
- writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
- ReadWriteByte(i2c);
+ }
break;
case I2C_READ:
if (addr && addr_len) {
+ /*
+ * Register address has been sent, now send slave chip
+ * address again to start the actual read transaction.
+ */
writel(chip, &i2c->iicds);
- /* send START */
- writel(I2C_MODE_MT | I2C_TXRX_ENA | I2C_START_STOP,
- &i2c->iicstat);
- result = WaitForXfer(i2c);
- if (IsACK(i2c)) {
- i = 0;
- while ((i < addr_len) && (result == I2C_OK)) {
- writel(addr[i], &i2c->iicds);
- ReadWriteByte(i2c);
- result = WaitForXfer(i2c);
- i++;
- }
-
- writel(chip, &i2c->iicds);
- /* resend START */
- writel(I2C_MODE_MR | I2C_TXRX_ENA |
- I2C_START_STOP, &i2c->iicstat);
- ReadWriteByte(i2c);
- result = WaitForXfer(i2c);
- i = 0;
- while ((i < data_len) && (result == I2C_OK)) {
- /* disable ACK for final READ */
- if (i == data_len - 1)
- writel(readl(&i2c->iiccon)
- & ~I2CCON_ACKGEN,
- &i2c->iiccon);
- ReadWriteByte(i2c);
- result = WaitForXfer(i2c);
- data[i] = readl(&i2c->iicds);
- i++;
- }
- } else {
- result = I2C_NACK;
- }
-
- } else {
- writel(chip, &i2c->iicds);
- /* send START */
+
+ /* Generate a re-START. */
writel(I2C_MODE_MR | I2C_TXRX_ENA | I2C_START_STOP,
&i2c->iicstat);
+ ReadWriteByte(i2c);
result = WaitForXfer(i2c);
- if (IsACK(i2c)) {
- i = 0;
- while ((i < data_len) && (result == I2C_OK)) {
- /* disable ACK for final READ */
- if (i == data_len - 1)
- writel(readl(&i2c->iiccon) &
- ~I2CCON_ACKGEN,
- &i2c->iiccon);
- ReadWriteByte(i2c);
- result = WaitForXfer(i2c);
- data[i] = readl(&i2c->iicds);
- i++;
- }
- } else {
- result = I2C_NACK;
- }
+ if (result != I2C_OK)
+ goto bailout;
}
- /* send STOP */
- writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
- ReadWriteByte(i2c);
+ while ((i < data_len) && (result == I2C_OK)) {
+ /* disable ACK for final READ */
+ if (i == data_len - 1)
+ writel(readl(&i2c->iiccon)
+ & ~I2CCON_ACKGEN,
+ &i2c->iiccon);
+ ReadWriteByte(i2c);
+ result = WaitForXfer(i2c);
+ data[i++] = readl(&i2c->iicds);
+ }
+ if (result == I2C_NACK)
+ result = I2C_OK; /* Normal terminated read. */
break;
default:
@@ -398,6 +359,11 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
break;
}
+bailout:
+ /* Send STOP. */
+ writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
+ ReadWriteByte(i2c);
+
return result;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/3 v3] exynos: i2c: Fix i2c driver to handle NACKs properly
2013-10-15 5:12 ` [PATCH 1/3 v3] exynos: i2c: Fix i2c driver to handle NACKs properly Naveen Krishna Chatradhi
@ 2013-10-15 6:06 ` Heiko Schocher
2013-10-15 6:12 ` Naveen Krishna Ch
0 siblings, 1 reply; 3+ messages in thread
From: Heiko Schocher @ 2013-10-15 6:06 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-i2c, linux-kernel, linux-samsung-soc, khali, ben-linux,
grant.likely, sjg, naveenkrishna.ch, d.mueller, trini, mk7.kang,
cpgs
Hello Naveen,
Am 15.10.2013 07:12, schrieb Naveen Krishna Chatradhi:
> The Exynos5 i2c driver does not handle NACKs properly. This change:
>
> - fixes the NACK processing problem (do not continue transaction if
> address cycle was NACKed)
>
> - eliminates a fair amount of duplicate code
>
> Signed-off-by: Vadim Bendebury<vbendeb@chromium.org>
> Reviewed-by: Simon Glass<sjg@google.com>
> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
> ---
> Changes since v1:
> Fixed complilation warning in function i2c_init()
>
> Changes since v2:
> None
>
> drivers/i2c/s3c24x0_i2c.c | 214 +++++++++++++++++++--------------------------
> 1 file changed, 90 insertions(+), 124 deletions(-)
Hmm.. I think your patchset is for u-boot, or? But I could not find
the u-boot ml on cc... instead some linux ml ... if so, please resend
to the u-boot ml ...
(matches for all three patches from you)
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/3 v3] exynos: i2c: Fix i2c driver to handle NACKs properly
2013-10-15 6:06 ` Heiko Schocher
@ 2013-10-15 6:12 ` Naveen Krishna Ch
0 siblings, 0 replies; 3+ messages in thread
From: Naveen Krishna Ch @ 2013-10-15 6:12 UTC (permalink / raw)
To: hs
Cc: Naveen Krishna Chatradhi, linux-i2c, linux-kernel,
linux-samsung-soc, khali, Ben Dooks, grant.likely, sjg,
d.mueller, trini, Minkyu Kang, cpgs
On 15 October 2013 11:36, Heiko Schocher <hs@denx.de> wrote:
> Hello Naveen,
>
> Am 15.10.2013 07:12, schrieb Naveen Krishna Chatradhi:
>
>> The Exynos5 i2c driver does not handle NACKs properly. This change:
>>
>> - fixes the NACK processing problem (do not continue transaction if
>> address cycle was NACKed)
>>
>> - eliminates a fair amount of duplicate code
>>
>> Signed-off-by: Vadim Bendebury<vbendeb@chromium.org>
>> Reviewed-by: Simon Glass<sjg@google.com>
>> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
>> ---
>> Changes since v1:
>> Fixed complilation warning in function i2c_init()
>>
>> Changes since v2:
>> None
>>
>> drivers/i2c/s3c24x0_i2c.c | 214
>> +++++++++++++++++++--------------------------
>> 1 file changed, 90 insertions(+), 124 deletions(-)
>
>
> Hmm.. I think your patchset is for u-boot, or? But I could not find
> the u-boot ml on cc... instead some linux ml ... if so, please resend
> to the u-boot ml ...
>
> (matches for all three patches from you)
I just checked that u-boot mailing list is missing.
I've done the checks you mentioned in the other mail and reposted with
u-boot@lists.denx.de in --to
Thanks for your review.
>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
Shine bright,
(: Nav :)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-10-15 6:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1380799354-14460-1-git-send-email-ch.naveen@samsung.com>
2013-10-15 5:12 ` [PATCH 1/3 v3] exynos: i2c: Fix i2c driver to handle NACKs properly Naveen Krishna Chatradhi
2013-10-15 6:06 ` Heiko Schocher
2013-10-15 6:12 ` Naveen Krishna Ch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).