2008-08-08 14:50:10

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 0/3] matroxfb fixes and improvements

Hi Antonino,

Not having heard of Petr in one month and a half, I am sending these 3
patches again. Patch 1/3 (matrox maven: Fix a broken error path) in
particular should be applied ASAP. Patch 2/3 (matroxfb: i2c structure
templates clean-up) would be good to have as well and I tested it.
Patch 3/3 (matrox maven: Convert to a new-style i2c driver) needs
testing before it can go upstream. I can't test it myself, but it has
been in linux-next for 6 weeks or so.

Thanks,
--
Jean Delvare


2008-08-08 14:58:43

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 1/3] matrox maven: Fix a broken error path

I broke an error path with d03c21ec0be7787ff6b75dcf56c0e96209ccbfbd,
sorry about that.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/video/matrox/matroxfb_maven.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.26-rc6.orig/drivers/video/matrox/matroxfb_maven.c 2008-06-17 17:22:42.000000000 +0200
+++ linux-2.6.26-rc6/drivers/video/matrox/matroxfb_maven.c 2008-06-17 17:23:18.000000000 +0200
@@ -1266,7 +1266,7 @@ static int maven_detect_client(struct i2
ERROR4:;
i2c_detach_client(new_client);
ERROR3:;
- kfree(new_client);
+ kfree(data);
ERROR0:;
return err;
}


--
Jean Delvare

2008-08-08 15:02:27

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 2/3] matroxfb: i2c structure templates clean-up

Clean up the use of structure templates in i2c-matroxfb. In this case
it's more efficient to initialize the few fields we need individually.
This makes i2c-matroxfb.ko 16% smaller on my system.

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

--- linux-2.6.26-rc6.orig/drivers/video/matrox/i2c-matroxfb.c 2008-06-17 20:23:41.000000000 +0200
+++ linux-2.6.26-rc6/drivers/video/matrox/i2c-matroxfb.c 2008-06-17 20:27:36.000000000 +0200
@@ -87,13 +87,7 @@ static int matroxfb_gpio_getscl(void* da
return (matroxfb_read_gpio(b->minfo) & b->mask.clock) ? 1 : 0;
}

-static struct i2c_adapter matrox_i2c_adapter_template =
-{
- .owner = THIS_MODULE,
- .id = I2C_HW_B_G400,
-};
-
-static struct i2c_algo_bit_data matrox_i2c_algo_template =
+static const struct i2c_algo_bit_data matrox_i2c_algo_template =
{
.setsda = matroxfb_gpio_setsda,
.setscl = matroxfb_gpio_setscl,
@@ -112,7 +106,8 @@ static int i2c_bus_reg(struct i2c_bit_ad
b->minfo = minfo;
b->mask.data = data;
b->mask.clock = clock;
- b->adapter = matrox_i2c_adapter_template;
+ b->adapter.owner = THIS_MODULE;
+ b->adapter.id = I2C_HW_B_G400;
snprintf(b->adapter.name, sizeof(b->adapter.name), name,
minfo->fbcon.node);
i2c_set_adapdata(&b->adapter, b);

--
Jean Delvare

2008-08-08 15:06:24

by Jean Delvare

[permalink] [raw]
Subject: [PATCH 3/3] matrox maven: Convert to a new-style i2c driver

The legacy i2c model is going away soon, so switch to the new model.

Signed-off-by: Jean Delvare <[email protected]>
---
drivers/video/matrox/i2c-matroxfb.c | 12 +++-
drivers/video/matrox/matroxfb_maven.c | 95 +++++++++++++--------------------
include/linux/i2c-id.h | 2
3 files changed, 50 insertions(+), 59 deletions(-)

--- linux-2.6.26-rc9.orig/drivers/video/matrox/i2c-matroxfb.c 2008-07-12 10:24:23.000000000 +0200
+++ linux-2.6.26-rc9/drivers/video/matrox/i2c-matroxfb.c 2008-07-12 10:24:26.000000000 +0200
@@ -107,7 +107,6 @@ static int i2c_bus_reg(struct i2c_bit_ad
b->mask.data = data;
b->mask.clock = clock;
b->adapter.owner = THIS_MODULE;
- b->adapter.id = I2C_HW_B_G400;
snprintf(b->adapter.name, sizeof(b->adapter.name), name,
minfo->fbcon.node);
i2c_set_adapdata(&b->adapter, b);
@@ -182,6 +181,17 @@ static void* i2c_matroxfb_probe(struct m
MAT_DATA, MAT_CLK, "MAVEN:fb%u", 0);
if (err)
printk(KERN_INFO "i2c-matroxfb: Could not register Maven i2c bus. Continuing anyway.\n");
+ else {
+ struct i2c_board_info maven_info = {
+ I2C_BOARD_INFO("maven", 0x1b),
+ };
+ unsigned short const addr_list[2] = {
+ 0x1b, I2C_CLIENT_END
+ };
+
+ i2c_new_probed_device(&m2info->maven.adapter,
+ &maven_info, addr_list);
+ }
}
return m2info;
fail_ddc1:;
--- linux-2.6.26-rc9.orig/drivers/video/matrox/matroxfb_maven.c 2008-07-12 10:24:21.000000000 +0200
+++ linux-2.6.26-rc9/drivers/video/matrox/matroxfb_maven.c 2008-07-12 10:24:41.000000000 +0200
@@ -19,8 +19,6 @@
#include <linux/matroxfb.h>
#include <asm/div64.h>

-#define MAVEN_I2CID (0x1B)
-
#define MGATVO_B 1
#define MGATVO_C 2

@@ -128,7 +126,7 @@ static int get_ctrl_id(__u32 v4l2_id) {

struct maven_data {
struct matrox_fb_info* primary_head;
- struct i2c_client client;
+ struct i2c_client *client;
int version;
};

@@ -974,7 +972,7 @@ static inline int maven_compute_timming(

static int maven_program_timming(struct maven_data* md,
const struct mavenregs* m) {
- struct i2c_client* c = &md->client;
+ struct i2c_client *c = md->client;

if (m->mode == MATROXFB_OUTPUT_MODE_MONITOR) {
LR(0x80);
@@ -1011,7 +1009,7 @@ static int maven_program_timming(struct
}

static inline int maven_resync(struct maven_data* md) {
- struct i2c_client* c = &md->client;
+ struct i2c_client *c = md->client;
maven_set_reg(c, 0x95, 0x20); /* start whole thing */
return 0;
}
@@ -1069,48 +1067,48 @@ static int maven_set_control (struct mav
maven_compute_bwlevel(md, &blacklevel, &whitelevel);
blacklevel = (blacklevel >> 2) | ((blacklevel & 3) << 8);
whitelevel = (whitelevel >> 2) | ((whitelevel & 3) << 8);
- maven_set_reg_pair(&md->client, 0x0e, blacklevel);
- maven_set_reg_pair(&md->client, 0x1e, whitelevel);
+ maven_set_reg_pair(md->client, 0x0e, blacklevel);
+ maven_set_reg_pair(md->client, 0x1e, whitelevel);
}
break;
case V4L2_CID_SATURATION:
{
- maven_set_reg(&md->client, 0x20, p->value);
- maven_set_reg(&md->client, 0x22, p->value);
+ maven_set_reg(md->client, 0x20, p->value);
+ maven_set_reg(md->client, 0x22, p->value);
}
break;
case V4L2_CID_HUE:
{
- maven_set_reg(&md->client, 0x25, p->value);
+ maven_set_reg(md->client, 0x25, p->value);
}
break;
case V4L2_CID_GAMMA:
{
const struct maven_gamma* g;
g = maven_compute_gamma(md);
- maven_set_reg(&md->client, 0x83, g->reg83);
- maven_set_reg(&md->client, 0x84, g->reg84);
- maven_set_reg(&md->client, 0x85, g->reg85);
- maven_set_reg(&md->client, 0x86, g->reg86);
- maven_set_reg(&md->client, 0x87, g->reg87);
- maven_set_reg(&md->client, 0x88, g->reg88);
- maven_set_reg(&md->client, 0x89, g->reg89);
- maven_set_reg(&md->client, 0x8a, g->reg8a);
- maven_set_reg(&md->client, 0x8b, g->reg8b);
+ maven_set_reg(md->client, 0x83, g->reg83);
+ maven_set_reg(md->client, 0x84, g->reg84);
+ maven_set_reg(md->client, 0x85, g->reg85);
+ maven_set_reg(md->client, 0x86, g->reg86);
+ maven_set_reg(md->client, 0x87, g->reg87);
+ maven_set_reg(md->client, 0x88, g->reg88);
+ maven_set_reg(md->client, 0x89, g->reg89);
+ maven_set_reg(md->client, 0x8a, g->reg8a);
+ maven_set_reg(md->client, 0x8b, g->reg8b);
}
break;
case MATROXFB_CID_TESTOUT:
{
unsigned char val
- = maven_get_reg(&md->client,0x8d);
+ = maven_get_reg(md->client, 0x8d);
if (p->value) val |= 0x10;
else val &= ~0x10;
- maven_set_reg(&md->client, 0x8d, val);
+ maven_set_reg(md->client, 0x8d, val);
}
break;
case MATROXFB_CID_DEFLICKER:
{
- maven_set_reg(&md->client, 0x93, maven_compute_deflicker(md));
+ maven_set_reg(md->client, 0x93, maven_compute_deflicker(md));
}
break;
}
@@ -1189,6 +1187,7 @@ static int maven_init_client(struct i2c_
MINFO_FROM(container_of(clnt->adapter, struct i2c_bit_adapter, adapter)->minfo);

md->primary_head = MINFO;
+ md->client = clnt;
down_write(&ACCESS_FBINFO(altout.lock));
ACCESS_FBINFO(outputs[1]).output = &maven_altout;
ACCESS_FBINFO(outputs[1]).src = ACCESS_FBINFO(outputs[1]).default_src;
@@ -1232,14 +1231,11 @@ static int maven_shutdown_client(struct
return 0;
}

-static const unsigned short normal_i2c[] = { MAVEN_I2CID, I2C_CLIENT_END };
-I2C_CLIENT_INSMOD;
-
-static struct i2c_driver maven_driver;
-
-static int maven_detect_client(struct i2c_adapter* adapter, int address, int kind) {
- int err = 0;
- struct i2c_client* new_client;
+static int maven_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ int err = -ENODEV;
struct maven_data* data;

if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
@@ -1250,50 +1246,37 @@ static int maven_detect_client(struct i2
err = -ENOMEM;
goto ERROR0;
}
- new_client = &data->client;
- i2c_set_clientdata(new_client, data);
- new_client->addr = address;
- new_client->adapter = adapter;
- new_client->driver = &maven_driver;
- new_client->flags = 0;
- strlcpy(new_client->name, "maven", I2C_NAME_SIZE);
- if ((err = i2c_attach_client(new_client)))
- goto ERROR3;
- err = maven_init_client(new_client);
+ i2c_set_clientdata(client, data);
+ err = maven_init_client(client);
if (err)
goto ERROR4;
return 0;
ERROR4:;
- i2c_detach_client(new_client);
-ERROR3:;
kfree(data);
ERROR0:;
return err;
}

-static int maven_attach_adapter(struct i2c_adapter* adapter) {
- if (adapter->id == I2C_HW_B_G400)
- return i2c_probe(adapter, &addr_data, &maven_detect_client);
- return 0;
-}
-
-static int maven_detach_client(struct i2c_client* client) {
- int err;
-
- if ((err = i2c_detach_client(client)))
- return err;
+static int maven_remove(struct i2c_client *client)
+{
maven_shutdown_client(client);
kfree(i2c_get_clientdata(client));
return 0;
}

+static const struct i2c_device_id maven_id[] = {
+ { "maven", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, maven_id);
+
static struct i2c_driver maven_driver={
.driver = {
.name = "maven",
},
- .id = I2C_DRIVERID_MGATVO,
- .attach_adapter = maven_attach_adapter,
- .detach_client = maven_detach_client,
+ .probe = maven_probe,
+ .remove = maven_remove,
+ .id_table = maven_id,
};

static int __init matroxfb_maven_init(void)
--- linux-2.6.26-rc9.orig/include/linux/i2c-id.h 2008-07-12 10:11:02.000000000 +0200
+++ linux-2.6.26-rc9/include/linux/i2c-id.h 2008-07-12 10:24:26.000000000 +0200
@@ -39,7 +39,6 @@
#define I2C_DRIVERID_SAA7111A 8 /* video input processor */
#define I2C_DRIVERID_SAA7185B 13 /* video encoder */
#define I2C_DRIVERID_SAA7110 22 /* video decoder */
-#define I2C_DRIVERID_MGATVO 23 /* Matrox TVOut */
#define I2C_DRIVERID_SAA5249 24 /* SAA5249 and compatibles */
#define I2C_DRIVERID_PCF8583 25 /* real time clock */
#define I2C_DRIVERID_SAB3036 26 /* SAB3036 tuner */
@@ -94,7 +93,6 @@
#define I2C_HW_B_BT848 0x010005 /* BT848 video boards */
#define I2C_HW_B_VIA 0x010007 /* Via vt82c586b */
#define I2C_HW_B_HYDRA 0x010008 /* Apple Hydra Mac I/O */
-#define I2C_HW_B_G400 0x010009 /* Matrox G400 */
#define I2C_HW_B_I810 0x01000a /* Intel I810 */
#define I2C_HW_B_VOO 0x01000b /* 3dfx Voodoo 3 / Banshee */
#define I2C_HW_B_SCX200 0x01000e /* Nat'l Semi SCx200 I2C */

--
Jean Delvare

2008-08-08 16:30:17

by Krzysztof Helt

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 1/3] matrox maven: Fix a broken error path

On Fri, 8 Aug 2008 16:58:23 +0200
Jean Delvare <[email protected]> wrote:

> I broke an error path with d03c21ec0be7787ff6b75dcf56c0e96209ccbfbd,
> sorry about that.
>
> Signed-off-by: Jean Delvare <[email protected]>
> ---

Acked-by: Krzysztof Helt <[email protected]>


> drivers/video/matrox/matroxfb_maven.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.26-rc6.orig/drivers/video/matrox/matroxfb_maven.c 2008-06-17 17:22:42.000000000 +0200
> +++ linux-2.6.26-rc6/drivers/video/matrox/matroxfb_maven.c 2008-06-17 17:23:18.000000000 +0200
> @@ -1266,7 +1266,7 @@ static int maven_detect_client(struct i2
> ERROR4:;
> i2c_detach_client(new_client);
> ERROR3:;
> - kfree(new_client);
> + kfree(data);
> ERROR0:;
> return err;
> }
>
>
> --
> Jean Delvare
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
>


----------------------------------------------------------------------
Tylko dla detektywow! Konkurs na Smaker.pl
Kliknij >>> http://link.interia.pl/f1eb1

2008-08-08 16:31:04

by Krzysztof Helt

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 2/3] matroxfb: i2c structure templates clean-up

On Fri, 8 Aug 2008 17:02:08 +0200
Jean Delvare <[email protected]> wrote:

> Clean up the use of structure templates in i2c-matroxfb. In this case
> it's more efficient to initialize the few fields we need individually.
> This makes i2c-matroxfb.ko 16% smaller on my system.
>
> Signed-off-by: Jean Delvare <[email protected]>
> ---

Acked-by: Krzysztof Helt <[email protected]>

> drivers/video/matrox/i2c-matroxfb.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> --- linux-2.6.26-rc6.orig/drivers/video/matrox/i2c-matroxfb.c 2008-06-17 20:23:41.000000000 +0200
> +++ linux-2.6.26-rc6/drivers/video/matrox/i2c-matroxfb.c 2008-06-17 20:27:36.000000000 +0200
> @@ -87,13 +87,7 @@ static int matroxfb_gpio_getscl(void* da
> return (matroxfb_read_gpio(b->minfo) & b->mask.clock) ? 1 : 0;
> }
>
> -static struct i2c_adapter matrox_i2c_adapter_template =
> -{
> - .owner = THIS_MODULE,
> - .id = I2C_HW_B_G400,
> -};
> -
> -static struct i2c_algo_bit_data matrox_i2c_algo_template =
> +static const struct i2c_algo_bit_data matrox_i2c_algo_template =
> {
> .setsda = matroxfb_gpio_setsda,
> .setscl = matroxfb_gpio_setscl,
> @@ -112,7 +106,8 @@ static int i2c_bus_reg(struct i2c_bit_ad
> b->minfo = minfo;
> b->mask.data = data;
> b->mask.clock = clock;
> - b->adapter = matrox_i2c_adapter_template;
> + b->adapter.owner = THIS_MODULE;
> + b->adapter.id = I2C_HW_B_G400;
> snprintf(b->adapter.name, sizeof(b->adapter.name), name,
> minfo->fbcon.node);
> i2c_set_adapdata(&b->adapter, b);
>
> --
> Jean Delvare
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
>


----------------------------------------------------------------------
Tylko dla detektywow! Konkurs na Smaker.pl
Kliknij >>> http://link.interia.pl/f1eb1

2008-08-08 16:42:39

by Krzysztof Helt

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 3/3] matrox maven: Convert to a new-style i2c driver

On Fri, 8 Aug 2008 17:05:56 +0200
Jean Delvare <[email protected]> wrote:

> The legacy i2c model is going away soon, so switch to the new model.
>
> Signed-off-by: Jean Delvare <[email protected]>
> ---

Acked-by: Krzysztof Helt <[email protected]>


> drivers/video/matrox/i2c-matroxfb.c | 12 +++-
> drivers/video/matrox/matroxfb_maven.c | 95 +++++++++++++--------------------
> include/linux/i2c-id.h | 2
> 3 files changed, 50 insertions(+), 59 deletions(-)
>
> --- linux-2.6.26-rc9.orig/drivers/video/matrox/i2c-matroxfb.c 2008-07-12 10:24:23.000000000 +0200
> +++ linux-2.6.26-rc9/drivers/video/matrox/i2c-matroxfb.c 2008-07-12 10:24:26.000000000 +0200
> @@ -107,7 +107,6 @@ static int i2c_bus_reg(struct i2c_bit_ad
> b->mask.data = data;
> b->mask.clock = clock;
> b->adapter.owner = THIS_MODULE;
> - b->adapter.id = I2C_HW_B_G400;
> snprintf(b->adapter.name, sizeof(b->adapter.name), name,
> minfo->fbcon.node);
> i2c_set_adapdata(&b->adapter, b);
> @@ -182,6 +181,17 @@ static void* i2c_matroxfb_probe(struct m
> MAT_DATA, MAT_CLK, "MAVEN:fb%u", 0);
> if (err)
> printk(KERN_INFO "i2c-matroxfb: Could not register Maven i2c bus. Continuing anyway.\n");
> + else {
> + struct i2c_board_info maven_info = {
> + I2C_BOARD_INFO("maven", 0x1b),
> + };
> + unsigned short const addr_list[2] = {
> + 0x1b, I2C_CLIENT_END
> + };
> +
> + i2c_new_probed_device(&m2info->maven.adapter,
> + &maven_info, addr_list);
> + }
> }
> return m2info;
> fail_ddc1:;
> --- linux-2.6.26-rc9.orig/drivers/video/matrox/matroxfb_maven.c 2008-07-12 10:24:21.000000000 +0200
> +++ linux-2.6.26-rc9/drivers/video/matrox/matroxfb_maven.c 2008-07-12 10:24:41.000000000 +0200
> @@ -19,8 +19,6 @@
> #include <linux/matroxfb.h>
> #include <asm/div64.h>
>
> -#define MAVEN_I2CID (0x1B)
> -
> #define MGATVO_B 1
> #define MGATVO_C 2
>
> @@ -128,7 +126,7 @@ static int get_ctrl_id(__u32 v4l2_id) {
>
> struct maven_data {
> struct matrox_fb_info* primary_head;
> - struct i2c_client client;
> + struct i2c_client *client;
> int version;
> };
>
> @@ -974,7 +972,7 @@ static inline int maven_compute_timming(
>
> static int maven_program_timming(struct maven_data* md,
> const struct mavenregs* m) {
> - struct i2c_client* c = &md->client;
> + struct i2c_client *c = md->client;
>
> if (m->mode == MATROXFB_OUTPUT_MODE_MONITOR) {
> LR(0x80);
> @@ -1011,7 +1009,7 @@ static int maven_program_timming(struct
> }
>
> static inline int maven_resync(struct maven_data* md) {
> - struct i2c_client* c = &md->client;
> + struct i2c_client *c = md->client;
> maven_set_reg(c, 0x95, 0x20); /* start whole thing */
> return 0;
> }
> @@ -1069,48 +1067,48 @@ static int maven_set_control (struct mav
> maven_compute_bwlevel(md, &blacklevel, &whitelevel);
> blacklevel = (blacklevel >> 2) | ((blacklevel & 3) << 8);
> whitelevel = (whitelevel >> 2) | ((whitelevel & 3) << 8);
> - maven_set_reg_pair(&md->client, 0x0e, blacklevel);
> - maven_set_reg_pair(&md->client, 0x1e, whitelevel);
> + maven_set_reg_pair(md->client, 0x0e, blacklevel);
> + maven_set_reg_pair(md->client, 0x1e, whitelevel);
> }
> break;
> case V4L2_CID_SATURATION:
> {
> - maven_set_reg(&md->client, 0x20, p->value);
> - maven_set_reg(&md->client, 0x22, p->value);
> + maven_set_reg(md->client, 0x20, p->value);
> + maven_set_reg(md->client, 0x22, p->value);
> }
> break;
> case V4L2_CID_HUE:
> {
> - maven_set_reg(&md->client, 0x25, p->value);
> + maven_set_reg(md->client, 0x25, p->value);
> }
> break;
> case V4L2_CID_GAMMA:
> {
> const struct maven_gamma* g;
> g = maven_compute_gamma(md);
> - maven_set_reg(&md->client, 0x83, g->reg83);
> - maven_set_reg(&md->client, 0x84, g->reg84);
> - maven_set_reg(&md->client, 0x85, g->reg85);
> - maven_set_reg(&md->client, 0x86, g->reg86);
> - maven_set_reg(&md->client, 0x87, g->reg87);
> - maven_set_reg(&md->client, 0x88, g->reg88);
> - maven_set_reg(&md->client, 0x89, g->reg89);
> - maven_set_reg(&md->client, 0x8a, g->reg8a);
> - maven_set_reg(&md->client, 0x8b, g->reg8b);
> + maven_set_reg(md->client, 0x83, g->reg83);
> + maven_set_reg(md->client, 0x84, g->reg84);
> + maven_set_reg(md->client, 0x85, g->reg85);
> + maven_set_reg(md->client, 0x86, g->reg86);
> + maven_set_reg(md->client, 0x87, g->reg87);
> + maven_set_reg(md->client, 0x88, g->reg88);
> + maven_set_reg(md->client, 0x89, g->reg89);
> + maven_set_reg(md->client, 0x8a, g->reg8a);
> + maven_set_reg(md->client, 0x8b, g->reg8b);
> }
> break;
> case MATROXFB_CID_TESTOUT:
> {
> unsigned char val
> - = maven_get_reg(&md->client,0x8d);
> + = maven_get_reg(md->client, 0x8d);
> if (p->value) val |= 0x10;
> else val &= ~0x10;
> - maven_set_reg(&md->client, 0x8d, val);
> + maven_set_reg(md->client, 0x8d, val);
> }
> break;
> case MATROXFB_CID_DEFLICKER:
> {
> - maven_set_reg(&md->client, 0x93, maven_compute_deflicker(md));
> + maven_set_reg(md->client, 0x93, maven_compute_deflicker(md));
> }
> break;
> }
> @@ -1189,6 +1187,7 @@ static int maven_init_client(struct i2c_
> MINFO_FROM(container_of(clnt->adapter, struct i2c_bit_adapter, adapter)->minfo);
>
> md->primary_head = MINFO;
> + md->client = clnt;
> down_write(&ACCESS_FBINFO(altout.lock));
> ACCESS_FBINFO(outputs[1]).output = &maven_altout;
> ACCESS_FBINFO(outputs[1]).src = ACCESS_FBINFO(outputs[1]).default_src;
> @@ -1232,14 +1231,11 @@ static int maven_shutdown_client(struct
> return 0;
> }
>
> -static const unsigned short normal_i2c[] = { MAVEN_I2CID, I2C_CLIENT_END };
> -I2C_CLIENT_INSMOD;
> -
> -static struct i2c_driver maven_driver;
> -
> -static int maven_detect_client(struct i2c_adapter* adapter, int address, int kind) {
> - int err = 0;
> - struct i2c_client* new_client;
> +static int maven_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct i2c_adapter *adapter = client->adapter;
> + int err = -ENODEV;
> struct maven_data* data;
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
> @@ -1250,50 +1246,37 @@ static int maven_detect_client(struct i2
> err = -ENOMEM;
> goto ERROR0;
> }
> - new_client = &data->client;
> - i2c_set_clientdata(new_client, data);
> - new_client->addr = address;
> - new_client->adapter = adapter;
> - new_client->driver = &maven_driver;
> - new_client->flags = 0;
> - strlcpy(new_client->name, "maven", I2C_NAME_SIZE);
> - if ((err = i2c_attach_client(new_client)))
> - goto ERROR3;
> - err = maven_init_client(new_client);
> + i2c_set_clientdata(client, data);
> + err = maven_init_client(client);
> if (err)
> goto ERROR4;
> return 0;
> ERROR4:;
> - i2c_detach_client(new_client);
> -ERROR3:;
> kfree(data);
> ERROR0:;
> return err;
> }
>
> -static int maven_attach_adapter(struct i2c_adapter* adapter) {
> - if (adapter->id == I2C_HW_B_G400)
> - return i2c_probe(adapter, &addr_data, &maven_detect_client);
> - return 0;
> -}
> -
> -static int maven_detach_client(struct i2c_client* client) {
> - int err;
> -
> - if ((err = i2c_detach_client(client)))
> - return err;
> +static int maven_remove(struct i2c_client *client)
> +{
> maven_shutdown_client(client);
> kfree(i2c_get_clientdata(client));
> return 0;
> }
>
> +static const struct i2c_device_id maven_id[] = {
> + { "maven", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, maven_id);
> +
> static struct i2c_driver maven_driver={
> .driver = {
> .name = "maven",
> },
> - .id = I2C_DRIVERID_MGATVO,
> - .attach_adapter = maven_attach_adapter,
> - .detach_client = maven_detach_client,
> + .probe = maven_probe,
> + .remove = maven_remove,
> + .id_table = maven_id,
> };
>
> static int __init matroxfb_maven_init(void)
> --- linux-2.6.26-rc9.orig/include/linux/i2c-id.h 2008-07-12 10:11:02.000000000 +0200
> +++ linux-2.6.26-rc9/include/linux/i2c-id.h 2008-07-12 10:24:26.000000000 +0200
> @@ -39,7 +39,6 @@
> #define I2C_DRIVERID_SAA7111A 8 /* video input processor */
> #define I2C_DRIVERID_SAA7185B 13 /* video encoder */
> #define I2C_DRIVERID_SAA7110 22 /* video decoder */
> -#define I2C_DRIVERID_MGATVO 23 /* Matrox TVOut */
> #define I2C_DRIVERID_SAA5249 24 /* SAA5249 and compatibles */
> #define I2C_DRIVERID_PCF8583 25 /* real time clock */
> #define I2C_DRIVERID_SAB3036 26 /* SAB3036 tuner */
> @@ -94,7 +93,6 @@
> #define I2C_HW_B_BT848 0x010005 /* BT848 video boards */
> #define I2C_HW_B_VIA 0x010007 /* Via vt82c586b */
> #define I2C_HW_B_HYDRA 0x010008 /* Apple Hydra Mac I/O */
> -#define I2C_HW_B_G400 0x010009 /* Matrox G400 */
> #define I2C_HW_B_I810 0x01000a /* Intel I810 */
> #define I2C_HW_B_VOO 0x01000b /* 3dfx Voodoo 3 / Banshee */
> #define I2C_HW_B_SCX200 0x01000e /* Nat'l Semi SCx200 I2C */
>
> --
> Jean Delvare
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
>


----------------------------------------------------------------------
Tylko dla detektywow! Konkurs na Smaker.pl
Kliknij >>> http://link.interia.pl/f1eb1

2008-08-08 21:01:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 0/3] matroxfb fixes and improvements

On Fri, 8 Aug 2008 16:49:37 +0200
Jean Delvare <[email protected]> wrote:

> Not having heard of Petr in one month and a half, I am sending these 3
> patches again. Patch 1/3 (matrox maven: Fix a broken error path) in
> particular should be applied ASAP. Patch 2/3 (matroxfb: i2c structure
> templates clean-up) would be good to have as well and I tested it.
> Patch 3/3 (matrox maven: Convert to a new-style i2c driver) needs
> testing before it can go upstream. I can't test it myself, but it has
> been in linux-next for 6 weeks or so.

I'll put all three into 2.6.27.

I suspect that #1 should be backported to 2.6.25.x and to 2.6.26.x but
I am unsure about that due to its poor changelogging.

I _think_ that the machine will crash if the i2c_attach_client() or
maven_init_client() calls fail. But I don't know under what
circumstances that can happen, nor whether the bug has been reported in
the field, etc.

And the -stable people also will be interested in such information when
making their decisions.

2008-08-09 08:58:27

by Jean Delvare

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 0/3] matroxfb fixes and improvements

Hi Andrew,

On Fri, 8 Aug 2008 14:00:19 -0700, Andrew Morton wrote:
> On Fri, 8 Aug 2008 16:49:37 +0200
> Jean Delvare <[email protected]> wrote:
>
> > Not having heard of Petr in one month and a half, I am sending these 3
> > patches again. Patch 1/3 (matrox maven: Fix a broken error path) in
> > particular should be applied ASAP. Patch 2/3 (matroxfb: i2c structure
> > templates clean-up) would be good to have as well and I tested it.
> > Patch 3/3 (matrox maven: Convert to a new-style i2c driver) needs
> > testing before it can go upstream. I can't test it myself, but it has
> > been in linux-next for 6 weeks or so.
>
> I'll put all three into 2.6.27.

Thank you.

> I suspect that #1 should be backported to 2.6.25.x and to 2.6.26.x but
> I am unsure about that due to its poor changelogging.
>
> I _think_ that the machine will crash if the i2c_attach_client() or
> maven_init_client() calls fail. But I don't know under what
> circumstances that can happen, nor whether the bug has been reported in
> the field, etc.

I found it while working on patch #3. I am not aware of it having been
reported in the field. The maven driver doesn't have that many users
(which is why I am struggling to get patch #3 tested.) And then the
errors in question are unlikely to happen. In fact, maven_init_client()
always returns 0, it simply cannot fail. So I'd say that the chances
that someone hits that bug are thin.

That would mean that we don't really need to backport the fix. But OTOH
it's a regression and the fix is pretty easy and not intrusive. So, I
think I would still backport it.

> And the -stable people also will be interested in such information when
> making their decisions.

I think I will let them decide.

--
Jean Delvare