| Message ID | 20090410170726.3616b6d5@hyperion.delvare (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Paul Mackerras |
| Headers | show |
On Fri, 10 Apr 2009 17:07:26 +0200, Jean Delvare wrote: > The legacy i2c binding model is going away soon, so convert the ppc > keywest sound driver to the new model or it will break. > > Signed-off-by: Jean Delvare <khali@linux-fr.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > sound/ppc/keywest.c | 81 +++++++++++++++++++++++++-------------------------- > 1 file changed, 40 insertions(+), 41 deletions(-) Ah, I forgot. You need the following patch applied before testing: ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-loosen-driver-check.patch > > --- linux-2.6.30-rc1.orig/sound/ppc/keywest.c 2009-04-10 15:22:18.000000000 +0200 > +++ linux-2.6.30-rc1/sound/ppc/keywest.c 2009-04-10 16:41:36.000000000 +0200 > @@ -33,26 +33,25 @@ > static struct pmac_keywest *keywest_ctx; > > > -static int keywest_attach_adapter(struct i2c_adapter *adapter); > -static int keywest_detach_client(struct i2c_client *client); > - > -struct i2c_driver keywest_driver = { > - .driver = { > - .name = "PMac Keywest Audio", > - }, > - .attach_adapter = &keywest_attach_adapter, > - .detach_client = &keywest_detach_client, > -}; > - > - > #ifndef i2c_device_name > #define i2c_device_name(x) ((x)->name) > #endif > > +static int keywest_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + i2c_set_clientdata(client, keywest_ctx); > + return 0; > +} > + > +/* > + * This is kind of a hack, best would be to turn powermac to fixed i2c > + * bus numbers and declare the sound device as part of platform > + * initialization > + */ > static int keywest_attach_adapter(struct i2c_adapter *adapter) > { > - int err; > - struct i2c_client *new_client; > + struct i2c_board_info info; > > if (! keywest_ctx) > return -EINVAL; > @@ -60,46 +59,46 @@ static int keywest_attach_adapter(struct > if (strncmp(i2c_device_name(adapter), "mac-io", 6)) > return 0; /* ignored */ > > - new_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL); > - if (! new_client) > - return -ENOMEM; > - > - new_client->addr = keywest_ctx->addr; > - i2c_set_clientdata(new_client, keywest_ctx); > - new_client->adapter = adapter; > - new_client->driver = &keywest_driver; > - new_client->flags = 0; > - > - strcpy(i2c_device_name(new_client), keywest_ctx->name); > - keywest_ctx->client = new_client; > + memset(&info, 0, sizeof(struct i2c_board_info)); > + strlcpy(info.type, "keywest", I2C_NAME_SIZE); > + info.addr = keywest_ctx->addr; > + keywest_ctx->client = i2c_new_device(adapter, &info); > > - /* Tell the i2c layer a new client has arrived */ > - if (i2c_attach_client(new_client)) { > - snd_printk(KERN_ERR "tumbler: cannot attach i2c client\n"); > - err = -ENODEV; > - goto __err; > - } > - > + /* > + * Let i2c-core delete that device on driver removal. > + * This is safe because i2c-core holds the core_lock mutex for us. > + */ > + list_add_tail(&keywest_ctx->client->detected, > + &keywest_ctx->client->driver->clients); > return 0; > - > - __err: > - kfree(new_client); > - keywest_ctx->client = NULL; > - return err; > } > > -static int keywest_detach_client(struct i2c_client *client) > +static int keywest_remove(struct i2c_client *client) > { > if (! keywest_ctx) > return 0; > if (client == keywest_ctx->client) > keywest_ctx->client = NULL; > > - i2c_detach_client(client); > - kfree(client); > return 0; > } > > + > +static const struct i2c_device_id keywest_i2c_id[] = { > + { "keywest", 0 }, > + { } > +}; > + > +struct i2c_driver keywest_driver = { > + .driver = { > + .name = "PMac Keywest Audio", > + }, > + .attach_adapter = keywest_attach_adapter, > + .probe = keywest_probe, > + .remove = keywest_remove, > + .id_table = keywest_i2c_id, > +}; > + > /* exported */ > void snd_pmac_keywest_cleanup(struct pmac_keywest *i2c) > { > >
On Fri, 10 Apr 2009 17:09:51 +0200, Jean Delvare wrote: > Ah, I forgot. You need the following patch applied before testing: > ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-loosen-driver-check.patch This patch is is Linus' tree now: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=935298696f469c0e07c73be687bd055878074ce0
Jean Delvare writes:
> The legacy i2c binding model is going away soon,
But not before 2.6.30, right?
Paul.
Hi Paul, On Wed, 15 Apr 2009 14:57:30 +1000, Paul Mackerras wrote: > Jean Delvare writes: > > > The legacy i2c binding model is going away soon, > > But not before 2.6.30, right? Ideally, yes, before 2.6.30. This is what Documentation/feature-removal-schedule.txt says: --------------------------- What: i2c_attach_client(), i2c_detach_client(), i2c_driver->detach_client(), i2c_adapter->client_register(), i2c_adapter->client_unregister When: 2.6.30 Check: i2c_attach_client i2c_detach_client Why: Deprecated by the new (standard) device driver binding model. Use i2c_driver->probe() and ->remove() instead. Who: Jean Delvare <khali@linux-fr.org> --------------------------- There are only 10 legacy i2c drivers remaining (not counting staging), 9 of which are ppc drivers. My hope was that ppc developers would see the deprecation warning and convert the drivers themselves, but it did not happen. Which is why I am trying to do it myself now, admittedly quite late in the 2.6.30 development cycle. This seems doable to me if my patches get proper testing fast (which seems to be the case, thanks!) and positive results (not true for onyx, but tas and therm_pm72 are apparently OK). Now I know I have to be realistic, if problems arise and/or ppc people feel too bad about me pushing these patches at about rc3 time, I will refrain from doing so and wait for 2.6.31. But there are a number or i2c-core improvements other developers are asking for for quite some times now, which depend on the removal of the legacy model, so delaying this wound not make me happy at all. Thanks,
--- linux-2.6.30-rc1.orig/sound/ppc/keywest.c 2009-04-10 15:22:18.000000000 +0200 +++ linux-2.6.30-rc1/sound/ppc/keywest.c 2009-04-10 16:41:36.000000000 +0200 @@ -33,26 +33,25 @@ static struct pmac_keywest *keywest_ctx; -static int keywest_attach_adapter(struct i2c_adapter *adapter); -static int keywest_detach_client(struct i2c_client *client); - -struct i2c_driver keywest_driver = { - .driver = { - .name = "PMac Keywest Audio", - }, - .attach_adapter = &keywest_attach_adapter, - .detach_client = &keywest_detach_client, -}; - - #ifndef i2c_device_name #define i2c_device_name(x) ((x)->name) #endif +static int keywest_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + i2c_set_clientdata(client, keywest_ctx); + return 0; +} + +/* + * This is kind of a hack, best would be to turn powermac to fixed i2c + * bus numbers and declare the sound device as part of platform + * initialization + */ static int keywest_attach_adapter(struct i2c_adapter *adapter) { - int err; - struct i2c_client *new_client; + struct i2c_board_info info; if (! keywest_ctx) return -EINVAL; @@ -60,46 +59,46 @@ static int keywest_attach_adapter(struct if (strncmp(i2c_device_name(adapter), "mac-io", 6)) return 0; /* ignored */ - new_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL); - if (! new_client) - return -ENOMEM; - - new_client->addr = keywest_ctx->addr; - i2c_set_clientdata(new_client, keywest_ctx); - new_client->adapter = adapter; - new_client->driver = &keywest_driver; - new_client->flags = 0; - - strcpy(i2c_device_name(new_client), keywest_ctx->name); - keywest_ctx->client = new_client; + memset(&info, 0, sizeof(struct i2c_board_info)); + strlcpy(info.type, "keywest", I2C_NAME_SIZE); + info.addr = keywest_ctx->addr; + keywest_ctx->client = i2c_new_device(adapter, &info); - /* Tell the i2c layer a new client has arrived */ - if (i2c_attach_client(new_client)) { - snd_printk(KERN_ERR "tumbler: cannot attach i2c client\n"); - err = -ENODEV; - goto __err; - } - + /* + * Let i2c-core delete that device on driver removal. + * This is safe because i2c-core holds the core_lock mutex for us. + */ + list_add_tail(&keywest_ctx->client->detected, + &keywest_ctx->client->driver->clients); return 0; - - __err: - kfree(new_client); - keywest_ctx->client = NULL; - return err; } -static int keywest_detach_client(struct i2c_client *client) +static int keywest_remove(struct i2c_client *client) { if (! keywest_ctx) return 0; if (client == keywest_ctx->client) keywest_ctx->client = NULL; - i2c_detach_client(client); - kfree(client); return 0; } + +static const struct i2c_device_id keywest_i2c_id[] = { + { "keywest", 0 }, + { } +}; + +struct i2c_driver keywest_driver = { + .driver = { + .name = "PMac Keywest Audio", + }, + .attach_adapter = keywest_attach_adapter, + .probe = keywest_probe, + .remove = keywest_remove, + .id_table = keywest_i2c_id, +}; + /* exported */ void snd_pmac_keywest_cleanup(struct pmac_keywest *i2c) {
The legacy i2c binding model is going away soon, so convert the ppc keywest sound driver to the new model or it will break. Signed-off-by: Jean Delvare <khali@linux-fr.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- sound/ppc/keywest.c | 81 +++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 41 deletions(-)