Re: [PATCH 1/5] em28xx: add support for em25xx i2c bus B read/write/check device operations
Am 04.03.2013 22:31, schrieb Frank Schäfer: > ... > I don't expect this chip to appear in one of the devices with the > currently supported generic IDs. Hmm... maybe I should add: it doesn't make sense to add the generic USB ID of this chip (eb1a:2765) to the driver, because most of the devices using it should be UVC compliant. The same should apply to all other newer Empia camera bridges (EM276x/7x/8x). Regards, Frank -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] em28xx: add support for em25xx i2c bus B read/write/check device operations
Am 04.03.2013 21:23, schrieb Mauro Carvalho Chehab:
> Em Mon, 4 Mar 2013 17:20:05 -0300
> Mauro Carvalho Chehab escreveu:
>
>> Em Sun, 3 Mar 2013 20:40:57 +0100
>> Frank Schäfer escreveu:
...
>>> @@ -277,7 +386,9 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
>>>struct i2c_msg msgs[], int num)
>>> {
>>> struct em28xx *dev = i2c_adap->algo_data;
>>> - int addr, rc, i, byte;
>>> + int addr, i, byte;
>>> + int rc = -EOPNOTSUPP;
>>> + enum em28xx_i2c_algo_type algo_type = dev->i2c_algo_type;
>>>
>>> if (num <= 0)
>>> return 0;
>>> @@ -290,10 +401,13 @@ static int em28xx_i2c_xfer(struct i2c_adapter
>>> *i2c_adap,
>>>i == num - 1 ? "stop" : "nonstop",
>>>addr, msgs[i].len );
>>> if (!msgs[i].len) { /* no len: check only for device presence */
>>> - if (dev->board.is_em2800)
>>> - rc = em2800_i2c_check_for_device(dev, addr);
>>> - else
>>> + if (algo_type == EM28XX_I2C_ALGO_EM28XX) {
>>> rc = em28xx_i2c_check_for_device(dev, addr);
>>> + } else if (algo_type == EM28XX_I2C_ALGO_EM2800) {
>>> + rc = em2800_i2c_check_for_device(dev, addr);
>>> + } else if (algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B) {
>>> + rc = em25xx_bus_B_check_for_device(dev, addr);
>>> + }
>> That seems too messy.
>>
>> IMO, the best is to create 3 different em28xx_i2c_xfer() routines:
>> one for em2800, one for "standard" I2c protocol, and another one for
>> this em25xx one. Then, all you need to do is to embed
>> static struct i2c_algorithm into struct em28xx and fill
>> em28xx_algo.master_transfer to point to the correct xfer.
>>
>> That makes the code more readable and remove a little faster by removing
>> the unneeded ifs from the code.
Hmmm that's how I did it initially.
But IIRC correctly, it also duplicates lots of code, making it much
bigger...
I will take a look at it again tomorrow evening.
>>> if (rc == -ENODEV) {
>>> if (i2c_debug)
>>> printk(" no device\n");
>>> @@ -301,14 +415,19 @@ static int em28xx_i2c_xfer(struct i2c_adapter
>>> *i2c_adap,
>>> }
>>> } else if (msgs[i].flags & I2C_M_RD) {
>>> /* read bytes */
>>> - if (dev->board.is_em2800)
>>> - rc = em2800_i2c_recv_bytes(dev, addr,
>>> + if (algo_type == EM28XX_I2C_ALGO_EM28XX) {
>>> + rc = em28xx_i2c_recv_bytes(dev, addr,
>>>msgs[i].buf,
>>>msgs[i].len);
>>> - else
>>> - rc = em28xx_i2c_recv_bytes(dev, addr,
>>> + } else if (algo_type == EM28XX_I2C_ALGO_EM2800) {
>>> + rc = em2800_i2c_recv_bytes(dev, addr,
>>>msgs[i].buf,
>>>msgs[i].len);
>>> + } else if (algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B) {
>>> + rc = em25xx_bus_B_recv_bytes(dev, addr,
>>> + msgs[i].buf,
>>> + msgs[i].len);
>>> + }
>>> if (i2c_debug) {
>>> for (byte = 0; byte < msgs[i].len; byte++)
>>> printk(" %02x", msgs[i].buf[byte]);
>>> @@ -319,15 +438,20 @@ static int em28xx_i2c_xfer(struct i2c_adapter
>>> *i2c_adap,
>>> for (byte = 0; byte < msgs[i].len; byte++)
>>> printk(" %02x", msgs[i].buf[byte]);
>>> }
>>> - if (dev->board.is_em2800)
>>> - rc = em2800_i2c_send_bytes(dev, addr,
>>> - msgs[i].buf,
>>> - msgs[i].len);
>>> - else
>>> + if (algo_type == EM28XX_I2C_ALGO_EM28XX) {
>>> rc = em28xx_i2c_send_bytes(dev, addr,
>>>msgs[i].buf,
>>>msgs[i].len,
>>>i == num - 1);
>>> + } else if (algo_type == EM28XX_I2C_ALGO_EM2800) {
>>> + rc = em2800_i2c_send_bytes(dev, addr,
>>> + msgs[i].buf,
>>> +
Re: [PATCH 1/5] em28xx: add support for em25xx i2c bus B read/write/check device operations
Em Mon, 4 Mar 2013 17:20:05 -0300
Mauro Carvalho Chehab escreveu:
> Em Sun, 3 Mar 2013 20:40:57 +0100
> Frank Schäfer escreveu:
>
> > The webcam "SpeedLink VAD Laplace" (em2765 + ov2640) uses a special
> > algorithm
> > for i2c communication with the sensor, which is connected to a second i2c
> > bus.
> >
> > We don't know yet how to find out which devices support/use it.
> > It's very likely used by all em25xx and em276x+ bridges.
> > Tests with other em28xx chips (em2820, em2882/em2883) show, that this
> > algorithm always succeeds there although no slave device is connected.
> >
> > The algorithm likely also works for real I2C client devices (OV2640 uses
> > SCCB),
> > because the Windows driver seems to use it for probing Samsung and Kodak
> > sensors.
> >
> > Signed-off-by: Frank Schäfer
> > ---
> > drivers/media/usb/em28xx/em28xx-cards.c |6 +-
> > drivers/media/usb/em28xx/em28xx-i2c.c | 164
> > +++
> > drivers/media/usb/em28xx/em28xx.h |7 ++
> > 3 Dateien geändert, 159 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
> > b/drivers/media/usb/em28xx/em28xx-cards.c
> > index 5a5b637..75d4aef 100644
> > --- a/drivers/media/usb/em28xx/em28xx-cards.c
> > +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> > @@ -3103,7 +3103,11 @@ static int em28xx_init_dev(struct em28xx *dev,
> > struct usb_device *udev,
> > return retval;
> > }
> > /* Configure i2c bus */
> > - if (!dev->board.is_em2800) {
> > + if (dev->board.is_em2800) {
> > + dev->i2c_algo_type = EM28XX_I2C_ALGO_EM2800;
> > + } else {
> > + dev->i2c_algo_type = EM28XX_I2C_ALGO_EM28XX;
> > +
> > retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK,
> > dev->board.i2c_speed);
> > if (retval < 0) {
> > em28xx_errdev("%s: em28xx_write_reg failed!"
> > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
> > b/drivers/media/usb/em28xx/em28xx-i2c.c
> > index 44bef43..6e86ffc 100644
> > --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> > @@ -5,6 +5,7 @@
> > Markus Rechberger
> > Mauro Carvalho Chehab
> > Sascha Sommer
> > + Copyright (C) 2013 Frank Schäfer
> >
> > This program is free software; you can redistribute it and/or modify
> > it under the terms of the GNU General Public License as published by
> > @@ -270,6 +271,114 @@ static int em28xx_i2c_check_for_device(struct em28xx
> > *dev, u16 addr)
> > }
> >
> > /*
> > + * em25xx_bus_B_send_bytes
> > + * write bytes to the i2c device
> > + */
> > +static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
> > + u16 len)
> > +{
> > + int ret;
> > +
> > + if (len < 1 || len > 64)
> > + return -EOPNOTSUPP;
> > + /* NOTE: limited by the USB ctrl message constraints
> > +* Zero length reads always succeed, even if no device is connected */
> > +
> > + /* Set register and write value */
> > + ret = dev->em28xx_write_regs_req(dev, 0x06, addr, buf, len);
> > + /* NOTE:
> > +* 0 byte writes always succeed, even if no device is connected. */
> > + if (ret != len) {
> > + if (ret < 0) {
> > + em28xx_warn("writing to i2c device at 0x%x failed "
> > + "(error=%i)\n", addr, ret);
> > + return ret;
> > + } else {
> > + em28xx_warn("%i bytes write to i2c device at 0x%x "
> > + "requested, but %i bytes written\n",
> > + len, addr, ret);
> > + return -EIO;
> > + }
> > + }
> > + /* Check success */
> > + ret = dev->em28xx_read_reg_req(dev, 0x08, 0x);
> > + /* NOTE: the only error we've seen so far is
> > +* 0x01 when the slave device is not present */
> > + if (ret == 0x00) {
> > + return len;
> > + } else if (ret > 0) {
> > + return -ENODEV;
> > + }
> > +
> > + return ret;
> > + /* NOTE: With chips which do not support this operation,
> > +* it seems to succeed ALWAYS ! (even if no device connected) */
> > +}
> > +
> > +/*
> > + * em25xx_bus_B_recv_bytes
> > + * read bytes from the i2c device
> > + */
> > +static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
> > + u16 len)
> > +{
> > + int ret;
> > +
> > + if (len < 1 || len > 64)
> > + return -EOPNOTSUPP;
> > + /* NOTE: limited by the USB ctrl message constraints
> > +* Zero length reads always succeed, even if no device is connected */
> > +
> > + /* Read value */
> > + ret = dev->em28xx_read_reg_req_len(dev, 0x06, addr, buf, len);
> > + /* NOTE:
> > +* 0 byte reads always succeed, even if no device is connected. */
> > + if (ret != len) {
> > + if (ret
Re: [PATCH 1/5] em28xx: add support for em25xx i2c bus B read/write/check device operations
Em Sun, 3 Mar 2013 20:40:57 +0100
Frank Schäfer escreveu:
> The webcam "SpeedLink VAD Laplace" (em2765 + ov2640) uses a special algorithm
> for i2c communication with the sensor, which is connected to a second i2c bus.
>
> We don't know yet how to find out which devices support/use it.
> It's very likely used by all em25xx and em276x+ bridges.
> Tests with other em28xx chips (em2820, em2882/em2883) show, that this
> algorithm always succeeds there although no slave device is connected.
>
> The algorithm likely also works for real I2C client devices (OV2640 uses
> SCCB),
> because the Windows driver seems to use it for probing Samsung and Kodak
> sensors.
>
> Signed-off-by: Frank Schäfer
> ---
> drivers/media/usb/em28xx/em28xx-cards.c |6 +-
> drivers/media/usb/em28xx/em28xx-i2c.c | 164
> +++
> drivers/media/usb/em28xx/em28xx.h |7 ++
> 3 Dateien geändert, 159 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
> b/drivers/media/usb/em28xx/em28xx-cards.c
> index 5a5b637..75d4aef 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -3103,7 +3103,11 @@ static int em28xx_init_dev(struct em28xx *dev, struct
> usb_device *udev,
> return retval;
> }
> /* Configure i2c bus */
> - if (!dev->board.is_em2800) {
> + if (dev->board.is_em2800) {
> + dev->i2c_algo_type = EM28XX_I2C_ALGO_EM2800;
> + } else {
> + dev->i2c_algo_type = EM28XX_I2C_ALGO_EM28XX;
> +
> retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK,
> dev->board.i2c_speed);
> if (retval < 0) {
> em28xx_errdev("%s: em28xx_write_reg failed!"
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
> b/drivers/media/usb/em28xx/em28xx-i2c.c
> index 44bef43..6e86ffc 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -5,6 +5,7 @@
> Markus Rechberger
> Mauro Carvalho Chehab
> Sascha Sommer
> + Copyright (C) 2013 Frank Schäfer
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -270,6 +271,114 @@ static int em28xx_i2c_check_for_device(struct em28xx
> *dev, u16 addr)
> }
>
> /*
> + * em25xx_bus_B_send_bytes
> + * write bytes to the i2c device
> + */
> +static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
> +u16 len)
> +{
> + int ret;
> +
> + if (len < 1 || len > 64)
> + return -EOPNOTSUPP;
> + /* NOTE: limited by the USB ctrl message constraints
> + * Zero length reads always succeed, even if no device is connected */
> +
> + /* Set register and write value */
> + ret = dev->em28xx_write_regs_req(dev, 0x06, addr, buf, len);
> + /* NOTE:
> + * 0 byte writes always succeed, even if no device is connected. */
> + if (ret != len) {
> + if (ret < 0) {
> + em28xx_warn("writing to i2c device at 0x%x failed "
> + "(error=%i)\n", addr, ret);
> + return ret;
> + } else {
> + em28xx_warn("%i bytes write to i2c device at 0x%x "
> + "requested, but %i bytes written\n",
> + len, addr, ret);
> + return -EIO;
> + }
> + }
> + /* Check success */
> + ret = dev->em28xx_read_reg_req(dev, 0x08, 0x);
> + /* NOTE: the only error we've seen so far is
> + * 0x01 when the slave device is not present */
> + if (ret == 0x00) {
> + return len;
> + } else if (ret > 0) {
> + return -ENODEV;
> + }
> +
> + return ret;
> + /* NOTE: With chips which do not support this operation,
> + * it seems to succeed ALWAYS ! (even if no device connected) */
> +}
> +
> +/*
> + * em25xx_bus_B_recv_bytes
> + * read bytes from the i2c device
> + */
> +static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
> +u16 len)
> +{
> + int ret;
> +
> + if (len < 1 || len > 64)
> + return -EOPNOTSUPP;
> + /* NOTE: limited by the USB ctrl message constraints
> + * Zero length reads always succeed, even if no device is connected */
> +
> + /* Read value */
> + ret = dev->em28xx_read_reg_req_len(dev, 0x06, addr, buf, len);
> + /* NOTE:
> + * 0 byte reads always succeed, even if no device is connected. */
> + if (ret != len) {
> + if (ret < 0) {
> + em28xx_warn("reading from i2c device at 0x%x failed "
> + "(error=%i)\n", addr, ret);
> + return ret;
> + } else {
[PATCH 1/5] em28xx: add support for em25xx i2c bus B read/write/check device operations
The webcam "SpeedLink VAD Laplace" (em2765 + ov2640) uses a special algorithm
for i2c communication with the sensor, which is connected to a second i2c bus.
We don't know yet how to find out which devices support/use it.
It's very likely used by all em25xx and em276x+ bridges.
Tests with other em28xx chips (em2820, em2882/em2883) show, that this
algorithm always succeeds there although no slave device is connected.
The algorithm likely also works for real I2C client devices (OV2640 uses SCCB),
because the Windows driver seems to use it for probing Samsung and Kodak
sensors.
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-cards.c |6 +-
drivers/media/usb/em28xx/em28xx-i2c.c | 164 +++
drivers/media/usb/em28xx/em28xx.h |7 ++
3 Dateien geändert, 159 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 5a5b637..75d4aef 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3103,7 +3103,11 @@ static int em28xx_init_dev(struct em28xx *dev, struct
usb_device *udev,
return retval;
}
/* Configure i2c bus */
- if (!dev->board.is_em2800) {
+ if (dev->board.is_em2800) {
+ dev->i2c_algo_type = EM28XX_I2C_ALGO_EM2800;
+ } else {
+ dev->i2c_algo_type = EM28XX_I2C_ALGO_EM28XX;
+
retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK,
dev->board.i2c_speed);
if (retval < 0) {
em28xx_errdev("%s: em28xx_write_reg failed!"
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 44bef43..6e86ffc 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -5,6 +5,7 @@
Markus Rechberger
Mauro Carvalho Chehab
Sascha Sommer
+ Copyright (C) 2013 Frank Schäfer
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -270,6 +271,114 @@ static int em28xx_i2c_check_for_device(struct em28xx
*dev, u16 addr)
}
/*
+ * em25xx_bus_B_send_bytes
+ * write bytes to the i2c device
+ */
+static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
+ u16 len)
+{
+ int ret;
+
+ if (len < 1 || len > 64)
+ return -EOPNOTSUPP;
+ /* NOTE: limited by the USB ctrl message constraints
+* Zero length reads always succeed, even if no device is connected */
+
+ /* Set register and write value */
+ ret = dev->em28xx_write_regs_req(dev, 0x06, addr, buf, len);
+ /* NOTE:
+* 0 byte writes always succeed, even if no device is connected. */
+ if (ret != len) {
+ if (ret < 0) {
+ em28xx_warn("writing to i2c device at 0x%x failed "
+ "(error=%i)\n", addr, ret);
+ return ret;
+ } else {
+ em28xx_warn("%i bytes write to i2c device at 0x%x "
+ "requested, but %i bytes written\n",
+ len, addr, ret);
+ return -EIO;
+ }
+ }
+ /* Check success */
+ ret = dev->em28xx_read_reg_req(dev, 0x08, 0x);
+ /* NOTE: the only error we've seen so far is
+* 0x01 when the slave device is not present */
+ if (ret == 0x00) {
+ return len;
+ } else if (ret > 0) {
+ return -ENODEV;
+ }
+
+ return ret;
+ /* NOTE: With chips which do not support this operation,
+* it seems to succeed ALWAYS ! (even if no device connected) */
+}
+
+/*
+ * em25xx_bus_B_recv_bytes
+ * read bytes from the i2c device
+ */
+static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
+ u16 len)
+{
+ int ret;
+
+ if (len < 1 || len > 64)
+ return -EOPNOTSUPP;
+ /* NOTE: limited by the USB ctrl message constraints
+* Zero length reads always succeed, even if no device is connected */
+
+ /* Read value */
+ ret = dev->em28xx_read_reg_req_len(dev, 0x06, addr, buf, len);
+ /* NOTE:
+* 0 byte reads always succeed, even if no device is connected. */
+ if (ret != len) {
+ if (ret < 0) {
+ em28xx_warn("reading from i2c device at 0x%x failed "
+ "(error=%i)\n", addr, ret);
+ return ret;
+ } else {
+ em28xx_warn("%i bytes requested from i2c device at "
+ "0x%x, but %i bytes received\n",
+ len, a
