diff mbox

keywest: Convert to new-style i2c driver

Message ID 20090420225659.7b92a184@hyperion.delvare (mailing list archive)
State Accepted, archived
Delegated to: Paul Mackerras
Headers show

Commit Message

Jean Delvare April 20, 2009, 8:56 p.m. UTC
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>
Cc: Takashi Iwai <tiwai@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(-)

Comments

Paul Mackerras April 20, 2009, 10:34 p.m. UTC | #1
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.
Takashi Iwai April 21, 2009, 6:31 a.m. UTC | #2
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 <khali@linux-fr.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Takashi Iwai <tiwai@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
Takashi Iwai April 21, 2009, 6:33 a.m. UTC | #3
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
Jean Delvare April 21, 2009, 9:23 a.m. UTC | #4
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 <khali@linux-fr.org>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Takashi Iwai <tiwai@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.
Takashi Iwai April 21, 2009, 9:30 a.m. UTC | #5
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 <khali@linux-fr.org>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Takashi Iwai <tiwai@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
Paul Mackerras April 21, 2009, 9:33 a.m. UTC | #6
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.
Takashi Iwai April 21, 2009, 9:36 a.m. UTC | #7
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
Jean Delvare April 21, 2009, 9:44 a.m. UTC | #8
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 <khali@linux-fr.org>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Takashi Iwai <tiwai@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 April 21, 2009, 9:48 a.m. UTC | #9
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,
Wolfram Sang April 21, 2009, 9:56 a.m. UTC | #10
> 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!
Paul Mackerras April 22, 2009, 9:34 a.m. UTC | #11
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.
Jean Delvare April 22, 2009, 12:04 p.m. UTC | #12
Hi Paul,

On Wed, 22 Apr 2009 19:34:40 +1000, Paul Mackerras wrote:
> 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.

We're past -rc3 because people discuss instead of testing my patches.
Otherwise everything would be merged already.

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.

> 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. :)

What do you think, that I don't know about linux-next? My i2c tree is
already there. It has been since linux-next exists.

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.

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. What matters is that the patches can't go anywhere before they
have been tested. So, can the powerpc people please test my
patches? You (powerpc developers, not you Paul in particular) didn't
pay attention to the deprecation warnings. I did warn Ben that this
would become a problem during the Kernel Summit last year in Portland
though, but apparently it did not help. I would expect that you at
least help me with testing once I have done all the conversion work.

Out of the 10 remaining legacy i2c drivers, 9 are powerpc drivers!

> > 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.

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. One of these two events will
happen still. The real world isn't as perfect as we'd like.

So, once again, can powermac developers/users please test my patches?

Thanks,
Paul Mackerras April 22, 2009, 12:56 p.m. UTC | #13
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.
Takashi Iwai April 22, 2009, 2:24 p.m. UTC | #14
At Wed, 22 Apr 2009 22:56:52 +1000,
Paul Mackerras wrote:
> 
> 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". :)

Well, I believe it's even better to merge this conversion patch now,
from practical viewpoint.

If we get a regression for this particular device, the patch can be
simply reverted as long as the legacy i2c framework exists.  If we
merge everything, i.e. the conversion and the removal of old i2c
binding, at the same time in 2.6.31 merge window, reverting is much
harder.

So, I agree with that the removal of old i2c binding can be postponed
to 2.6.31.  It can be done on linux-next from now on, and we can keep
watching.  But, this small conversion patch can be more easily managed
in 2.6.30.


thanks,

Takashi
Jean Delvare April 23, 2009, 9:54 a.m. UTC | #15
Hi Paul,

On Wed, 22 Apr 2009 22:56:52 +1000, Paul Mackerras wrote:
> Jean Delvare writes:
> > 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.

I had the hope that the developers/maintainers involved would not
ignore the warnings and had patches ready, that would go to Linus
during the merge window. This didn't happen, call me naive if you want.
So right after rc1 I started working on the conversion myself. 

> > 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". :)

Regressions happen all the time, no matter how hard we all try to avoid
them. As long as they are fixed before -final, this isn't a big problem
though.

> > 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. :)

I don't consider breaking the linux-next build a good practice, sorry.
I suspect this tree doesn't get a lot of testing, breaking it isn't
going to improve the situation. Not to mention that this will make
Stephen's life harder, which isn't my goal. I do have a list of drivers
that aren't yet converted upstream, I don't need linux-next to tell me.
Except for new drivers, of course, there I agree your strategy has
value (but has a flaw, see below.)

> > 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?

3 months ago, people told me "what's so special about 2.6.29". The
result is that we made very little progress and the legacy interface
was still used by 10 (non-staging) drivers in 2.6.30-rc1. There's
nothing special about 2.6.30 other than the fact that I am fed up
waiting for all drivers to drop using a deprecated API so that I can
remove it. Again, further i2c developments are blocked by the existence
of this deprecated API.

> As for blocking development, you can remove the legacy interfaces
> today in your next branch and get on with development immediately.

No, I can't, that would break the linux-next build, as explained above.
I would also have to include all the driver conversion patches there.
The problem is that this still touches drivers/macintosh,
drivers/media/video and sound/ppc at the moment, each of which is
supposed to be maintained in a different tree. If I include these
patches in my i2c tree, chances of collisions with other trees are big,
which means more work for Stephen. If the patches are instead included
in their respective trees (as Takashi just did for sound/ppc), you
avoid the collisions, but then I can't remove the legacy API in
linux-next, because the i2c tree is listed relatively early, so
bisecting (or just incrementally building) linux-next would fail
horribly.

So, as you can see, the problem you think is so easily solved by
linux-next, isn't. The only thing I can do at this point is keep the
patches in a local tree and point other interested developers thereto.
Basically I'll tell them "you need to use linux-next plus a number of
public patches that can't go to linux-next plus the development patches
we are discussing." This makes the work and discussions about further
developments much more difficult, and results in zero testing outside
the development group, which makes it unclear if they will be ready for
2.6.31.

Compare this with the case where all driver conversions are already in
Linus' tree (even without removing the legacy interface): I can put the
legacy interface removal and all the development patches in linux-next
and interested people can test just this, and this is build tested on
several architectures, and possibly even runtime-tested by a few random
users. This is way less work for everyone and a much better quality in
the end.

> 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.

But I would be able to get the new features in 2.6.31. With your plan,
I doubt this can happen.

> And this is a particularly bad time to be hastily trying to get
> powermac driver changes upstream, since Ben H. is on vacation.

Yes, that I admit is pretty bad luck :(

> > 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.

"Before the 2.6.31 merge window" is way too late if I want any i2c
development to make it into 2.6.31 as well. The conversions must go in
linux-next as soon as possible. Even that isn't ideal as I explained
above, but that's the bare minimum to make sure everything (driver
conversions, interface removal and i2c developments) has a chance to be
ready for 2.6.31.

Note that I did get some more test results meanwhile. Johannes Berg
was able to test the windfarm drivers conversion, and Andreas Schwab
tested the therm_adt746x conversion, both successfully. So the only two
drivers which didn't get any testing at this point are keywest
(snd-powermac) and therm_windtunnel. That being said, more testing for
every driver is certainly welcome. You can check the current status at:
http://i2c.wiki.kernel.org/index.php/Legacy_drivers_to_be_converted

I agree to stop pushing driver conversions to Linus for 2.6.30, which
means the legacy API will stay there. Some drivers have already been
converted though (go7007 in rc3) and some conversions are scheduled to
go to Linus already (sound drivers in rc4, Takashi said.) This means 7
remaining unconverted drivers in 2.6.30 (6 powermac thermal management
drivers, and ir-kbd-i2c.)
diff mbox

Patch

--- linux-2.6.30-rc2.orig/sound/ppc/keywest.c	2009-04-20 22:40:27.000000000 +0200
+++ linux-2.6.30-rc2/sound/ppc/keywest.c	2009-04-20 22:47:34.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,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)
 {