| Message ID | 1388832951-11195-9-git-send-email-m.chehab@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab: > The I2C wait completion/timeout logic currently assumes that > msleep(5) will wait exaclty 5 ms. This is not true at all, > as it depends on CONFIG_HZ. > > Convert it to use jiffies, in order to not wait for more time > than needed. > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > --- > drivers/media/usb/em28xx/em28xx-i2c.c | 65 ++++++++++++++++++----------------- > 1 file changed, 34 insertions(+), 31 deletions(-) > > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c > index 9e6a11d01858..9fa7ed51e5b1 100644 > --- a/drivers/media/usb/em28xx/em28xx-i2c.c > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c > @@ -26,6 +26,7 @@ > #include <linux/kernel.h> > #include <linux/usb.h> > #include <linux/i2c.h> > +#include <linux/jiffies.h> > > #include "em28xx.h" > #include "tuner-xc2028.h" > @@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]"); > */ > static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > { > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > int ret; > - int write_timeout; > u8 b2[6]; > > if (len < 1 || len > 4) > @@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > return (ret < 0) ? ret : -EIO; > } > /* wait for completion */ > - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; > - write_timeout -= 5) { > + while (time_is_after_jiffies(timeout)) { AFAIU, it must be time_is_before_jiffies(timeout). > ret = dev->em28xx_read_reg(dev, 0x05); > - if (ret == 0x80 + len - 1) { > + if (ret == 0x80 + len - 1) > return len; > - } else if (ret == 0x94 + len - 1) { > + if (ret == 0x94 + len - 1) { > em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); > return -ENODEV; > - } else if (ret < 0) { > + } > + if (ret < 0) { > em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > ret); > return ret; > @@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > */ > static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > { > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > u8 buf2[4]; > int ret; > - int read_timeout; > int i; > > if (len < 1 || len > 4) > @@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > } > > /* wait for completion */ > - for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0; > - read_timeout -= 5) { > + while (time_is_after_jiffies(timeout)) { The same here... > ret = dev->em28xx_read_reg(dev, 0x05); > - if (ret == 0x84 + len - 1) { > + if (ret == 0x84 + len - 1) > break; > - } else if (ret == 0x94 + len - 1) { > + if (ret == 0x94 + len - 1) { > em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); > return -ENODEV; > - } else if (ret < 0) { > + } > + if (ret < 0) { > em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > ret); > return ret; > @@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr) > static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > u16 len, int stop) > { > - int write_timeout, ret; > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > + int ret; > > if (len < 1 || len > 64) > return -EOPNOTSUPP; > @@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > } > } > > - /* Check success of the i2c operation */ > - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; > - write_timeout -= 5) { > + /* wait for completion */ > + while (time_is_after_jiffies(timeout)) { ... and here. > ret = dev->em28xx_read_reg(dev, 0x05); > - if (ret == 0) { /* success */ > + if (ret == 0) /* success */ > return len; > - } else if (ret == 0x10) { > - em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr); > + if (ret == 0x10) { > + em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", > + addr); > return -ENODEV; > - } else if (ret < 0) { > - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", > + } > + if (ret < 0) { > + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > ret); > return ret; > } > @@ -214,6 +217,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > * (even with high payload) ... > */ > } > + > em28xx_warn("write to i2c device at 0x%x timed out\n", addr); > return -EIO; > } > @@ -251,21 +255,20 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len) > > /* Check success of the i2c operation */ > ret = dev->em28xx_read_reg(dev, 0x05); > + if (ret == 0) /* success */ > + return len; > if (ret < 0) { > - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", > + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > ret); > return ret; > } > - if (ret > 0) { > - if (ret == 0x10) { > - em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); > - return -ENODEV; > - } else { > - em28xx_warn("unknown i2c error (status=%i)\n", ret); > - return -EIO; > - } > + if (ret == 0x10) { > + em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); > + return -ENODEV; > } > - return len; > + > + em28xx_warn("unknown i2c error (status=%i)\n", ret); > + return -EIO; > } > > /* -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Sun, 05 Jan 2014 12:03:51 +0100 Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: > Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab: > > The I2C wait completion/timeout logic currently assumes that > > msleep(5) will wait exaclty 5 ms. This is not true at all, > > as it depends on CONFIG_HZ. > > > > Convert it to use jiffies, in order to not wait for more time > > than needed. > > > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > --- > > drivers/media/usb/em28xx/em28xx-i2c.c | 65 ++++++++++++++++++----------------- > > 1 file changed, 34 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c > > index 9e6a11d01858..9fa7ed51e5b1 100644 > > --- a/drivers/media/usb/em28xx/em28xx-i2c.c > > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c > > @@ -26,6 +26,7 @@ > > #include <linux/kernel.h> > > #include <linux/usb.h> > > #include <linux/i2c.h> > > +#include <linux/jiffies.h> > > > > #include "em28xx.h" > > #include "tuner-xc2028.h" > > @@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]"); > > */ > > static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > > { > > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > > int ret; > > - int write_timeout; > > u8 b2[6]; > > > > if (len < 1 || len > 4) > > @@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > > return (ret < 0) ? ret : -EIO; > > } > > /* wait for completion */ > > - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; > > - write_timeout -= 5) { > > + while (time_is_after_jiffies(timeout)) { > AFAIU, it must be time_is_before_jiffies(timeout). This is tricky, but it is right. See its description at jiffies.h: /* time_is_after_jiffies(a) return true if a is after jiffies */ #define time_is_after_jiffies(a) time_before(jiffies, a) > > > ret = dev->em28xx_read_reg(dev, 0x05); > > - if (ret == 0x80 + len - 1) { > > + if (ret == 0x80 + len - 1) > > return len; > > - } else if (ret == 0x94 + len - 1) { > > + if (ret == 0x94 + len - 1) { > > em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); > > return -ENODEV; > > - } else if (ret < 0) { > > + } > > + if (ret < 0) { > > em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > > ret); > > return ret; > > @@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > > */ > > static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > > { > > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > > u8 buf2[4]; > > int ret; > > - int read_timeout; > > int i; > > > > if (len < 1 || len > 4) > > @@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) > > } > > > > /* wait for completion */ > > - for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0; > > - read_timeout -= 5) { > > + while (time_is_after_jiffies(timeout)) { > The same here... > > > ret = dev->em28xx_read_reg(dev, 0x05); > > - if (ret == 0x84 + len - 1) { > > + if (ret == 0x84 + len - 1) > > break; > > - } else if (ret == 0x94 + len - 1) { > > + if (ret == 0x94 + len - 1) { > > em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); > > return -ENODEV; > > - } else if (ret < 0) { > > + } > > + if (ret < 0) { > > em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > > ret); > > return ret; > > @@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr) > > static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > > u16 len, int stop) > > { > > - int write_timeout, ret; > > + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); > > + int ret; > > > > if (len < 1 || len > 64) > > return -EOPNOTSUPP; > > @@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > > } > > } > > > > - /* Check success of the i2c operation */ > > - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; > > - write_timeout -= 5) { > > + /* wait for completion */ > > + while (time_is_after_jiffies(timeout)) { > ... and here. > > > ret = dev->em28xx_read_reg(dev, 0x05); > > - if (ret == 0) { /* success */ > > + if (ret == 0) /* success */ > > return len; > > - } else if (ret == 0x10) { > > - em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr); > > + if (ret == 0x10) { > > + em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", > > + addr); > > return -ENODEV; > > - } else if (ret < 0) { > > - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", > > + } > > + if (ret < 0) { > > + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > > ret); > > return ret; > > } > > @@ -214,6 +217,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, > > * (even with high payload) ... > > */ > > } > > + > > em28xx_warn("write to i2c device at 0x%x timed out\n", addr); > > return -EIO; > > } > > @@ -251,21 +255,20 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len) > > > > /* Check success of the i2c operation */ > > ret = dev->em28xx_read_reg(dev, 0x05); > > + if (ret == 0) /* success */ > > + return len; > > if (ret < 0) { > > - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", > > + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", > > ret); > > return ret; > > } > > - if (ret > 0) { > > - if (ret == 0x10) { > > - em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); > > - return -ENODEV; > > - } else { > > - em28xx_warn("unknown i2c error (status=%i)\n", ret); > > - return -EIO; > > - } > > + if (ret == 0x10) { > > + em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); > > + return -ENODEV; > > } > > - return len; > > + > > + em28xx_warn("unknown i2c error (status=%i)\n", ret); > > + return -EIO; > > } > > > > /* >
Am 05.01.2014 14:10, schrieb Mauro Carvalho Chehab: > Em Sun, 05 Jan 2014 12:03:51 +0100 > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: > >> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab: >>> The I2C wait completion/timeout logic currently assumes that >>> msleep(5) will wait exaclty 5 ms. This is not true at all, >>> as it depends on CONFIG_HZ. >>> >>> Convert it to use jiffies, in order to not wait for more time >>> than needed. >>> >>> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> >>> --- >>> drivers/media/usb/em28xx/em28xx-i2c.c | 65 ++++++++++++++++++----------------- >>> 1 file changed, 34 insertions(+), 31 deletions(-) >>> >>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c >>> index 9e6a11d01858..9fa7ed51e5b1 100644 >>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c >>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c >>> @@ -26,6 +26,7 @@ >>> #include <linux/kernel.h> >>> #include <linux/usb.h> >>> #include <linux/i2c.h> >>> +#include <linux/jiffies.h> >>> >>> #include "em28xx.h" >>> #include "tuner-xc2028.h" >>> @@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]"); >>> */ >>> static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) >>> { >>> + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); >>> int ret; >>> - int write_timeout; >>> u8 b2[6]; >>> >>> if (len < 1 || len > 4) >>> @@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) >>> return (ret < 0) ? ret : -EIO; >>> } >>> /* wait for completion */ >>> - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; >>> - write_timeout -= 5) { >>> + while (time_is_after_jiffies(timeout)) { >> AFAIU, it must be time_is_before_jiffies(timeout). > This is tricky, but it is right. > > See its description at jiffies.h: > > /* time_is_after_jiffies(a) return true if a is after jiffies */ > #define time_is_after_jiffies(a) time_before(jiffies, a) Urgh... yes, you are right. I've read this, but didn't notice that we check the timeout jiffies and not the current jiffies. :/ Sorry for the noise. > >>> ret = dev->em28xx_read_reg(dev, 0x05); >>> - if (ret == 0x80 + len - 1) { >>> + if (ret == 0x80 + len - 1) >>> return len; >>> - } else if (ret == 0x94 + len - 1) { >>> + if (ret == 0x94 + len - 1) { >>> em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); >>> return -ENODEV; >>> - } else if (ret < 0) { >>> + } >>> + if (ret < 0) { >>> em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", >>> ret); >>> return ret; >>> @@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) >>> */ >>> static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) >>> { >>> + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); >>> u8 buf2[4]; >>> int ret; >>> - int read_timeout; >>> int i; >>> >>> if (len < 1 || len > 4) >>> @@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) >>> } >>> >>> /* wait for completion */ >>> - for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0; >>> - read_timeout -= 5) { >>> + while (time_is_after_jiffies(timeout)) { >> The same here... >> >>> ret = dev->em28xx_read_reg(dev, 0x05); >>> - if (ret == 0x84 + len - 1) { >>> + if (ret == 0x84 + len - 1) >>> break; >>> - } else if (ret == 0x94 + len - 1) { >>> + if (ret == 0x94 + len - 1) { >>> em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); >>> return -ENODEV; >>> - } else if (ret < 0) { >>> + } >>> + if (ret < 0) { >>> em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", >>> ret); >>> return ret; >>> @@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr) >>> static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, >>> u16 len, int stop) >>> { >>> - int write_timeout, ret; >>> + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); >>> + int ret; >>> >>> if (len < 1 || len > 64) >>> return -EOPNOTSUPP; >>> @@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, >>> } >>> } >>> >>> - /* Check success of the i2c operation */ >>> - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; >>> - write_timeout -= 5) { >>> + /* wait for completion */ >>> + while (time_is_after_jiffies(timeout)) { >> ... and here. >> >>> ret = dev->em28xx_read_reg(dev, 0x05); >>> - if (ret == 0) { /* success */ >>> + if (ret == 0) /* success */ >>> return len; >>> - } else if (ret == 0x10) { >>> - em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr); >>> + if (ret == 0x10) { >>> + em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", >>> + addr); >>> return -ENODEV; >>> - } else if (ret < 0) { >>> - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", >>> + } >>> + if (ret < 0) { >>> + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", >>> ret); >>> return ret; >>> } >>> @@ -214,6 +217,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, >>> * (even with high payload) ... >>> */ >>> } >>> + >>> em28xx_warn("write to i2c device at 0x%x timed out\n", addr); >>> return -EIO; >>> } >>> @@ -251,21 +255,20 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len) >>> >>> /* Check success of the i2c operation */ >>> ret = dev->em28xx_read_reg(dev, 0x05); >>> + if (ret == 0) /* success */ >>> + return len; >>> if (ret < 0) { >>> - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", >>> + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", >>> ret); >>> return ret; >>> } >>> - if (ret > 0) { >>> - if (ret == 0x10) { >>> - em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); >>> - return -ENODEV; >>> - } else { >>> - em28xx_warn("unknown i2c error (status=%i)\n", ret); >>> - return -EIO; >>> - } >>> + if (ret == 0x10) { >>> + em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); >>> + return -ENODEV; >>> } >>> - return len; >>> + >>> + em28xx_warn("unknown i2c error (status=%i)\n", ret); >>> + return -EIO; >>> } >>> >>> /* > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c index 9e6a11d01858..9fa7ed51e5b1 100644 --- a/drivers/media/usb/em28xx/em28xx-i2c.c +++ b/drivers/media/usb/em28xx/em28xx-i2c.c @@ -26,6 +26,7 @@ #include <linux/kernel.h> #include <linux/usb.h> #include <linux/i2c.h> +#include <linux/jiffies.h> #include "em28xx.h" #include "tuner-xc2028.h" @@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]"); */ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) { + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); int ret; - int write_timeout; u8 b2[6]; if (len < 1 || len > 4) @@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) return (ret < 0) ? ret : -EIO; } /* wait for completion */ - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; - write_timeout -= 5) { + while (time_is_after_jiffies(timeout)) { ret = dev->em28xx_read_reg(dev, 0x05); - if (ret == 0x80 + len - 1) { + if (ret == 0x80 + len - 1) return len; - } else if (ret == 0x94 + len - 1) { + if (ret == 0x94 + len - 1) { em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); return -ENODEV; - } else if (ret < 0) { + } + if (ret < 0) { em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", ret); return ret; @@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) */ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) { + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); u8 buf2[4]; int ret; - int read_timeout; int i; if (len < 1 || len > 4) @@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len) } /* wait for completion */ - for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0; - read_timeout -= 5) { + while (time_is_after_jiffies(timeout)) { ret = dev->em28xx_read_reg(dev, 0x05); - if (ret == 0x84 + len - 1) { + if (ret == 0x84 + len - 1) break; - } else if (ret == 0x94 + len - 1) { + if (ret == 0x94 + len - 1) { em28xx_warn("R05 returned 0x%02x: I2C timeout", ret); return -ENODEV; - } else if (ret < 0) { + } + if (ret < 0) { em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", ret); return ret; @@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr) static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len, int stop) { - int write_timeout, ret; + unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT); + int ret; if (len < 1 || len > 64) return -EOPNOTSUPP; @@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, } } - /* Check success of the i2c operation */ - for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0; - write_timeout -= 5) { + /* wait for completion */ + while (time_is_after_jiffies(timeout)) { ret = dev->em28xx_read_reg(dev, 0x05); - if (ret == 0) { /* success */ + if (ret == 0) /* success */ return len; - } else if (ret == 0x10) { - em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr); + if (ret == 0x10) { + em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", + addr); return -ENODEV; - } else if (ret < 0) { - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", + } + if (ret < 0) { + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", ret); return ret; } @@ -214,6 +217,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf, * (even with high payload) ... */ } + em28xx_warn("write to i2c device at 0x%x timed out\n", addr); return -EIO; } @@ -251,21 +255,20 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len) /* Check success of the i2c operation */ ret = dev->em28xx_read_reg(dev, 0x05); + if (ret == 0) /* success */ + return len; if (ret < 0) { - em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n", + em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n", ret); return ret; } - if (ret > 0) { - if (ret == 0x10) { - em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); - return -ENODEV; - } else { - em28xx_warn("unknown i2c error (status=%i)\n", ret); - return -EIO; - } + if (ret == 0x10) { + em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr); + return -ENODEV; } - return len; + + em28xx_warn("unknown i2c error (status=%i)\n", ret); + return -EIO; } /*
The I2C wait completion/timeout logic currently assumes that msleep(5) will wait exaclty 5 ms. This is not true at all, as it depends on CONFIG_HZ. Convert it to use jiffies, in order to not wait for more time than needed. Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> --- drivers/media/usb/em28xx/em28xx-i2c.c | 65 ++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 31 deletions(-)