Enumerable I2C busses

[John Baldwin]

On Tuesday 06 January 2009 2:24:19 pm Nathan Whitehorn wrote:
> M. Warner Losh wrote:
> > In message: <492AC8DE.6050902 at freebsd.org>
> >             Nathan Whitehorn <nwhitehorn at freebsd.org> writes:
> > : M. Warner Losh wrote:
> > : > In message: <4929C6D8.7090305 at freebsd.org>
> > : >             Nathan Whitehorn <nwhitehorn at freebsd.org> writes:
> > : > : Rafał Jaworowski wrote:
> > : > : > 
> > : > : > On 2008-11-23, at 19:18, Dag-Erling Smørgrav wrote:
> > : > : > 
> > : > : >> Nathan Whitehorn <nwhitehorn at freebsd.org> writes:
> > : > : >>> The current I2C bus mechanism does not support the bus adding 
its own
> > : > : >>> children [...]
> > : > : >>
> > : > : >> That's because the I2C protocol does not support device 
enumeration or
> > : > : >> identification.  You have to know in advance what kind of devices 
are
> > : > : >> attached and at what address.  Even worse, it is not uncommon for
> > : > : >> similar but not entirely compatible devices to use the same I2C 
address
> > : > : >> (for instance, every I2C-capable RTC chip uses the same address, 
even
> > : > : >> though they have different feature sets)
> > : > : > 
> > : > : > Well, hard-coded addresses and conflicting assignments between 
vendors 
> > : > : > do not technically prevent from scanning the bus; actually, our 
current 
> > : > : > iicbus code can do bus scaning when compiled with a diag define. 
The 
> > : > : > problem however is some slave devices are not well-behaved, and 
they 
> > : > : > don't like to be read/written to other than in very specific 
scenario: 
> > : > : > if polled during bus scan strange effects occur e.g. they 
disappear from 
> > : > : > the bus, or do not react to consecutive requests etc.
> > : > : 
> > : > : All of this is true, but perhaps my question was badly worded. What 
I am 
> > : > : trying to figure out is how to shove information from an out-of-band 
> > : > : source (Open Firmware, in this case) into newbus without disrupting 
> > : > : existing code. In that way, my question is not I2C specific -- we 
run 
> > : > : into the same issue with the Open Firmware nexus node and 
pseudo-devices 
> > : > : like cryptosoft that attach themselves.
> > : > 
> > : > You are looking at the problem incorrectly.  In newbus, a case like
> > : > this the i2c bus should be a subclass (say i2c_of) that is derived
> > : > from the normal i2c bus stuff, but replaces the hints insertion of
> > : > devices with OF enumeration of devices.  The OF higher levels will
> > : > already know to attach this kind of i2c bus to certain i2c
> > : > controllers, or always on certain platforms.
> > : 
> > : Yes, this is exactly what I wanted to do, like how ofw_pci works.
> > : 
> > : > : What I want to do is to have the I2C bus add the children that the 
> > : > : firmware says it has. What the firmware cannot tell in advance, 
however, 
> > : > : is which FreeBSD driver is responsible for those devices, and so the 
I2C 
> > : > : bus driver can't know that without a translation table that I would 
> > : > : prefer not to hack in to the bus driver.
> > : > 
> > : > This is the bigger problem.  Today, we are stuck with a lame table
> > : > that will translate the OF provided properties into FreeBSD driver
> > : > names.
> > : 
> > : At the moment, I don't believe Apple uses any of the current very small 
> > : number of I2C device drivers in tree. So I may skip the table for the 
> > : time being, assuming the hack below is OK. In future, this may change, 
> > : since G5 systems require software thermal control. But that will be the 
> > : subject of another mail to this list...
> > : 
> > : > : It seems reasonable to allow devices to use a real probe routine to 
look 
> > : > : at the firmware's name and compatible properties, like we allow on 
other 
> > : > : Open Firmware busses. The trouble is that existing drivers don't do 
> > : > : this, because they expect to be attached with hints, so they will 
attach 
> > : > : to all devices. I'm trying to figure out how to avoid this.
> > : > : 
> > : > : My basic question comes down to whether there is a good way in 
newbus to 
> > : > : handle busses that may be wholly or partially enumerated by firmware 
or 
> > : > : some other method, and may also have devices that can only attach 
> > : > : themselves if told to by hints.
> > : > 
> > : > Yes.  This is a bit of a problem.  The problem is that the existing
> > : > hints mechanism combines device tree and driver tree into one, and in
> > : > such a scenario, we wind up with the problem that you have.
> > : > 
> > : > One could make the probe routines return BUS_PROBE_GENERIC, and that
> > : > would help somewhat.  One could also have the probe routine check to
> > : > see if a specific driver is assigned to the device or not.  That would
> > : > also help, but does mean changing all the i2c bus attached drivers in
> > : > the tree.
> > : 
> > : I think changing existing I2C drivers may be unavoidable. Would there be 
> > : any objection to changing the MI iicbus drivers to return 
> > : BUS_PROBE_NOWILDCARD in their probe routines? It seems to have been 
> > : introduced (by you) to solve more or less exactly this problem. By my 
> > : count, the relevant files are:
> > : dev/iicbus/ds133x.c
> > : dev/iicbus/icee.c
> > : dev/iicbus/ad7418.c
> > : dev/iicbus/iicsmb.c
> > : dev/iicbus/ds1672.c
> > : dev/iicbus/if_ic.c
> > : dev/iicbus/iic.c
> > : 
> > : I would also like to change iicbus_probe to return -1000 like 
> > : dev/pci/pci.c to allow it to be overridden by a subclass. Please let me 
> > : know if this is a terrible idea or if I have forgotten any I2C device 
> > : drivers.
> >
> > Short term, this is the right fix.  There was an objection, I think by
> > Marcel, to this approach.  However, his objections were part of a
> > larger set of objections and I think that we're working to solve
> > those.
> >
> > Warner
> >   
> This is now in the tree. Now for part 2, which I had not considered 
> previously: connecting the I2C bus layer to the I2C host adapters.
> 
> Right now, we have the following:
> kiic/other i2c adapters
> ---iicbus
> ---ofw_iicbus
> 
> Since kiic provides an Open Firmware node, ofw_iicbus gets priority, 
> attaches, and everything after that is wonderful. The issue is how best 
> to attach the iicbus modules to kiic. Current I2C controllers contain a 
> line like this:
> DRIVER_MODULE(iicbus, me, iicbus_driver, iicbus_devclass, 0, 0);
> 
> This explicitly specifies that you want the standard driver, so we need 
> additional glue to allow the ofw_iicbus driver to attach. One solution 
> is that each relevant host adapter can add an additional DRIVER_MODULE 
> line with the ofw_iicbus driver and class, which would have to exported 
> in a header somewhere. This is pretty ugly. Another solution is for the 
> ofw_iicbus module to grow a list of the names of interesting adapters. 
> This is worse.
> 
> The third option is to do what we do for pci, where all PCI adapters are 
> named 'pcib'. So we could make new I2C host adapters named 'iichb' or 
> something, and then move the DRIVER_MODULE logic for new code into the 
> bus modules, as is done for PCI. This decreases the amount of 
> information in the device names, but seems a bit cleaner. Thoughts?
> -Nathan

If ofw_iicbus is simply an OF-aware version of iicbus (i.e. same 
functionality) similar to the OF-aware PCI bus, then I would go the PCI route 
and just call it iicbus but give it a higher probe priority.

-- 
John Baldwin