Re: [PATCH] keywest: Convert to new-style i2c driver
Jean Delvare writes: Not removing it now has a high risk of developers continuing to ignore the deprecation warnings and adding new legacy drivers, which I then must convert to the new model. This never ends. I know my behavior may seem a bit rude, but apparently this is the only way to get things to actually happen. I've been waiting for over a year already! I sympathize, but throwing disruptive changes into Linus' tree when we're past -rc3 is not the way to solve the problem. The way to solve the problem is to (a) publish a branch where you put the stuff you're going to ask Linus to pull in the next merge window, (b) push a commit there that removes the legacy interfaces, (c) ask Stephen Rothwell to include that branch in linux-next. The linux-next tree gets built for a wide range of architectures and configs, and any breakages get noticed and fixed pretty quickly. Getting the removal of the legacy interfaces into linux-next will do more in a week than a year's worth of deprecation warnings. :) I don't think the risk is that high, at least not for sound drivers. The conversions are fairly easy and if something really went wrong, fixing it is a matter of minutes. We're past -rc3 now. This is not the time for pushing this sort of change into Linus' tree. Ask Linus if you don't believe me. I have converted all remaining drivers by now: http://i2c.wiki.kernel.org/index.php/Legacy_drivers_to_be_converted It's really only a matter of getting them tested in time now. Given that most drivers are powermac ones, what I really need here is powermac users/maintainers to test my patches and report success or failure. OK, but please work in your next branch and linux-next, not Linus' tree. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
Jean Delvare writes: I sympathize, but throwing disruptive changes into Linus' tree when we're past -rc3 is not the way to solve the problem. We're past -rc3 because people discuss instead of testing my patches. Otherwise everything would be merged already. Well, no. The first conversion patch that I saw was posted after the merge window had already closed, on 8 April. And really, these changes (sound drivers) don't qualify as disruptive. You might argue about the thermal management driver changes if you want. But sound drivers, nothing bad will happen if they accidentally break. That's what we call a regression. :) But linux-next will only build-test the code. That I have already done, so it really doesn't buy my anything. Developers (including me) don't look at warnings in linux-next, and I don't think linux-next gets a lot of testing. If you remove the legacy interfaces then even a build test will point out the drivers that still need to be converted. :) Also, I can't push all untested patches to linux-next. In particular the 4 patches that touch thermal management on powermac, need to be tested successfully by at least one person before I can push them. You did test the therm_pm72 patch, and I thank you for that, so that one is in linux-next, but the other 3 ones need testing. So, really, you're trying to solve the wrong problem. Whether the patches go to Linus now or in the next merge window, doesn't really matter. It does matter, actually. And 2.6.31 isn't the kernel to remove an interface which was scheduled for removal in 2.6.29 and then 2.6.30 and which is blocking the development of features people need badly. What's so special about 2.6.30 that it matters whether the legacy interfaces are still there or not? As for blocking development, you can remove the legacy interfaces today in your next branch and get on with development immediately. So the blocking development argument has zero relevance to what goes into Linus' tree for 2.6.30. Even if you got the legacy interfaces removed for 2.6.30 you still wouldn't be able to get any new features based on that into 2.6.30. And this is a particularly bad time to be hastily trying to get powermac driver changes upstream, since Ben H. is on vacation. So, once again, can powermac developers/users please test my patches? Can we compromise on this? I'll do everything I reasonably can to help you get the powermac driver patches tested and working before the 2.6.31 merge window, if you agree to leave the drivers and interfaces in Linus' tree as they are for 2.6.30. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
At Mon, 20 Apr 2009 22:56:59 +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 kh...@linux-fr.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Takashi Iwai ti...@suse.de --- Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I did not get any test report for this one, but it's relatively simple so I am confident I got it right. Applied this one, too. Thanks. Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
At Tue, 21 Apr 2009 08:34:02 +1000, Paul Mackerras wrote: Jean Delvare writes: Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I really don't think you can remove it from Linus' tree at this stage in the 2.6.30 cycle. If it was going to be removed it should have been removed in the merge window. Removing it now has too much risk of introducing regressions in my opinion. I presume you have a development tree where you queue up commits for the i2c subsystem for the next merge window. I suggest you do the removal there now (or whenever you like) and push it to Linus in the next merge window. At least, the conversion patch Jean posted can be in 2.6.30, I think. As the old API is marked deprecated, it should be fixed sooner or later. Whether to remove the whole old i2c-binding in 2.6.30 is a different question, although I myself feel it's feasible. thanks, Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
Takashi Iwai writes: At least, the conversion patch Jean posted can be in 2.6.30, I think. Really? What regression, security hole or serious bug are you going to tell Linus that it fixes? :) Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
On Tue, 21 Apr 2009 11:23:00 +0200, Jean Delvare wrote: On Tue, 21 Apr 2009 08:31:00 +0200, Takashi Iwai wrote: At Mon, 20 Apr 2009 22:56:59 +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 kh...@linux-fr.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Takashi Iwai ti...@suse.de --- Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I did not get any test report for this one, but it's relatively simple so I am confident I got it right. Applied this one, too. Thanks. Thanks Takashi. BTW, I forgot to mention again that the new i2c binding model is functional since kernel 2.6.25, so for the external alsa driver tree you will want to guard these changes with appropriate ifdef magic. Err, make that 2.6.30, sorry. While the infrastructure was there since 2.6.25, the way I converted the sound drivers doesn't fit in what earlier kernels considered acceptable (the checks on which driver methods were implemented was a little too strict IMHO). So the converted drivers can only be used with kernels = 2.6.30. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
Hi Paul, Takashi, On Tue, 21 Apr 2009 08:33:43 +0200, Takashi Iwai wrote: At Tue, 21 Apr 2009 08:34:02 +1000, Paul Mackerras wrote: Jean Delvare writes: Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I really don't think you can remove it from Linus' tree at this stage in the 2.6.30 cycle. If it was going to be removed it should have been removed in the merge window. It would have happened if the developers/maintainers who see deprecation warnings when they build their drivers had paid attention to them. And actually some did, but not all. The remaining drivers are the ones nobody cared about, and this is the reason why I have to take care of them myself, that late in the development cycle. Note that the removal had already been scheduled for 2.6.29, and it did not happen. The set of legacy drivers did shrink a bit between 2.6.29-rc1 and 2.6.30-rc1 (thanks to Hans Verkuil) but not as much as it should have. Basically the number of remaining driver was halved. If the number of remaining drivers is halved with each release cycle, the legacy model is never going to be removed ;) Removing it now has too much risk of introducing regressions in my opinion. Not removing it now has a high risk of developers continuing to ignore the deprecation warnings and adding new legacy drivers, which I then must convert to the new model. This never ends. I know my behavior may seem a bit rude, but apparently this is the only way to get things to actually happen. I've been waiting for over a year already! I don't think the risk is that high, at least not for sound drivers. The conversions are fairly easy and if something really went wrong, fixing it is a matter of minutes. Please note that the removal of the legacy model isn't my goal per se. The fact is that the legacy model needs to be removed for further developments of i2c-core to happen, in particular the support of i2C bus multiplexers. There are patches waiting for inclusion since early February, which I can't take as long as the legacy i2c model is in. This is why I am pushing. I presume you have a development tree where you queue up commits for the i2c subsystem for the next merge window. I suggest you do the removal there now (or whenever you like) and push it to Linus in the next merge window. At least, the conversion patch Jean posted can be in 2.6.30, I think. As the old API is marked deprecated, it should be fixed sooner or later. Whether to remove the whole old i2c-binding in 2.6.30 is a different question, although I myself feel it's feasible. I have converted all remaining drivers by now: http://i2c.wiki.kernel.org/index.php/Legacy_drivers_to_be_converted It's really only a matter of getting them tested in time now. Given that most drivers are powermac ones, what I really need here is powermac users/maintainers to test my patches and report success or failure. Thanks, -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
Not removing it now has a high risk of developers continuing to ignore the deprecation warnings and adding new legacy drivers, which I then must convert to the new model. This never ends. I know my behavior may seem a bit rude, but apparently this is the only way to get things to actually happen. I've been waiting for over a year already! I don't think the risk is that high, at least not for sound drivers. The conversions are fairly easy and if something really went wrong, fixing it is a matter of minutes. Please note that the removal of the legacy model isn't my goal per se. The fact is that the legacy model needs to be removed for further developments of i2c-core to happen, in particular the support of i2C bus multiplexers. There are patches waiting for inclusion since early February, which I can't take as long as the legacy i2c model is in. This is why I am pushing. Full ACK! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
At Tue, 21 Apr 2009 19:33:00 +1000, Paul Mackerras wrote: Takashi Iwai writes: At least, the conversion patch Jean posted can be in 2.6.30, I think. Really? What regression, security hole or serious bug are you going to tell Linus that it fixes? :) Build warning fixes :) Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
On Tue, 21 Apr 2009 08:31:00 +0200, Takashi Iwai wrote: At Mon, 20 Apr 2009 22:56:59 +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 kh...@linux-fr.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Takashi Iwai ti...@suse.de --- Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I did not get any test report for this one, but it's relatively simple so I am confident I got it right. Applied this one, too. Thanks. Thanks Takashi. BTW, I forgot to mention again that the new i2c binding model is functional since kernel 2.6.25, so for the external alsa driver tree you will want to guard these changes with appropriate ifdef magic. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
At Tue, 21 Apr 2009 11:23:00 +0200, Jean Delvare wrote: On Tue, 21 Apr 2009 08:31:00 +0200, Takashi Iwai wrote: At Mon, 20 Apr 2009 22:56:59 +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 kh...@linux-fr.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Takashi Iwai ti...@suse.de --- Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I did not get any test report for this one, but it's relatively simple so I am confident I got it right. Applied this one, too. Thanks. Thanks Takashi. BTW, I forgot to mention again that the new i2c binding model is functional since kernel 2.6.25, so for the external alsa driver tree you will want to guard these changes with appropriate ifdef magic. I thought some patch (applied on 2.6.30) was needed to get them working properly? Anyway, I already added ifdef in alsa-driver external tree to make 2.6.29 and earlier building with the old i2c code. thanks, Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] keywest: Convert to new-style i2c driver
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 kh...@linux-fr.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Takashi Iwai ti...@suse.de
---
Takashi, please push this patch to Linus quickly, as this is blocking
the removal of the legacy i2c binding model, which is scheduled for
2.6.30.
I did not get any test report for this one, but it's relatively simple
so I am confident I got it right.
sound/ppc/keywest.c | 82 +--
1 file changed, 41 insertions(+), 41 deletions(-)
--- linux-2.6.30-rc2.orig/sound/ppc/keywest.c 2009-04-20 22:40:27.0
+0200
+++ linux-2.6.30-rc2/sound/ppc/keywest.c2009-04-20 22:47:34.0
+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,47 @@ 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)
{
+ i2c_set_clientdata(client, NULL);
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)
{
--
Jean Delvare
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] keywest: Convert to new-style i2c driver
Jean Delvare writes: Takashi, please push this patch to Linus quickly, as this is blocking the removal of the legacy i2c binding model, which is scheduled for 2.6.30. I really don't think you can remove it from Linus' tree at this stage in the 2.6.30 cycle. If it was going to be removed it should have been removed in the merge window. Removing it now has too much risk of introducing regressions in my opinion. I presume you have a development tree where you queue up commits for the i2c subsystem for the next merge window. I suggest you do the removal there now (or whenever you like) and push it to Linus in the next merge window. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
