2006-03-13 20:09:35

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 0/8] Zoran drivers updates

Hi all,

Here comes an 8-piece patch set for the zoran drivers. The first 7 of
these were already posted to the mjpeg-users and video4linux lists two
months ago [1], the last one is new. There are both fixes and cleanups.

Andrew, Ronald says he is too busy these days to deal with my patches,
but otherwise he was OK with them. Could you please take them in -mm so
that they get wider testing? Then we'd be able to merge them.

[1] http://www.mail-archive.com/[email protected]/msg06451.html

Summary:

Jean Delvare:
o saa7110: Fix array overrun
o saa7111: Prevent array overrun
o saa7114: Fix i2c block write
o adv7175: Drop unused encoder dump command
o adv7175: Drop unused register cache
o zoran: Use i2c_master_send when possible
o bt856: Spare memory
o zoran: Init cleanups

Statistics:

drivers/media/video/adv7170.c | 17 +++++--------
drivers/media/video/adv7175.c | 51 ++++++--------------------------------
drivers/media/video/bt819.c | 17 +++++--------
drivers/media/video/bt856.c | 13 ++++------
drivers/media/video/saa7110.c | 19 ++++----------
drivers/media/video/saa7111.c | 25 +++++++++----------
drivers/media/video/saa7114.c | 23 ++++++-----------
drivers/media/video/saa7185.c | 17 +++++--------
drivers/media/video/zoran_card.c | 38 +++++++++++++---------------
9 files changed, 77 insertions(+), 143 deletions(-)

The patches are also available from there until they get merged:
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-saa7110-fix-array-overrun.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-saa7111-prevent-array-overrun.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-saa7114-fix-i2c-block-write.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-adv7175-drop-encoder-dump.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-adv7175-drop-unused-register-cache.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-zoran-use-i2c-master-send.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-bt856-spare-memory.patch
http://khali.linux-fr.org/devel/i2c/linux-2.6/media-video-zoran-init-cleanup.patch

Thanks,
--
Jean Delvare


2006-03-13 20:29:17

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 1/8] saa7110: Fix array overrun

Fix a (probably harmless) array overrun in the DECODER_DUMP command
of the saa7110 driver. No big deal as this command is not used
anywhere anyway. Also reformat the dump so that it displays nicely.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7110.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/saa7110.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7110.c 2006-03-01 21:10:01.000000000 +0100
@@ -431,15 +431,13 @@
break;

case DECODER_DUMP:
- for (v = 0; v < 0x34; v += 16) {
+ for (v = 0; v < SAA7110_NR_REG; v += 16) {
int j;
- dprintk(1, KERN_INFO "%s: %03x\n", I2C_NAME(client),
+ dprintk(1, KERN_DEBUG "%s: %02x:", I2C_NAME(client),
v);
- for (j = 0; j < 16; j++) {
- dprintk(1, KERN_INFO " %02x",
- decoder->reg[v + j]);
- }
- dprintk(1, KERN_INFO "\n");
+ for (j = 0; j < 16 && v + j < SAA7110_NR_REG; j++)
+ dprintk(1, " %02x", decoder->reg[v + j]);
+ dprintk(1, "\n");
}
break;


--
Jean Delvare

2006-03-13 20:30:54

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 3/8] saa7114: Fix i2c block write

Fix the i2c block write mode of the saa7114 driver. A previous code
change accidentally commented out a local variable increment, which
should have been kept, causing the register writes over the I2C bus
to never be batched, replacing any attempted block write by slower,
individual write transactions.

Also drop the commented out code, as it only adds to confusion.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7114.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/saa7114.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7114.c 2006-03-01 21:10:10.000000000 +0100
@@ -138,9 +138,6 @@
u8 reg,
u8 value)
{
- /*struct saa7114 *decoder = i2c_get_clientdata(client);*/
-
- /*decoder->reg[reg] = value;*/
return i2c_smbus_write_byte_data(client, reg, value);
}

@@ -156,7 +153,6 @@
* the adapter understands raw I2C */
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
- /*struct saa7114 *decoder = i2c_get_clientdata(client);*/
struct i2c_msg msg;
u8 block_data[32];

@@ -167,8 +163,8 @@
msg.len = 0;
block_data[msg.len++] = reg = data[0];
do {
- block_data[msg.len++] =
- /*decoder->reg[reg++] =*/ data[1];
+ block_data[msg.len++] = data[1];
+ reg++;
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&

--
Jean Delvare

2006-03-13 20:30:06

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 2/8] saa7111: Prevent array overrun

Explicitely state the number of registers the SAA7111 has, and use
that defined value where relevant. This should prevent any future
array overrun like the one I just fixed in the saa7110 driver.

This patch also saves 8 bytes of memory as a side effect, as the
register cache was larger than needed.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7111.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/saa7111.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7111.c 2006-03-01 21:10:07.000000000 +0100
@@ -69,8 +69,10 @@

/* ----------------------------------------------------------------------- */

+#define SAA7111_NR_REG 0x18
+
struct saa7111 {
- unsigned char reg[32];
+ unsigned char reg[SAA7111_NR_REG];

int norm;
int input;
@@ -226,11 +228,11 @@
{
int i;

- for (i = 0; i < 32; i += 16) {
+ for (i = 0; i < SAA7111_NR_REG; i += 16) {
int j;

printk(KERN_DEBUG "%s: %03x", I2C_NAME(client), i);
- for (j = 0; j < 16; ++j) {
+ for (j = 0; j < 16 && i + j < SAA7111_NR_REG; ++j) {
printk(" %02x",
saa7111_read(client, i + j));
}

--
Jean Delvare

2006-03-13 20:31:56

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 4/8] adv7175: Drop unused encoder dump command

Drop support for the ENCODER_DUMP command in the adv7175 driver.
ENCODER_DUMP was never actually defined as far as I can see, so the
code was ifdef'd out, and I suspect it was never used, not even once,
as it includes an obvious array overrun.

The register values of this specific chip can be dumped in a generic
way using the i2c-dev driver and the "i2cdump" user-space tool if it
is ever really needed.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/adv7175.c | 26 --------------------------
1 file changed, 26 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/adv7175.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/adv7175.c 2006-03-01 21:10:12.000000000 +0100
@@ -170,24 +170,6 @@
adv7175_write(client, 0x05, 0x25);
}

-#ifdef ENCODER_DUMP
-static void
-dump (struct i2c_client *client)
-{
- struct adv7175 *encoder = i2c_get_clientdata(client);
- int i, j;
-
- printk(KERN_INFO "%s: registry dump\n", I2C_NAME(client));
- for (i = 0; i < 182 / 8; i++) {
- printk("%s: 0x%02x -", I2C_NAME(client), i * 8);
- for (j = 0; j < 8; j++) {
- printk(" 0x%02x", encoder->reg[i * 8 + j]);
- }
- printk("\n");
- }
-}
-#endif
-
/* ----------------------------------------------------------------------- */
// Output filter: S-Video Composite

@@ -406,14 +388,6 @@
}
break;

-#ifdef ENCODER_DUMP
- case ENCODER_DUMP:
- {
- dump(client);
- }
- break;
-#endif
-
default:
return -EINVAL;
}

--
Jean Delvare

2006-03-13 20:32:49

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 5/8] adv7175: Drop unused register cache

Drop the adv7175 register cache, as it is only written to and never
read back from. This saves 128 bytes of memory and slightly speeds up
the register writes.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/adv7175.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/adv7175.c 2006-03-01 21:10:12.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/adv7175.c 2006-03-01 21:10:14.000000000 +0100
@@ -67,8 +67,6 @@
/* ----------------------------------------------------------------------- */

struct adv7175 {
- unsigned char reg[128];
-
int norm;
int input;
int enable;
@@ -94,9 +92,6 @@
u8 reg,
u8 value)
{
- struct adv7175 *encoder = i2c_get_clientdata(client);
-
- encoder->reg[reg] = value;
return i2c_smbus_write_byte_data(client, reg, value);
}

@@ -119,7 +114,6 @@
* the adapter understands raw I2C */
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
- struct adv7175 *encoder = i2c_get_clientdata(client);
struct i2c_msg msg;
u8 block_data[32];

@@ -130,8 +124,8 @@
msg.len = 0;
block_data[msg.len++] = reg = data[0];
do {
- block_data[msg.len++] =
- encoder->reg[reg++] = data[1];
+ block_data[msg.len++] = data[1];
+ reg++;
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&

--
Jean Delvare

2006-03-13 20:34:48

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 6/8] zoran: Use i2c_master_send when possible

Change all the Zoran (ZR36050/ZR36060) drivers to use i2c_master_send
instead of i2c_transfer when possible. This simplifies the code by a
few lines in each driver.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/adv7170.c | 17 +++++++----------
drivers/media/video/adv7175.c | 17 +++++++----------
drivers/media/video/bt819.c | 17 +++++++----------
drivers/media/video/saa7110.c | 7 +------
drivers/media/video/saa7111.c | 17 +++++++----------
drivers/media/video/saa7114.c | 17 +++++++----------
drivers/media/video/saa7185.c | 17 +++++++----------
7 files changed, 43 insertions(+), 66 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/saa7110.c 2006-03-01 21:10:01.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7110.c 2006-03-01 21:10:16.000000000 +0100
@@ -107,13 +107,8 @@
* the adapter understands raw I2C */
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
struct saa7110 *decoder = i2c_get_clientdata(client);
- struct i2c_msg msg;

- msg.len = len;
- msg.buf = (char *) data;
- msg.addr = client->addr;
- msg.flags = 0;
- ret = i2c_transfer(client->adapter, &msg, 1);
+ ret = i2c_master_send(client, data, len);

/* Cache the written data */
memcpy(decoder->reg + reg, data + 1, len - 1);
--- linux-2.6.16-rc5.orig/drivers/media/video/adv7175.c 2006-03-01 21:10:14.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/adv7175.c 2006-03-01 21:10:16.000000000 +0100
@@ -114,24 +114,21 @@
* the adapter understands raw I2C */
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] = data[1];
+ block_data[block_len++] = data[1];
reg++;
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {
--- linux-2.6.16-rc5.orig/drivers/media/video/saa7111.c 2006-03-01 21:10:07.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7111.c 2006-03-01 21:10:16.000000000 +0100
@@ -111,24 +111,21 @@
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
struct saa7111 *decoder = i2c_get_clientdata(client);
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] =
+ block_data[block_len++] =
decoder->reg[reg++] = data[1];
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {
--- linux-2.6.16-rc5.orig/drivers/media/video/saa7185.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7185.c 2006-03-01 21:10:16.000000000 +0100
@@ -112,24 +112,21 @@
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
struct saa7185 *encoder = i2c_get_clientdata(client);
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] =
+ block_data[block_len++] =
encoder->reg[reg++] = data[1];
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {
--- linux-2.6.16-rc5.orig/drivers/media/video/bt819.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/bt819.c 2006-03-01 21:10:16.000000000 +0100
@@ -140,24 +140,21 @@
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
struct bt819 *decoder = i2c_get_clientdata(client);
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] =
+ block_data[block_len++] =
decoder->reg[reg++] = data[1];
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {
--- linux-2.6.16-rc5.orig/drivers/media/video/adv7170.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/adv7170.c 2006-03-01 21:10:16.000000000 +0100
@@ -124,24 +124,21 @@
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
struct adv7170 *encoder = i2c_get_clientdata(client);
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] =
+ block_data[block_len++] =
encoder->reg[reg++] = data[1];
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {
--- linux-2.6.16-rc5.orig/drivers/media/video/saa7114.c 2006-03-01 21:10:10.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/saa7114.c 2006-03-01 21:10:16.000000000 +0100
@@ -153,24 +153,21 @@
* the adapter understands raw I2C */
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
/* do raw I2C, not smbus compatible */
- struct i2c_msg msg;
u8 block_data[32];
+ int block_len;

- msg.addr = client->addr;
- msg.flags = 0;
while (len >= 2) {
- msg.buf = (char *) block_data;
- msg.len = 0;
- block_data[msg.len++] = reg = data[0];
+ block_len = 0;
+ block_data[block_len++] = reg = data[0];
do {
- block_data[msg.len++] = data[1];
+ block_data[block_len++] = data[1];
reg++;
len -= 2;
data += 2;
} while (len >= 2 && data[0] == reg &&
- msg.len < 32);
- if ((ret = i2c_transfer(client->adapter,
- &msg, 1)) < 0)
+ block_len < 32);
+ if ((ret = i2c_master_send(client, block_data,
+ block_len)) < 0)
break;
}
} else {

--
Jean Delvare

2006-03-13 20:35:31

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 7/8] bt856: Spare memory

The bt856 driver has a register cache much larger than needed. We
really only write to 3 registers, so a 32-byte cache is a bit too
much. We can be just as efficient with a 6-byte cache. We could even
do with a 3-byte cache, but at the cost of additional arithmetics
arguably not worth the spared 3 bytes.

Also, 4 of the 6 other members of the bt856 data structure were not
used anywhere, so we can as well drop them for an additional 16 bytes
of memory spared.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/bt856.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

--- linux-2.6.16-rc5.orig/drivers/media/video/bt856.c 2006-03-01 21:09:59.000000000 +0100
+++ linux-2.6.16-rc5/drivers/media/video/bt856.c 2006-03-01 21:10:19.000000000 +0100
@@ -70,17 +70,14 @@

/* ----------------------------------------------------------------------- */

-#define REG_OFFSET 0xCE
+#define REG_OFFSET 0xDA
+#define BT856_NR_REG 6

struct bt856 {
- unsigned char reg[32];
+ unsigned char reg[BT856_NR_REG];

int norm;
int enable;
- int bright;
- int contrast;
- int hue;
- int sat;
};

#define I2C_BT856 0x88
@@ -119,8 +116,8 @@
struct bt856 *encoder = i2c_get_clientdata(client);

printk(KERN_INFO "%s: register dump:", I2C_NAME(client));
- for (i = 0xd6; i <= 0xde; i += 2)
- printk(" %02x", encoder->reg[i - REG_OFFSET]);
+ for (i = 0; i < BT856_NR_REG; i += 2)
+ printk(" %02x", encoder->reg[i]);
printk("\n");
}


--
Jean Delvare

2006-03-13 20:36:51

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 8/8] zoran: Init cleanups

Cleanups to the zr36057 initialization:
* Drop intermediate local variables.
* Single error path.
Also drop a needless cast on kfree.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/zoran_card.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)

--- linux-2.6.15-git.orig/drivers/media/video/zoran_card.c 2006-01-12 21:08:57.000000000 +0100
+++ linux-2.6.15-git/drivers/media/video/zoran_card.c 2006-01-12 21:12:05.000000000 +0100
@@ -995,10 +995,7 @@
static int __devinit
zr36057_init (struct zoran *zr)
{
- u32 *mem;
- void *vdev;
- unsigned mem_needed;
- int j;
+ int j, err;
int two = 2;
int zero = 0;

@@ -1049,19 +1046,16 @@

/* allocate memory *before* doing anything to the hardware
* in case allocation fails */
- mem_needed = BUZ_NUM_STAT_COM * 4;
- mem = kzalloc(mem_needed, GFP_KERNEL);
- vdev = (void *) kmalloc(sizeof(struct video_device), GFP_KERNEL);
- if (!mem || !vdev) {
+ zr->stat_com = kzalloc(BUZ_NUM_STAT_COM * 4, GFP_KERNEL);
+ zr->video_dev = kmalloc(sizeof(struct video_device), GFP_KERNEL);
+ if (!zr->stat_com || !zr->video_dev) {
dprintk(1,
KERN_ERR
"%s: zr36057_init() - kmalloc (STAT_COM) failed\n",
ZR_DEVNAME(zr));
- kfree(vdev);
- kfree(mem);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto exit_free;
}
- zr->stat_com = mem;
for (j = 0; j < BUZ_NUM_STAT_COM; j++) {
zr->stat_com[j] = 1; /* mark as unavailable to zr36057 */
}
@@ -1069,16 +1063,11 @@
/*
* Now add the template and register the device unit.
*/
- zr->video_dev = vdev;
memcpy(zr->video_dev, &zoran_template, sizeof(zoran_template));
strcpy(zr->video_dev->name, ZR_DEVNAME(zr));
- if (video_register_device(zr->video_dev, VFL_TYPE_GRABBER,
- video_nr) < 0) {
- zoran_unregister_i2c(zr);
- kfree((void *) zr->stat_com);
- kfree(vdev);
- return -1;
- }
+ err = video_register_device(zr->video_dev, VFL_TYPE_GRABBER, video_nr);
+ if (err < 0)
+ goto exit_unregister;

zoran_init_hardware(zr);
if (*zr_debug > 2)
@@ -1092,6 +1081,13 @@
zr->zoran_proc = NULL;
zr->initialized = 1;
return 0;
+
+exit_unregister:
+ zoran_unregister_i2c(zr);
+exit_free:
+ kfree(zr->stat_com);
+ kfree(zr->video_dev);
+ return err;
}

static void
@@ -1121,7 +1117,7 @@
btwrite(0, ZR36057_SPGPPCR);
free_irq(zr->pci_dev->irq, zr);
/* unmap and free memory */
- kfree((void *) zr->stat_com);
+ kfree(zr->stat_com);
zoran_proc_cleanup(zr);
iounmap(zr->zr36057_mem);
pci_disable_device(zr->pci_dev);

--
Jean Delvare

2006-03-13 20:53:51

by Ronald S. Bultje

[permalink] [raw]
Subject: Re: [PATCH 0/8] Zoran drivers updates

Hi Andrew / all,

On Mon, 13 Mar 2006, Jean Delvare wrote:
> Andrew, Ronald says he is too busy these days to deal with my patches,
> but otherwise he was OK with them. Could you please take them in -mm so
> that they get wider testing? Then we'd be able to merge them.

I second that for the first 7 patches. I'll look at the 8th one (it's new,
didn't look yet :-) ) shortly. Please accept the first 7, and hold on for
#8 for a second longer, thanks.

And Jean, sorry for taking so long, the patches are of great quality,
thanks for contributing them.

Ronald

2006-03-27 05:16:17

by Ronald S. Bultje

[permalink] [raw]
Subject: Re: [PATCH 8/8] zoran: Init cleanups

Hi Andrew,

On Mon, 2006-03-13 at 21:36 +0100, Jean Delvare wrote:
> Cleanups to the zr36057 initialization:
> * Drop intermediate local variables.
> * Single error path.
> Also drop a needless cast on kfree.

Please take this patch into your tree, it looks good. Jean, thanks for
contributing (again!).

Cheers,
Ronald

> Signed-off-by: Jean Delvare <[email protected]>
> ---
> drivers/media/video/zoran_card.c | 38 +++++++++++++++++---------------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> --- linux-2.6.15-git.orig/drivers/media/video/zoran_card.c 2006-01-12 21:08:57.000000000 +0100
> +++ linux-2.6.15-git/drivers/media/video/zoran_card.c 2006-01-12 21:12:05.000000000 +0100
> @@ -995,10 +995,7 @@
> static int __devinit
> zr36057_init (struct zoran *zr)
> {
> - u32 *mem;
> - void *vdev;
> - unsigned mem_needed;
> - int j;
> + int j, err;
> int two = 2;
> int zero = 0;
>
> @@ -1049,19 +1046,16 @@
>
> /* allocate memory *before* doing anything to the hardware
> * in case allocation fails */
> - mem_needed = BUZ_NUM_STAT_COM * 4;
> - mem = kzalloc(mem_needed, GFP_KERNEL);
> - vdev = (void *) kmalloc(sizeof(struct video_device), GFP_KERNEL);
> - if (!mem || !vdev) {
> + zr->stat_com = kzalloc(BUZ_NUM_STAT_COM * 4, GFP_KERNEL);
> + zr->video_dev = kmalloc(sizeof(struct video_device), GFP_KERNEL);
> + if (!zr->stat_com || !zr->video_dev) {
> dprintk(1,
> KERN_ERR
> "%s: zr36057_init() - kmalloc (STAT_COM) failed\n",
> ZR_DEVNAME(zr));
> - kfree(vdev);
> - kfree(mem);
> - return -ENOMEM;
> + err = -ENOMEM;
> + goto exit_free;
> }
> - zr->stat_com = mem;
> for (j = 0; j < BUZ_NUM_STAT_COM; j++) {
> zr->stat_com[j] = 1; /* mark as unavailable to zr36057 */
> }
> @@ -1069,16 +1063,11 @@
> /*
> * Now add the template and register the device unit.
> */
> - zr->video_dev = vdev;
> memcpy(zr->video_dev, &zoran_template, sizeof(zoran_template));
> strcpy(zr->video_dev->name, ZR_DEVNAME(zr));
> - if (video_register_device(zr->video_dev, VFL_TYPE_GRABBER,
> - video_nr) < 0) {
> - zoran_unregister_i2c(zr);
> - kfree((void *) zr->stat_com);
> - kfree(vdev);
> - return -1;
> - }
> + err = video_register_device(zr->video_dev, VFL_TYPE_GRABBER, video_nr);
> + if (err < 0)
> + goto exit_unregister;
>
> zoran_init_hardware(zr);
> if (*zr_debug > 2)
> @@ -1092,6 +1081,13 @@
> zr->zoran_proc = NULL;
> zr->initialized = 1;
> return 0;
> +
> +exit_unregister:
> + zoran_unregister_i2c(zr);
> +exit_free:
> + kfree(zr->stat_com);
> + kfree(zr->video_dev);
> + return err;
> }
>
> static void
> @@ -1121,7 +1117,7 @@
> btwrite(0, ZR36057_SPGPPCR);
> free_irq(zr->pci_dev->irq, zr);
> /* unmap and free memory */
> - kfree((void *) zr->stat_com);
> + kfree(zr->stat_com);
> zoran_proc_cleanup(zr);
> iounmap(zr->zr36057_mem);
> pci_disable_device(zr->pci_dev);
>