[i2c] [patch/rfc 2.6.19-rc6 1/3] I2C "new style" driver model type binding

David Brownell david-b at pacbell.net
Mon Nov 20 00:50:57 CET 2006


> Date: Sun, 19 Nov 2006 16:38:48 +0100
> From: Jean Delvare <khali at linux-fr.org>
>
> On Fri, 17 Nov 2006 13:31:43 -0800, David Brownell wrote:
> > On Friday 17 November 2006 7:37 am, Jean Delvare wrote:
> > 
> > > >  - Update i2c_device_match() to handle new style driver binding
> > > >    by comparing
> > > >    client and driver names (like platform and SPI busses).
> > > 
> > > This won't work in the general case, as i2c chip drivers can support up
> > > to 8 different chip types.
> > 
> > Where did "8" come from?  My at24c eeprom driver handles 13 in its
> > current incarnation, and I know there are others it could handle.
>
> Because of the I2C_CLIENT_INSMOD mess, we needed one different macro for
> each number of devices we wanted to support in a driver, and at 8 we
> felt it was enough. It's an arbitrary number, I didn't mention it here
> as being meaningful. And I misexpressed myself, it's not actually the
> number of devices a driver can support, but rather the number of
> devices the driver can handle differently.

More particularly, it's the number of device types the CLIENT_INSMOD
stuff handles.  For now, drivers using the driver model infrastructure
have no need for that "legacy" CLIENT_INSMOD probe infrastructure, so
that particular issue can't affect them.


>		My point is simply that you
> can't rely on driver.name == device.name in i2c_device_match(), it is
> too restrictive for our needs.

That's arguable.  In fact you already agreed with some of the basic
counter-arguments!


> > Chip type information is best suited to the platform_data field, in
> > the common case where hardware probes are insufficient (or unsafe).
>
> Granted, but in the case where hardware probes are sufficient, no
> platform data will be provided.

And if they're "sufficient", then platform_data isn't needed either...
The driver's probe() could poke the i2c chip to find out whatever extra
information it needs to find.


>	We just can't provide platform data
> information for the 2000+ PC motherboards out there.
>
> I don't know yet how this will be handled when we migrate such code to
> the driver model-compliant code you're adding now,

That's going to take some time to sort out.

My immediate priority is to (finally) have a solution that works on
the embedded platforms that have been experiencing such trouble.
(OMAP comes to mind.)  They will convert ASAP.

Conveniently, it seems to me that the chips they use don't really
overlap with those used on (LOTS more than 2k!) PC motherboards.
Except for EEPROMs, and as you know the current "eeprom" driver
doesn't support those well.  (It assumes all eeproms are 24c02 chips,
without write support, etc.)

Then comes the question of how/whether to update the "legacy" code.
You've evidently had some of the same thoughts I've had in that area...


>		but one possibility
> would be to have a dedicated kernel module probing the motherboard
> SMBus, identifying the supported devices, and instanciating the
> devices. Then the required drivers could be automatically loaded.

That's the model I'd rather see investigated first.  The proposals
from Nathan and Allessandro both added addressing information to
the driver struct (though still in bloated CLIENT_INSMOD format).
For busses that support SMBUS_QUICK, that information should suffice
to support speedy driver conversions.


> Another way would be to let the drivers probe and instanciate their own
> device, old-style, but then we would not be able to load the needed
> drivers automatically.

That's true; and it would also be error prone.  I'd be happier saying
that drivers which "need" to instantiate their own devices should stick
to the legacy APIs.  (And that we'd work to minimize that need.)


> > I2C doesn't have such a managaged ID namespace.  Which is why the
> > simple solution seemed to be to reuse the approach Linux uses in
> > similar cases.  It's not necessarily the _best_ approach, but it's
> > one that works well and has precedents in Linux.  Plus it needs
> > no design effort, and no special education/training.
>
> I'm fine with using simple strings to identify I2C device types, but
> then each i2c driver must provide a list of supported device types (as a
> list of strings) and device.name must be matched against this list,
> rather than driver.name.

Well, not "must" (as you agreed above); that can all be done by a
driver checking information in the platform_data.  Or in some cases,
potentially by talking to the device during driver probe().


> > (Also, adding new device id table types gets really messy, fast,
> > since it requires modutils updates.  That complicates deployment
> > by quite a lot.)
>
> But how do you automatically load the right drivers without these 
> tables?

That's working already, courtesy of the MODALIAS hook in the uevent()
call, and the modalias attribute.  When userspace is running, hotplug
does "modprobe -q $MODALIAS".  For devices configured before then,
coldplug does "modprobe -q $(cat modalias)".  How?  Because that works
out directly to "modprobe -q driver_name" ... don't be deceived by
the fancy match algorithms that USB or PCI use, or the way they use
module aliases that embed wildcards.


> > > >  - Add device modalias attribute, for coldplug support; switch
> > > >    over to use
> > > >    attribute groups ("name" and "modalias", contents are the same).
> > > 
> > > Why duplicate it if it's the same?
> > 
> > The convention is to call it "modalias", so that for example coldplug
> > scripts (like the ones I use) can just scan /sys/devices after userspace
> > is running and just "modprobe $(cat modalias)" for driverless devices,
> > without needing to add bus-specific logic.
>
> I see, it's a nice feature. Should we then (much) later delete the
> "name" sysfs file?

Probably.  Though it might be that the client->name field isn't
always being used in quite this way; we might want to rename that
to "modalias" and make sure there's no longer potential confusion.


> Granted, splitting related things doesn't help. I just meant that 20+
> kB patches aren't easy to review and test, and making them smaller
> where possible will make it easier for me to review and merge your work.

You'll like the upcoming patches better then; most are under 9K.


> So, SMBus adds some semantics on top of I2C, but there's still some
> margin for chips to behave differently. And many I2C chips use the
> SMBus semantics too. So there again, the boundary between I2C chips and
> SMBus chips is a fuzzy one.

Other than electrical constraints, sure.  Are you now saying that
the messaging protocol is fuzzy in only one way though ... that
SMBus is always a message-wise subset of I2C?  (Excepting ARP.)

- Dave





More information about the i2c mailing list