Salut Olivier, On Fri, 20 Jan 2012 12:46:54 +0100, Olivier Sobrie wrote: > Generally it is not needed to wait for 1 msec, the SMBus get often ready > in less than 200 usecs.
The code change looks OK but the patch description not really. The loop you're changing is waiting for command completion, it isn't checking for bus business, regardless of what the comment in the code says. What about: i2c-isch: Decrease delay in command completion check loop If this is OK with you I'll apply your patch with this description. > msleep(1) can wait up to 20 msecs... It has a significant impact when > there is a burst of transactions on the bus. To be honest I didn't know about usleep_range(). Probably the same change could apply to a number of polled SMBus controller drivers, starting with i2c-i801. I'll give it a try... Of course it's nowhere as good as switching the drivers to be interrupt-driven. Did you check if you patch had any impact in terms of CPU load? I'm also curious what happens on systems without high resolution timer support, as apparently usleep_range() is implemented in terms of these. I admit I am surprised that usleep_range() is unconditionally available given that. > Signed-off-by: Olivier Sobrie <[email protected]> > --- > drivers/i2c/busses/i2c-isch.c | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c > index 6561d27..f90a605 100644 > --- a/drivers/i2c/busses/i2c-isch.c > +++ b/drivers/i2c/busses/i2c-isch.c > @@ -47,7 +47,7 @@ > #define SMBBLKDAT (0x20 + sch_smba) > > /* Other settings */ > -#define MAX_TIMEOUT 500 > +#define MAX_RETRIES 5000 > > /* I2C constants */ > #define SCH_QUICK 0x00 > @@ -68,7 +68,7 @@ static int sch_transaction(void) > { > int temp; > int result = 0; > - int timeout = 0; > + int retries = 0; > > dev_dbg(&sch_adapter.dev, "Transaction (pre): CNT=%02x, CMD=%02x, " > "ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb(SMBHSTCNT), > @@ -100,12 +100,12 @@ static int sch_transaction(void) > outb(inb(SMBHSTCNT) | 0x10, SMBHSTCNT); > > do { > - msleep(1); > + usleep_range(100, 200); > temp = inb(SMBHSTSTS) & 0x0f; > - } while ((temp & 0x08) && (timeout++ < MAX_TIMEOUT)); > + } while ((temp & 0x08) && (retries++ < MAX_RETRIES)); > > /* If the SMBus is still busy, we give up */ > - if (timeout > MAX_TIMEOUT) { > + if (retries > MAX_RETRIES) { > dev_err(&sch_adapter.dev, "SMBus Timeout!\n"); > result = -ETIMEDOUT; > } -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
