* [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy
@ 2013-03-25 11:46 Akshay Saraswat
2013-04-01 11:18 ` Minkyu Kang
2013-04-09 6:30 ` Hung-ying Tyan
0 siblings, 2 replies; 4+ messages in thread
From: Akshay Saraswat @ 2013-03-25 11:46 UTC (permalink / raw)
To: u-boot
From: Gabe Black <gabeblack@google.com>
The exynos s3c24x0 i2c driver wouldn't do the right thing when a NACK was
received on a read because it didn't attempt a read before waiting for the
read to finish. Putting a call to ReadWriteByte in the NACK path fixed a
problem where getting a NACK reading from a device would jam up the bus and
prevent future accesses like probing from working.
Because other than the ReadWriteByte call the NACK and normal paths were
almost the same thing, and to avoid future instances of the NACK path not
working because it's not exercised normally, this change also consolidates
those two paths.
Signed-off-by: Gabe Black <gabeblack@google.com>
Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
---
drivers/i2c/s3c24x0_i2c.c | 53 ++++++++++++++++-------------------------------
1 file changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index d2b4eb0..91298a7 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -366,21 +366,25 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
break;
case I2C_READ:
- if (result == I2C_OK) {
- writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
- writel(chip, &i2c->iicds);
- /* send START */
- writel(readl(&i2c->iicstat) | I2C_START_STOP,
- &i2c->iicstat);
- ReadWriteByte(i2c);
- result = WaitForXfer(i2c);
+ {
+ int was_ok = (result == I2C_OK);
+
+ writel(chip, &i2c->iicds);
+ /* resend START */
+ writel(I2C_MODE_MR | I2C_TXRX_ENA |
+ I2C_START_STOP, &i2c->iicstat);
+ ReadWriteByte(i2c);
+ result = WaitForXfer(i2c);
+
+ if (was_ok || 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);
+ if (i == data_len - 1) {
+ writel(readl(&i2c->iiccon) &
+ ~I2CCON_ACKGEN,
+ &i2c->iiccon);
+ }
ReadWriteByte(i2c);
result = WaitForXfer(i2c);
data[i] = readl(&i2c->iicds);
@@ -388,35 +392,14 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
}
} else {
- writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
- writel(chip, &i2c->iicds);
- /* send START */
- writel(readl(&i2c->iicstat) | I2C_START_STOP,
- &i2c->iicstat);
- 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;
- }
+ result = I2C_NACK;
}
/* send STOP */
writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
ReadWriteByte(i2c);
break;
+ }
default:
debug("i2c_transfer: bad call\n");
--
1.8.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy
2013-03-25 11:46 [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy Akshay Saraswat
@ 2013-04-01 11:18 ` Minkyu Kang
2013-04-09 6:30 ` Hung-ying Tyan
1 sibling, 0 replies; 4+ messages in thread
From: Minkyu Kang @ 2013-04-01 11:18 UTC (permalink / raw)
To: u-boot
On 25/03/13 20:46, Akshay Saraswat wrote:
> From: Gabe Black <gabeblack@google.com>
>
> The exynos s3c24x0 i2c driver wouldn't do the right thing when a NACK was
> received on a read because it didn't attempt a read before waiting for the
> read to finish. Putting a call to ReadWriteByte in the NACK path fixed a
> problem where getting a NACK reading from a device would jam up the bus and
> prevent future accesses like probing from working.
>
> Because other than the ReadWriteByte call the NACK and normal paths were
> almost the same thing, and to avoid future instances of the NACK path not
> working because it's not exercised normally, this change also consolidates
> those two paths.
>
> Signed-off-by: Gabe Black <gabeblack@google.com>
> Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
> ---
> drivers/i2c/s3c24x0_i2c.c | 53 ++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> index d2b4eb0..91298a7 100644
> --- a/drivers/i2c/s3c24x0_i2c.c
> +++ b/drivers/i2c/s3c24x0_i2c.c
> @@ -366,21 +366,25 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> break;
>
> case I2C_READ:
> - if (result == I2C_OK) {
> - writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> - writel(chip, &i2c->iicds);
> - /* send START */
> - writel(readl(&i2c->iicstat) | I2C_START_STOP,
> - &i2c->iicstat);
> - ReadWriteByte(i2c);
> - result = WaitForXfer(i2c);
> + {
> + int was_ok = (result == I2C_OK);
do you really need the was_ok?
If not, please remove it.
> +
> + writel(chip, &i2c->iicds);
> + /* resend START */
> + writel(I2C_MODE_MR | I2C_TXRX_ENA |
> + I2C_START_STOP, &i2c->iicstat);
> + ReadWriteByte(i2c);
> + result = WaitForXfer(i2c);
> +
> + if (was_ok || 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);
> + if (i == data_len - 1) {
> + writel(readl(&i2c->iiccon) &
> + ~I2CCON_ACKGEN,
> + &i2c->iiccon);
> + }
> ReadWriteByte(i2c);
> result = WaitForXfer(i2c);
> data[i] = readl(&i2c->iicds);
> @@ -388,35 +392,14 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> }
>
> } else {
> - writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> - writel(chip, &i2c->iicds);
> - /* send START */
> - writel(readl(&i2c->iicstat) | I2C_START_STOP,
> - &i2c->iicstat);
> - 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;
> - }
> + result = I2C_NACK;
> }
>
> /* send STOP */
> writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> ReadWriteByte(i2c);
> break;
> + }
>
> default:
> debug("i2c_transfer: bad call\n");
>
Thanks,
Minkyu Kang.
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy
2013-03-25 11:46 [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy Akshay Saraswat
2013-04-01 11:18 ` Minkyu Kang
@ 2013-04-09 6:30 ` Hung-ying Tyan
1 sibling, 0 replies; 4+ messages in thread
From: Hung-ying Tyan @ 2013-04-09 6:30 UTC (permalink / raw)
To: u-boot
On which branch is this patch based? It looks quite off from ToT.
On Mon, Mar 25, 2013 at 7:46 PM, Akshay Saraswat <akshay.s@samsung.com>wrote:
> From: Gabe Black <gabeblack@google.com>
>
> The exynos s3c24x0 i2c driver wouldn't do the right thing when a NACK was
> received on a read because it didn't attempt a read before waiting for the
> read to finish. Putting a call to ReadWriteByte in the NACK path fixed a
> problem where getting a NACK reading from a device would jam up the bus and
> prevent future accesses like probing from working.
>
> Because other than the ReadWriteByte call the NACK and normal paths were
> almost the same thing, and to avoid future instances of the NACK path not
> working because it's not exercised normally, this change also consolidates
> those two paths.
>
> Signed-off-by: Gabe Black <gabeblack@google.com>
> Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
> ---
> drivers/i2c/s3c24x0_i2c.c | 53
> ++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> index d2b4eb0..91298a7 100644
> --- a/drivers/i2c/s3c24x0_i2c.c
> +++ b/drivers/i2c/s3c24x0_i2c.c
> @@ -366,21 +366,25 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> break;
>
> case I2C_READ:
> - if (result == I2C_OK) {
> - writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> - writel(chip, &i2c->iicds);
> - /* send START */
> - writel(readl(&i2c->iicstat) | I2C_START_STOP,
> - &i2c->iicstat);
> - ReadWriteByte(i2c);
> - result = WaitForXfer(i2c);
> + {
> + int was_ok = (result == I2C_OK);
> +
> + writel(chip, &i2c->iicds);
> + /* resend START */
> + writel(I2C_MODE_MR | I2C_TXRX_ENA |
> + I2C_START_STOP, &i2c->iicstat);
> + ReadWriteByte(i2c);
> + result = WaitForXfer(i2c);
> +
> + if (was_ok || 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);
> + if (i == data_len - 1) {
> + writel(readl(&i2c->iiccon) &
> + ~I2CCON_ACKGEN,
> + &i2c->iiccon);
> + }
> ReadWriteByte(i2c);
> result = WaitForXfer(i2c);
> data[i] = readl(&i2c->iicds);
> @@ -388,35 +392,14 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> }
>
> } else {
> - writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> - writel(chip, &i2c->iicds);
> - /* send START */
> - writel(readl(&i2c->iicstat) | I2C_START_STOP,
> - &i2c->iicstat);
> - 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;
> - }
> + result = I2C_NACK;
> }
>
> /* send STOP */
> writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> ReadWriteByte(i2c);
> break;
> + }
>
> default:
> debug("i2c_transfer: bad call\n");
> --
> 1.8.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy
@ 2013-04-09 8:08 Akshay Saraswat
0 siblings, 0 replies; 4+ messages in thread
From: Akshay Saraswat @ 2013-04-09 8:08 UTC (permalink / raw)
To: u-boot
Hi Hung-ying Tyan,
>On which branch is this patch based? It looks quite off from ToT.
>
I rebased this patch on u-boot-samsung and posted. But it's been quite some
time since I posted it, so I am not sure if it could be applied straightway now.
>
>On Mon, Mar 25, 2013 at 7:46 PM, Akshay Saraswat <akshay.s@samsung.com> wrote:
>
>From: Gabe Black <gabeblack@google.com>
>
>The exynos s3c24x0 i2c driver wouldn't do the right thing when a NACK was
>received on a read because it didn't attempt a read before waiting for the
>read to finish. Putting a call to ReadWriteByte in the NACK path fixed a
>problem where getting a NACK reading from a device would jam up the bus and
>prevent future accesses like probing from working.
>
>Because other than the ReadWriteByte call the NACK and normal paths were
>almost the same thing, and to avoid future instances of the NACK path not
>working because it's not exercised normally, this change also consolidates
>those two paths.
>
>Signed-off-by: Gabe Black <gabeblack@google.com>
>Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
>---
> drivers/i2c/s3c24x0_i2c.c | 53 ++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
>index d2b4eb0..91298a7 100644
>--- a/drivers/i2c/s3c24x0_i2c.c
>+++ b/drivers/i2c/s3c24x0_i2c.c
>@@ -366,21 +366,25 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> break;
>
> case I2C_READ:
>- if (result == I2C_OK) {
>- writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
>- writel(chip, &i2c->iicds);
>- /* send START */
>- writel(readl(&i2c->iicstat) | I2C_START_STOP,
>- &i2c->iicstat);
>- ReadWriteByte(i2c);
>- result = WaitForXfer(i2c);
>+ {
>+ int was_ok = (result == I2C_OK);
>+
>+ writel(chip, &i2c->iicds);
>+ /* resend START */
>+ writel(I2C_MODE_MR | I2C_TXRX_ENA |
>+ I2C_START_STOP, &i2c->iicstat);
>+ ReadWriteByte(i2c);
>+ result = WaitForXfer(i2c);
>+
>+ if (was_ok || 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);
>+ if (i == data_len - 1) {
>+ writel(readl(&i2c->iiccon) &
>+ ~I2CCON_ACKGEN,
>+ &i2c->iiccon);
>+ }
> ReadWriteByte(i2c);
> result = WaitForXfer(i2c);
> data[i] = readl(&i2c->iicds);
>@@ -388,35 +392,14 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
> }
>
> } else {
>- writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
>- writel(chip, &i2c->iicds);
>- /* send START */
>- writel(readl(&i2c->iicstat) | I2C_START_STOP,
>- &i2c->iicstat);
>- 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;
>- }
>+ result = I2C_NACK;
> }
>
> /* send STOP */
> writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> ReadWriteByte(i2c);
> break;
>+ }
>
> default:
> debug("i2c_transfer: bad call\n");
>--
>1.8.0
>
>
Regards,
Akshay Saraswat
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-09 8:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-25 11:46 [U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy Akshay Saraswat
2013-04-01 11:18 ` Minkyu Kang
2013-04-09 6:30 ` Hung-ying Tyan
-- strict thread matches above, loose matches on Subject: below --
2013-04-09 8:08 Akshay Saraswat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox