* [PATCH] i2c: test off by one in {piix4,vt596}_transaction()
@ 2009-12-27 14:49 Roel Kluin
[not found] ` <4B3773F2.3010703-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Roel Kluin @ 2009-12-27 14:49 UTC (permalink / raw)
To: Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Andrew Morton
With `while (timeout++ < MAX_TIMEOUT)' timeout reaches MAX_TIMEOUT + 1 after the loop
This is probably unlikely to produce a problem.
Signed-off-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/i2c/busses/i2c-piix4.c | 2 +-
drivers/i2c/busses/i2c-viapro.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 1e245e9..d8e0df0 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -329,7 +329,7 @@ static int piix4_transaction(void)
msleep(1);
/* If the SMBus is still busy, we give up */
- if (timeout >= MAX_TIMEOUT) {
+ if (timeout > MAX_TIMEOUT) {
dev_err(&piix4_adapter.dev, "SMBus Timeout!\n");
result = -ETIMEDOUT;
}
diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c
index e4b1543..8a2e0d5 100644
--- a/drivers/i2c/busses/i2c-viapro.c
+++ b/drivers/i2c/busses/i2c-viapro.c
@@ -168,7 +168,7 @@ static int vt596_transaction(u8 size)
} while ((temp & 0x01) && (timeout++ < MAX_TIMEOUT));
/* If the SMBus is still busy, we give up */
- if (timeout >= MAX_TIMEOUT) {
+ if (timeout > MAX_TIMEOUT) {
result = -ETIMEDOUT;
dev_err(&vt596_adapter.dev, "SMBus timeout!\n");
}
^ permalink raw reply related [flat|nested] 4+ messages in thread[parent not found: <4B3773F2.3010703-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] i2c: test off by one in {piix4,vt596}_transaction() [not found] ` <4B3773F2.3010703-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2010-01-05 16:55 ` Jean Delvare [not found] ` <20100105175511.39dc6c42-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Jean Delvare @ 2010-01-05 16:55 UTC (permalink / raw) To: Roel Kluin Cc: Ben Dooks (embedded platforms), linux-i2c-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, LKML On Sun, 27 Dec 2009 15:49:22 +0100, Roel Kluin wrote: > With `while (timeout++ < MAX_TIMEOUT)' timeout reaches MAX_TIMEOUT + 1 after the loop > This is probably unlikely to produce a problem. > > Signed-off-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > drivers/i2c/busses/i2c-piix4.c | 2 +- > drivers/i2c/busses/i2c-viapro.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index 1e245e9..d8e0df0 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -329,7 +329,7 @@ static int piix4_transaction(void) > msleep(1); > > /* If the SMBus is still busy, we give up */ > - if (timeout >= MAX_TIMEOUT) { > + if (timeout > MAX_TIMEOUT) { > dev_err(&piix4_adapter.dev, "SMBus Timeout!\n"); > result = -ETIMEDOUT; > } > diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c > index e4b1543..8a2e0d5 100644 > --- a/drivers/i2c/busses/i2c-viapro.c > +++ b/drivers/i2c/busses/i2c-viapro.c > @@ -168,7 +168,7 @@ static int vt596_transaction(u8 size) > } while ((temp & 0x01) && (timeout++ < MAX_TIMEOUT)); > > /* If the SMBus is still busy, we give up */ > - if (timeout >= MAX_TIMEOUT) { > + if (timeout > MAX_TIMEOUT) { > result = -ETIMEDOUT; > dev_err(&vt596_adapter.dev, "SMBus timeout!\n"); > } That's right... but I'd rather change the loops to use "++timeout" and leave the conditions as is (or maybe change it to "=="). I think it's easier to read that way. Would that be OK with you? -- Jean Delvare ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20100105175511.39dc6c42-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>]
* Re: [PATCH] i2c: test off by one in {piix4,vt596}_transaction() [not found] ` <20100105175511.39dc6c42-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> @ 2010-01-05 21:22 ` Roel Kluin [not found] ` <4B43AD8F.2030809-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Roel Kluin @ 2010-01-05 21:22 UTC (permalink / raw) To: Jean Delvare Cc: Ben Dooks (embedded platforms), linux-i2c-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, LKML With `while (timeout++ < MAX_TIMEOUT)' timeout reaches MAX_TIMEOUT + 1 after the loop This is probably unlikely to produce a problem. Signed-off-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- > That's right... but I'd rather change the loops to use "++timeout" and > leave the conditions as is (or maybe change it to "=="). I think it's > easier to read that way. Would that be OK with you? Ok, drivers/i2c/busses/i2c-piix4.c | 4 ++-- drivers/i2c/busses/i2c-viapro.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index 1e245e9..e56e4b6 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -324,12 +324,12 @@ static int piix4_transaction(void) else msleep(1); - while ((timeout++ < MAX_TIMEOUT) && + while ((++timeout < MAX_TIMEOUT) && ((temp = inb_p(SMBHSTSTS)) & 0x01)) msleep(1); /* If the SMBus is still busy, we give up */ - if (timeout >= MAX_TIMEOUT) { + if (timeout == MAX_TIMEOUT) { dev_err(&piix4_adapter.dev, "SMBus Timeout!\n"); result = -ETIMEDOUT; } diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c index e4b1543..a84a909 100644 --- a/drivers/i2c/busses/i2c-viapro.c +++ b/drivers/i2c/busses/i2c-viapro.c @@ -165,10 +165,10 @@ static int vt596_transaction(u8 size) do { msleep(1); temp = inb_p(SMBHSTSTS); - } while ((temp & 0x01) && (timeout++ < MAX_TIMEOUT)); + } while ((temp & 0x01) && (++timeout < MAX_TIMEOUT)); /* If the SMBus is still busy, we give up */ - if (timeout >= MAX_TIMEOUT) { + if (timeout == MAX_TIMEOUT) { result = -ETIMEDOUT; dev_err(&vt596_adapter.dev, "SMBus timeout!\n"); } ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <4B43AD8F.2030809-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] i2c: test off by one in {piix4,vt596}_transaction() [not found] ` <4B43AD8F.2030809-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2010-01-06 11:14 ` Jean Delvare 0 siblings, 0 replies; 4+ messages in thread From: Jean Delvare @ 2010-01-06 11:14 UTC (permalink / raw) To: Roel Kluin Cc: Ben Dooks (embedded platforms), linux-i2c-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, LKML On Tue, 05 Jan 2010 22:22:23 +0100, Roel Kluin wrote: > With `while (timeout++ < MAX_TIMEOUT)' timeout reaches MAX_TIMEOUT + 1 after the loop > This is probably unlikely to produce a problem. > > Signed-off-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > > That's right... but I'd rather change the loops to use "++timeout" and > > leave the conditions as is (or maybe change it to "=="). I think it's > > easier to read that way. Would that be OK with you? > > Ok, > > drivers/i2c/busses/i2c-piix4.c | 4 ++-- > drivers/i2c/busses/i2c-viapro.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index 1e245e9..e56e4b6 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -324,12 +324,12 @@ static int piix4_transaction(void) > else > msleep(1); > > - while ((timeout++ < MAX_TIMEOUT) && > + while ((++timeout < MAX_TIMEOUT) && > ((temp = inb_p(SMBHSTSTS)) & 0x01)) > msleep(1); > > /* If the SMBus is still busy, we give up */ > - if (timeout >= MAX_TIMEOUT) { > + if (timeout == MAX_TIMEOUT) { > dev_err(&piix4_adapter.dev, "SMBus Timeout!\n"); > result = -ETIMEDOUT; > } > diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c > index e4b1543..a84a909 100644 > --- a/drivers/i2c/busses/i2c-viapro.c > +++ b/drivers/i2c/busses/i2c-viapro.c > @@ -165,10 +165,10 @@ static int vt596_transaction(u8 size) > do { > msleep(1); > temp = inb_p(SMBHSTSTS); > - } while ((temp & 0x01) && (timeout++ < MAX_TIMEOUT)); > + } while ((temp & 0x01) && (++timeout < MAX_TIMEOUT)); > > /* If the SMBus is still busy, we give up */ > - if (timeout >= MAX_TIMEOUT) { > + if (timeout == MAX_TIMEOUT) { > result = -ETIMEDOUT; > dev_err(&vt596_adapter.dev, "SMBus timeout!\n"); > } Applied, thanks. -- Jean Delvare ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-01-06 11:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-27 14:49 [PATCH] i2c: test off by one in {piix4,vt596}_transaction() Roel Kluin
[not found] ` <4B3773F2.3010703-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-01-05 16:55 ` Jean Delvare
[not found] ` <20100105175511.39dc6c42-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2010-01-05 21:22 ` Roel Kluin
[not found] ` <4B43AD8F.2030809-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-01-06 11:14 ` Jean Delvare
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).