From owner-freebsd-arch@FreeBSD.ORG Tue Jan 6 19:54:39 2009 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 33F60106574E for ; Tue, 6 Jan 2009 19:54:39 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from mail.icecube.wisc.edu (trout.icecube.wisc.edu [128.104.255.119]) by mx1.freebsd.org (Postfix) with ESMTP id 7387B8FC19 for ; Tue, 6 Jan 2009 19:54:37 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.icecube.wisc.edu (Postfix) with ESMTP id D85CE5823A; Tue, 6 Jan 2009 13:24:31 -0600 (CST) X-Virus-Scanned: amavisd-new at icecube.wisc.edu Received: from mail.icecube.wisc.edu ([127.0.0.1]) by localhost (trout.icecube.wisc.edu [127.0.0.1]) (amavisd-new, port 10030) with ESMTP id AgUHkTUCG6Iz; Tue, 6 Jan 2009 13:24:31 -0600 (CST) Received: from wanderer.tachypleus.net (i3-dhcp-172-16-223-165.icecube.wisc.edu [172.16.223.165]) by mail.icecube.wisc.edu (Postfix) with ESMTP id 9EF0E5823D; Tue, 6 Jan 2009 13:24:31 -0600 (CST) Message-ID: <4963AFE3.5080607@freebsd.org> Date: Tue, 06 Jan 2009 13:24:19 -0600 From: Nathan Whitehorn User-Agent: Thunderbird 2.0.0.19 (X11/20090105) MIME-Version: 1.0 To: "M. Warner Losh" References: <4929C6D8.7090305@freebsd.org> <20081123.170854.-626772149.imp@bsdimp.com> <492AC8DE.6050902@freebsd.org> <20081124.105800.-267230932.imp@bsdimp.com> In-Reply-To: <20081124.105800.-267230932.imp@bsdimp.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Cc: freebsd-arch@FreeBSD.org Subject: Re: Enumerable I2C busses X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Jan 2009 19:54:43 -0000 M. Warner Losh wrote: > In message: <492AC8DE.6050902@freebsd.org> > Nathan Whitehorn writes: > : M. Warner Losh wrote: > : > In message: <4929C6D8.7090305@freebsd.org> > : > Nathan Whitehorn writes: > : > : Rafał Jaworowski wrote: > : > : > > : > : > On 2008-11-23, at 19:18, Dag-Erling Smørgrav wrote: > : > : > > : > : >> Nathan Whitehorn 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