[i2c] [patch 2.6.20-rc1 6/6] i2c driver suspend()/resume()/shutdown() support

David Brownell david-b at pacbell.net
Fri Jan 5 02:16:52 CET 2007


> >  - Add new suspend(), resume(), and shutdown() methods.  Use them in the
> >    standard driver model style; document them.
>
> Is there any driver using this? Usually we want at least one driver to
> use a feature before we add that feature to the kernel. Although here I
> understand that what you're adding is pretty standard.

More ... it's *MISSING BASIC FUNCTIONALITY* for the driver model.

Because of the chicken/egg scenario, drivers won't be updated
(and get system-wide integration retesting) until after the
mechanism is first put in place.

Over the past couple years there are probably at least half a dozen
I2C drivers I've touched that should be using suspend()/resume()
calls, but that never got updated since I2C doesn't support that.
I don't have access to most of that hardware any more.  Not all
those drivers are upstream, either.  Realistically, none of them
can start using i2c suspend/resume until it arrives via "git pull". 

And the hardware _I've_ got which needs I2C suspend/resume support
and has been merged upstream can't possibly kick it in until after
those "Phase II" patches merge...



> > -* [Copyright] Use (C), not (c), for copyright.
> > +* [Copyright] Use (C), not (c), for copyright.  Provide dates and names:
> > +  "Copyright (C) 2006 you", "Copyright (C) 1998,2004-2006 company", etc
>
> Please back this one out, it's really not specific to i2c drivers and
> at any rate doesn't belong to this patch.

In that case the comment about (C) vs (c) doesn't belong in that
file either ... maybe you should just remove it entirely?  That
file is full of advice not specific to I2C, for that matter.


> >  static int i2c_device_match(struct device *dev, struct device_driver *drv)
> >  {
> > -	return 1;
> > +	/* (for now) bypass driver model probing entirely; drivers
> > +	 * scan each i2c adapter/bus themselves.
> > +	 */
> > +	return 0;
> >  }
>
> I'm a bit confused by this one, the match was always succeeding, now
> it's never succeeding and... it doesn't make any difference?

Exactly.  The previous comment was wrong.  This particular patch is
about implementing core driver model mechanisms correctly.

And the reason to have it in this patch was so that these trivial
fixes could be merged into 2.6.20 ... it was originally submitted
before the 2.6.20 merge window even opened.  I was hoping to see
I2C drivers start to obey the driver model, at least in these small
but often-important ways, before next summer. :(


>	This
> change doesn't appear to belong to this patch, if it makes any sense at
> all.

I suppose I can reissue this patch.  For the sixth (?) time.
Then this particular change would be the first part of "Part II"

Redoing all those "Part II" patches (again) will be a real PITA.


> What about moving i2c_device_suspend() and i2c_device_resume() down
> after i2c_device_shutdown() to group related functions together?

ISTR that doing that doubled the size of this patch, and made it
all but unreadable, for some reason known only to "diff".

The shutdown() isn't actually related to suspend()/resume(), except
that this patch adds all three missing driver model mechanisms at
the same time.

- Dave




More information about the i2c mailing list