Date: Wed, 21 Jan 2009 15:42:08 -0500 From: John Baldwin <jhb@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: nwhitehorn@freebsd.org, freebsd-arch@freebsd.org Subject: Re: Enumerable I2C busses Message-ID: <200901211542.08461.jhb@freebsd.org> In-Reply-To: <20090121.132805.1108816677.imp@bsdimp.com> References: <200901210843.33247.jhb@freebsd.org> <200901211110.33961.jhb@freebsd.org> <20090121.132805.1108816677.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 21 January 2009 3:28:05 pm M. Warner Losh wrote: > In message: <200901211110.33961.jhb@freebsd.org> > John Baldwin <jhb@FreeBSD.org> writes: > : 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= =20 adding=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 o= f=20 > : devices=20 > : > > are > : > >>> : > : >> attached and at what address. Even worse, it is not=20 uncommon=20 > : for > : > >>> : > : >> similar but not entirely compatible devices to use the s= ame=20 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=20 between=20 > : > > vendors=20 > : > >>> : > : > do not technically prevent from scanning the bus; actuall= y,=20 our=20 > : > > current=20 > : > >>> : > : > iicbus code can do bus scaning when compiled with a diag= =20 define.=20 > : > > The=20 > : > >>> : > : > problem however is some slave devices are not well-behave= d,=20 and=20 > : > > they=20 > : > >>> : > : > don't like to be read/written to other than in very speci= fic=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 word= ed.=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= --=20 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 cas= e=20 like > : > >>> : > this the i2c bus should be a subclass (say i2c_of) that is=20 derived > : > >>> : > from the normal i2c bus stuff, but replaces the hints inserti= on=20 of > : > >>> : > devices with OF enumeration of devices. The OF higher levels= =20 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 work= s. > : > >>> :=20 > : > >>> : > : What I want to do is to have the I2C bus add the children t= hat=20 the=20 > : > >>> : > : firmware says it has. What the firmware cannot tell in=20 advance,=20 > : > > however,=20 > : > >>> : > : is which FreeBSD driver is responsible for those devices, a= nd=20 so=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= =20 table > : > >>> : > that will translate the OF provided properties into FreeBSD=20 driver > : > >>> : > names. > : > >>> :=20 > : > >>> : At the moment, I don't believe Apple uses any of the current ve= ry=20 > : small=20 > : > >>> : number of I2C device drivers in tree. So I may skip the table f= or=20 the=20 > : > >>> : time being, assuming the hack below is OK. In future, this may= =20 change,=20 > : > >>> : since G5 systems require software thermal control. But that wil= l=20 be=20 > : the=20 > : > >>> : subject of another mail to this list... > : > >>> :=20 > : > >>> : > : It seems reasonable to allow devices to use a real probe=20 routine=20 > : to=20 > : > > look=20 > : > >>> : > : at the firmware's name and compatible properties, like we=20 allow on=20 > : > > other=20 > : > >>> : > : Open Firmware busses. The trouble is that existing drivers= =20 don't=20 > : do=20 > : > >>> : > : this, because they expect to be attached with hints, so the= y=20 will=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= =20 attach=20 > : > >>> : > : themselves if told to by hints. > : > >>> : >=20 > : > >>> : > Yes. This is a bit of a problem. The problem is that the=20 existing > : > >>> : > hints mechanism combines device tree and driver tree into one= ,=20 and=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, a= nd=20 that > : > >>> : > would help somewhat. One could also have the probe routine=20 check to > : > >>> : > see if a specific driver is assigned to the device or not. T= hat=20 > : would > : > >>> : > also help, but does mean changing all the i2c bus attached=20 drivers=20 > : in > : > >>> : > the tree. > : > >>> :=20 > : > >>> : I think changing existing I2C drivers may be unavoidable. Would= =20 there=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= =20 been=20 > : > >>> : introduced (by you) to solve more or less exactly this problem.= By=20 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. Pleas= e=20 let=20 > : me=20 > : > >>> : know if this is a terrible idea or if I have forgotten any I2C= =20 device=20 > : > >>> : drivers. > : > >>> > : > >>> Short term, this is the right fix. There was an objection, I thi= nk=20 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 considere= d=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 priorit= y,=20 > : > >> attaches, and everything after that is wonderful. The issue is how= =20 best=20 > : > >> to attach the iicbus modules to kiic. Current I2C controllers cont= ain=20 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= =20 need=20 > : > >> additional glue to allow the ofw_iicbus driver to attach. One=20 solution=20 > : > >> is that each relevant host adapter can add an additional=20 DRIVER_MODULE=20 > : > >> line with the ofw_iicbus driver and class, which would have to=20 exported=20 > : > >> in a header somewhere. This is pretty ugly. Another solution is fo= r=20 the=20 > : > >> ofw_iicbus module to grow a list of the names of interesting=20 adapters.=20 > : > >> This is worse. > : > >> > : > >> The third option is to do what we do for pci, where all PCI adapte= rs=20 are=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= =20 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= =20 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 th= e=20 > : > standard iicbus to itself, instead of letting it and any firmware-awa= re=20 > : > versions probe. > :=20 > : I'm a bit torn on that one, especially since you have weird cases like = one=20 of=20 > : the via parts that has both smbus and iicbus children. > :=20 > : The other option would be to fix the attaching to subclasses thing (tha= t=20 > : should make all pci drivers attach to cardbus0 devices since cardbus=20 inherits=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 driv= ers=20 > : inherit from, and iicbus would attach to that. >=20 > Right now, the only bug that I know about with the cardbus vs pci > thing is that kldload doesn't necessarily probe/attach pci drivers on > a cardbus bus. Otherwise, it works perfectly. This is the only > reason that we have driver lines with cardbus attachments in the pci > drivers at all... That is the bug though. :) I've looked at it and I think I know how to fix= =20 it, I just haven't gotten around to it. I think device_probe_and_attach()= =20 needs to actually walk up the inheritance list of the current 'bus' driver,= =20 but only if all of the drivers for the current 'bus' failed to probe (or=20 there are no drivers). =2D-=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901211542.08461.jhb>