by Hello all,
this patch fixes errors that have been introduced while porting the i2c
drivers in drivers/media/video to the new i2c layer: the i2c addresses
have not been ported appropriately.
It changes the files bt819.c, bt856.c, saa5249.c, saa7110.c, saa7111.c and
saa7185.c to follow the guidelines in "Documentation/i2c/writing-clients".
Additionally, it fixes a problem in saa7111.c that prevents the device to
be programmed with an inital programming. A FIXME message is added
to saa7185.c and bt819.c, because similar changes have to be done there.
I cannot do this, because this would require bigger changes which I could
not test.
(All these bugs make the drivers non usable -- are the drivers maintained
at all? I only need saa7111.c for my new driver...)
Linus, please apply. It has been tested with 2.5.54.
A detailed description of the problem and the fix is below.
CU
Michael.
-------------------------------------------------------------------
A small grep shows the following problem.
---------------------------------------------------------------------------
[root@michael video]# pwd
/usr/src/linux-2.5.52/drivers/media/video
[root@michael video]# grep -ir "34>>1" *
bt819.c:static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
bt856.c:static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
saa5249.c:static unsigned short normal_i2c[] = {34>>1,I2C_CLIENT_END};
saa7110.c:static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
saa7111.c:static unsigned short normal_i2c[] = { 34>>1, I2C_CLIENT_END };
saa7185.c:static unsigned short normal_i2c[] = { 34>>1, I2C_CLIENT_END };
---------------------------------------------------------------------------
So, all the bt819, bt856, saa5249, saa7110, saa7111 and saa7185 can be
found at the same i2c address 34>>1 = 0x22? I doubt that...
Apparently, someone fixed the saa5249 (which has the i2c address 0x22)
then wrote the "unofficial" tutorial Documentation/i2c/i2c-old-porting and
states there:
---------------------------------------------------------------------------
Step 2: Add and set the i2c modes
Add the following code near the top of the driver
static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
static unsigned short probe[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
static unsigned short probe_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END
static unsigned short ignore_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END
[...]
---------------------------------------------------------------------------
So somebody screwed up and really put this into every i2c driver, without
changing the actual i2c address, resulting in non-functional i2c drivers
out there...
Additionally, i2c-old-porting contradicts "writing-clients" in several
points. The most obvious is:
---------------------------------------------------------------------------
Fortunately, as a module writer, you just have to define the `normal'
and/or `normal_range' parameters. The complete declaration could look
like this:
/* Scan 0x20 to 0x2f, 0x37, and 0x40 to 0x4f */
static unsigned short normal_i2c[] = { 0x37,I2C_CLIENT_END };
static unsigned short normal_i2c_range[] = { 0x20, 0x2f, 0x40, 0x4f,
I2C_CLIENT_END };
/* Magic definition of all other variables and things */
I2C_CLIENT_INSMOD;
---------------------------------------------------------------------------
So you never, ever should define things like "ignore" above, because they
are handled by the i2c-core and won't work if you define them by hand...
And another problem: the same somebody who did this change, also changed
the way the initial programming is transferred into the device. Have a
look at saa7110.c. There, we have:
---------------------------------------------------------------------------
static const unsigned char init[] = {
0x00, 0x00, /* 00 - ID byte */
0x01, 0x00, /* 01 - reserved */
/*front end */
0x02, 0xd0, /* 02 - FUSE=3, GUDL=2, MODE=0 */
0x03, 0x23, /* 03 - HLNRS=0, VBSL=1, WPOFF=0,
[...]
---------------------------------------------------------------------------
init is used directly as an argument
for i2c_master_send() later on, which simply puts this into the buffer
argument of an i2c message.
But the init data in this case (ie. pairs
of addresses and data-bytes) is *not* a valid i2c message, so this
won't work. you need: subadress, then follow the data bytes for
subaddress, subaddress+1 and so on, if the device supports auto-increment.
In this case, removing the address information helps:
---------------------------------------------------------------------------
static const unsigned char init[] = {
0x00 /* starting address */
0x00, /* 00 - ID byte */
0x00, /* 01 - reserved */
/*front end */
0xd0, /* 02 - FUSE=3, GUDL=2, MODE=0 */
0x23, /* 03 - HLNRS=0, VBSL=1, WPOFF=0,
[...]
---------------------------------------------------------------------------
saa7185 and bt819 have the same problem, but I don't have such devices and
check if it works now.
--
diff -urN linux-2.5.52/drivers/media/video/bt819.c linux-2.5.52.i2c/drivers/media/video/bt819.c
--- linux-2.5.52/drivers/media/video/bt819.c Mon Dec 16 03:07:45 2002
+++ linux-2.5.52.i2c/drivers/media/video/bt819.c Mon Dec 23 20:03:57 2002
@@ -45,22 +45,6 @@
#define DEBUG(x) x /* Debug driver */
/* ----------------------------------------------------------------------- */
-
-static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
-static unsigned short probe[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned force[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-
-static struct i2c_client_address_data addr_data = {
- normal_i2c , normal_i2c_range,
- probe , probe_range,
- ignore , ignore_range,
- force
-};
-
static struct i2c_client client_template;
struct bt819 {
@@ -96,7 +80,11 @@
#define I2C_BT819 0x8a
-#define I2C_DELAY 10
+unsigned short normal_i2c[] = {I2C_BT819>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
+
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
/* ----------------------------------------------------------------------- */
@@ -114,6 +102,13 @@
{
struct bt819 *decoder;
+ /* FIXME: init[] is used directly as an argument
+ for i2c_master_send(), which simply puts this into the buffer
+ argument of an i2c message. the following init data (ie. pairs
+ of addresses and data-bytes) is *not* a valid i2c message, so this
+ won't work. you need: subadress, then follow the data bytes for
+ subaddress, subaddress+1 and so on, if the device supports
+ auto-increment. */
static unsigned char init[] = {
//0x1f, 0x00, /* Reset */
0x01, 0x59, /* 0x01 input format */
diff -urN linux-2.5.52/drivers/media/video/bt856.c linux-2.5.52.i2c/drivers/media/video/bt856.c
--- linux-2.5.52/drivers/media/video/bt856.c Mon Dec 16 03:07:43 2002
+++ linux-2.5.52.i2c/drivers/media/video/bt856.c Mon Dec 23 19:59:32 2002
@@ -53,22 +53,6 @@
#define DEBUG(x) x /* Debug driver */
/* ----------------------------------------------------------------------- */
-
-static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
-static unsigned short probe[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short force[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-
-static struct i2c_client_address_data addr_data = {
- normal_i2c , normal_i2c_range,
- probe , probe_range,
- ignore , ignore_range,
- force
-};
-
static struct i2c_client client_template;
struct bt856 {
@@ -86,7 +70,11 @@
#define I2C_BT856 0x88
-#define I2C_DELAY 10
+unsigned short normal_i2c[] = {I2C_BT856>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
+
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
/* ----------------------------------------------------------------------- */
diff -urN linux-2.5.52/drivers/media/video/saa5249.c linux-2.5.52.i2c/drivers/media/video/saa5249.c
--- linux-2.5.52/drivers/media/video/saa5249.c Mon Dec 16 03:08:14 2002
+++ linux-2.5.52.i2c/drivers/media/video/saa5249.c Mon Dec 23 19:59:32 2002
@@ -132,20 +132,11 @@
static struct video_device saa_template; /* Declared near bottom */
/* Addresses to scan */
-static unsigned short normal_i2c[] = {34>>1,I2C_CLIENT_END};
-static unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
-static unsigned short probe[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short force[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
+unsigned short normal_i2c[] = {0x22>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
-static struct i2c_client_address_data addr_data = {
- normal_i2c, normal_i2c_range,
- probe, probe_range,
- ignore, ignore_range,
- force
-};
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
static struct i2c_client client_template;
diff -urN linux-2.5.52/drivers/media/video/saa7110.c linux-2.5.52.i2c/drivers/media/video/saa7110.c
--- linux-2.5.52/drivers/media/video/saa7110.c Mon Dec 16 03:07:42 2002
+++ linux-2.5.52.i2c/drivers/media/video/saa7110.c Mon Dec 23 19:59:32 2002
@@ -38,22 +38,12 @@
#define I2C_SAA7110 0x9C /* or 0x9E */
#define IF_NAME "saa7110"
-#define I2C_DELAY 10 /* 10 us or 100khz */
-static unsigned short normal_i2c[] = {34>>1, I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
-static unsigned short probe[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short force[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
+unsigned short normal_i2c[] = {I2C_SAA7110>>1, 0x9E>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
-static struct i2c_client_address_data addr_data = {
- normal_i2c, normal_i2c_range,
- probe, probe_range,
- ignore, ignore_range,
- force
-};
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
static struct i2c_client client_template;
diff -urN linux-2.5.52/drivers/media/video/saa7111.c linux-2.5.52.i2c/drivers/media/video/saa7111.c
--- linux-2.5.52/drivers/media/video/saa7111.c Mon Dec 16 03:07:54 2002
+++ linux-2.5.52.i2c/drivers/media/video/saa7111.c Mon Dec 23 19:59:32 2002
@@ -59,25 +59,11 @@
#define I2C_SAA7111 0x48
-#define I2C_DELAY 10
+unsigned short normal_i2c[] = {I2C_SAA7111>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
-static unsigned short normal_i2c[] = { 34>>1, I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
-static unsigned short probe[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short force[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-
-static struct i2c_client_address_data addr_data = {
- .normal_i2c = normal_i2c,
- .normal_i2c_range = normal_i2c_range,
- .probe = probe,
- .probe_range = probe_range,
- .ignore = ignore,
- .ignore_range = ignore_range,
- .force = force
-};
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
static struct i2c_client client_template;
/* ----------------------------------------------------------------------- */
@@ -88,34 +74,36 @@
struct saa7111 *decoder;
struct i2c_client *client;
static const unsigned char init[] = {
- 0x00, 0x00, /* 00 - ID byte */
- 0x01, 0x00, /* 01 - reserved */
+ 0x00, /* start address */
+
+ 0x00, /* 00 - ID byte */
+ 0x00, /* 01 - reserved */
/*front end */
- 0x02, 0xd0, /* 02 - FUSE=3, GUDL=2, MODE=0 */
- 0x03, 0x23, /* 03 - HLNRS=0, VBSL=1, WPOFF=0, HOLDG=0, GAFIX=0, GAI1=256, GAI2=256 */
- 0x04, 0x00, /* 04 - GAI1=256 */
- 0x05, 0x00, /* 05 - GAI2=256 */
+ 0xd0, /* 02 - FUSE=3, GUDL=2, MODE=0 */
+ 0x23, /* 03 - HLNRS=0, VBSL=1, WPOFF=0, HOLDG=0, GAFIX=0, GAI1=256, GAI2=256 */
+ 0x00, /* 04 - GAI1=256 */
+ 0x00, /* 05 - GAI2=256 */
/* decoder */
- 0x06, 0xf3, /* 06 - HSB at 13(50Hz) / 17(60Hz) pixels after end of last line */
- 0x07, 0x13, /* 07 - HSS at 113(50Hz) / 117(60Hz) pixels after end of last line */
- 0x08, 0xc8, /* 08 - AUFD=1, FSEL=1, EXFIL=0, VTRC=1, HPLL=0, VNOI=0 */
- 0x09, 0x01, /* 09 - BYPS=0, PREF=0, BPSS=0, VBLB=0, UPTCV=0, APER=1 */
- 0x0a, 0x80, /* 0a - BRIG=128 */
- 0x0b, 0x47, /* 0b - CONT=1.109 */
- 0x0c, 0x40, /* 0c - SATN=1.0 */
- 0x0d, 0x00, /* 0d - HUE=0 */
- 0x0e, 0x01, /* 0e - CDTO=0, CSTD=0, DCCF=0, FCTC=0, CHBW=1 */
- 0x0f, 0x00, /* 0f - reserved */
- 0x10, 0x48, /* 10 - OFTS=1, HDEL=0, VRLN=1, YDEL=0 */
- 0x11, 0x1c, /* 11 - GPSW=0, CM99=0, FECO=0, COMPO=1, OEYC=1, OEHV=1, VIPB=0, COLO=0 */
- 0x12, 0x00, /* 12 - output control 2 */
- 0x13, 0x00, /* 13 - output control 3 */
- 0x14, 0x00, /* 14 - reserved */
- 0x15, 0x00, /* 15 - VBI */
- 0x16, 0x00, /* 16 - VBI */
- 0x17, 0x00, /* 17 - VBI */
+ 0xf3, /* 06 - HSB at 13(50Hz) / 17(60Hz) pixels after end of last line */
+ 0x13, /* 07 - HSS at 113(50Hz) / 117(60Hz) pixels after end of last line */
+ 0xc8, /* 08 - AUFD=1, FSEL=1, EXFIL=0, VTRC=1, HPLL=0, VNOI=0 */
+ 0x01, /* 09 - BYPS=0, PREF=0, BPSS=0, VBLB=0, UPTCV=0, APER=1 */
+ 0x80, /* 0a - BRIG=128 */
+ 0x47, /* 0b - CONT=1.109 */
+ 0x40, /* 0c - SATN=1.0 */
+ 0x00, /* 0d - HUE=0 */
+ 0x01, /* 0e - CDTO=0, CSTD=0, DCCF=0, FCTC=0, CHBW=1 */
+ 0x00, /* 0f - reserved */
+ 0x48, /* 10 - OFTS=1, HDEL=0, VRLN=1, YDEL=0 */
+ 0x1c, /* 11 - GPSW=0, CM99=0, FECO=0, COMPO=1, OEYC=1, OEHV=1, VIPB=0, COLO=0 */
+ 0x00, /* 12 - output control 2 */
+ 0x00, /* 13 - output control 3 */
+ 0x00, /* 14 - reserved */
+ 0x00, /* 15 - VBI */
+ 0x00, /* 16 - VBI */
+ 0x00, /* 17 - VBI */
};
client = kmalloc(sizeof(*client), GFP_KERNEL);
if(client == NULL)
diff -urN linux-2.5.52/drivers/media/video/saa7185.c linux-2.5.52.i2c/drivers/media/video/saa7185.c
--- linux-2.5.52/drivers/media/video/saa7185.c Mon Dec 16 03:07:43 2002
+++ linux-2.5.52.i2c/drivers/media/video/saa7185.c Mon Dec 23 20:05:05 2002
@@ -67,29 +67,22 @@
#define I2C_SAA7185 0x88
-#define I2C_DELAY 10
-
/* ----------------------------------------------------------------------- */
-static unsigned short normal_i2c[] = { 34>>1, I2C_CLIENT_END };
-static unsigned short normal_i2c_range[] = { I2C_CLIENT_END };
-static unsigned short probe[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short probe_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short ignore[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-static unsigned short ignore_range[2] = { I2C_CLIENT_END , I2C_CLIENT_END };
-static unsigned short force[2] = { I2C_CLIENT_END, I2C_CLIENT_END };
-
-static struct i2c_client_address_data addr_data = {
- .normal_i2c = normal_i2c,
- .normal_i2c_range = normal_i2c_range,
- .probe = probe,
- .probe_range = probe_range,
- .ignore = ignore,
- .ignore_range = ignore_range,
- .force = force
-};
+unsigned short normal_i2c[] = {I2C_SAA7185>>1, I2C_CLIENT_END};
+unsigned short normal_i2c_range[] = {I2C_CLIENT_END};
+
+/* magic definition of all other variables and things */
+I2C_CLIENT_INSMOD;
static struct i2c_client client_template;
+/* FIXME: init_common[] is used directly as an argument
+ for i2c_master_send(), which simply puts this into the buffer
+ argument of an i2c message. the following init data (ie. pairs
+ of addresses and data-bytes) is *not* a valid i2c message, so this
+ won't work. you need: subadress, then follow the data bytes for
+ subaddress, subaddress+1 and so on, if the device supports
+ auto-increment. */
static const unsigned char init_common[] = {
0x3a, 0x0f, /* CBENB=0, V656=0, VY2C=1, YUV2C=1, MY2C=1, MUV2C=1 */