Date: Wed, 21 Jan 2009 11:10:33 -0500 From: John Baldwin <jhb@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: Re: Enumerable I2C busses Message-ID: <200901211110.33961.jhb@freebsd.org> In-Reply-To: <49773596.2050700@freebsd.org> References: <4929C6D8.7090305@freebsd.org> <200901210843.33247.jhb@freebsd.org> <49773596.2050700@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 21 January 2009 9:47:50 am Nathan Whitehorn wrote: > John Baldwin wrote: > > On Tuesday 06 January 2009 2:24:19 pm Nathan Whitehorn wrote: > >> M. Warner Losh wrote: > >>> In message: <492AC8DE.6050902@freebsd.org> > >>> Nathan Whitehorn <nwhitehorn@freebsd.org> writes: > >>> : M. Warner Losh wrote: > >>> : > In message: <4929C6D8.7090305@freebsd.org> > >>> : > Nathan Whitehorn <nwhitehorn@freebsd.org> writes: > >>> : > : Rafa=C5=82 Jaworowski wrote: > >>> : > : >=20 > >>> : > : > On 2008-11-23, at 19:18, Dag-Erling Sm=C3=B8rgrav wrote: > >>> : > : >=20 > >>> : > : >> Nathan Whitehorn <nwhitehorn@freebsd.org> writes: > >>> : > : >>> The current I2C bus mechanism does not support the bus addi= ng=20 > > its own > >>> : > : >>> children [...] > >>> : > : >> > >>> : > : >> That's because the I2C protocol does not support device=20 > > enumeration or > >>> : > : >> identification. You have to know in advance what kind of=20 devices=20 > > are > >>> : > : >> attached and at what address. Even worse, it is not uncommo= n=20 for > >>> : > : >> similar but not entirely compatible devices to use the same = I2C=20 > > address > >>> : > : >> (for instance, every I2C-capable RTC chip uses the same=20 address,=20 > > even > >>> : > : >> though they have different feature sets) > >>> : > : >=20 > >>> : > : > Well, hard-coded addresses and conflicting assignments betwee= n=20 > > vendors=20 > >>> : > : > do not technically prevent from scanning the bus; actually, o= ur=20 > > current=20 > >>> : > : > iicbus code can do bus scaning when compiled with a diag defi= ne.=20 > > The=20 > >>> : > : > problem however is some slave devices are not well-behaved, a= nd=20 > > they=20 > >>> : > : > don't like to be read/written to other than in very specific= =20 > > scenario:=20 > >>> : > : > if polled during bus scan strange effects occur e.g. they=20 > > disappear from=20 > >>> : > : > the bus, or do not react to consecutive requests etc. > >>> : > :=20 > >>> : > : All of this is true, but perhaps my question was badly worded.= =20 What=20 > > I am=20 > >>> : > : trying to figure out is how to shove information from an=20 out-of-band=20 > >>> : > : source (Open Firmware, in this case) into newbus without=20 disrupting=20 > >>> : > : existing code. In that way, my question is not I2C specific -- = we=20 > > run=20 > >>> : > : into the same issue with the Open Firmware nexus node and=20 > > pseudo-devices=20 > >>> : > : like cryptosoft that attach themselves. > >>> : >=20 > >>> : > You are looking at the problem incorrectly. In newbus, a case li= ke > >>> : > 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. > >>> :=20 > >>> : Yes, this is exactly what I wanted to do, like how ofw_pci works. > >>> :=20 > >>> : > : What I want to do is to have the I2C bus add the children that = the=20 > >>> : > : firmware says it has. What the firmware cannot tell in advance,= =20 > > however,=20 > >>> : > : is which FreeBSD driver is responsible for those devices, and s= o=20 the=20 > > I2C=20 > >>> : > : bus driver can't know that without a translation table that I=20 would=20 > >>> : > : prefer not to hack in to the bus driver. > >>> : >=20 > >>> : > This is the bigger problem. Today, we are stuck with a lame table > >>> : > that will translate the OF provided properties into FreeBSD driver > >>> : > names. > >>> :=20 > >>> : At the moment, I don't believe Apple uses any of the current very=20 small=20 > >>> : number of I2C device drivers in tree. So I may skip the table for t= he=20 > >>> : time being, assuming the hack below is OK. In future, this may chan= ge,=20 > >>> : since G5 systems require software thermal control. But that will be= =20 the=20 > >>> : subject of another mail to this list... > >>> :=20 > >>> : > : It seems reasonable to allow devices to use a real probe routin= e=20 to=20 > > look=20 > >>> : > : at the firmware's name and compatible properties, like we allow= on=20 > > other=20 > >>> : > : Open Firmware busses. The trouble is that existing drivers don'= t=20 do=20 > >>> : > : this, because they expect to be attached with hints, so they wi= ll=20 > > attach=20 > >>> : > : to all devices. I'm trying to figure out how to avoid this. > >>> : > :=20 > >>> : > : My basic question comes down to whether there is a good way in= =20 > > newbus to=20 > >>> : > : handle busses that may be wholly or partially enumerated by=20 firmware=20 > > or=20 > >>> : > : some other method, and may also have devices that can only atta= ch=20 > >>> : > : themselves if told to by hints. > >>> : >=20 > >>> : > Yes. This is a bit of a problem. The problem is that the existi= ng > >>> : > hints mechanism combines device tree and driver tree into one, an= d=20 in > >>> : > such a scenario, we wind up with the problem that you have. > >>> : >=20 > >>> : > One could make the probe routines return BUS_PROBE_GENERIC, and t= hat > >>> : > 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= =20 would > >>> : > also help, but does mean changing all the i2c bus attached driver= s=20 in > >>> : > the tree. > >>> :=20 > >>> : I think changing existing I2C drivers may be unavoidable. Would the= re=20 be=20 > >>> : any objection to changing the MI iicbus drivers to return=20 > >>> : BUS_PROBE_NOWILDCARD in their probe routines? It seems to have been= =20 > >>> : introduced (by you) to solve more or less exactly this problem. By = my=20 > >>> : 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 > >>> :=20 > >>> : I would also like to change iicbus_probe to return -1000 like=20 > >>> : dev/pci/pci.c to allow it to be overridden by a subclass. Please le= t=20 me=20 > >>> : know if this is a terrible idea or if I have forgotten any I2C devi= ce=20 > >>> : 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 > >>> =20 > >> This is now in the tree. Now for part 2, which I had not considered=20 > >> 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,=20 > >> attaches, and everything after that is wonderful. The issue is how bes= t=20 > >> to attach the iicbus modules to kiic. Current I2C controllers contain = a=20 > >> line like this: > >> DRIVER_MODULE(iicbus, me, iicbus_driver, iicbus_devclass, 0, 0); > >> > >> This explicitly specifies that you want the standard driver, so we nee= d=20 > >> additional glue to allow the ofw_iicbus driver to attach. One solution= =20 > >> is that each relevant host adapter can add an additional DRIVER_MODULE= =20 > >> line with the ofw_iicbus driver and class, which would have to exporte= d=20 > >> in a header somewhere. This is pretty ugly. Another solution is for th= e=20 > >> ofw_iicbus module to grow a list of the names of interesting adapters.= =20 > >> This is worse. > >> > >> The third option is to do what we do for pci, where all PCI adapters a= re=20 > >> named 'pcib'. So we could make new I2C host adapters named 'iichb' or= =20 > >> something, and then move the DRIVER_MODULE logic for new code into the= =20 > >> bus modules, as is done for PCI. This decreases the amount of=20 > >> information in the device names, but seems a bit cleaner. Thoughts? > >> -Nathan > >=20 > > If ofw_iicbus is simply an OF-aware version of iicbus (i.e. same=20 > > functionality) similar to the OF-aware PCI bus, then I would go the PCI= =20 route=20 > > and just call it iicbus but give it a higher probe priority. > >=20 >=20 > Which it is. What I meant was the bridge devices to which iicbus=20 > attaches. For pci, they all end up with the same name (pcib) so that the= =20 > pci layer knows to attach to them. For I2C, they are called=20 > iicbb/pcf/at91_twi/etc. and each bridge device explicitly attaches the=20 > standard iicbus to itself, instead of letting it and any firmware-aware=20 > versions probe. I'm a bit torn on that one, especially since you have weird cases like one = of=20 the via parts that has both smbus and iicbus children. The other option would be to fix the attaching to subclasses thing (that=20 should make all pci drivers attach to cardbus0 devices since cardbus inheri= ts=20 from pci, for example) and then you could have what would basically be an=20 abstract base class "iicbridge" with no devmethods that all bridge drivers= =20 inherit from, and iicbus would attach to that. =2D-=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901211110.33961.jhb>