Em Sun, 05 Jan 2014 21:38:31 +0100 Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: > Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab: > > In the lack of a better spec, let's assume the timeout > > values compatible with SMBus spec: > > http://smbus.org/specs/smbus110.pdf > > > > at chapter 8 - Electrical Characteristics of SMBus devices > > > > Ok, SMBus is a subset of I2C, and not all devices will be > > following it, but the timeout value before this patch was not > > even following the spec. > > > > So, while we don't have a better guess for it, use 35 + 1 > > ms as the timeout. > > > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@xxxxxxxxxxx> > > --- > > drivers/media/usb/em28xx/em28xx.h | 17 +++++++++++++++-- > > 1 file changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h > > index db47c2236ca4..9af19332b0f1 100644 > > --- a/drivers/media/usb/em28xx/em28xx.h > > +++ b/drivers/media/usb/em28xx/em28xx.h > > @@ -183,8 +183,21 @@ > > > > #define EM28XX_INTERLACED_DEFAULT 1 > > > > -/* time in msecs to wait for i2c xfers to finish */ > > -#define EM2800_I2C_XFER_TIMEOUT 20 > > +/* > > + * Time in msecs to wait for i2c xfers to finish. > > + * 35ms is the maximum time a SMBUS device could wait when > > + * clock stretching is used. As the transfer itself will take > > + * some time to happen, set it to 35 ms. > > + * > > + * Ok, I2C doesn't specify any limit. So, eventually, we may need > > + * to increase this timeout. > > + * > > + * FIXME: this assumes that an I2C message is not longer than 1ms. > > + * This is actually dependent on the I2C bus speed, although most > > + * devices use a 100kHz clock. So, this assumtion is true most of > > + * the time. > > + */ > > +#define EM2800_I2C_XFER_TIMEOUT 36 > > > > /* time in msecs to wait for AC97 xfers to finish */ > > #define EM2800_AC97_XFER_TIMEOUT 100 > Mauro... > What exactly are you fixing with this patch ? It fixes some of the timeouts I noticed here with HVR-950. > Which devices are not working with the current timeout value ? > > You really shouldn't increase the timout to 172% for all devices based > on such a fragile pure theory. It is not fragile. It is the SMBUS spec. It should _at_least_ wait up to the timeout specified there. Btw, it is not increasing the timeout. It is actually reducing it. See, this is the code before the patch: for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0; read_timeout -= 5) { ret = dev->em28xx_read_reg(dev, 0x05); if (ret == 0x84 + len - 1) { break; } else if (ret == 0x94 + len - 1) { return -ENODEV; } else if (ret < 0) { em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", ret); return ret; } msleep(5); } msleep(5) actually sleeps up to 20 ms, as the minimal time is the schedule() time - being 10 ms a typical value (CONFIG_HZ equal to 100). So, the current code has a timeout of up to 100 ms. This patch is actually reducing from 100 ms to 36 ms. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
