[i2c] [patch 2.6.19-rc6 0/11] I2C driver model updates

[David Brownell]

On Friday 15 December 2006 8:51 am, Jean Delvare wrote:
> Hi David,
> 
> On Sat, 25 Nov 2006 13:40:41 -0800, David Brownell wrote:
> > Here's a refresh of some previous patches, updated per feedback, and
> > headed by some new core cleanups.
> > 
> > The first few should IMO be candidates for 2.6.20-early:
> > 
> >  - Core cleanup:  remove i2c_adapter.dev and non-class "i2c-adapter" class
> >  - i2-isa updates, it (needlessly) relied on those interfaces
> >  - I2C adapter updates, use i2c_adapter.class_dev.dev not &i2c_adapter.dev
> >  - I2C misc updates, non-adapter drivers need the same changes.
> 
> There's an organizational problem in this series. Each patch must
> constitute an incremental update where everything still compiles and
> works afterwards.

That is certainly the ideal; but as I'm sure you know, sometimes
patchsets get in where "works afterwards" holds after the whole
series but not individual patches (and thus "git bisect" loses).

That's especially true with API changes, which often can't be
done in digestible/reviewable chunks without such breakage.

Those particular patches started out with that "API change"; later
I noticed it wasn't needed in this case, which can be a LOT nicer.


> Here you start by deleting i2c_adapter.dev, and the 
> subsequent patches remove its users. That's the wrong way around, you
> must remove the users first, and only then you can remove
> i2c_adapter.dev.

The "series" file was updated later on, to address that very issue.
The tarball I sent puts the "remove" bits first ... for i2c drivers
outside the core:  h2mon, rtcs, and misc bits in drivers/rtc too.
Patch #13 in the series noted that re-ordering explicitly.

For adapters there's a little glitch I noticed a few days ago, and
you mention below.


> So basically your patch #1 should come after all the "don't use
> i2c_adapter.dev" patches.

And does, in more recent incarnations.  Modulo the glitch, which
I didn't fix since I was waiting on feedback for that first change


> But then there's another problem in patch #3: 
> this patch doesn't only remove users of i2c_adapter.dev, it also
> changes the way adapters declare their parent device. This family of
> changes really belongs to the i2c-core patch (#1), as one cannot go
> without the other.

Well, #3 should be split in two:  (a) the "remove users", which is most
of it, and (b) the change-init bits.  With (b) going with old #1, not (a),
to preserve the "still runs after each patch" rule.


> So, please rework this first patchset into something that I can apply
> incrementally.

OK.  And one that applies on top of 2.6.19-rc1 too.  Cleanups to
the i2c-nforce and it87 driver needed a bit of attention; also the
class device changes affected i2c-dev.


> The other patches we should discuss later, this first 
> change is large enough by itself.

Yeah, I thought they'd go best with a cleaner slate to build on.
And the "remove i2c_adapter.dev" bit should be the only _change_
since the rest of the patch is a bunch of additions.  :)

However, those additions were discussed a lot more, so I think they
don't merit as much discussion.


Thanks for the feedback.

- Dave


 
> >  - Add suspend()/resume()/shutdown() methods; document them
> > 
> > Any drivers that missed catching up to that core cleanup will generate
> > nice clean compile-time errors.  Stuff in drivers/i2c that compiles for
> > ARM or x86 PCs should behave already.
> > 
> > The others add the "new style" driver binding, and might need a bit more
> > cooking yet ... though IMO merging soon would be good, since that's needed
> > to start converting drivers (and by definition this can't affect "legacy"
> > drivers):
> > 
> >  - Add probe() method and associated hotplug/coldplug support
> >  - Add remove() method
> >  - Doc updates to match those new methods
> >  - Add and use i2c_board_info infrastructure, for "new style" drivers
> >  - i2c-omap update, uses new i2c_register_adapter()
> >  - Convert one board (OMAP5912 OSK) to use the new style drivers
> > 
> > There are of course other patches that should follow, including a few
> > more i2c core cleanups and updates to make various boards and drivers
> > use this new infrastructure, but this is enough to get the ball rolling.
> > 
> > Once it's rolling, serious discussions about how to convert legacy drivers
> > to be more compatible with the driver model can usefully proceed.  I expect
> > such discussions will take quite a while to sort out.
> 
> Thanks,
> -- 
> Jean Delvare
>